[PATCH] D122256: [clang][ABI] New C++20 module mangling scheme
Fangrui Song via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed May 18 10:25:15 PDT 2022
MaskRay added a comment.
The differential has the same intention/subject as D118352 <https://reviews.llvm.org/D118352>. In such a case reusing the preexisting differential would have been better. I have left more comments on https://reviews.llvm.org/D125825#3522911
Note: a revert was committed (https://reviews.llvm.org/D118352#3368921) in 11 hours after the problem was reported.
It was fairly legitimate. The user pushing the revert helps maintain build bots green. (Some groups may be more aggressive, e.g. they may leave a comment and perform a revert in 10 minutes.
Technically that is also fine, but sometimes can be done better.)
In this particular case, I believe the problem was just a Mach-O platform differential that Mach-O IR output doesn't use `dso_local `. This is very common when adding new tests to `test/clang/CodeGen*` and contributors often neglect it.
If I had seen the failure in time, I would have tried fixing the test as it is quite straightforward, instead of reverting it.
(Thanks for adding #clang-language-wg <https://reviews.llvm.org/tag/clang-language-wg/> since this was a patch that some folks would like to subscribe to.)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122256/new/
https://reviews.llvm.org/D122256
More information about the cfe-commits
mailing list