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

[Richard Smith via cfe-dev]

On 13 October 2017 at 02:02, Vassil Vassilev <v.g.vassilev at gmail.com> 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 pretty badly break some of our invariants (eg, that walking over
PreviousDecl edges and then from the first decl to the MostRecentDecl will
eventually return you to the declaration where you started, all
declarations of an entity have the same semantic DeclContext, all
declarations have equivalent template parameter lists, ...). This would
bring back some of the problems we fixed when we removed these declarations
from the redecl chain (though, granted, not all of the, because walking the
redeclaration chain starting from somewhere other than the depedent-context
friend would never take you to the dependent-context friend).

And I don't see much benefit. Any correct code looking at a dependent
friend already needs to cope with the possibility that it has not been
wired into the redeclaration chain (because some portion of the friend
declaration itself might be dependent). Given that, it seems much simpler
to say that *no* friends in dependent contexts are wired into a
redeclaration chain.

That said, I can imagine that some (particularly, tooling) users of Clang
might want a best-effort "what is this friend declaration redeclaring, if
you happen to know?" query. And as it happens, we do work that out while
we're initially parsing the friend declaration, so I don't see a problem
with storing it and later making it available to such uses, but not as the
canonical / previous declaration (or anywhere that suggests it's reliable
and not merely best-effort).
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20171013/f66d8c42/attachment.html>