[clang-cl] Fix value of FUNCTION in MSVC mode. #67592

zahiraam · 2023-09-27T19:01:45Z

Predefined macro FUNCTION in clang is not returning the same string than MS for templated functions.

For the same test case MSVC is returning:

function: TestClass::TestClass
function: TestStruct::TestStruct
function: TestEnum::TestEnum

shafik · 2023-10-02T15:20:07Z

Could you add a description explaining what is currently wrong with the value of __FUNCTION__ is MSVC mode? I think we also need a release note.

zahiraam · 2023-10-02T15:50:38Z

Could you add a description explaining what is currently wrong with the value of __FUNCTION__ is MSVC mode? I think we also need a release note.

Hope the RN is at the right place? Thanks.

clang/test/SemaCXX/source_location.cpp

clang/lib/AST/TypePrinter.cpp

clang/include/clang/AST/PrettyPrinter.h

AaronBallman

I'd appreciate some unit tests showing that we print the elaboration in other circumstances. e.g.,

struct S { int x; };
namespace NS {
class C {};
}

S foo(S s1, NS::C c1) {
  S s12{12};
  using namespace NS;
  C c;
}

ensuring that we pretty print that to:

struct S { int x; };
namespace NS {
class C {};
}

struct S foo(struct S s1, class NS::C c1) {
  struct S s12{12};
  using namespace NS;
  class NS::C c;
}

(Could probably use more test coverage for other situations where the type name can appear, like within sizeof, etc)

zahiraam · 2023-10-05T12:55:24Z

struct S { int x; };
namespace NS {
class C {};
}

S foo(S s1, NS::C c1) {
S s12{12};
using namespace NS;
C c;
}

@AaronBallman By pretty printed you mean the use of PRETTY_FUNCTION right?
In MSVC this macro is not defined. The equivalent one is FUNCSIG, although maybe not quite the same.
On Linux GCC and clang are generating the same string. See https://godbolt.org/z/zd37orvdG
I can't figure out from godbolt, what string is generated for clang on Windows. Running on the command line (without my patch), in the asm output I see
.asciz "S foo(S, NS::C)"
That's different than what MSVC is generating with FUNCSIG. If we want to generate struct S foo(struct S,class NS::C then this seems to always have been broken. Right?

AaronBallman · 2023-10-05T14:37:56Z

@AaronBallman By pretty printed you mean the use of PRETTY_FUNCTION right?

No, I mean using the DeclPrinter and TypePrinter interfaces that are used to implement -ast-print:

llvm-project/clang/lib/AST/DeclPrinter.cpp

Line 65 in 32d16b6

