[Sema] Fix bug to do with lookup of template friend function in namespace

[erik.pilkington]

Summary

Clang erroneously rejects the following code because it does not find the default argument for makeType :

namespace n {
  template <class> struct Type {
    template <class T> friend Type<T> makeType();
  };

  template <class T = void> Type<T> makeType();

  void f() {
    Type<void> t;
    n::makeType<>(); // Error, lookup finds wrong version of makeType
  }
} // end namespace n

The problem is that during instantiation of Type<void> on the previous line the default template argument version of makeType is incorrectly swapped out of DeclContext::LookupPtr for the friend version. This patch fixes that by disallowing the swap in NamedDecl::declarationReplaces().

This patch looks like it's a fix for PR10856, PR18038, PR20877, and PR26962.

[meadori]

I see failure when running the Clang test suite with this patch:

Command Output (stderr):
--
error: 'error' diagnostics expected but not seen: 
  File /Users/meadori/Code/src/llvm/llvm-mainline/tools/clang/test/SemaTemplate/friend-template.cpp Line 80: template non-type parameter has a different type 'long' in template redeclaration
  File /Users/meadori/Code/src/llvm/llvm-mainline/tools/clang/test/SemaTemplate/friend-template.cpp Line 304: different type 'char' in template redeclaration
error: 'error' diagnostics seen but not expected: 
  File /Users/meadori/Code/src/llvm/llvm-mainline/tools/clang/test/SemaTemplate/friend-template.cpp Line 79: template non-type parameter has a different type 'long' in template redeclaration
  File /Users/meadori/Code/src/llvm/llvm-mainline/tools/clang/test/SemaTemplate/friend-template.cpp Line 303: template non-type parameter has a different type 'char' in template redeclaration
error: 'note' diagnostics expected but not seen: 
  File /Users/meadori/Code/src/llvm/llvm-mainline/tools/clang/test/SemaTemplate/friend-template.cpp Line 73: previous non-type template parameter with type 'int' is here
  File /Users/meadori/Code/src/llvm/llvm-mainline/tools/clang/test/SemaTemplate/friend-template.cpp Line 300: previous non-type template parameter with type 'int' is here
error: 'note' diagnostics seen but not expected: 
  File /Users/meadori/Code/src/llvm/llvm-mainline/tools/clang/test/SemaTemplate/friend-template.cpp Line 72: previous non-type template parameter with type 'int' is here
  File /Users/meadori/Code/src/llvm/llvm-mainline/tools/clang/test/SemaTemplate/friend-template.cpp Line 299: previous non-type template parameter with type 'int' is here
8 errors generated.

[erik.pilkington]

Very weird, the test suite runs fine on my machine. Looks like the expected line for the diagnostics on your run is always one more than the line of the actual diagnostic. Could you please update your build to top-of-trunk, reapply the patch, and run the suite again?
If that doesn't work, maybe trying running the test on this file:
https://gist.github.com/EPilkington/662b516d2448919aaeebe1e4dce9d92b

Thanks for pointing this out, and please let me know what happens!

[meadori]

Hmmm, it works on trunk r269671. Not sure what happened before.

[erik.pilkington]

Ping!!

[rsmith]

It sounds like we're failing to properly inherit default template arguments onto redeclarations generated by template instantiation, resulting in a violation of our AST invariants. This patch will hide the problem in some cases, but the right thing to do is to fix the root cause: we should call CheckTemplateParameterList to inherit the default template arguments when instantiating a function template declaration. (Note that TemplateDeclInstantiator::VisitClassTemplateDecl remembers to do this but TemplateDeclInstantiator::VisitFunctionTemplateDecl forgets -- as does our handling for variable templates, and for partial specializations of class templates and variable templates.)

lib/AST/Decl.cpp

 bool NamedDecl::declarationReplaces(NamedDecl *OldD, bool IsKnownNewer) const {
   assert(getDeclName() == OldD->getDeclName() && "Declaration name mismatch");
 
   // Never replace one imported declaration with another; we need both results
   // when re-exporting.
   if (OldD->isFromASTFile() && isFromASTFile())
     return false;
 
+  // Do not replace a declaration with a friend.
+  if (getFriendObjectKind() != FOK_None)
+    return false;
+
   // 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;
 

test/SemaTemplate/friend-template.cpp

-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify %s
 // PR5057
 namespace test0 {
   namespace std {
     class X {
     public:
       template<typename T> friend struct Y;
     };
   }

     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

   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 non-type template parameter with type 'int' is here}}
   };
   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}}
+    template<T> friend struct F::G; // 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
 // been parsed.
 namespace rdar12350696 {
   template <class T> struct A {
     void foo() {
       A<int> a;
     }
     template <class U> friend void foo(const A<U> & a) {
       int array[sizeof(T) == sizeof(U) ? -1 : 1]; // expected-error {{negative size}}
     }
   };
 
   void test() {
     A<int> b;
     foo(b); // expected-note {{in instantiation}}
   }
 }
+
+namespace PR26962 {
+template <class> struct Type {
+  template <class T> friend Type<T> makeType();
+};
+
+template <class T = void> Type<T> makeType();
+
+void f() {
+  Type<void> t;
+  PR26962::makeType<>();
+}
+} // end PR26962