Fix regression introduced by r279164: only pass definitions as the PatternDef

to DiagnoseUninstantiableTemplate, teach hasVisibleDefinition to correctly
determine whether a function definition is visible, and mark both the function
and the template as visible when merging function template definitions to
provide hasVisibleDefinition with the relevant information.

The change to always pass the right declaration as the PatternDef to
DiagnoseUninstantiableTemplate also caused those checks to happen before other
diagnostics in InstantiateFunctionDefinition, giving worse diagnostics for the
same situations, so I sunk the relevant diagnostics into
DiagnoseUninstantiableTemplate. Those parts of this patch are based on changes
in reviews.llvm.org/D23492 by Vassil Vassilev.

This reinstates r279486, reverted in r279500, with a fix to
DiagnoseUninstantiableTemplate to only mark uninstantiable explicit
instantiation declarations as invalid if we actually diagnosed them. (When we
trigger an explicit instantiation of a class member from an explicit
instantiation declaration for the class, it's OK if there is no corresponding
definition and we certainly don't want to mark the member invalid in that
case.) This previously caused a build failure during bootstrap.

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

diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index ca4de55fb51c57315eaacccc00d5b7fcccd4a636..9122030e16d7634dd2b623ed7c170f12ae0dc8bf 100644 (file)
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -11274,9 +11274,8 @@ Sema::CheckForFunctionRedefinition(FunctionDecl *FD,
     SkipBody->ShouldSkip = true;
     if (auto *TD = Definition->getDescribedFunctionTemplate())
       makeMergedDefinitionVisible(TD, FD->getLocation());
-    else
-      makeMergedDefinitionVisible(const_cast<FunctionDecl*>(Definition),
-                                  FD->getLocation());
+    makeMergedDefinitionVisible(const_cast<FunctionDecl*>(Definition),
+                                FD->getLocation());
     return;
   }
 

diff --git a/lib/Sema/SemaTemplate.cpp b/lib/Sema/SemaTemplate.cpp
index ede1c5bcc64166c6233396cdf7a88a3511b2890c..ea0357a33599e67be10a4d1d996a21dd7a44b2d6 100644 (file)
--- a/lib/Sema/SemaTemplate.cpp
+++ b/lib/Sema/SemaTemplate.cpp
@@ -483,15 +483,13 @@ bool Sema::DiagnoseUninstantiableTemplate(SourceLocation PointOfInstantiation,
     return false;
   }
 
+  if (!Complain || (PatternDef && PatternDef->isInvalidDecl()))
+    return true;
 
   QualType InstantiationTy;
   if (TagDecl *TD = dyn_cast<TagDecl>(Instantiation))
     InstantiationTy = Context.getTypeDeclType(TD);
