[cfe-dev] [Possible regression?] Canonical decl of friend template decls in dependent contexts

[John McCall via cfe-dev]

> On Oct 13, 2017, at 6:16 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> On 13 October 2017 at 14:28, John McCall via cfe-dev <cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>> wrote:
>> On Oct 13, 2017, at 5:02 AM, Vassil Vassilev via cfe-dev <cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>> wrote:
>> 
>> On 13/10/17 00:17, slycelote wrote:
>>> Consider the following:
>>> 
>>> 
>>> template<typename T>
>>> struct C; // d1
>>> 
>>> template<typename T>
>>> class D {
>>>     static void f() {}
>>> 
>>>     template<typename>
>>>     friend struct C; // d2
>>> };
>>> 
>>> template<typename T>
>>> struct C { }; // d3
>>> 
>>> int main() { }
>>> 
>>> 
>>> Here there are 3 declarations corresponding to class template C (marked
>>> as d1, d2 and d3).
>>> 
>>> In clang 3.9 getCanonicalDecl() for all 3 of them returns d1. Starting
>>> from clang 4.0, d2->getCanonicalDecl() == d2. As far as I can tell, this
>>> is caused by commit 291753 [1].
>>   That was intentional because keeping friend declarations in the redecl chain confuses the template instantiator. That's visible especially with modules.
>>> 
>>> Is this intended? It looks to me, that, for example,
>>> clang::declaresSameEntity(d1, d2) [2] will return a wrong result.
>>   Richard, would it be feasible to not add the fiend decl to the redecl chain but to still rewire it to the 'right' canonical decl. That way getMostRecentDecl() won't return d2 (in some cases) and declaresSameEntity(d1,d2) will return true?
> 
> This would've been useful when I was implementing access control originally.  There's some pretty low-level complexity forced by the attempt to maintain a single chain of redeclarations.  I'm thinking particularly of how friend declarations have to inherit the right IDNS from the original declaration because they replace it as the most recent declaration.
> 
> And in practice it's possible for lookup to find an older declaration anyway (because of things like UsingShadowDecls).
> 
> Hmm, I think you're talking about a change to *all* friends, to not make them reachable from other declarations through the redeclaration chain, right? Perhaps I misunderstood, but I thought Vassil was referring to the case of friends in dependent contexts (which we currently don't put into the redeclaration chain at all), which I think is a somewhat distinct issue, because from a semantic point of view, they're not supposed to exist *at all* outside their template until instantiation. Eg:
> 
>   template<typename T> struct A { friend int f(); };
>   template<typename T> struct B { friend float f(); };
> 
> ... is presumably valid unless both templates are instantiated in the same program.

Right.  There's a lot of silliness like this with friends in templates.

> But now you mention it... I'm imagining an approach where a FriendDecl would hold a pointer to the befriended entity (outside the class), and also contain an (isolated, never part of a redecl chain) declaration that lexically appeared within it, with the befriended entity being lazily computed by the access checking code or similar. Is that something like what you had in mind, or were you really thinking of making the redecl chain not be a single circular list?

I was thinking that the redecl chain doesn't really need to be a circular list, yeah.  But I haven't thought very deeply about it; it might be completely unreasonable.

I know that we can't necessarily completely remove friends from the redeclaration chain because they might be the first declaration of something that's later explicitly declared, or even the definition.

John.

> I don't think the same approach would work for local extern declarations, which use an analogous IDNS mechanism to be redeclarable in the enclosing namespace scope without being visible to ordinary lookup there, since we really do need them to be in the same redeclaration chain.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20171013/910c4c0b/attachment.html>