[PATCH] D147655: Implement mangling rules for C++20 concepts and requires-expressions.

Mon Apr 10 06:14:14 PDT 2023

erichkeane added a comment.

In D147655#4251984 <https://reviews.llvm.org/D147655#4251984>, @rsmith wrote:

> In D147655#4251042 <https://reviews.llvm.org/D147655#4251042>, @aaron.ballman wrote:
>
>> In D147655#4250922 <https://reviews.llvm.org/D147655#4250922>, @royjacobson wrote:
>>
>>> In D147655#4250056 <https://reviews.llvm.org/D147655#4250056>, @rsmith wrote:
>>>
>>>> There has not been any stable ABI from any compiler targeting the Itanium C++ ABI for constrained templates prior to this change. I don't think we need to worry too much about people using unfinished compiler features being broken when those features are finished.
>>>
>>> The ABI has been stable for quite some time and people have been using concepts with it for almost 3 years now. I'm not sure we can still break ABI this easily. But then, I also have no data to say we can't.
>>
>> We've never claimed full support for concepts, so those folks would be relying on an unstable ABI. However, if it turns out this causes significant pain in practice, perhaps we could use ABI tags to give folks the older ABI? I'd prefer to avoid that in this case given that the feature isn't yet fully supported (I don't like the idea of setting a precedent for relying on the ABI of incomplete features in general), but concepts is a sufficiently important use case that I could imagine doing it as a one-off if needed.
>
> This patch already extends `-fclang-abi-compat` to retain the old manglings so that users can stay on an old (broken) ABI if they need to. I don't think we need to do more than that for the concepts mangling changes.
>
>>>> The ABI proposals haven't been accepted yet; I'm not intending to land this change until the proposals have reached consensus in the Itanium C++ ABI group.
>
> The corresponding (confirmed) GCC bug report that they don't implement these mangling rules yet is: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100825
>
> This patch supersedes https://reviews.llvm.org/D126818 (apologies for the duplicated work, @erichkeane -- I didn't find that before I started working on this).

No problem!  This is a much more complete solution, and mine was held up on waiting for the Itanium ABI group to come to a consensus anyway (and fell by the wayside).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147655