[clang] Apply -fmacro-prefix-map to anonymous tags in template arguments

[dankm]

Summary

When expanding template arguments for pretty function printing,
such as for PRETTY_FUNCTION, make TypePrinter apply
macro-prefix-map remapping to anonymous tags such as lambdas.

Fixes https://github.com/llvm/llvm-project/issues/63219

[dankm]

I still need to make a unit test for this. Should be more-or-less a cleaned up version of the testcase in https://github.com/llvm/llvm-project/issues/63219.

[dankm]

Add release notes.

[dankm]

Despite needing unit tests, I'd like some eyes on this change.

[shafik]

clang/lib/AST/Expr.cpp

789

This looks consistent with how other places that set CallBacks but I am not familiar with this area.

791

This may not be a well known idiom but is widely used within llvm.

[dankm]

clang/lib/AST/Expr.cpp

791

Makes sense, I was going for unambiguity between the class's member name and the constructor argument name, but if it's widely used that's fine. I'll add that change when I add unit tests.

[dankm]

Added unit test

[dankm]

Fix typo in release notes

[dankm]

Rebase and hopefully fix builds.

[dankm]

Fix unit tests on windows

[MaskRay]

Looks great!

[clang] Fix filename remapping in template arguments

Personally I don't consider this a bug fix (AnonymousTagLocations behavior in TypePrinter; GCC doesn't print the filename), so I'd avoid "Fix", but this changes does move into a desired direction, by making -fmacro-prefix-map= add local determinism.
I wonder whether Make -fmacro-prefix-map= apply to anonymous tag in TypePrinter will be more meaningful subject without being too verbose.

clang/test/CodeGenCXX/file-prefix-map-lambda.cpp

1 ↗

