[PATCH] D152570: [clang] Fix filename remapping in template arguments

[Dan McGregor via Phabricator via cfe-commits]

dankm added a comment.

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.



================
Comment at: clang/test/CodeGenCXX/file-prefix-map-lambda.cpp:1
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -fmacro-prefix-map=%p=./UNLIKELY_PATH/empty -S %s -emit-llvm -o - | FileCheck %s
+
----------------
MaskRay wrote:
> 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).
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.


================
Comment at: clang/test/CodeGenCXX/file-prefix-map-lambda.cpp:2
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -fmacro-prefix-map=%p=./UNLIKELY_PATH/empty -S %s -emit-llvm -o - | FileCheck %s
+
+template<typename f>
----------------
MaskRay wrote:
> 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=`.
Yes, I was just quickly thinking of a new filename; macro-prefix-map makes more sense to me too. I'll rename the file.


================
Comment at: clang/test/CodeGenCXX/file-prefix-map-lambda.cpp:11
+  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:10:24)]\00", align 1
+
----------------
MaskRay wrote:
> 10 should be replaced with `[[#@LINE-1]]` so that adding more tests will not easily make this stale.
Thanks, I was looking for how to do that. Not too familiar with filecheck and lit.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152570/new/

https://reviews.llvm.org/D152570