Handle simple friend-class decls in class templates better by ensuring that

we look for shadow friend decls in the appropriate scope before injecting
a new declaration.



git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@99552 91177308-0d34-0410-b5e6-96231b3b80d8

3 files changed, 113 insertions, 61 deletions

diff --git a/lib/Sema/Lookup.h b/lib/Sema/Lookup.h
index 6b2694591f..f310c253ab 100644
--- a/lib/Sema/Lookup.h
+++ b/lib/Sema/Lookup.h
@@ -397,6 +397,12 @@ public:
     configure();
   }
 
+  /// \brief Change this lookup's redeclaration kind.
+  void setRedeclarationKind(Sema::RedeclarationKind RK) {
+    Redecl = RK;
+    configure();
+  }
+
   void print(llvm::raw_ostream &);
 
   /// Suppress the diagnostics that would normally fire because of this
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 87b2f80886..d449951c98 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -4774,6 +4774,79 @@ Sema::DeclPtrTy Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
     }
   }
 
+  // If we didn't find a previous declaration, and this is a reference
+  // (or friend reference), move to the correct scope.  In C++, we
+  // also need to do a redeclaration lookup there, just in case
+  // there's a shadow friend decl.
+  if (Name && Previous.empty() &&
+      (TUK == TUK_Reference || TUK == TUK_Friend)) {
+    if (Invalid) goto CreateNewDecl;
+    assert(SS.isEmpty());
+
+    if (TUK == TUK_Reference) {
+      // C++ [basic.scope.pdecl]p5:
+      //   -- for an elaborated-type-specifier of the form
+      //
+      //          class-key identifier
+      //
+      //      if the elaborated-type-specifier is used in the
+      //      decl-specifier-seq or parameter-declaration-clause of a
+      //      function defined in namespace scope, the identifier is
+      //      declared as a class-name in the namespace that contains
+      //      the declaration; otherwise, except as a friend
+      //      declaration, the identifier is declared in the smallest
+      //      non-class, non-function-prototype scope that contains the
+      //      declaration.
+      //
+      // C99 6.7.2.3p8 has a similar (but not identical!) provision for
+      // C structs and unions.
+      //
+      // It is an error in C++ to declare (rather than define) an enum
+      // type, including via an elaborated type specifier.  We'll
+      // diagnose that later; for now, declare the enum in the same
+      // scope as we would have picked for any other tag type.
+      //
+      // GNU C also supports this behavior as part of its incomplete
+      // enum types extension, while GNU C++ does not.
+      //
+      // Find the context where we'll be declaring the tag.
+      // FIXME: We would like to maintain the current DeclContext as the
+      // lexical context,
+      while (SearchDC->isRecord())
+        SearchDC = SearchDC->getParent();
+
+      // Find the scope where we'll be declaring the tag.
+      while (S->isClassScope() ||
+             (getLangOptions().CPlusPlus &&
+              S->isFunctionPrototypeScope()) ||
+             ((S->getFlags() & Scope::DeclScope) == 0) ||
+             (S->getEntity() &&
+              ((DeclContext *)S->getEntity())->isTransparentContext()))
+        S = S->getParent();
+    } else {
+      assert(TUK == TUK_Friend);
+      // C++ [namespace.memdef]p3:
+      //   If a friend declaration in a non-local class first declares a
+      //   class or function, the friend class or function is a member of
+      //   the innermost enclosing namespace.
+      SearchDC = SearchDC->getEnclosingNamespaceContext();
+
+      // Look up through our scopes until we find one with an entity which
+      // matches our declaration context.
+      while (S->getEntity() &&
+             ((DeclContext *)S->getEntity())->getPrimaryContext() != SearchDC) {
+        S = S->getParent();
+        assert(S && "No enclosing scope matching the enclosing namespace.");
+      }
+    }
+
+    // In C++, look for a shadow friend decl.
+    if (getLangOptions().CPlusPlus) {
+      Previous.setRedeclarationKind(ForRedeclaration);
+      LookupQualifiedName(Previous, SearchDC);
+    }
+  }
+
   if (!Previous.empty()) {
     assert(Previous.isSingleResult());
     NamedDecl *PrevDecl = Previous.getFoundDecl();
@@ -4871,7 +4944,8 @@ Sema::DeclPtrTy Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
     } else {
       // PrevDecl is a namespace, template, or anything else
       // that lives in the IDNS_Tag identifier namespace.
-      if (isDeclInScope(PrevDecl, SearchDC, S)) {
+      if (TUK == TUK_Reference || TUK == TUK_Friend ||
+          isDeclInScope(PrevDecl, SearchDC, S)) {
         // The tag name clashes with a namespace name, issue an error and
         // recover by making this tag be anonymous.
         Diag(NameLoc, diag::err_redefinition_different_kind) << Name;
@@ -4885,60 +4959,6 @@ Sema::DeclPtrTy Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
         Previous.clear();
       }
     }
-  } else if (TUK == TUK_Reference && SS.isEmpty() && Name) {
-    // C++ [basic.scope.pdecl]p5:
-    //   -- for an elaborated-type-specifier of the form
-    //
-    //          class-key identifier
-    //
-    //      if the elaborated-type-specifier is used in the
-    //      decl-specifier-seq or parameter-declaration-clause of a
-    //      function defined in namespace scope, the identifier is
-    //      declared as a class-name in the namespace that contains
-    //      the declaration; otherwise, except as a friend
-    //      declaration, the identifier is declared in the smallest
-    //      non-class, non-function-prototype scope that contains the
-    //      declaration.
-    //
-    // C99 6.7.2.3p8 has a similar (but not identical!) provision for
-    // C structs and unions.
-    //
-    // It is an error in C++ to declare (rather than define) an enum
-    // type, including via an elaborated type specifier.  We'll
-    // diagnose that later; for now, declare the enum in the same
-    // scope as we would have picked for any other tag type.
-    //
-    // GNU C also supports this behavior as part of its incomplete
-    // enum types extension, while GNU C++ does not.
-    //
-    // Find the context where we'll be declaring the tag.
-    // FIXME: We would like to maintain the current DeclContext as the
-    // lexical context,
-    while (SearchDC->isRecord())
-      SearchDC = SearchDC->getParent();
-
-    // Find the scope where we'll be declaring the tag.
-    while (S->isClassScope() ||
-           (getLangOptions().CPlusPlus && S->isFunctionPrototypeScope()) ||
-           ((S->getFlags() & Scope::DeclScope) == 0) ||
-           (S->getEntity() &&
-            ((DeclContext *)S->getEntity())->isTransparentContext()))
-      S = S->getParent();
-
-  } else if (TUK == TUK_Friend && SS.isEmpty() && Name) {
-    // C++ [namespace.memdef]p3:
-    //   If a friend declaration in a non-local class first declares a
-    //   class or function, the friend class or function is a member of
-    //   the innermost enclosing namespace.
-    SearchDC = SearchDC->getEnclosingNamespaceContext();
-
-    // Look up through our scopes until we find one with an entity which
-    // matches our declaration context.
-    while (S->getEntity() &&
-           ((DeclContext *)S->getEntity())->getPrimaryContext() != SearchDC) {
-      S = S->getParent();
-      assert(S && "No enclosing scope matching the enclosing namespace.");
-    }
   }
 
 CreateNewDecl:
