DR674, PR38883, PR40238: Qualified friend lookup should look for a

template specialization if there is no matching non-template function.

This exposed a couple of related bugs:
 - we would sometimes substitute into a friend template instead of a
   suitable non-friend declaration; this would now crash because we'd
   decide the specialization of the friend is a redeclaration of itself
 - ADL failed to properly handle the case where an invisible local
   extern declaration redeclares an invisible friend

Both are fixed herein: in particular, we now never make invisible
friends or local extern declarations visible to name lookup unless
they are the only declaration of the entity. (We already mostly did
this for local extern declarations.)

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

include/clang/Basic/DiagnosticSemaKinds.td

@@ -1298,8 +1298,8 @@ def ext_unelaborated_friend_type : ExtWarn<
 def warn_cxx98_compat_unelaborated_friend_type : Warning<
   "befriending %1 without '%select{struct|interface|union|class|enum}0' "
   "keyword is incompatible with C++98">, InGroup<CXX98Compat>, DefaultIgnore;
-def err_qualified_friend_not_found : Error<
-  "no function named %0 with type %1 was found in the specified scope">;
+def err_qualified_friend_no_match : Error<
+  "friend declaration of %0 does not match any declaration in %1">;
 def err_introducing_special_friend : Error<
   "%plural{[0,2]:must use a qualified name when declaring|3:cannot declare}0"
   " a %select{constructor|destructor|conversion operator|deduction guide}0 "

include/clang/Sema/Sema.h

@@ -6356,9 +6356,9 @@ public:
                     const TemplateArgumentListInfo &ExplicitTemplateArgs,
                                                     LookupResult &Previous);
 
-  bool CheckFunctionTemplateSpecialization(FunctionDecl *FD,
-                         TemplateArgumentListInfo *ExplicitTemplateArgs,
-                                           LookupResult &Previous);
+  bool CheckFunctionTemplateSpecialization(
+      FunctionDecl *FD, TemplateArgumentListInfo *ExplicitTemplateArgs,
+      LookupResult &Previous, bool QualifiedFriend = false);
   bool CheckMemberSpecialization(NamedDecl *Member, LookupResult &Previous);
   void CompleteMemberSpecialization(NamedDecl *Member, LookupResult &Previous);
 

lib/AST/DeclBase.cpp

@@ -1405,6 +1405,12 @@ static bool shouldBeHidden(NamedDecl *D) {
       D->isTemplateParameter())
     return true;
 
+  // Skip friends and local extern declarations unless they're the first
+  // declaration of the entity.
+  if ((D->isLocalExternDecl() || D->getFriendObjectKind()) &&
+      D != D->getCanonicalDecl())
+    return true;
+
   // Skip template specializations.
   // FIXME: This feels like a hack. Should DeclarationName support
   // template-ids, or is there a better way to keep specializations

lib/Sema/SemaDecl.cpp

