Fix a diagnoses-valid bug with using declarations

[aaron.ballman]

Summary

The following was found by a customer and is accepted by the other primary C++ compilers, but fails to compile in Clang:

namespace sss {
double foo(int, double);
template <class T>
T foo(T); // note: target of using declaration
}  // namespace sss

namespace oad {
void foo();
}

namespace oad {
using ::sss::foo;
}

namespace sss {
using oad::foo; // note: using declaration
}

namespace sss {
double foo(int, double) { return 0; }
template <class T>
T foo(T t) { // error: declaration conflicts with target of using declaration already in scope
  return t;
}
}  // namespace sss

I believe the issue is that MergeFunctionDecl() was calling checkUsingShadowRedecl() but only considering a FunctionDecl as a possible shadow and not FunctionTemplateDecl. The changes in this patch largely mirror how variable declarations were being handled by also catching FunctionTemplateDecl.

[erichkeane]

The patch looks fine to me, though I don't particularly get the test changes that move a diagnostic by 1 line?

[aaron.ballman]

In D103485#2791940, @erichkeane wrote:

The patch looks fine to me, though I don't particularly get the test changes that move a diagnostic by 1 line?

The global-vs-module.cpp test had a line removed from it for the FIXME comment. The module-vs-module.cpp and module-vs-global.cpp files both reference diagnostics by line within global-vs-module.cpp, so those lines all had to be adjusted accordingly.

[erichkeane]

Ah! Thats what I missed!

[aaron.ballman]

Thanks for the review! I've commit in ca68f3bc48e48f839142de1461e95d87ae48e9df.

clang/lib/Sema/SemaDecl.cpp

if (OldLinkage == CXXLanguageLinkage && New->isInExternCContext())
     return true;
   if (OldLinkage == CLanguageLinkage && New->isInExternCXXContext())
     return true;
   return false;
 }
 
 template<typename T> static bool isExternC(T *D) { return D->isExternC(); }
 static bool isExternC(VarTemplateDecl *) { return false; }
+static bool isExternC(FunctionTemplateDecl *) { return false; }
 
 /// Check whether a redeclaration of an entity introduced by a
 /// using-declaration is valid, given that we know it's not an overload
 /// (nor a hidden tag declaration).
 template<typename ExpectedDecl>
 static bool checkUsingShadowRedecl(Sema &S, UsingShadowDecl *OldS,
                                    ExpectedDecl *New) {
   // C++11 [basic.scope.declarative]p4:
if (UsingShadowDecl *Shadow = dyn_cast<UsingShadowDecl>(OldD)) {
         Diag(New->getLocation(), diag::err_using_decl_friend);
         Diag(Shadow->getTargetDecl()->getLocation(),
              diag::note_using_decl_target);
         Diag(Shadow->getUsingDecl()->getLocation(),
              diag::note_using_decl) << 0;
         return true;
       }
 
-      // Check whether the two declarations might declare the same function.
+      // Check whether the two declarations might declare the same function or
+      // function template.
+      if (FunctionTemplateDecl *NewTemplate =
+              New->getDescribedFunctionTemplate()) {
+        if (checkUsingShadowRedecl<FunctionTemplateDecl>(*this, Shadow,
+                                                         NewTemplate))
+          return true;
+        OldD = Old = cast<FunctionTemplateDecl>(Shadow->getTargetDecl())
+                         ->getAsFunction();
+      } else {
         if (checkUsingShadowRedecl<FunctionDecl>(*this, Shadow, New))
           return true;
         OldD = Old = cast<FunctionDecl>(Shadow->getTargetDecl());
+      }
     } else {
       Diag(New->getLocation(), diag::err_redefinition_different_kind)
         << New->getDeclName();
       notePreviousDefinition(OldD, New->getLocation());
       return true;
     }
   }
 

clang/test/CXX/modules-ts/basic/basic.def.odr/p6/global-vs-module.cpp

 // RUN: %clang_cc1 -fmodules-ts -verify -std=c++17 %s 
 // RUN: %clang_cc1 -fmodules-ts -verify -std=c++17 %s -DEXPORT
 // RUN: %clang_cc1 -fmodules-ts -verify -std=c++17 %s -DUSING
 
 #ifndef NO_GLOBAL
 extern int var; // expected-note {{previous declaration is here}}
 int func(); // expected-note {{previous declaration is here}}
 struct str; // expected-note {{previous declaration is here}}
 using type = int;
 
 template<typename> extern int var_tpl; // expected-note {{previous declaration is here}}
-template<typename> int func_tpl(); // expected-note-re {{{{previous declaration is here|target of using declaration}}}}
+template<typename> int func_tpl(); // expected-note {{previous declaration is here}}
 template<typename> struct str_tpl; // expected-note {{previous declaration is here}}
 template<typename> using type_tpl = int; // expected-note {{previous declaration is here}}
 
 typedef int type;
 namespace ns { using ::func; }
 namespace ns_alias = ns;
 #endif
 
 export module M;
 
 #ifdef USING
 using ::var;
 using ::func;
 using ::str;
 using ::type;
 using ::var_tpl;
