[PATCH] D126818: Itanium ABI: Implement mangling for constrained friends

Mon Nov 21 06:54:16 PST 2022

dfrib added a comment.

In D126818#3941036 <https://reviews.llvm.org/D126818#3941036>, @erichkeane wrote:

> In D126818#3940898 <https://reviews.llvm.org/D126818#3940898>, @dfrib wrote:
>
>> In D126818#3935740 <https://reviews.llvm.org/D126818#3935740>, @rjmccall wrote:
>>
>>> I'm too often slow to actually apply edits to the ABI document.  There's been plenty of time for feedback on this one; go ahead and act like it's accepted.
>>
>> CWG 2596 was discussed at Kona and, afaict, CWG is opting for a path of least effort, with a different result <https://wg21.cmeerw.net/cwg/issue2596> than what is implemented this patch and previously discussed in the ABI issue <https://github.com/itanium-cxx-abi/cxx-abi/issues/24#issuecomment-934713719>:
>>
>>> **CWG 2022-11-10**
>>>
>>> The friend definitions **should conflict with friend definitions from other instantiations of the same class template**, consistent with how non-constrained friends would work. Note that the enclosing dependent class type does not appear in the friend function's signature, which is unusual.
>
> Can you clarify the difference here?  What did we choose different?  The example from that CWG issue is exactly in the test for this (see the top of `useS`) so I'm not sure what difference we're missing? Can you clarify what I'm not matching here?

I wrote the CWG issue and the example therein indeed flags exactly what is being addressed by this patch, but I'm referring particularly to the comment added from CWGs Kona meeting 2022-11-10 (see now **emphasized** quote above). I may be mistaken, but I read it as proposing to make `useS` ill-formed, emphasizing that you need to include the class somewhere in the hidden friend's signature, whereas overloading as shown in the `useS` example would not be supported.

For more details, see the Kona minutes <https://wiki.edg.com/bin/view/Wg21kona2022/CoreWorkingGroup#2596_Instantiation_of_constraine> at EDG-wiki.

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

https://reviews.llvm.org/D126818