[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