Handle dependent friends more explicitly and deal with the possibility

of templated-scope friends by marking them invalid and white-listing all
accesses until such time as we implement them.  Fixes a crash, this time
without a broken test case.



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

5 files changed, 128 insertions, 52 deletions

diff --git a/lib/Sema/SemaAccess.cpp b/lib/Sema/SemaAccess.cpp
index e629f0fd35..ea6481bd8a 100644
--- a/lib/Sema/SemaAccess.cpp
+++ b/lib/Sema/SemaAccess.cpp
@@ -516,6 +516,10 @@ static AccessResult MatchesFriend(Sema &S,
 static AccessResult MatchesFriend(Sema &S,
                                   const EffectiveContext &EC,
                                   FriendDecl *FriendD) {
+  // Whitelist accesses if there's an invalid friend declaration.
+  if (FriendD->isInvalidDecl())
+    return AR_accessible;
+
   if (TypeSourceInfo *T = FriendD->getFriendType())
     return MatchesFriend(S, EC, T->getType()->getCanonicalTypeUnqualified());
 
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 7f844056a3..cb7656865c 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -3471,7 +3471,9 @@ Sema::ActOnFunctionDeclarator(Scope* S, Declarator& D, DeclContext* DC,
 
   if (isFriend) {
     // DC is the namespace in which the function is being declared.
-    assert((DC->isFileContext() || !Previous.empty()) &&
+    assert((DC->isFileContext() || !Previous.empty() ||
+            (D.getCXXScopeSpec().isSet() &&
+             D.getCXXScopeSpec().getScopeRep()->isDependent())) &&
            "previously-undeclared friend function being created "
            "in a non-namespace context");
 
diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index ec02f75ecb..eb62b6a9db 100644
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -6285,10 +6285,8 @@ Decl *Sema::ActOnFriendTypeDecl(Scope *S, const DeclSpec &DS,
   return D;
 }
 
-Decl *Sema::ActOnFriendFunctionDecl(Scope *S,
-                                         Declarator &D,
-                                         bool IsDefinition,
-                              MultiTemplateParamsArg TemplateParams) {
+Decl *Sema::ActOnFriendFunctionDecl(Scope *S, Declarator &D, bool IsDefinition,
+                                    MultiTemplateParamsArg TemplateParams) {
   const DeclSpec &DS = D.getDeclSpec();
 
   assert(DS.isFriendSpecified());
@@ -6331,7 +6329,7 @@ Decl *Sema::ActOnFriendFunctionDecl(Scope *S,
   //    declared as a friend, scopes outside the innermost enclosing
   //    namespace scope are not considered.
 
-  CXXScopeSpec &ScopeQual = D.getCXXScopeSpec();
+  CXXScopeSpec &SS = D.getCXXScopeSpec();
   DeclarationNameInfo NameInfo = GetNameForDeclarator(D);
   DeclarationName Name = NameInfo.getName();
   assert(Name);
@@ -6339,47 +6337,18 @@ Decl *Sema::ActOnFriendFunctionDecl(Scope *S,
   // The context we found the declaration in, or in which we should
   // create the declaration.
   DeclContext *DC;
-
-  // FIXME: handle local classes
-
-  // Recover from invalid scope qualifiers as if they just weren't there.
   LookupResult Previous(*this, NameInfo, LookupOrdinaryName,
                         ForRedeclaration);
-  if (!ScopeQual.isInvalid() && ScopeQual.isSet()) {
-    DC = computeDeclContext(ScopeQual);
-
-    // FIXME: handle dependent contexts
-    if (!DC) return 0;
-    if (RequireCompleteDeclContext(ScopeQual, DC)) return 0;
-
-    LookupQualifiedName(Previous, DC);
-
-    // Ignore things found implicitly in the wrong scope.
-    // TODO: better diagnostics for this case.  Suggesting the right
-    // qualified scope would be nice...
-    LookupResult::Filter F = Previous.makeFilter();
-    while (F.hasNext()) {
-      NamedDecl *D = F.next();
-      if (!DC->InEnclosingNamespaceSetOf(
-              D->getDeclContext()->getRedeclContext()))
-        F.erase();
-    }
-    F.done();
-
-    if (Previous.empty()) {
-      D.setInvalidType();
-      Diag(Loc, diag::err_qualified_friend_not_found) << Name << T;
-      return 0;
-    }
 
-    // C++ [class.friend]p1: A friend of a class is a function or
-    //   class that is not a member of the class . . .
-    if (DC->Equals(CurContext))
-      Diag(DS.getFriendSpecLoc(), diag::err_friend_is_member);
+  // FIXME: there are different rules in local classes
 
-  // Otherwise walk out to the nearest namespace scope looking for matches.
-  } else {
-    // TODO: handle local class contexts.
+  // There are four cases here.
+  //   - There's no scope specifier, in which case we just go to the
+  //     appropriate namespace and create a function or function template
+  //     there as appropriate.
+  // Recover from invalid scope qualifiers as if they just weren't there.
+  if (SS.isInvalid() || !SS.isSet()) {
+    // Walk out to the nearest namespace scope looking for matches.
 
     DC = CurContext;
     while (true) {
@@ -6411,6 +6380,49 @@ Decl *Sema::ActOnFriendFunctionDecl(Scope *S,
     if (!Previous.empty() && DC->Equals(CurContext)
         && !getLangOptions().CPlusPlus0x)
       Diag(DS.getFriendSpecLoc(), diag::err_friend_is_member);
+
+  //   - There's a non-dependent scope specifier, in which case we
+  //     compute it and do a previous lookup there for a function
+  //     or function template.
+  } else if (!SS.getScopeRep()->isDependent()) {
+    DC = computeDeclContext(SS);
+    if (!DC) return 0;
+
+    if (RequireCompleteDeclContext(SS, DC)) return 0;
+
+    LookupQualifiedName(Previous, DC);
+
+    // Ignore things found implicitly in the wrong scope.
+    // TODO: better diagnostics for this case.  Suggesting the right
+    // qualified scope would be nice...
+    LookupResult::Filter F = Previous.makeFilter();
+    while (F.hasNext()) {
+      NamedDecl *D = F.next();
+      if (!DC->InEnclosingNamespaceSetOf(
+              D->getDeclContext()->getRedeclContext()))
+        F.erase();
+    }
+    F.done();
+
+    if (Previous.empty()) {
+      D.setInvalidType();
+      Diag(Loc, diag::err_qualified_friend_not_found) << Name << T;
+      return 0;
+    }
+
+    // C++ [class.friend]p1: A friend of a class is a function or
+    //   class that is not a member of the class . . .
+    if (DC->Equals(CurContext))
+      Diag(DS.getFriendSpecLoc(), diag::err_friend_is_member);
+
+  //   - There's a scope specifier that does not match any template
+  //     parameter lists, in which case we use some arbitrary context,
+  //     create a method or method template, and wait for instantiation.
+  //   - There's a scope specifier that does match some template
+  //     parameter lists, which we don't handle right now.
+  } else {
+    DC = CurContext;
+    assert(isa<CXXRecordDecl>(DC) && "friend declaration not in class?");
   }
 
   if (DC->isFileContext()) {
@@ -6454,6 +6466,9 @@ Decl *Sema::ActOnFriendFunctionDecl(Scope *S,
   FrD->setAccess(AS_public);
   CurContext->addDecl(FrD);
 
+  if (ND->isInvalidDecl())
+    FrD->setInvalidDecl();
+
   return ND;
 }
 
diff --git a/lib/Sema/SemaTemplate.cpp b/lib/Sema/SemaTemplate.cpp
index cceb9677b9..ec928ea4b7 100644
--- a/lib/Sema/SemaTemplate.cpp
+++ b/lib/Sema/SemaTemplate.cpp
@@ -1374,8 +1374,13 @@ Sema::MatchTemplateParametersToScopeSpecifier(SourceLocation DeclStartLoc,
   // If there were at least as many template-ids as there were template
   // parameter lists, then there are no template parameter lists remaining for
   // the declaration itself.
-  if (Idx >= NumParamLists)
+  if (Idx >= NumParamLists) {
+    // Silently drop template member friend declarations.
+    // TODO: implement these
+    if (IsFriend && NumParamLists) Invalid = true;
+
     return 0;
+  }
 
   // If there were too many template parameter lists, complain about that now.
   if (Idx != NumParamLists - 1) {
@@ -1404,6 +1409,11 @@ Sema::MatchTemplateParametersToScopeSpecifier(SourceLocation DeclStartLoc,
     }
   }
 
+  // Silently drop template member template friend declarations.
+  // TODO: implement these
+  if (IsFriend && NumParamLists > 1)
+    Invalid = true;
+
   // Return the last template parameter list, which corresponds to the
   // entity being declared.
   return ParamLists[NumParamLists - 1];
diff --git a/test/CXX/temp/temp.decls/temp.friend/p5.cpp b/test/CXX/temp/temp.decls/temp.friend/p5.cpp
index f23611bd50..82c2b3169d 100644
--- a/test/CXX/temp/temp.decls/temp.friend/p5.cpp
+++ b/test/CXX/temp/temp.decls/temp.friend/p5.cpp
@@ -1,13 +1,58 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
 
-template <class T> class A {
-  class Member {
+namespace test0 {
+  template <class T> class A {
+    class Member {};
   };
-};
 
-class B {
-  template <class T> friend class A<T>::Member;
-};
+  class B {
+    template <class T> friend class A<T>::Member;
+  };
+
+  A<int> a;
+  B b;
+}
+
+// rdar://problem/8204127
+namespace test1 {
+  template <class T> struct A;
+
+  class C {
+    static void foo();
+    template <class T> friend void A<T>::f();
+  };
 
-A<int> a;
-B b;
+  template <class T> struct A {
+    void f() { C::foo(); }
+  };
+
+  template <class T> struct A<T*> {
+    void f() { C::foo(); }
+  };
+
+  template <> struct A<char> {
+    void f() { C::foo(); }
+  };
+}
+
+// FIXME: these should fail!
+namespace test2 {
+  template <class T> struct A;
+
+  class C {
+    static void foo();
+    template <class T> friend void A<T>::g();
+  };
+
+  template <class T> struct A {
+    void f() { C::foo(); }
+  };
+
+  template <class T> struct A<T*> {
+    void f() { C::foo(); }
+  };
+
+  template <> struct A<char> {
+    void f() { C::foo(); }
+  };
+}