Template parameters of friend declarations.

Hello.

In clang AST, we currently have two nodes representing friend declarations, namely FriendDecl and FriendTemplateDecl. The documentation for the second class says that it is currently unused:

/// \note This class is not currently in use. All of the above
/// will yield a FriendDecl, not a FriendTemplateDecl.

Hence, all friend declarations having template parameter lists end up building a FriendDecl node. Currently, these FriendDecl nodes have no way to store the (optional) template parameter lists.

For source-code fidelity purposes *only*, would it be OK if FriendDecl is changed so as to also store this missing info?

Currently, the FriendDecl node uses a pointer union:

   typedef llvm::PointerUnion<NamedDecl*,TypeSourceInfo*> FriendUnion;

A possibility would be to add an ExtInfo inner class (similar to what was done for DeclaratorDecl) and then change the union above to become:

   typedef llvm::PointerUnion3<NamedDecl*,TypeSourceInfo*,ExtInfo*> FriendUnion;

This way, we would incur a memory space penalty only when there actually are template parameter lists. Are you willing to accept patches following this approach?

Enea

Are the template parameter lists not attached to the target declaration?

John.

I am actually referring to a friend *type* declaration such as

class C {
   template<class T> friend struct S<T>::B;
};

That is, I am interested in the "outer" template parameter lists (such as those of out-of-line definitions of non-template members of class templates). Afaict, this kind of friend declarations is handled in

   Sema::ActOnTemplatedFriendTag

where I see the comment:

   // [...] TODO: for source fidelity, remember the headers.

Enea.

Ah, yes, I see. This is actually the "unsupported friend" case, for which
we do not actually implement correct semantic analysis at all — which
should be obvious from the fact that we drop the template parameter
lists, which would otherwise be necessary for correctness.

These outer template parameter lists can be present for both cases of a
friend declaration:
  template <class T> class A<T>::B;
  template <class T> void A<T>::b::foo();

So I don't suggest adding this as ExtInfo in the FriendUnion. Fortunately,
FriendDecl has no subclasses, and we know how many outer template
parameter lists there are when parsing it, so you can actually just store
the number of parameter lists in the class and tail-allocate the array of
template parameter lists.

John.

For the second case

   template <class T> friend void A<T>::b::foo();

the outer template parameter list(s) are already stored inside the function declaration, namely in the QualifierInfo base class of DeclaratorDecl::ExtInfo.

I think that friend *types* are the only cases where these outer template parameter lists are missing. Anyway, tail-allocation as suggested will be fine.

Enea.

[...]

These outer template parameter lists can be present for both cases of a
friend declaration:
   template <class T> class A<T>::B;
   template <class T> void A<T>::b::foo();

So I don't suggest adding this as ExtInfo in the FriendUnion.
Fortunately,
FriendDecl has no subclasses, and we know how many outer template
parameter lists there are when parsing it, so you can actually just store
the number of parameter lists in the class and tail-allocate the array of
template parameter lists.

John.

For the second case

   template <class T> friend void A<T>::b::foo();

the outer template parameter list(s) are already stored inside the
function declaration, namely in the QualifierInfo base class of
DeclaratorDecl::ExtInfo.

I think that friend *types* are the only cases where these outer
template parameter lists are missing. Anyway, tail-allocation as
suggested will be fine.

Please find attached a patch for review, including (de-)serialization and pretty printing of the new info attached to friend type declarations. On a second though, we kept the PointerUnion3 approach, which seems to be simpler (at least, to my eyes) when it comes to deserializing the declaration node.

Enea.

FriendDecl.patch (9.75 KB)

Please do use post-storage. You can look at how the template parameter packs deserialize themselves for an example of how to get the information for how much extra space to allocate; it's really quite straightforward.

John.

[...]

Please find attached a patch for review, including
(de-)serialization and pretty printing of the new info attached to
friend type declarations. On a second though, we kept the
PointerUnion3 approach, which seems to be simpler (at least, to my
eyes) when it comes to deserializing the declaration node.

Please do use post-storage. You can look at how the template
parameter packs deserialize themselves for an example of how to get
the information for how much extra space to allocate; it's really
quite straightforward.

John.

Here is the revised patch for review.
(Also fixes method getSourceRange).

Enea.

FriendDecl-2.patch (11.6 KB)

+ // The number of "outer" template parameter lists in non-templatic
+ // (currently unsupported) friend type declarations, such as
+ // template <class T> friend class A<T>::B;
+ unsigned NumTPLists;

Please make 'UnsupportedFriend' an unsigned : 1 bitfield and
pack this into the remaining 31 bits.

+ // The tail-allocated friend type template parameter lists (if any).
+ TemplateParameterList **getTPLists() const {
+ return reinterpret_cast<TemplateParameterList**>
+ (const_cast<FriendDecl*>(this) + 1);
+ }

Committed in r174050.

Enea.