@@ -5055,7 +5075,7 @@ CreateNewDecl:
     New->setObjectOfFriendDecl(/* PreviouslyDeclared = */ !Previous.empty());
 
   // Set the access specifier.
-  if (!Invalid && TUK != TUK_Friend)
+  if (!Invalid && SearchDC->isRecord())
     SetMemberAccessSpecifier(New, PrevDecl, AS);
 
   if (TUK == TUK_Definition)
@@ -5068,13 +5088,11 @@ CreateNewDecl:
     if (PrevDecl)
       New->setAccess(PrevDecl->getAccess());
 
-    // Friend tag decls are visible in fairly strange ways.
-    if (!CurContext->isDependentContext()) {
-      DeclContext *DC = New->getDeclContext()->getLookupContext();
-      DC->makeDeclVisibleInContext(New, /* Recoverable = */ false);
+    DeclContext *DC = New->getDeclContext()->getLookupContext();
+    DC->makeDeclVisibleInContext(New, /* Recoverable = */ false);
+    if (Name) // can be null along some error paths
       if (Scope *EnclosingScope = getScopeForDeclContext(S, DC))
         PushOnScopeChains(New, EnclosingScope, /* AddToContext = */ false);
-    }
   } else if (Name) {
     S = getNonFieldDeclScope(S);
     PushOnScopeChains(New, S);
diff --git a/test/CXX/temp/temp.decls/temp.friend/p1.cpp b/test/CXX/temp/temp.decls/temp.friend/p1.cpp
index 9d5589646d..39818028bb 100644
--- a/test/CXX/temp/temp.decls/temp.friend/p1.cpp
+++ b/test/CXX/temp/temp.decls/temp.friend/p1.cpp
@@ -1,4 +1,6 @@
 // RUN: %clang_cc1 -faccess-control -verify -emit-llvm-only %s
+
+namespace test0 {
 template <typename T> struct Num {
   T value_;
 
@@ -53,6 +55,7 @@ int calc2() {
   Num<int> result = x * n;
   return result.get();
 }
+}
 
 // Reduced from GNU <locale>
 namespace test1 {
@@ -150,3 +153,28 @@ namespace Dependent {
     friend X operator+<>(const X&, const value_type*);
   };
 }
+
+namespace test7 {
+  template <class T> class A { // expected-note {{previous definition is here}}
+    friend class B;
+    int x; // expected-note {{declared private here}}
+  };
+
+  class B {
+    int foo(A<int> &a) {
+      return a.x;
+    }
+  };
+
+  class C {
+    int foo(A<int> &a) {
+      return a.x; // expected-error {{'x' is a private member of 'test7::A<int>'}}
+    }
+  };
+
+  // This shouldn't crash.
+  template <class T> class D {
+    friend class A; // expected-error {{redefinition of 'A' as different kind of symbol}}
+  };
+  template class D<int>;
+}