Given the code namespace boost { template <class> class shared_ptr { template <class> friend class shared_ptr; }; } shared_ptr<char> c; shared_ptr<int> i; shared_ptr<char> is created with its TemplateDecl being the class shared_ptr while shared_ptr<int> TemplateDecl is the friend TemplateDecl created in shared_ptr<char>. This happens because the friend TemplateDecl created in shared_ptr<char> replaces the original class TemplateDecl. It seems inconsistent that both ClassTemplateSpecializationDecl have different TemplateDecl. To solve this, we could demand same friendness in NamedDecl::declarationReplaces bool NamedDecl::declarationReplaces(NamedDecl *OldD, bool IsKnownNewer) const { ... // Friend should not replace non-friend. if (OldD->getFriendObjectKind() != getFriendObjectKind()) return false;
Hi Yaron! This would make sense to me. Let's wait for Richard's opinion. This is a busy week because of the C++ standards committee meeting. Cheers, Vassil
I believe this fixes two FIXME in SemaTemplate/friend-template.cpp: // FIXME: the redeclaration note ends up here because redeclaration // lookup ends up finding the friend target from X3<int>. // FIXME: As with cases above, the note here is on an unhelpful declaration, // and should point to the declaration of G within F.
I don't think the proposed change is correct; we should use something like if (getFriendObjectKind() > OldD->getFriendObjectKind() && !isThisDeclarationADefinition()) return false; The > is needed so that a non-friend can properly replace a friend, and a visible friend can properly replace an invisible friend. The definition check is needed for cases like: void f(); struct A { [[noreturn]] void f() { exit(1); } }; void g() { f(); } ... where we want the 'noreturn' attribute to be visible in g(). Beyond that, I'm concerned that this may lead to us silently losing GNU attributes (or other properties that might be altered by the friend declaration) in cases like: void f(); struct A { friend __attribute__((deprecated)) void f(); }; void g() { f(); } ... because name lookup would only find the non-friend version of 'f'. That said, perhaps we should consider rejecting GNU attributes on non-defining friend declarations, in the same way that C++11 attributes are ill-formed on such declarations, as a prerequisite for this change. We'd need to do some investigation to see if there is any significant amount of real code relying on our current behaviour.
This makes sense. isThisDeclarationADefinition is available for VarDecl, FunctionDecl, TagDecl, FunctionTemplateDecl, ClassTemplateDecl, VarTemplateDecl. For other Decls which has no meaning for definition and no isThisDeclarationADefinition, is this condition always false or decays to if (getFriendObjectKind() > OldD->getFriendObjectKind()) return false; or maybe it makes no sense because they won't have friend kind and hence it will be always false?
(In reply to comment #4) > maybe it makes no sense because they won't have friend kind and hence it > will be always false? That seems correct.
https://reviews.llvm.org/D31540
Created attachment 18298 [details] reduced case from Richard Smith! r300443 breaking internal build of libc++ with modules: While building module 'x': In file included from <module-includes>:2: In file included from ./c.h:1: ./a.h:3:32: error: inline declaration of 'f' follows non-inline definition template<typename> inline void f() {} ^ ./a.h:3:32: note: previous definition is here template<typename> inline void f() {} r300443 was reverted in r300497.
the replacement condition may need to consider modules? with the reproducer, it prevents this replacement: this= FunctionTemplateDecl 0x23515b2ab28 parent 0x23515a1d918 prev 0x23515b2a718 <./a.h:2:12, col:45> col:43 hidden f |-TemplateTypeParmDecl 0x23515b2a9c8 <col:21> col:21 hidden typename depth 0 index 0 `-FunctionDecl 0x23515b2aa78 parent 0x23515a1d918 prev 0x23515a1e578 <col:31, col:45> col:43 hidden f 'void (void)' OldD= FunctionTemplateDecl 0x23515b2a718 <./a.h:1:1, col:27> col:25 in (local) x.b hidden f |-TemplateTypeParmDecl 0x23515a1e460 <col:10> col:10 hidden typename depth 0 index 0 `-FunctionDecl 0x23515a1e578 <col:20, col:27> col:25 in (local) x.b hidden f 'void (void)' causing: |-FunctionTemplateDecl 0x23515b2ae58 prev 0x23515b2ab28 <line:3:1, col:37> col:32 in (local) x.b hidden f | |-TemplateTypeParmDecl 0x23515b2ad08 <col:10> col:10 hidden typename depth 0 index 0 | `-FunctionDecl 0x23515b2adb8 prev 0x23515b2aa78 <col:20, col:37> col:32 hidden f 'void (void)' inline | `-CompoundStmt 0x23515b2aea8 <col:36, col:37> but FunctionDecl 0x23515b2adb8 should have been in (local) x.b same as the FunctionTemplateDecl 0x23515b2ae58. Richard, what do you think?