Re-revert r86040, which was un-reverted in r186199.

This breaks the build of basic patterns with repeated friend
declarations. See the added test case in SemaCXX/friend.cpp or the test
case reported to the original commit log.

Original commit log:
  If we friend a declaration twice, that should not make it visible to
  name lookup in the surrounding context. Slightly rework how we handle
  friend declarations to inherit the visibility of the prior
  declaration, rather than setting a friend declaration to be visible
  whenever there was a prior declaration.

llvm-svn: 186331

clang/include/clang/AST/Decl.h

@@ -3415,12 +3415,6 @@ void Redeclarable<decl_type>::setPreviousDeclaration(decl_type *PrevDecl) {
   First->RedeclLink = LatestDeclLink(static_cast<decl_type*>(this));
   assert(!isa<NamedDecl>(static_cast<decl_type*>(this)) ||
          cast<NamedDecl>(static_cast<decl_type*>(this))->isLinkageValid());
-
-  // If the declaration was previously visible, a redeclaration of it remains
-  // visible even if it wouldn't be visible by itself.
-  static_cast<decl_type*>(this)->IdentifierNamespace |=
-    First->getIdentifierNamespace() &
-    (Decl::IDNS_Ordinary | Decl::IDNS_Tag | Decl::IDNS_Type);
 }
 
 // Inline function definitions.

clang/include/clang/AST/DeclBase.h

@@ -295,8 +295,6 @@ protected:
   friend class ASTReader;
   friend class LinkageComputer;
 
-  template<typename decl_type> friend class Redeclarable;
-
 private:
   void CheckAccessDeclContext() const;
 
@@ -826,7 +824,7 @@ public:
   /// class, but in the semantic context of the actual entity.  This property
   /// applies only to a specific decl object;  other redeclarations of the
   /// same entity may not (and probably don't) share this property.
-  void setObjectOfFriendDecl(bool PerformFriendInjection = false) {
+  void setObjectOfFriendDecl(bool PreviouslyDeclared) {
     unsigned OldNS = IdentifierNamespace;
     assert((OldNS & (IDNS_Tag | IDNS_Ordinary |
                      IDNS_TagFriend | IDNS_OrdinaryFriend)) &&
@@ -835,20 +833,15 @@ public:
                        IDNS_TagFriend | IDNS_OrdinaryFriend)) &&
            "namespace includes other than ordinary or tag");
 
-    Decl *Prev = getPreviousDecl();
     IdentifierNamespace = 0;
     if (OldNS & (IDNS_Tag | IDNS_TagFriend)) {
       IdentifierNamespace |= IDNS_TagFriend;
-      if (PerformFriendInjection || 
-          (Prev && Prev->getIdentifierNamespace() & IDNS_Tag))
-        IdentifierNamespace |= IDNS_Tag | IDNS_Type;
+      if (PreviouslyDeclared) IdentifierNamespace |= IDNS_Tag | IDNS_Type;
     }
 
     if (OldNS & (IDNS_Ordinary | IDNS_OrdinaryFriend)) {
       IdentifierNamespace |= IDNS_OrdinaryFriend;
-      if (PerformFriendInjection ||
-          (Prev && Prev->getIdentifierNamespace() & IDNS_Ordinary))
-        IdentifierNamespace |= IDNS_Ordinary;
+      if (PreviouslyDeclared) IdentifierNamespace |= IDNS_Ordinary;
     }
   }
 

clang/lib/Sema/SemaDecl.cpp

@@ -6328,11 +6328,12 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
     }
 
     if (isFriend) {
+      // For now, claim that the objects have no previous declaration.
       if (FunctionTemplate) {
-        FunctionTemplate->setObjectOfFriendDecl();
+        FunctionTemplate->setObjectOfFriendDecl(false);
         FunctionTemplate->setAccess(AS_public);
       }
-      NewFD->setObjectOfFriendDecl();
+      NewFD->setObjectOfFriendDecl(false);
       NewFD->setAccess(AS_public);
     }
 
@@ -6651,6 +6652,8 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
 
       NewFD->setAccess(Access);
       if (FunctionTemplate) FunctionTemplate->setAccess(Access);
