erichkeane added a comment. In D126818#3563376 <https://reviews.llvm.org/D126818#3563376>, @erichkeane wrote:
> In D126818#3562355 <https://reviews.llvm.org/D126818#3562355>, @tahonermann > wrote: > >>> Note we might be confused, the parens there aren't completely clear as to >>> what your intent is. >> >> Well, I know that I'm confused and not clear what my intent is :) >> >> I asked the question because there appears to be an asymmetry between >> (temp.friend)p9 sentence 1 <http://eel.is/c++draft/temp.friend#9.sentence-1> >> and (temp.friend)p9 sentence 2 >> <http://eel.is/c++draft/temp.friend#9.sentence-2>. Sentence 1 applies to all >> constrained non-template friend declarations regardless of any template >> argument dependence while sentence 2 applies to just a subset of constrained >> friend function templates; those that have some amount of template >> dependence. The difference impacts when mangling differences are required. >> >> I spent some time analyzing how gcc, clang, and MSVC handle these different >> cases. See https://godbolt.org/z/85E5eMh3x. Search for FIXME? to find cases >> where I think one or more of the compilers is misbehaving or where it is >> unclear to me whether or how [temp.friend]p9 applies. Some highlights: >> >> - Some compilers diagnose some ill-formed cases when parsing the class >> template, others don't until the class template is instantiated. Not >> surprising. >> - All of the compilers reject non-template friend function definitions with >> non-dependent constraints due to duplicate definitions, presumably in >> violation of [temp.friend]p9; see the `ff2()` example. >> - All of the compilers reject non-template friend function definitions with >> dependent constraints due to duplicate definitions, presumably in violation >> of [temp.friend]p9; see the `ff6()` example. >> - Name mangling is currently insufficient to differentiate (otherwise) >> non-dependent friend function templates with dependent constraints; see the >> `fft5()` and `fft6()` examples. >> - None of the compilers reject some cases of non-definitions that should be >> rejected by [temp.friend]p9; see the `fft5()` and `fft7()` examples. > > Yes, I understand that Clang doesn't currently properly implement > [temp.friend]p9, and it is my belief too that none of the other compilers get > it right at the moment either. Clang approximates it by just directly > comparing partially instantiated constraints, so it gets it MOSTLY right. > > I believe I have all of your test cases in my implementation of > [temp.friend]p9 as a part of the deferred concepts patch here: > https://reviews.llvm.org/D126907 > > Aaron, my, and at least 1 other core expert's reading was: > > -A non template friend declaration with a requires-clause shall be a > definition. > > >> That is, ALL non-template friends, no matter what the requires-clause > depends on. > > -A friend function template with a constraint that depends on the template > parameter from an enclosing template shall be a definition. > > >> That is, when the declaration is a friend function template, it must be > a definition ONLY IF it depends on a template param from an enclosing > template. > > THOSE two sentences aren't particularly relevant, except for setting up the > two groups (that is, ALL constrained non-template friends and constrained > friend function templates where the constraint depends on an enclosing > template). > > The third sentence is the valuable one to both of these: > > -Such a constrained friend function or function template declaration does not > declare the same function or function template as a declaration in any other > scope. > > >> That is, any friend in either of those two groups from sentence 1 and > sentence 2 do not conflict with the same lexical declaration in a different > instantiation. > > THIS patch attempts to implement the mangling required in that last sentence, > however it doesn't differentiate the "ONLY if it depends on a template param > from an enclosing template", which I intend to fix in a future revision, once > the infrastructure from my deferred concepts patch (see link above) has made > it into the repo. SO I believe my 'future revision' part in the above is actually no longer necessary. We prohibit that with a Sema 'redefinition error': https://godbolt.org/z/1r549vfK4 So the question here is whether we want 'baz' in that godbolt link to use the 'F' mangling or not. @rjmccall? The 'easiest' thing to do I think (based on this patch) is to ALWAYS use the 'F' mangling for a friend with a constraint. WDYT? 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