-  else
-    InstantiationTy = cast<FunctionDecl>(Instantiation)->getType();
-  if (!Complain || (PatternDef && PatternDef->isInvalidDecl())) {
-    // Say nothing
-  } else if (PatternDef) {
+  if (PatternDef) {
     Diag(PointOfInstantiation,
          diag::err_template_instantiate_within_definition)
       << (TSK != TSK_ImplicitInstantiation)
@@ -500,15 +498,30 @@ bool Sema::DiagnoseUninstantiableTemplate(SourceLocation PointOfInstantiation,
     // we're lexically inside it.
     Instantiation->setInvalidDecl();
   } else if (InstantiatedFromMember) {
-    Diag(PointOfInstantiation,
-         diag::err_implicit_instantiate_member_undefined)
-      << InstantiationTy;
-    Diag(Pattern->getLocation(), diag::note_member_declared_at);
+    if (isa<FunctionDecl>(Instantiation)) {
+      Diag(PointOfInstantiation,
+           diag::err_explicit_instantiation_undefined_member)
+        << 1 << Instantiation->getDeclName() << Instantiation->getDeclContext();
+    } else {
+      Diag(PointOfInstantiation,
+           diag::err_implicit_instantiate_member_undefined)
+        << InstantiationTy;
+    }
+    Diag(Pattern->getLocation(), isa<FunctionDecl>(Instantiation)
+                                     ? diag::note_explicit_instantiation_here
+                                     : diag::note_member_declared_at);
   } else {
-    Diag(PointOfInstantiation, diag::err_template_instantiate_undefined)
-      << (TSK != TSK_ImplicitInstantiation)
-      << InstantiationTy;
-    Diag(Pattern->getLocation(), diag::note_template_decl_here);
+    if (isa<FunctionDecl>(Instantiation))
+      Diag(PointOfInstantiation,
+           diag::err_explicit_instantiation_undefined_func_template)
+        << Pattern;
+    else
+      Diag(PointOfInstantiation, diag::err_template_instantiate_undefined)
+        << (TSK != TSK_ImplicitInstantiation)
+        << InstantiationTy;
+    Diag(Pattern->getLocation(), isa<FunctionDecl>(Instantiation)
+                                     ? diag::note_explicit_instantiation_here
+                                     : diag::note_template_decl_here);
   }
 
   // In general, Instantiation isn't marked invalid to get more than one

diff --git a/lib/Sema/SemaTemplateInstantiateDecl.cpp b/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 3f4a0f6f39a673765d628484e6ed79c7612f7caf..75d7f70a6368e2f3cf0f6ae005a722385377b4c2 100644 (file)
--- a/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -3554,23 +3554,38 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
   const FunctionDecl *PatternDecl = Function->getTemplateInstantiationPattern();
   assert(PatternDecl && "instantiating a non-template");
 
-  Stmt *Pattern = PatternDecl->getBody(PatternDecl);
-  assert(PatternDecl && "template definition is not a template");
-  if (!Pattern) {
-    // Try to find a defaulted definition
-    PatternDecl->isDefined(PatternDecl);
-  }
-  assert(PatternDecl && "template definition is not a template");
+  const FunctionDecl *PatternDef = PatternDecl->getDefinition();
+  Stmt *Pattern = PatternDef->getBody(PatternDef);
+  if (PatternDef)
+    PatternDecl = PatternDef;
 
   // FIXME: We need to track the instantiation stack in order to know which
   // definitions should be visible within this instantiation.
   if (DiagnoseUninstantiableTemplate(PointOfInstantiation, Function,
                                 Function->getInstantiatedFromMemberFunction(),
-                                     PatternDecl, PatternDecl, TSK,
-                                     /*Complain*/DefinitionRequired))
-     return;
-
+                                     PatternDecl, PatternDef, TSK,
+                                     /*Complain*/DefinitionRequired)) {
+    if (DefinitionRequired)
+      Function->setInvalidDecl();
+    else if (TSK == TSK_ExplicitInstantiationDefinition) {
+      // Try again at the end of the translation unit (at which point a
+      // definition will be required).
+      assert(!Recursive);
+      PendingInstantiations.push_back(
+        std::make_pair(Function, PointOfInstantiation));
+    } else if (TSK == TSK_ImplicitInstantiation) {
+      if (AtEndOfTU && !getDiagnostics().hasErrorOccurred()) {
+        Diag(PointOfInstantiation, diag::warn_func_template_missing)
+          << Function;
+        Diag(PatternDecl->getLocation(), diag::note_forward_template_decl);
+        if (getLangOpts().CPlusPlus11)
+          Diag(PointOfInstantiation, diag::note_inst_declaration_hint)
+            << Function;
+      }
+    }
 
+    return;
+  }
 
   // Postpone late parsed template instantiations.
   if (PatternDecl->isLateTemplateParsed() &&
@@ -3604,40 +3619,9 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
     Pattern = PatternDecl->getBody(PatternDecl);
   }
 
-  // FIXME: Check if we could sink these diagnostics in
-  // DiagnoseUninstantiableTemplate.
-  if (!Pattern && !PatternDecl->isDefaulted()) {
-    if (DefinitionRequired) {
-      if (Function->getPrimaryTemplate())
-        Diag(PointOfInstantiation,
-             diag::err_explicit_instantiation_undefined_func_template)
-          << Function->getPrimaryTemplate();
-      else
-        Diag(PointOfInstantiation,
-             diag::err_explicit_instantiation_undefined_member)
-          << 1 << Function->getDeclName() << Function->getDeclContext();
-
-      if (PatternDecl)
-        Diag(PatternDecl->getLocation(),
-             diag::note_explicit_instantiation_here);
-      Function->setInvalidDecl();
-    } else if (TSK == TSK_ExplicitInstantiationDefinition) {
-      assert(!Recursive);
-      PendingInstantiations.push_back(
-        std::make_pair(Function, PointOfInstantiation));
-    } else if (TSK == TSK_ImplicitInstantiation) {
-      if (AtEndOfTU && !getDiagnostics().hasErrorOccurred()) {
-        Diag(PointOfInstantiation, diag::warn_func_template_missing)
-          << Function;
-        Diag(PatternDecl->getLocation(), diag::note_forward_template_decl);
-        if (getLangOpts().CPlusPlus11)
-          Diag(PointOfInstantiation, diag::note_inst_declaration_hint)
-            << Function;
-      }
-    }
-
-    return;
-  }
+  // Note, we should never try to instantiate a deleted function template.
+  assert((Pattern || PatternDecl->isDefaulted()) &&
+         "unexpected kind of function template definition");
 
   // C++1y [temp.explicit]p10:
   //   Except for inline functions, declarations with types deduced from their

diff --git a/lib/Sema/SemaType.cpp b/lib/Sema/SemaType.cpp
index 091a4e34e19543c490e77bc4c40d9cd73fc5bcce..8a49f6120cf8e82254c8184004884044366614ca 100644 (file)
--- a/lib/Sema/SemaType.cpp
+++ b/lib/Sema/SemaType.cpp
@@ -6890,6 +6890,10 @@ bool Sema::hasVisibleDefinition(NamedDecl *D, NamedDecl **Suggested,
       return false;
     }
     D = ED->getDefinition();
+  } else if (auto *FD = dyn_cast<FunctionDecl>(D)) {
+    if (auto *Pattern = FD->getTemplateInstantiationPattern())
+      FD = Pattern;
+    D = FD->getDefinition();
   }
   assert(D && "missing definition for pattern of instantiated definition");
 
@@ -6897,7 +6901,7 @@ bool Sema::hasVisibleDefinition(NamedDecl *D, NamedDecl **Suggested,
   if (isVisible(D))
     return true;
 
-  // The external source may have additional definitions of this type that are
+  // The external source may have additional definitions of this entity that are
   // visible, so complete the redeclaration chain now and ask again.
   if (auto *Source = Context.getExternalSource()) {
     Source->CompleteRedeclChain(D);

diff --git a/test/Modules/Inputs/merge-template-pattern-visibility/a.h b/test/Modules/Inputs/merge-template-pattern-visibility/a.h
index 7f9b6497e7250cdb8f54984435384aac91b9b57b..e6592027611c2f91d7d09dfc6ca0d7dd1a3724d5 100644 (file)
--- a/test/Modules/Inputs/merge-template-pattern-visibility/a.h
+++ b/test/Modules/Inputs/merge-template-pattern-visibility/a.h
@@ -3,3 +3,4 @@ template<typename T> struct B;
 
 template<typename, typename> struct A {};
 template<typename T> struct B : A<T> {};
+template<typename T> inline auto C(T) {}

diff --git a/test/Modules/Inputs/merge-template-pattern-visibility/b.h b/test/Modules/Inputs/merge-template-pattern-visibility/b.h
index 5ed18e7e7c5d9745925b343c305e43ad2cb07c68..6db3c2c6c9f873a7007a95199785f05c914f6c16 100644 (file)
--- a/test/Modules/Inputs/merge-template-pattern-visibility/b.h
+++ b/test/Modules/Inputs/merge-template-pattern-visibility/b.h
@@ -3,7 +3,9 @@ template<typename T> struct B;
 
 template<typename, typename> struct A {};
 template<typename T> struct B : A<T> {};
+template<typename T> inline auto C(T) {}
 
 inline void f() {
   B<int> bi;
+  C(0);
 }

diff --git a/test/Modules/merge-template-pattern-visibility.cpp b/test/Modules/merge-template-pattern-visibility.cpp
index db759b5a46a12e179d048f2ba9062206cf85bf0e..b97f9f113e3b96f40213415d6603cb66ed57bde3 100644 (file)
--- a/test/Modules/merge-template-pattern-visibility.cpp
+++ b/test/Modules/merge-template-pattern-visibility.cpp
@@ -1,4 +1,4 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fno-modules-error-recovery \
+// RUN: %clang_cc1 -fmodules -fno-modules-error-recovery -std=c++14 \
 // RUN:            -fmodule-name=X -emit-module %S/Inputs/merge-template-pattern-visibility/module.modulemap -x c++ \
-// RUN:            -fmodules-local-submodule-visibility
+// RUN:            -fmodules-local-submodule-visibility -o %t/X.pcm