MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land.
Looks great! > [clang] Fix filename remapping in template arguments Personally I don't consider this a bug 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. ================ 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 + ---------------- 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). ================ 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> ---------------- 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=`. ================ 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 + ---------------- 10 should be replaced with `[[#@LINE-1]]` so that adding more tests will not easily make this stale. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152570/new/ https://reviews.llvm.org/D152570 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