DeclPrinter(raw_ostream &Out, const PrintingPolicy &Policy,

llvm-project/clang/lib/AST/TypePrinter.cpp

Line 120 in 32d16b6

explicit TypePrinter(const PrintingPolicy &Policy, unsigned Indentation = 0)

AaronBallman · 2023-10-05T14:38:45Z

We have unit tests for these that live at: https://github.com/llvm/llvm-project/blob/32d16b64d3125e76f65d7d88a302a33618eb0e6e/clang/unittests/AST/TypePrinterTest.cpp and https://github.com/llvm/llvm-project/blob/32d16b64d3125e76f65d7d88a302a33618eb0e6e/clang/unittests/AST/DeclPrinterTest.cpp

github-actions · 2024-02-15T13:27:15Z

✅ With the latest revision this PR passed the C/C++ code formatter.

AaronBallman · 2024-02-15T14:03:49Z

clang/lib/AST/DeclPrinter.cpp

@@ -897,7 +897,8 @@ void DeclPrinter::VisitFunctionDecl(FunctionDecl *D) {
        D->getBody()->printPrettyControlled(Out, nullptr, SubPolicy, Indentation, "\n",
                                  &Context);
    } else {
-      if (!Policy.TerseOutput && isa<CXXConstructorDecl>(*D))
+      if (Policy.ForcePrintingAsElaboratedType ||


What does forcefully printing an elaborate type have to do with printing out {}?

AaronBallman · 2024-02-15T14:11:28Z

clang/lib/AST/TypePrinter.cpp

+      if (Qualifier)
+        OS << "class ";
+      else
+        OS << "struct ";


union? enum?

AaronBallman · 2024-02-15T14:19:10Z

clang/include/clang/AST/PrettyPrinter.h

@@ -326,6 +326,10 @@ struct PrintingPolicy {
  LLVM_PREFERRED_TYPE(bool)
  unsigned AlwaysIncludeTypeForTemplateArgument : 1;

+  // Whether to print the type as an elaborated type. This is used when
+  // printing a function via the _FUNCTION__ macro in MSVC mode.
+  unsigned ForcePrintingAsElaboratedType : 1;


It's not clear to me how this new option should interact with SuppressElaboration and SuppressTagKeyword or whether we should have it at all. I would have expected that the existing type printing options (like FullyQualifiedName) would be sufficient and that we were failing to honor those in template arguments?

AaronBallman

Precommit CI has relevant failures.

AaronBallman · 2024-02-16T13:14:49Z

clang/lib/AST/TypePrinter.cpp

@@ -1635,6 +1635,14 @@ void TypePrinter::printElaboratedBefore(const ElaboratedType *T,
    if (T->getKeyword() != ElaboratedTypeKeyword::None)
      OS << " ";
    NestedNameSpecifier *Qualifier = T->getQualifier();
+    if (Policy.FullyQualifiedName) {


FullyQualifiedName controls printing of identifier qualification, not elaboration of type names. IOW, it controls printing Bar vs Foo::Bar rather than B vs struct B.

I think you want to be looking at !SuppressTagKeyword right?

zahiraam · 2024-02-21T14:45:09Z

@AaronBallman I removed the field that I added in the policy and tried to use the existing ones. That broke a few LIT tests but before fixing them I want to check with you if the combination of policy I have added in the unittest is correct. Thanks.

zahiraam · 2024-02-27T16:29:14Z

@AaronBallman I removed the field that I added in the policy and tried to use the existing ones. That broke a few LIT tests but before fixing them I want to check with you if the combination of policy I have added in the unittest is correct. Thanks.

ping?

AaronBallman · 2024-02-27T17:32:14Z

clang/lib/AST/TypePrinter.cpp

+    if (Policy.SuppressTagKeyword && Policy.SuppressScope) {
+      std::string prefix = T->isClassType()       ? "class "
+                           : T->isStructureType() ? "struct "
+                           : T->isUnionType()     ? "union "
+                                                  : "";


Hmmm this still seems wrong to me. When SuppressTagKeyword is true, tags (struct, union, etc) should not be printed.

AaronBallman · 2024-02-27T17:32:56Z

clang/lib/Sema/SemaExpr.cpp

+    bool ForceElaboratedPrinting = false;
+    if (IK == PredefinedIdentKind::Function && getLangOpts().MicrosoftExt)
+      ForceElaboratedPrinting = true;


Suggested change

bool ForceElaboratedPrinting = false;

if (IK == PredefinedIdentKind::Function && getLangOpts().MicrosoftExt)

ForceElaboratedPrinting = true;

bool ForceElaboratedPrinting = IK == PredefinedIdentKind::Function && getLangOpts().MicrosoftExt;

AaronBallman · 2024-02-27T17:50:37Z

clang/lib/AST/Expr.cpp

@@ -665,7 +665,8 @@ StringRef PredefinedExpr::getIdentKindName(PredefinedIdentKind IK) {
 // FIXME: Maybe this should use DeclPrinter with a special "print predefined


The more complex we make this, the more I think this FIXME sounds like a somewhat better idea though it would potentially be an involved refactoring...

AaronBallman · 2024-02-27T17:58:44Z

clang/lib/AST/Expr.cpp

+    const auto &LO = Context.getLangOpts();
+    if (ForceElaboratedPrinting) {
+      if ((IK == PredefinedIdentKind::Func ||
+           IK == PredefinedIdentKind ::Function) &&
+          !LO.MicrosoftExt)
+        return FD->getNameAsString();
+      if (IK == PredefinedIdentKind::LFunction && LO.MicrosoftExt)
+        return FD->getNameAsString();
+    } else {
+      if (IK != PredefinedIdentKind::PrettyFunction &&
+          IK != PredefinedIdentKind::PrettyFunctionNoVirtual &&
+          IK != PredefinedIdentKind::FuncSig &&
+          IK != PredefinedIdentKind::LFuncSig)
+        return FD->getNameAsString();
+    }


These are all doing the same thing -- calling FD->getNameAsString(); can we simplify the logic?

AaronBallman · 2024-02-27T18:03:24Z

clang/lib/AST/TypePrinter.cpp

-                        Policy, TPL, ParmIndex));
+      if (!Policy.SuppressTagKeyword &&
+          Argument.getKind() == TemplateArgument::Type &&
+          !Argument.getAsType()->isBuiltinType())


Should this be looking at isa<TagType>(getAsType())?

AaronBallman

The failure found by CI looks to be related:

<stdin>:422:1: note: possible intended match here
VarDecl:{ResultType struct struct X}{TypedText f1} (50) (deprecated)

(note the struct struct).

zahiraam · 2024-02-29T18:47:35Z

struct struct X

Interesting! The LIT test is passing on a debug build on Windows and generating the right output:

VarDecl:{ResultType struct X}{TypedText f1} (50) (deprecated)

I will try a release build.

zahiraam · 2024-02-29T20:11:15Z

struct struct X

Interesting! The LIT test is passing on a debug build on Windows and generating the right output:

VarDecl:{ResultType struct X}{TypedText f1} (50) (deprecated)

I will try a release build.

It looks like I can reproduce it with the release build.

Fixes the `sanitizer-x86_64-linux-android` buildbot.

These were last used in the fcopysign lowering, which now uses AArch64ISD::BSP.

If the buildvector node contains extract, which later should be combined with some other nodes by shuffling, need to estimate the cost of this shuffle before building the mask after shuffle. Reviewers: RKSimon Reviewed By: RKSimon Pull Request: llvm#83442

This adds operations for bitwise operators. Furthermore, an UnaryOp class and a helper to print unary operations are introduced.

When do the analysis for the (potential) masked gather node, we check that not greater than half of the pointer operands are loop invariants or potentially vectorizable. Need to check actually, that we have a loop at first and do better check for the potentially vectorizable pointers. Reviewers: RKSimon Reviewed By: RKSimon Pull Request: llvm#83472

Fix typo introduced by 6e36ceb

…ow their own template parameters (llvm#78274)" (llvm#79683) Reapplies llvm#78274 with the addition of a default-error warning (`strict-primary-template-shadow`) that is issued for instances of shadowing which were previously accepted prior to this patch. I couldn't find an established convention for naming diagnostics related to compatibility with previous versions of clang, so I just used the prefix `ext_compat_`.

…unctions (second try). (llvm#80457) Default value of checker option `ModelPOSIX` is changed to `true`. Documentation is updated. This is a re-apply of commit 7af4e8b that was reverted because a test failure (this is fixed now).

The class `CallDescription` is used to define patterns that are used for matching `CallEvent`s. For example, a `CallDescription{{"std", "find_if"}, 3}` matches a call to `std::find_if` with 3 arguments. However, these patterns are somewhat fuzzy, so this pattern could also match something like `std::__1::find_if` (with an additional namespace layer), or, unfortunately, a `CallDescription` for the well-known function `free()` can match a C++ method named `free()`: llvm#81597 To prevent this kind of ambiguity this commit introduces the enum `CallDescription::Mode` which can limit the pattern matching to non-method function calls (or method calls etc.). After this NFC change, one or more follow-up commits will apply the right pattern matching modes in the ~30 checkers that use `CallDescription`s. Note that `CallDescription` previously had a `Flags` field which had only two supported values: - `CDF_None` was the default "match anything" mode, - `CDF_MaybeBuiltin` was a "match only C library functions and accept some inexact matches" mode. This commit preserves `CDF_MaybeBuiltin` under the more descriptive name `CallDescription::Mode::CLibrary` (or `CDM::CLibrary`). Instead of this "Flags" model I'm switching to a plain enumeration becasue I don't think that there is a natural usecase to combine the different matching modes. (Except for the default "match anything" mode, which is currently kept for compatibility, but will be phased out in the follow-up commits.)

…ded (llvm#83818) Umbrella bug llvm#83182

This patch is a minor formatting fix in the OpenMP ops documentation wherein the examples section in ``omp.private`` was breaking out into a top level list item.

…ed Vectors (llvm#83038) Legalize smaller/larger than legal vectors with i8 and i16 element sizes. Vectors with elements smaller than i8 will get widened to i8 elements.

…3583) This should have been removed in 8dcb8ea, which removed support for .fail.cpp tests in the libc++ test suite.

into RangeReducDiv

zahiraam changed the title ~~Fix value of __FUNCTION__ and __func__ in MSVC mode.~~ [clang-cl] Fix value of __FUNCTION__ and __func__ in MSVC mode. Sep 27, 2023

zahiraam changed the title ~~[clang-cl] Fix value of __FUNCTION__ and __func__ in MSVC mode.~~ [clang-cl] Fix value of __FUNCTION__ in MSVC mode. Sep 30, 2023

zahiraam requested a review from AaronBallman October 2, 2023 11:53

AaronBallman reviewed Oct 2, 2023

View reviewed changes

clang/test/SemaCXX/source_location.cpp Outdated Show resolved Hide resolved

clang/lib/AST/TypePrinter.cpp Outdated Show resolved Hide resolved

clang/include/clang/AST/PrettyPrinter.h Outdated Show resolved Hide resolved

zahiraam requested a review from AaronBallman October 3, 2023 18:04

AaronBallman reviewed Oct 4, 2023

View reviewed changes

zahiraam closed this Jan 3, 2024

zahiraam deleted the RangeReducDiv branch January 3, 2024 20:23

zahiraam restored the RangeReducDiv branch January 4, 2024 14:04

zahiraam reopened this Jan 4, 2024

AaronBallman reviewed Feb 15, 2024

View reviewed changes

zahiraam requested a review from AaronBallman February 15, 2024 20:59

AaronBallman reviewed Feb 16, 2024

View reviewed changes

AaronBallman reviewed Feb 27, 2024

View reviewed changes

AaronBallman reviewed Feb 29, 2024

View reviewed changes

labrinea and others added 5 commits March 1, 2024 10:57

[clang] Remove unused lambda capture. (llvm#83550)

8511b32

Fixes the `sanitizer-x86_64-linux-android` buildbot.

[AArch64] Remove unused AArch64ISD::BIT. NFC

0e9a102

These were last used in the fcopysign lowering, which now uses AArch64ISD::BSP.

[mlir][EmitC] Add bitwise operators (llvm#83387)

7ac03e8

This adds operations for bitwise operators. Furthermore, an UnaryOp class and a helper to print unary operations are introduced.

cor3ntin and others added 30 commits March 4, 2024 15:16

[Clang][Docs] Fix issue number

1e429ff

Fix typo introduced by 6e36ceb

[libc][NFC] Don't expose how float16 and float128 types are provi…

07f8efa

…ded (llvm#83818) Umbrella bug llvm#83182

[mlir][OpenMP][NFC] - Minor fix in OpenMP ops documentation (llvm#83598)

c54064d

This patch is a minor formatting fix in the OpenMP ops documentation wherein the examples section in ``omp.private`` was breaking out into a top level list item.

AMDGPU: Test a few more cycle counts for mfma ops

f407f2d

[AArch64][GlobalISel] Re-commit Legalize G_SHUFFLE_VECTOR for Odd-Siz…

13a78fd

…ed Vectors (llvm#83038) Legalize smaller/larger than legal vectors with i8 and i16 element sizes. Vectors with elements smaller than i8 will get widened to i8 elements.

[SLP][NFC]Make canVectorizeLoads member of BoUpSLP class, NFC.

8982786

[libc++] Remove leftover .fail.cpp matcher in Lit test format (llvm#8…

d052148

…3583) This should have been removed in 8dcb8ea, which removed support for .fail.cpp tests in the libc++ test suite.

Fix value of __FUNCTION__ and __func__ in MSVC mode.

f513c6f

Fix format.

2e0dc54

Add undefined field.

f5ef478

Fixed a few issues.

994ba97

Fix LIT test literals.cpp.

a731882

Reverted LIT Test.

29bd183

Added release note as requested by reviewer.

1526dbc

Addressed review comments.

ddf0567

Addressed review comments.

d63d336

Cleaned up RN file.

f09a0c4

Fixed merge errors.

4729fd7

Fixed RN.

90d7864

Fixed more mreging issues.

330114e

Adding unittest.

96615d7

Removed ForcePrintingAsElaboratedType and fixed unittest.

0a7c176

Fixed the policy.

31e9bd6

Removed the policy field ForcePrintingAsElaboratedType.

fd278a0

Addressed review comments.

d499a57

Fix CI failure.

b296128

Merge branch 'RangeReducDiv' of https://github.com/zahiraam/llvm-project

e6b1822

into RangeReducDiv

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[clang-cl] Fix value of FUNCTION in MSVC mode. #67592

[clang-cl] Fix value of FUNCTION in MSVC mode. #67592

zahiraam commented Sep 27, 2023 •

edited

Loading

shafik commented Oct 2, 2023

zahiraam commented Oct 2, 2023

AaronBallman left a comment

zahiraam commented Oct 5, 2023

AaronBallman commented Oct 5, 2023

AaronBallman commented Oct 5, 2023

github-actions bot commented Feb 15, 2024 •

edited

Loading

AaronBallman Feb 15, 2024

AaronBallman Feb 15, 2024

AaronBallman Feb 15, 2024

AaronBallman left a comment

AaronBallman Feb 16, 2024

zahiraam commented Feb 21, 2024

zahiraam commented Feb 27, 2024

AaronBallman Feb 27, 2024

AaronBallman Feb 27, 2024

AaronBallman Feb 27, 2024

AaronBallman Feb 27, 2024

AaronBallman Feb 27, 2024

AaronBallman left a comment

zahiraam commented Feb 29, 2024

zahiraam commented Feb 29, 2024

		@@ -665,7 +665,8 @@ StringRef PredefinedExpr::getIdentKindName(PredefinedIdentKind IK) {
		// FIXME: Maybe this should use DeclPrinter with a special "print predefined

[clang-cl] Fix value of __FUNCTION__ in MSVC mode. #67592

Are you sure you want to change the base?

[clang-cl] Fix value of __FUNCTION__ in MSVC mode. #67592

Conversation

zahiraam commented Sep 27, 2023 • edited Loading

shafik commented Oct 2, 2023

zahiraam commented Oct 2, 2023

AaronBallman left a comment

Choose a reason for hiding this comment

zahiraam commented Oct 5, 2023

AaronBallman commented Oct 5, 2023

AaronBallman commented Oct 5, 2023

github-actions bot commented Feb 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AaronBallman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zahiraam commented Feb 21, 2024

zahiraam commented Feb 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AaronBallman left a comment

Choose a reason for hiding this comment

zahiraam commented Feb 29, 2024

zahiraam commented Feb 29, 2024

[clang-cl] Fix value of FUNCTION in MSVC mode. #67592

[clang-cl] Fix value of FUNCTION in MSVC mode. #67592

zahiraam commented Sep 27, 2023 •

edited

Loading

github-actions bot commented Feb 15, 2024 •

edited

Loading