+
+      PrincipalDecl->setObjectOfFriendDecl(true);
     }
 
     if (NewFD->isOverloadedOperator() && !DC->isRecord() &&
@@ -10381,8 +10384,9 @@ CreateNewDecl:
   // declaration so we always pass true to setObjectOfFriendDecl to make
   // the tag name visible.
   if (TUK == TUK_Friend)
-    New->setObjectOfFriendDecl(!FriendSawTagOutsideEnclosingNamespace &&
-                               getLangOpts().MicrosoftExt);
+    New->setObjectOfFriendDecl(/* PreviouslyDeclared = */ !Previous.empty() ||
+                               (!FriendSawTagOutsideEnclosingNamespace &&
+                                getLangOpts().MicrosoftExt));
 
   // Set the access specifier.
   if (!Invalid && SearchDC->isRecord())

clang/lib/Sema/SemaLookup.cpp

@@ -2757,15 +2757,8 @@ void Sema::ArgumentDependentLookup(DeclarationName Name, bool Operator,
       // If the only declaration here is an ordinary friend, consider
       // it only if it was declared in an associated classes.
       if (D->getIdentifierNamespace() == Decl::IDNS_OrdinaryFriend) {
-        bool DeclaredInAssociatedClass = false;
-        for (Decl *DI = D; DI; DI = DI->getPreviousDecl()) {
-          DeclContext *LexDC = DI->getLexicalDeclContext();
-          if (AssociatedClasses.count(cast<CXXRecordDecl>(LexDC))) {
-            DeclaredInAssociatedClass = true;
-            break;
-          }
-        }
-        if (!DeclaredInAssociatedClass)
+        DeclContext *LexDC = D->getLexicalDeclContext();
+        if (!AssociatedClasses.count(cast<CXXRecordDecl>(LexDC)))
           continue;
       }
 

clang/lib/Sema/SemaTemplate.cpp

@@ -1120,7 +1120,8 @@ Sema::CheckClassTemplate(Scope *S, unsigned TagSpec, TagUseKind TUK,
       NewClass->setAccess(PrevClassTemplate->getAccess());
     }
 
-    NewTemplate->setObjectOfFriendDecl();
+    NewTemplate->setObjectOfFriendDecl(/* PreviouslyDeclared = */
+                                       PrevClassTemplate != NULL);
 
     // Friend templates are visible in fairly strange ways.
     if (!CurContext->isDependentContext()) {

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

@@ -960,7 +960,7 @@ Decl *TemplateDeclInstantiator::VisitClassTemplateDecl(ClassTemplateDecl *D) {
     else
       Inst->setAccess(D->getAccess());
 
-    Inst->setObjectOfFriendDecl();
+    Inst->setObjectOfFriendDecl(PrevClassTemplate != 0);
     // TODO: do we want to track the instantiation progeny of this
     // friend target decl?
   } else {
@@ -1110,8 +1110,8 @@ Decl *TemplateDeclInstantiator::VisitCXXRecordDecl(CXXRecordDecl *D) {
 
   // If the original function was part of a friend declaration,
   // inherit its namespace state.
-  if (D->getFriendObjectKind())
-    Record->setObjectOfFriendDecl();
+  if (Decl::FriendObjectKind FOK = D->getFriendObjectKind())
+    Record->setObjectOfFriendDecl(FOK == Decl::FOK_Declared);
 
   // Make sure that anonymous structs and unions are recorded.
   if (D->isAnonymousStructOrUnion()) {
@@ -1315,7 +1315,7 @@ Decl *TemplateDeclInstantiator::VisitFunctionDecl(FunctionDecl *D,
     assert(isFriend && "non-friend has dependent specialization info?");
 
     // This needs to be set now for future sanity.
-    Function->setObjectOfFriendDecl();
+    Function->setObjectOfFriendDecl(/*HasPrevious*/ true);
 
     // Instantiate the explicit template arguments.
     TemplateArgumentListInfo ExplicitArgs(Info->getLAngleLoc(),
@@ -1365,7 +1365,13 @@ Decl *TemplateDeclInstantiator::VisitFunctionDecl(FunctionDecl *D,
   // If the original function was part of a friend declaration,
   // inherit its namespace state and add it to the owner.
   if (isFriend) {
-    PrincipalDecl->setObjectOfFriendDecl();
+    NamedDecl *PrevDecl;
+    if (TemplateParams)
+      PrevDecl = FunctionTemplate->getPreviousDecl();
+    else
+      PrevDecl = Function->getPreviousDecl();
+
+    PrincipalDecl->setObjectOfFriendDecl(PrevDecl != 0);
     DC->makeDeclVisibleInContext(PrincipalDecl);
 
     bool queuedInstantiation = false;
@@ -1633,7 +1639,7 @@ TemplateDeclInstantiator::VisitCXXMethodDecl(CXXMethodDecl *D,
                                                     TemplateParams, Method);
     if (isFriend) {
       FunctionTemplate->setLexicalDeclContext(Owner);
-      FunctionTemplate->setObjectOfFriendDecl();
+      FunctionTemplate->setObjectOfFriendDecl(true);
     } else if (D->isOutOfLine())
       FunctionTemplate->setLexicalDeclContext(D->getLexicalDeclContext());
     Method->setDescribedFunctionTemplate(FunctionTemplate);
@@ -1660,7 +1666,7 @@ TemplateDeclInstantiator::VisitCXXMethodDecl(CXXMethodDecl *D,
                                             TempParamLists.data());
 
     Method->setLexicalDeclContext(Owner);
-    Method->setObjectOfFriendDecl();
+    Method->setObjectOfFriendDecl(true);
   } else if (D->isOutOfLine())
     Method->setLexicalDeclContext(D->getLexicalDeclContext());
 

clang/test/CXX/temp/temp.decls/temp.friend/p4.cpp

@@ -26,20 +26,3 @@ void g() {
   X2<float> xf; 
   f(xf);
 }
-
-template<typename T>
-struct X3 {
-  operator int();
-
-  friend void h(int x);
-};
-
-int array2[sizeof(X3<int>)]; 
-int array3[sizeof(X3<float>)];
-
-void i() {
-  X3<int> xi;
-  h(xi);
-  X3<float> xf; 
-  h(xf);
-}

clang/test/SemaCXX/friend.cpp

@@ -166,24 +166,13 @@ namespace test9 {
 
 namespace test10 {
   struct A {
-    friend void f10();
+    friend void f();
   };
+  extern void f();
   struct B {
-    friend void f10();
+    friend void f();
   };
   void g() {
-    f10(); // expected-error {{undeclared identifier}}
+    ::test10::f();
   }
 }
-
-namespace PR16597 {
-  struct A {
-    friend void f_16597();
-  };
-  struct B {
-    friend void f_16597();
-  };
-  struct C {
-  };
-  void g(C a) { f_16597(a); } // expected-error {{undeclared identifier}}
-}