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

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Eric Fiselier via cfe-dev
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].

Is this intended? It looks to me, that, for example,
clang::declaresSameEntity(d1, d2) [2] will return a wrong result.


[1] https://llvm.org/viewvc/llvm-project?view=revision&revision=291753
[2]
https://clang.llvm.org/doxygen/namespaceclang.html#ad9d926b16adbdbc93705737b69d47cae


--
Sly.
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [Possible regression?] Canonical decl of friend template decls in dependent contexts

Eric Fiselier via cfe-dev
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?
>
>
> [1] https://llvm.org/viewvc/llvm-project?view=revision&revision=291753
> [2]
> https://clang.llvm.org/doxygen/namespaceclang.html#ad9d926b16adbdbc93705737b69d47cae
>
>

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [Possible regression?] Canonical decl of friend template decls in dependent contexts

Eric Fiselier via cfe-dev

On Oct 13, 2017, at 5:02 AM, Vassil Vassilev via cfe-dev <[hidden email]> 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).

John.


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [Possible regression?] Canonical decl of friend template decls in dependent contexts

Eric Fiselier via cfe-dev
In reply to this post by Eric Fiselier via cfe-dev
On 13 October 2017 at 02:02, Vassil Vassilev <[hidden email]> 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).

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [Possible regression?] Canonical decl of friend template decls in dependent contexts

Eric Fiselier via cfe-dev
In reply to this post by Eric Fiselier via cfe-dev
On 13 October 2017 at 14:28, John McCall via cfe-dev <[hidden email]> wrote:
On Oct 13, 2017, at 5:02 AM, Vassil Vassilev via cfe-dev <[hidden email]> 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.

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

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [Possible regression?] Canonical decl of friend template decls in dependent contexts

Eric Fiselier via cfe-dev
On Oct 13, 2017, at 6:16 PM, Richard Smith <[hidden email]> wrote:
On 13 October 2017 at 14:28, John McCall via cfe-dev <[hidden email]> wrote:
On Oct 13, 2017, at 5:02 AM, Vassil Vassilev via cfe-dev <[hidden email]> 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.


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev