Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang-cl] __FUNCTION__ used in templated function is not returning the same string than MSVC. #66114

Open
zahiraam opened this issue Sep 12, 2023 · 6 comments
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang-cl

Comments

@zahiraam
Copy link
Contributor

zahiraam commented Sep 12, 2023

__FUNCTION__ in clang is not returning the same string than MS for templated functions.

See https://godbolt.org/z/88n1rGs3b

For this test case MSVC is returning:
function: TestClass<class UnitTestNative>::TestClass
func: TestClass

I have looked at it briefly. It looks like a fix can be implemented in PredefinedExpr::ComputeName by adding a case for IK==PredefinedExpr::Function? However, I couldn't find a way to stick the "class" keyword in front of the class name. May be some policy argument should be used to print the function?
Unless everything should be computed in PredefinedExpr::ComputeName using some string manipulation (meaning getting the string from printQualifiedName and manipulating it to get the "class" keyword?

@RIscRIpt I think you have been working on MS compatibility. I am wondering if you are currently working on this or if you might have some suggestion on the fix I am about to work on?
Thanks.

@RIscRIpt
Copy link
Member

@zahiraam I am currently working at semi-related problem (support of __LPREFIX and stuff). Changes won't touch PredefinedExpr::ComputeName significantly, so there should no be conflicts in code.
I'll post an update once I have time to take a closer look at the problem and possible solutions.

@cor3ntin cor3ntin added clang:frontend Language frontend issues, e.g. anything involving "Sema" clang-cl and removed new issue labels Sep 13, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 13, 2023

@llvm/issue-subscribers-clang-frontend

__FUNCTION__ in clang is not returning the same string than MS for templated functions.

See https://godbolt.org/z/88n1rGs3b

For this test case MSVC is returning:
function: TestClass<class UnitTestNative>::TestClass
func: TestClass

I have looked at it briefly. It looks like a fix can be implemented in PredefinedExpr::ComputeName by adding a case for IK==PredefinedExpr::Function? However, I couldn't find a way to stick the "class" keyword in front of the class name. May be some policy argument should be used to print the function?
Unless everything should be computed in PredefinedExpr::ComputeName using some string manipulation (meaning getting the string from printQualifiedName and manipulating it to get the "class" keyword?

@RIscRIpt I think you have been working on MS compatibility. I am wondering if you are currently working on this or if you might have some suggestion on the fix I am about to work on?
Thanks.

@zahiraam
Copy link
Contributor Author

zahiraam commented Sep 15, 2023

@zahiraam I am currently working at semi-related problem (support of __LPREFIX and stuff). Changes won't touch PredefinedExpr::ComputeName significantly, so there should no be conflicts in code. I'll post an update once I have time to take a closer look at the problem and possible solutions.

@RIscRIpt I have a draft (unfinished) PR here: #66120. When you have a minute take a look to see if this is the right way to tackle it. Thanks.

@RIscRIpt
Copy link
Member

RIscRIpt commented Sep 17, 2023

@zahiraam I don't have access neither of PR approval nor PR merge, so I won't pollute your PR with my comments.

But as you've requested, here're my two cents:

  • Consider a shorter check for MSVC mode: Context.getLangOpts().MicrosoftExt
  • Printing of "class " should be wrapped around with Policy.MSVCFormatting, and this policy should be set to true (or getLangOpts().MicrosoftExt) (I don't know why it's not equal to getLangOpts().MicrosoftExt by default; a bug?)
  • Consider merging your new if branch with existing code below (i.e. if (const FunctionDecl *FD ... branch) (e.g. see my example), tho I don't know which approach would be better.
  • Clang tests shall be updated to check for a new value of __FUNCTION__ in MSVC-mode.

Overall you are in the right direction.

@zahiraam
Copy link
Contributor Author

@RIscRIpt I wasn't able to add you as a reviewer in the PR? I suppose you have read/write access to the PR right? Don't hesitate to add comment on it. At any case I have updated the PR but still have a few doubts about the addition of "class" because it's changing the behavior of the debuginfo LIT tests. May adding "class" should be done somewhere else, or have a stricter condition or that's simply correct (will need to ask a debugger person).

@RIscRIpt
Copy link
Member

RIscRIpt commented Sep 23, 2023

I guess this issue should've been closed by #66120.

But, I found a bug in the merged implementation. Consider the following code: https://godbolt.org/z/KWTKPcG6G

#include <iostream>

template<class T> class A {
    public:
        A() {
            std::cout << __FUNCTION__ << '\n';
        }
};

class C {};
struct S {};
union U {};

int main() {
    A<int> i;
    A<C> c;
    A<S> s;
    A<U> u;
    return 0;
}

MSVC:

A<int>::A
A<class C>::A
A<struct S>::A
A<union U>::A

Clang-cl:

A<class int>::A
A<class C>::A
A<class S>::A
A<class U>::A

Shortly, MSVC distinguishes between class-key keywords, and it does not print any class-key in case of primitive types.

Another problem with new implementation, is that it makes no difference between __func__ and __FUNCTION__, whereas MSVC does: https://godbolt.org/z/7q64MjE6K

MSVC:

A
A
A
A

Clang-cl:

A<class int>::A
A<class C>::A
A<class S>::A
A<class U>::A

Edit: MSVC also prints enum for enum-types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang-cl
Projects
None yet
Development

No branches or pull requests

4 participants