(On Diff #530638)

You can use -triple x86_64 to test generic ELF behavior, if you don't want to write linux-gnu :)

For this test, perhaps -triple %itanium_abi_triple is better (coverage for non-x86 targets).

2 ↗

(On Diff #530638)

For the new test file, I was thinking of https://maskray.me/blog/2021-08-08-toolchain-testing#the-test-checks-at-the-wrong-layer#i-dont-know-an-existing-test-can-be-enhanced.

The main C++ test CodeGenCXX/predefined-expr.cpp does not say about __PRETTY_FUNCTION__ in its filename, so a new file seems fine.

If we are going to retain this new test, I think macro-prefix-map- is a better prefix.
-fmacro-prefix-map= is more specific to -ffile-prefix-map=.

11 ↗

(On Diff #530638)

10 should be replaced with [[#@LINE-1]] so that adding more tests will not easily make this stale.

[dankm]

Avoiding "Fix" in the description is a good suggestion. Whether it's a bugfix or not is a matter of perspective, and what's really happening here is I'm adjusting compliant implementation defined behavior, not really fixing it.

clang/test/CodeGenCXX/file-prefix-map-lambda.cpp

1 ↗

(On Diff #530638)

Generic ELF (or rather Itanium ABI) is probably fine, I just found Windows didn't work, so used the platform I was testing on. But I like the itanium abi triple suggestion.

2 ↗

(On Diff #530638)

Yes, I was just quickly thinking of a new filename; macro-prefix-map makes more sense to me too. I'll rename the file.

11 ↗

(On Diff #530638)

Thanks, I was looking for how to do that. Not too familiar with filecheck and lit.

[dankm]

Renamed & cleaned up unit tests

[dankm]

Maybe for the description something like:

Apply -fmacro-prefix-map to anonymous tags in template arguments

Do I need to mention TypePrinter?

[dankm]

@MaskRay if this meets your approval, are you able to commit it for me?

[MaskRay]

In D152570#4417752, @dankm wrote:

@MaskRay if this meets your approval, are you able to commit it for me?

LG, but give @aaron.ballman and @shafik some time if they have opinions.

In D152570#4416201, @dankm wrote:

Maybe for the description something like:

Apply -fmacro-prefix-map to anonymous tags in template arguments

Do I need to mention TypePrinter?

I think it's fine to not mention it in the subject. You can mention TypePrinter in the description, though.

clang/docs/ReleaseNotes.rst

514

This needs adjustment as the subject has changed.

[aaron.ballman]

LGTM aside from the issue with the release note.

[dankm]

Thanks, I'll update the release notes & description right away.

[dankm]

Update release notes

[MaskRay]

clang/docs/ReleaseNotes.rst

515

I'll add a period to be consistent with other entries. Thanks!

clang/test/CodeGenCXX/macro-prefix-map-lambda.cpp

2

I'll remove -S since -emit-llvm wins for CC1 options.

[zixuan-wu]

Hi,

clang/test/CodeGenCXX/macro-prefix-map-lambda.cpp

11

Hi, it's weird that the align in CSKY target is not 1, instead it's 4. Anybody know the key point?

[tuliom]

clang/test/CodeGenCXX/macro-prefix-map-lambda.cpp

11

And on SystemZ it's 2.

clang/docs/ReleaseNotes.rst

- Fix rejects-valid when consteval operator appears inside of a template.
   (`#62886 <https://github.com/llvm/llvm-project/issues/62886>`_).
 - Fix crash for code using ``_Atomic`` types in C++
   (`See patch <https://reviews.llvm.org/D152303>`_).
 - Fix crash when passing a value larger then 64 bits to the aligned attribute.
   (`#50534 <https://github.com/llvm/llvm-project/issues/50534>`_).
 - CallExpr built for C error-recovery now is always type-dependent. Fixes a
   crash when we encounter a unresolved TypoExpr during diagnostic emission.
   (`#50244 <https://github.com/llvm/llvm-project/issues/50244>_`).
+- Apply ``-fmacro-prefix-map`` to anonymous tags in template arguments
+  (`#63219 <https://github.com/llvm/llvm-project/issues/63219>`_).
 
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Bug Fixes to Attribute Support
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 - Fixed a bug where attribute annotations on type specifiers (enums, classes,
   structs, unions, and scoped enums) were not properly ignored, resulting in

clang/lib/AST/Expr.cpp

if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(CurrentDecl)) {
 
     if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(FD)) {
       if (MD->isVirtual() && IK != PrettyFunctionNoVirtual)
         Out << "virtual ";
       if (MD->isStatic())
         Out << "static ";
     }
 
+    class PrettyCallbacks final : public PrintingCallbacks {
+    public:
+      PrettyCallbacks(const LangOptions &LO) : LO(LO) {}
+      std::string remapPath(StringRef Path) const override {
+        SmallString<128> p(Path);
+        LO.remapPathPrefix(p);
+        return std::string(p);
+      }
+
+    private:
+      const LangOptions &LO;
+    };
     PrintingPolicy Policy(Context.getLangOpts());
+    PrettyCallbacks PrettyCB(Context.getLangOpts());
+    Policy.Callbacks = &PrettyCB;
     std::string Proto;
     llvm::raw_string_ostream POut(Proto);
 
     const FunctionDecl *Decl = FD;
     if (const FunctionDecl* Pattern = FD->getTemplateInstantiationPattern())
       Decl = Pattern;
     const FunctionType *AFT = Decl->getType()->getAs<FunctionType>();
     const FunctionProtoType *FT = nullptr;

clang/test/CodeGenCXX/macro-prefix-map-lambda.cpp

+// RUN: %clang_cc1 -triple %itanium_abi_triple -fmacro-prefix-map=%p=./UNLIKELY_PATH/empty %s -emit-llvm -o - | FileCheck %s
+
+template<typename f>
+auto lambdatest(f&& cb) {
+  const char *s = __PRETTY_FUNCTION__;
+  return s;
+}
+
+int main() {
+  auto *s = lambdatest([](){});
+// CHECK: @"__PRETTY_FUNCTION__._Z10lambdatestIZ4mainE3$_0EDaOT_" = private unnamed_addr constant [{{[0-9]+}} x i8] c"auto lambdatest(f &&) [f = (lambda at ./UNLIKELY_PATH/empty{{/|\\\\}}{{.*}}.cpp:[[#@LINE-1]]:24)]\00", align 1
+
+  return 0;
+}