erichkeane planned changes to this revision.
erichkeane added a comment.
In D126818#3551504 <https://reviews.llvm.org/D126818#3551504>, @erichkeane
wrote:
> Note this was mentioned as a 'must also do' in my discussion on deferred
> constraints with @rsmith, however as this already fails, I don't see this as
> a prerequisite. However, I think this is easy enough to just do.
>
> THAT SAID: The proposal on the itanium-abi github wasn't particularly clear
> as to which of the following three options were the 'right' answer. I'm
> hoping @rjmccall can decide whether this is acceptable:
>
> Option 1: Change the mangling of ALL friend functions. I determined this is
> likely an ABI break on otherwise 'settled' code, and figured we wouldn't want
> to do this.
>
> Option 2: Change the mangling for ALL constrained friend functions. This
> seemed easy enough to do, AND changes the mangling for things only covered by
> 'experimental' concepts support. So I think changing the ABI for current
> things is OK here.
>
> Option 3: Change the mangling for constrained friend functions that fall
> under temp.friend p9. The only difference between this and Option2 is that
> this would need to check the 'template friend function depends on containing
> scope'. I don't believe this is necessary so long as we are consistent with
> it. Presumably this could end up with multiple symbols for functions that
> are the 'same declaration' by rule in separate translation units, such as:
>
> struct Base{};
> template<int N>
> struct S {
> template<typename U>
> friend void func(Base&) requires(U::value) {}
> };
> void uses() {
> S<1> s;
> S<2> s2; // This is an error during Sema, since 'func' is a duplicate
> here.
> func(s); // all S<N>s SHOULD end up with the same declaration, but in
> different TUs they might end up with different copies here?
> }
>
> In typing this comment up, I believe I got the 'option' wrong here. Should I
> have gone with #3 above? I'm going to think about it overnight (and hope
> you reviewer folks can chime in!) and get to it in the morning.
After thinking about it, I'm about 99% sure I made the wrong choice here. I
should have gone with #3. That likely requires the infrastructure from the
deferred concepts patch, so I'll get that ready, then rebase this to be ready.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126818/new/
https://reviews.llvm.org/D126818
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