@@ -5518,15 +5518,8 @@ NamedDecl *Sema::HandleDeclarator(Scope *S, Declarator &D,
 
   // If this has an identifier and is not a function template specialization,
   // add it to the scope stack.
-  if (New->getDeclName() && AddToScope) {
-    // Only make a locally-scoped extern declaration visible if it is the first
-    // declaration of this entity. Qualified lookup for such an entity should
-    // only find this declaration if there is no visible declaration of it.
-    bool AddToContext = !D.isRedeclaration() || !New->isLocalExternDecl();
-    PushOnScopeChains(New, S, AddToContext);
-    if (!AddToContext)
-      CurContext->addHiddenDecl(New);
-  }
+  if (New->getDeclName() && AddToScope)
+    PushOnScopeChains(New, S);
 
   if (isInOpenMPDeclareTargetContext())
     checkDeclIsAllowedInOpenMPTarget(nullptr, New);
@@ -7728,8 +7721,10 @@ static NamedDecl *DiagnoseInvalidRedeclaration(
   SmallVector<std::pair<FunctionDecl *, unsigned>, 1> NearMatches;
   TypoCorrection Correction;
   bool IsDefinition = ExtraArgs.D.isFunctionDefinition();
-  unsigned DiagMsg = IsLocalFriend ? diag::err_no_matching_local_friend
-                                   : diag::err_member_decl_does_not_match;
+  unsigned DiagMsg =
+    IsLocalFriend ? diag::err_no_matching_local_friend :
+    NewFD->getFriendObjectKind() ? diag::err_qualified_friend_no_match :
+    diag::err_member_decl_does_not_match;
   LookupResult Prev(SemaRef, Name, NewFD->getLocation(),
                     IsLocalFriend ? Sema::LookupLocalFriendName
                                   : Sema::LookupOrdinaryName,

lib/Sema/SemaDeclCXX.cpp

@@ -14414,25 +14414,6 @@ NamedDecl *Sema::ActOnFriendFunctionDecl(Scope *S, Declarator &D,
 
     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 << TInfo->getType();
-      return nullptr;
-    }
-
     // 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))
@@ -14446,6 +14427,10 @@ NamedDecl *Sema::ActOnFriendFunctionDecl(Scope *S, Declarator &D,
       //   A function can be defined in a friend declaration of a class if and
       //   only if the class is a non-local class (9.8), the function name is
       //   unqualified, and the function has namespace scope.
+      //
+      // FIXME: We should only do this if the scope specifier names the
+      // innermost enclosing namespace; otherwise the fixit changes the
+      // meaning of the code.
       SemaDiagnosticBuilder DB
         = Diag(SS.getRange().getBegin(), diag::err_qualified_friend_def);
 

lib/Sema/SemaLookup.cpp

@@ -3337,38 +3337,29 @@ void Sema::ArgumentDependentLookup(DeclarationName Name, SourceLocation Loc,
           !isa<FunctionTemplateDecl>(Underlying))
         continue;
 
-      if (!isVisible(D)) {
-        D = findAcceptableDecl(
-            *this, D, (Decl::IDNS_Ordinary | Decl::IDNS_OrdinaryFriend));
-        if (!D)
-          continue;
-        if (auto *USD = dyn_cast<UsingShadowDecl>(D))
-          Underlying = USD->getTargetDecl();
-      }
-
-      // 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_Ordinary) == 0) {
-        // If it's neither ordinarily visible nor a friend, we can't find it.
-        if ((D->getIdentifierNamespace() & Decl::IDNS_OrdinaryFriend) == 0)
-          continue;
-
-        bool DeclaredInAssociatedClass = false;
-        for (Decl *DI = D; DI; DI = DI->getPreviousDecl()) {
-          DeclContext *LexDC = DI->getLexicalDeclContext();
-          if (isa<CXXRecordDecl>(LexDC) &&
-              AssociatedClasses.count(cast<CXXRecordDecl>(LexDC)) &&
-              isVisible(cast<NamedDecl>(DI))) {
-            DeclaredInAssociatedClass = true;
+      // The declaration is visible to argument-dependent lookup if either
+      // it's ordinarily visible or declared as a friend in an associated
+      // class.
+      bool Visible = false;
+      for (D = D->getMostRecentDecl(); D;
+           D = cast_or_null<NamedDecl>(D->getPreviousDecl())) {
+        if (D->getIdentifierNamespace() & Decl::IDNS_Ordinary) {
+          if (isVisible(D)) {
+            Visible = true;
+            break;
+          }
+        } else if (D->getFriendObjectKind()) {
+          auto *RD = cast<CXXRecordDecl>(D->getLexicalDeclContext());
+          if (AssociatedClasses.count(RD) && isVisible(D)) {
+            Visible = true;
             break;
           }
         }
-        if (!DeclaredInAssociatedClass)
-          continue;
       }
 
       // FIXME: Preserve D as the FoundDecl.
-      Result.insert(Underlying);
+      if (Visible)
+        Result.insert(Underlying);
     }
   }
 }

lib/Sema/SemaOverload.cpp

@@ -1041,6 +1041,35 @@ Sema::CheckOverload(Scope *S, FunctionDecl *New, const LookupResult &Old,
     }
   }
 
+  // C++ [temp.friend]p1:
+  //   For a friend function declaration that is not a template declaration:
+  //    -- if the name of the friend is a qualified or unqualified template-id,
+  //       [...], otherwise
+  //    -- if the name of the friend is a qualified-id and a matching
+  //       non-template function is found in the specified class or namespace,
+  //       the friend declaration refers to that function, otherwise,
+  //    -- if the name of the friend is a qualified-id and a matching function
+  //       template is found in the specified class or namespace, the friend
+  //       declaration refers to the deduced specialization of that function
+  //       template, otherwise
+  //    -- the name shall be an unqualified-id [...]
+  // If we get here for a qualified friend declaration, we've just reached the
+  // third bullet. If the type of the friend is dependent, skip this lookup
+  // until instantiation.
+  if (New->getFriendObjectKind() && New->getQualifier() &&
+      !New->getType()->isDependentType()) {
+    LookupResult TemplateSpecResult(LookupResult::Temporary, Old);
+    TemplateSpecResult.addAllDecls(Old);
+    if (CheckFunctionTemplateSpecialization(New, nullptr, TemplateSpecResult,
+                                            /*QualifiedFriend*/true)) {
+      New->setInvalidDecl();
+      return Ovl_Overload;
+    }
+
+    Match = TemplateSpecResult.getAsSingle<FunctionDecl>();
+    return Ovl_Match;
+  }
+
   return Ovl_Overload;
 }
 

lib/Sema/SemaTemplate.cpp

@@ -8104,9 +8104,13 @@ Sema::CheckDependentFunctionTemplateSpecialization(FunctionDecl *FD,
 ///
 /// \param Previous the set of declarations that may be specialized by
 /// this function specialization.
+///
+/// \param QualifiedFriend whether this is a lookup for a qualified friend
+/// declaration with no explicit template argument list that might be
+/// befriending a function template specialization.
 bool Sema::CheckFunctionTemplateSpecialization(
     FunctionDecl *FD, TemplateArgumentListInfo *ExplicitTemplateArgs,
-    LookupResult &Previous) {
+    LookupResult &Previous, bool QualifiedFriend) {
   // The set of function template specializations that could match this
   // explicit function template specialization.
   UnresolvedSet<8> Candidates;
@@ -8193,10 +8197,25 @@ bool Sema::CheckFunctionTemplateSpecialization(
     }
   }
 
+  // For a qualified friend declaration (with no explicit marker to indicate
+  // that a template specialization was intended), note all (template and
+  // non-template) candidates.
+  if (QualifiedFriend && Candidates.empty()) {
+    Diag(FD->getLocation(), diag::err_qualified_friend_no_match)
+        << FD->getDeclName() << FDLookupContext;
+    // FIXME: We should form a single candidate list and diagnose all
+    // candidates at once, to get proper sorting and limiting.
+    for (auto *OldND : Previous) {
+      if (auto *OldFD = dyn_cast<FunctionDecl>(OldND->getUnderlyingDecl()))
+        NoteOverloadCandidate(OldND, OldFD, FD->getType(), false);
+    }
+    FailedCandidates.NoteCandidates(*this, FD->getLocation());
+    return true;
+  }
+
   // Find the most specialized function template.
   UnresolvedSetIterator Result = getMostSpecialized(
-      Candidates.begin(), Candidates.end(), FailedCandidates,
-      FD->getLocation(),
+      Candidates.begin(), Candidates.end(), FailedCandidates, FD->getLocation(),
       PDiag(diag::err_function_template_spec_no_match) << FD->getDeclName(),
       PDiag(diag::err_function_template_spec_ambiguous)
           << FD->getDeclName() << (ExplicitTemplateArgs != nullptr),

test/CXX/class.access/class.friend/p1.cpp

@@ -11,12 +11,11 @@
 //   friends members of the befriending class.
 
 struct S { static void f(); }; // expected-note 2 {{'S' declared here}}
-S* g() { return 0; }
+S* g() { return 0; } // expected-note 2 {{'g' declared here}}
 
 struct X {
   friend struct S;
-  friend S* g(); // expected-note 2 {{'g' declared here}}
-  // FIXME: The above two notes would be better attached to line 11.
+  friend S* g();
 };
 
 void test1() {

test/CXX/class.access/class.friend/p11.cpp

@@ -19,17 +19,16 @@ namespace test1 {
 }
 
 namespace test2 {
-  void bar(); // expected-note {{'::test2::bar' declared here}}
+  void bar(); // expected-note 3{{'::test2::bar' declared here}}
 
-  void foo() { // expected-note {{'::test2::foo' declared here}}
+  void foo() { // expected-note 2{{'::test2::foo' declared here}}
     struct S1 {
       friend void foo(); // expected-error {{no matching function 'foo' found in local scope; did you mean '::test2::foo'?}}
     };
 
     void foo(); // expected-note {{local declaration nearly matches}}
     struct S2 {
-      friend void foo(); // expected-note{{'::test2::foo' declared here}}
-      // TODO: the above note should go on line 24
+      friend void foo();
     };
 
     {
@@ -47,8 +46,6 @@ namespace test2 {
 
     struct S4 {
       friend void bar(); // expected-error {{no matching function 'bar' found in local scope; did you mean '::test2::bar'?}}
-      // expected-note@-1 {{'::test2::bar' declared here}}
-      // TODO: the above note should go on line 22
     };
 
     { void bar(); }
@@ -81,8 +78,6 @@ namespace test2 {
     struct S9 {
       struct Inner {
         friend void baz(); // expected-error {{no matching function 'baz' found in local scope; did you mean 'bar'?}}
-        // expected-note@-1 {{'::test2::bar' declared here}}
-        // TODO: the above note should go on line 22
       };
     };
 

test/CXX/class/class.friend/p1.cpp

@@ -51,9 +51,9 @@ class A {
   friend class A::AInner; // this is okay as an extension
   friend class AInner; // okay, refers to ::AInner
 
-  friend void Derived::missing_member(); // expected-error {{no function named 'missing_member' with type 'void ()' was found in the specified scope}}
+  friend void Derived::missing_member(); // expected-error {{friend declaration of 'missing_member' does not match any declaration in 'Derived'}}
 
-  friend void Derived::base_member(); // expected-error {{no function named 'base_member' with type 'void ()' was found in the specified scope}}
+  friend void Derived::base_member(); // expected-error {{friend declaration of 'base_member' does not match any declaration in 'Derived'}}
 
   friend int Base::typedeffed_member(); // okay: should look through typedef
 

test/CXX/drs/dr1xx.cpp

@@ -401,13 +401,12 @@ namespace dr136 { // dr136: 3.4
   extern "C" void k(int, int, int, int); // expected-note {{previous declaration is here}}
   namespace NSA {
   struct A {
-    friend void dr136::k(int, int, int, int = 0); // expected-error {{friend declaration specifying a default argument must be the only declaration}} \
-                                                  // expected-note {{previous declaration is here}}
+    friend void dr136::k(int, int, int, int = 0); // expected-error {{friend declaration specifying a default argument must be the only declaration}}
   };
   }
   namespace NSB {
   struct A {
-    friend void dr136::k(int, int, int = 0, int); // expected-error {{friend declaration specifying a default argument must be the only declaration}}
+    friend void dr136::k(int, int, int = 0, int); // expected-error {{missing default argument on parameter}}
   };
   }
   struct B {

test/CXX/drs/dr5xx.cpp

@@ -740,17 +740,17 @@ namespace dr573 { // dr573: no
 
 namespace dr574 { // dr574: yes
   struct A {
-    A &operator=(const A&) const; // expected-note {{does not match because it is const}}
+    A &operator=(const A&) const; // expected-note {{different qualifiers}}
   };
   struct B {
-    B &operator=(const B&) volatile; // expected-note {{nearly matches}}
+    B &operator=(const B&) volatile; // expected-note {{different qualifiers}}
   };
 #if __cplusplus >= 201103L
   struct C {
-    C &operator=(const C&) &; // expected-note {{not viable}} expected-note {{nearly matches}} expected-note {{here}}
+    C &operator=(const C&) &; // expected-note {{not viable}} expected-note {{candidate}} expected-note {{here}}
   };
   struct D {
-    D &operator=(const D&) &&; // expected-note {{not viable}} expected-note {{nearly matches}} expected-note {{here}}
+    D &operator=(const D&) &&; // expected-note {{not viable}} expected-note {{candidate}} expected-note {{here}}
   };
   void test(C c, D d) {
     c = c;

test/CXX/drs/dr6xx.cpp

@@ -839,7 +839,7 @@ namespace dr673 { // dr673: yes
   F *f; // expected-error {{unknown type name}}
 }
 
-namespace dr674 { // dr674: no
+namespace dr674 { // dr674: 8
   template<typename T> int f(T);
 
   int g(int);
@@ -849,22 +849,50 @@ namespace dr674 { // dr674: no
   template<typename T> int h(T);
 
   class X {
-    // FIXME: This should deduce dr674::f<int>.
-    friend int dr674::f(int); // expected-error {{does not match any}}
+    friend int dr674::f(int);
     friend int dr674::g(int);
     friend int dr674::h<>(int);
-    int n;
+    int n; // expected-note 2{{private}}
   };
 
   template<typename T> int f(T) { return X().n; }
   int g(int) { return X().n; }
-  template<typename T> int g(T) { return X().n; }
-  int h(int) { return X().n; }
+  template<typename T> int g(T) { return X().n; } // expected-error {{private}}
+  int h(int) { return X().n; } // expected-error {{private}}
   template<typename T> int h(T) { return X().n; }
 
   template int f(int);
-  template int g(int);
+  template int g(int); // expected-note {{in instantiation of}}
   template int h(int);
+
+
+  struct Y {
+    template<typename T> int f(T);
+
+    int g(int);
+    template<typename T> int g(T);
+
+    int h(int);
+    template<typename T> int h(T);
+  };
+
+  class Z {
+    friend int Y::f(int);
+    friend int Y::g(int);
+    friend int Y::h<>(int);
+    int n; // expected-note 2{{private}}
+  };
+
+  template<typename T> int Y::f(T) { return Z().n; }
+  int Y::g(int) { return Z().n; }
+  template<typename T> int Y::g(T) { return Z().n; } // expected-error {{private}}
+  int Y::h(int) { return Z().n; } // expected-error {{private}}
+  template<typename T> int Y::h(T) { return Z().n; }
+
+  // FIXME: Should the <> be required here?
+  template int Y::f<>(int);
+  template int Y::g<>(int); // expected-note {{in instantiation of}}
+  template int Y::h<>(int);
 }
 
 namespace dr675 { // dr675: dup 739

test/SemaCXX/cxx1y-deduced-return-type.cpp

@@ -565,7 +565,7 @@ namespace PR33222 {
     static auto f1();
     static auto f2();
 
-    template<typename T> static decltype(auto) g0(T x) { return x.n; } // FIXME (PR38883): expected-error {{private}}
+    template<typename T> static decltype(auto) g0(T x) { return x.n; }
     template<typename T> static decltype(auto) g1(T);
     template<typename T> static decltype(auto) g2(T);
   };
@@ -574,8 +574,6 @@ namespace PR33222 {
     friend auto f1();
     friend auto f2();
 
-    // FIXME (PR38883): This friend declaration doesn't actually work, because
-    // we fail to look up the named function properly during instantiation.
     friend decltype(auto) g0<>(A);
     template<typename T> friend decltype(auto) g1(T);
     template<typename T> friend decltype(auto) g2(T);
@@ -589,7 +587,7 @@ namespace PR33222 {
     template<typename T_> friend decltype(auto) X::g1(T_);
     template<typename T_> friend decltype(auto) X::g2(T_);
 
-    int n; // FIXME: expected-note {{here}}
+    int n;
   };
 
   auto f1() { return A<int>().n; }
@@ -600,7 +598,7 @@ namespace PR33222 {
 
   A<int> ai;
   int k1 = g0(ai);
-  int k2 = X::g0(ai); // FIXME: expected-note {{in instantiation of}}
+  int k2 = X::g0(ai);
 
   int k3 = g1(ai);
   int k4 = X::g1(ai);

test/SemaCXX/friend.cpp

@@ -162,7 +162,7 @@ namespace test9 {
   class C {
   };
   struct A {
-    friend void C::f(int, int, int) {}  // expected-error {{no function named 'f' with type 'void (int, int, int)' was found in the specified scope}}
+    friend void C::f(int, int, int) {}  // expected-error {{friend function definition cannot be qualified with 'C::'}}
   };
 }
 
@@ -230,6 +230,10 @@ namespace test10 {
     friend void f10_d(X);
   };
 
+  struct W {
+    friend void f10_d(W);
+  };
+
   void g(X x, Y y, Z z) {
     f10_d(); // expected-error {{undeclared identifier}}
     ::test10::f10_d(); // expected-error {{no member named 'f10_d'}}
@@ -245,14 +249,13 @@ namespace test10 {
     ::test10::f10_d(z); // expected-error {{no type named 'f10_d'}}
   }
 
-  void local_externs(X x, Y y) {
-    extern void f10_d();
-    extern void f10_d(X);
+  void local_externs(W w, X x, Y y) {
+    extern void f10_d(); // expected-note {{candidate}}
+    extern void f10_d(X); // expected-note {{candidate}}
     f10_d();
     f10_d(x);
-    // FIXME: This lookup should fail, because the local extern declaration
-    // should suppress ADL.
     f10_d(y);
+    f10_d(w); // expected-error {{no matching}}
     {
       int f10_d;
       f10_d(); // expected-error {{not a function}}
@@ -402,12 +405,27 @@ namespace PR33222 {
   };
   Y<float> yf; // expected-note {{instantiation}}
 
-  int h();
+  int h(); // expected-note {{previous}}
   template<typename T> struct Z {
-    // FIXME: The note here should point at the non-friend declaration, not the
-    // instantiation in Z<int>.
-    friend T h(); // expected-error {{return type}} expected-note {{previous}}
+    friend T h(); // expected-error {{return type}}
   };
   Z<int> zi;
   Z<float> zf; // expected-note {{instantiation}}
 }
+
+namespace qualified_friend_no_match {
+  void f(int); // expected-note {{type mismatch at 1st parameter}}
+  template<typename T> void f(T*); // expected-note {{could not match 'type-parameter-0-0 *' against 'double'}}
+  struct X {
+    friend void qualified_friend_no_match::f(double); // expected-error {{friend declaration of 'f' does not match any declaration in namespace 'qualified_friend_no_match'}}
+    friend void qualified_friend_no_match::g(); // expected-error {{friend declaration of 'g' does not match any declaration in namespace 'qualified_friend_no_match'}}
+  };
+
+  struct Y {
+    void f(int); // expected-note {{type mismatch at 1st parameter}}
+    template<typename T> void f(T*); // expected-note {{could not match 'type-parameter-0-0 *' against 'double'}}
+  };
+  struct Z {
+    friend void Y::f(double); // expected-error {{friend declaration of 'f' does not match any declaration in 'qualified_friend_no_match::Y'}}
+  };
+}

www/cxx_dr_status.html

@@ -4087,7 +4087,7 @@ and <I>POD class</I></td>
     <td><a href="http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#674">674</a></td>
     <td>C++11</td>
     <td>&#8220;matching specialization&#8221; for a friend declaration</td>
-    <td class="none" align="center">No</td>
+    <td class="svn" align="center">SVN</td>
   </tr>
   <tr id="675">
     <td><a href="http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#675">675</a></td>