Prefer non-friend to friend in in redeclaration chain

[yaron.keren]

Summary

Address http://bugs.llvm.org/pr30994 so that a non-friend can properly replace a friend, and a visible friend can properly replace an invisible friend but not vice verse. This also fixes two FIXME in SemaTemplate/friend-template.cpp.

[v.g.vassilev]

Could you also add the testcase where the friend declaration has some attributes?

[yaron.keren]

Thanks, I'll make a test.

[yaron.keren]

Added attribute test.

[v.g.vassilev]

LGTM!

[yaron.keren]

reverted in r300497

tools/clang/include/clang/AST/DeclBase.h

const Decl *getNonClosureContext() const {
     return const_cast<Decl*>(this)->getNonClosureContext();
   }
 
   TranslationUnitDecl *getTranslationUnitDecl();
   const TranslationUnitDecl *getTranslationUnitDecl() const {
     return const_cast<Decl*>(this)->getTranslationUnitDecl();
   }
 
+  bool isThisDeclarationADefinition() const;
+
   bool isInAnonymousNamespace() const;
 
   bool isInStdNamespace() const;
 
   ASTContext &getASTContext() const LLVM_READONLY;
 
   void setAccess(AccessSpecifier AS) {
     Access = AS;

tools/clang/lib/AST/Decl.cpp

bool NamedDecl::declarationReplaces(NamedDecl *OldD, bool IsKnownNewer) const {
   // A kind mismatch implies that the declaration is not replaced.
   if (OldD->getKind() != getKind())
     return false;
 
   // For method declarations, we never replace. (Why?)
   if (isa<ObjCMethodDecl>(this))
     return false;
 
+  if (getFriendObjectKind() > OldD->getFriendObjectKind() &&
+      !isThisDeclarationADefinition())
+    return false;
+
   // For parameters, pick the newer one. This is either an error or (in
   // Objective-C) permitted as an extension.
   if (isa<ParmVarDecl>(this))
     return true;
 
   // Inline namespaces can give us two declarations with the same
   // name and kind in the same scope but different contexts; we should
   // keep both declarations in this case.

tools/clang/lib/AST/DeclBase.cpp

const FunctionType *Decl::getFunctionType(bool BlocksToo) const {
   if (Ty->isFunctionPointerType())
     Ty = Ty->getAs<PointerType>()->getPointeeType();
   else if (BlocksToo && Ty->isBlockPointerType())
     Ty = Ty->getAs<BlockPointerType>()->getPointeeType();
 
   return Ty->getAs<FunctionType>();
 }
 
+bool Decl::isThisDeclarationADefinition() const {
+  if (auto *TD = dyn_cast<TagDecl>(this))
+    return TD->isThisDeclarationADefinition();
+  if (auto *FD = dyn_cast<FunctionDecl>(this))
+    return FD->isThisDeclarationADefinition();
+  if (auto *VD = dyn_cast<VarDecl>(this))
+    return VD->isThisDeclarationADefinition();
+  if (auto *CTD = dyn_cast<ClassTemplateDecl>(this))
+    return CTD->isThisDeclarationADefinition();
+  if (auto *FTD = dyn_cast<FunctionTemplateDecl>(this))
+    return FTD->isThisDeclarationADefinition();
+  if (auto *VTD = dyn_cast<VarTemplateDecl>(this))
+    return VTD->isThisDeclarationADefinition();
+  return false;
+}
 
 /// Starting at a given context (a Decl or DeclContext), look for a
 /// code context that is not a closure (a lambda, block, etc.).
 template <class T> static Decl *getNonClosureContext(T *D) {
   if (getKind(D) == Decl::CXXMethod) {
     CXXMethodDecl *MD = cast<CXXMethodDecl>(D);
     if (MD->getOverloadedOperator() == OO_Call &&
         MD->getParent()->isLambda())

tools/clang/test/SemaTemplate/friend-template.cpp

namespace test3 {
   template <typename T> class Foo {
     template <typename U>
     friend class Foo;
   };
 
   Foo<int> foo;
 
   template<typename T, T Value> struct X2a;
-
-  template<typename T, int Size> struct X2b;
+  template<typename T, int Size> struct X2b;    // expected-note {{previous non-type template parameter with type 'int' is here}}
 
   template<typename T>
   class X3 {
     template<typename U, U Value> friend struct X2a;
-
-    // FIXME: the redeclaration note ends up here because redeclaration
-    // lookup ends up finding the friend target from X3<int>.
-    template<typename U, T Value> friend struct X2b; // expected-error {{template non-type parameter has a different type 'long' in template redeclaration}} \
-      // expected-note {{previous non-type template parameter with type 'int' is here}}
+    template<typename U, T Value> friend struct X2b; // expected-error {{template non-type parameter has a different type 'long' in template redeclaration}} 
   };
 
   X3<int> x3i; // okay
 
   X3<long> x3l; // expected-note {{in instantiation}}
 }
 
 // PR5716
namespace PR12585 {
   template<typename T> struct C::D {
     int f() {
       return E<int>().n;
     }
   };
   int n = C::D<void*>().f();
 
   struct F {
-    template<int> struct G;
+    template<int> struct G; // expected-note {{previous}}
   };
   template<typename T> struct H {
-    // FIXME: As with cases above, the note here is on an unhelpful declaration,
-    // and should point to the declaration of G within F.
     template<T> friend struct F::G; // \
-      // expected-error {{different type 'char' in template redeclaration}} \
-      // expected-note {{previous}}
+      // expected-error {{different type 'char' in template redeclaration}}
   };
   H<int> h1; // ok
   H<char> h2; // expected-note {{instantiation}}
 }
 
 // Ensure that we can still instantiate a friend function template
 // after the friend declaration is instantiated during the delayed
 // parsing of a member function, but before the friend function has