-using ::func_tpl; // expected-note {{using declaration}}
+using ::func_tpl;
 using ::str_tpl;
 using ::type_tpl;
 #endif
 
 #ifdef EXPORT
 export {
 #endif
 
 extern int var; // expected-error {{declaration of 'var' in module M follows declaration in the global module}}
 int func(); // expected-error {{declaration of 'func' in module M follows declaration in the global module}}
 struct str; // expected-error {{declaration of 'str' in module M follows declaration in the global module}}
 using type = int;
 
 template<typename> extern int var_tpl; // expected-error {{declaration of 'var_tpl' in module M follows declaration in the global module}}
-// FIXME: Is this the right diagnostic in the -DUSING case?
-template<typename> int func_tpl(); // expected-error-re {{{{declaration of 'func_tpl' in module M follows declaration in the global module|conflicts with target of using declaration}}}}
+template<typename> int func_tpl(); // expected-error {{declaration of 'func_tpl' in module M follows declaration in the global module}}
 template<typename> struct str_tpl; // expected-error {{declaration of 'str_tpl' in module M follows declaration in the global module}}
 template<typename> using type_tpl = int; // expected-error {{declaration of 'type_tpl' in module M follows declaration in the global module}}
 
 typedef int type;
 namespace ns { using ::func; }
 namespace ns_alias = ns;
 
 #ifdef EXPORT
 }
 #endif

clang/test/CXX/modules-ts/basic/basic.def.odr/p6/module-vs-global.cpp

 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fmodules-ts -emit-module-interface -std=c++17 %S/global-vs-module.cpp -o %t -DNO_GLOBAL -DEXPORT
 // RUN: %clang_cc1 -fmodules-ts -verify -std=c++17 %s -fmodule-file=%t
 
 import M;
 
 extern int var; // expected-error {{declaration of 'var' in the global module follows declaration in module M}} [e-mail]:38 {{previous}}
 int func(); // expected-error {{declaration of 'func' in the global module follows declaration in module M}} [e-mail]:39 {{previous}}
 struct str; // expected-error {{declaration of 'str' in the global module follows declaration in module M}} [e-mail]:40 {{previous}}
 using type = int;
 
 template<typename> extern int var_tpl; // expected-error {{declaration of 'var_tpl' in the global module follows declaration in module M}} [e-mail]:43 {{previous}}
-template<typename> int func_tpl(); // expected-error {{declaration of 'func_tpl' in the global module follows declaration in module M}} [e-mail]:45 {{previous}}
-template<typename> struct str_tpl; // expected-error {{declaration of 'str_tpl' in the global module follows declaration in module M}} [e-mail]:46 {{previous}}
-template<typename> using type_tpl = int; // expected-error {{declaration of 'type_tpl' in the global module follows declaration in module M}} [e-mail]:47 {{previous}}
+template<typename> int func_tpl(); // expected-error {{declaration of 'func_tpl' in the global module follows declaration in module M}} [e-mail]:44 {{previous}}
+template<typename> struct str_tpl; // expected-error {{declaration of 'str_tpl' in the global module follows declaration in module M}} [e-mail]:45 {{previous}}
+template<typename> using type_tpl = int; // expected-error {{declaration of 'type_tpl' in the global module follows declaration in module M}} [e-mail]:46 {{previous}}
 
 typedef int type;
 namespace ns { using ::func; }
 namespace ns_alias = ns;

clang/test/CXX/modules-ts/basic/basic.def.odr/p6/module-vs-module.cpp

 
 #ifndef NO_ERRORS
 extern int var; // expected-error {{declaration of 'var' in module N follows declaration in module M}} [e-mail]:38 {{previous}}
 int func(); // expected-error {{declaration of 'func' in module N follows declaration in module M}} [e-mail]:39 {{previous}}
 struct str; // expected-error {{declaration of 'str' in module N follows declaration in module M}} [e-mail]:40 {{previous}}
 using type = int;
 
 template<typename> extern int var_tpl; // expected-error {{declaration of 'var_tpl' in module N follows declaration in module M}} [e-mail]:43 {{previous}}
-template<typename> int func_tpl(); // expected-error {{declaration of 'func_tpl' in module N follows declaration in module M}} [e-mail]:45 {{previous}}
-template<typename> struct str_tpl; // expected-error {{declaration of 'str_tpl' in module N follows declaration in module M}} [e-mail]:46 {{previous}}
-template<typename> using type_tpl = int; // expected-error {{declaration of 'type_tpl' in module N follows declaration in module M}} [e-mail]:47 {{previous}}
+template<typename> int func_tpl(); // expected-error {{declaration of 'func_tpl' in module N follows declaration in module M}} [e-mail]:44 {{previous}}
+template<typename> struct str_tpl; // expected-error {{declaration of 'str_tpl' in module N follows declaration in module M}} [e-mail]:45 {{previous}}
+template<typename> using type_tpl = int; // expected-error {{declaration of 'type_tpl' in module N follows declaration in module M}} [e-mail]:46 {{previous}}
 
 typedef int type;
 namespace ns { using ::func; }
 namespace ns_alias = ns;
 #endif

clang/test/SemaCXX/using-decl-templates.cpp

 namespace DontDiagnoseInvalidTest {
 template <bool Value> struct Base {
   static_assert(Value, ""); // expected-error {{static_assert failed}}
 };
 struct Derived : Base<false> { // expected-note {{requested here}}
   using Base<false>::Base; // OK. Don't diagnose that 'Base' isn't a base class of Derived.
 };
 } // namespace DontDiagnoseInvalidTest
+
+namespace func_templ {
+namespace sss {
+double foo(int, double);
+template <class T>
+T foo(T);
+} // namespace sss
+
+namespace oad {
+void foo();
+}
+
+namespace oad {
+using sss::foo;
+}
+
+namespace sss {
+using oad::foo;
+}
+
+namespace sss {
+double foo(int, double) { return 0; }
+// There used to be an error with the below declaration when the example should
+// be accepted.
+template <class T>
+T foo(T t) { // OK
+  return t;
+}
+} // namespace sss
+} // namespace func_templ