erichkeane added a comment.

In D126818#3941165 <https://reviews.llvm.org/D126818#3941165>, @dfrib wrote:

> 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.

Hmm... I'm still a bit confused and hoping someone else can show up and let me 
know how to fix this.  There is quite a bit of work to do temp.friend p9 that I 
implemented to mean a constrained-non-template friend or a 
constrained-template-friend-that-refers-to-enclosing-scope don't conflict, 
based on the wording.

The minutes aren't particularly clear to me unfortunately as to what they 
really want, particularly since the suggested wording says the opposite of what 
I THOUGHT the discussion was doing at the end?  The wording/example are still 
consistent with my interpretation/implementation, so I have no idea what is 
going on.  We might find ourselves wanting to hold off until CWG comes up with 
actual wording?

The "Refers to an enclosing template" part seems sensible enough (and I have 
work in place to detect and store that), so the question is whether we want 
non-template friends to conflict ever.  IMO, the current rule (implemented in 
clang and here, where constrained non-template friends are somehow different) 
is a little odd-ball, but at least easy enough to reason about.

I think perhaps we need to wait on CWG to clarify what they mean, at least by 
including a wording consistent with that top thing.  The unfortunate part here 
is that Clang implements 1/2 of this at the moment: we implement the SEMA 
changes, but not the mangling changes for the current wording.


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

https://reviews.llvm.org/D126818

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to