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

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 23 12:41:39 PDT 2016


Author: rsmith
Date: Tue Aug 23 14:41:39 2016
New Revision: 279557

URL: http://llvm.org/viewvc/llvm-project?rev=279557&view=rev
Log:
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.

Modified:
    cfe/trunk/lib/Sema/SemaDecl.cpp
    cfe/trunk/lib/Sema/SemaTemplate.cpp
    cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
    cfe/trunk/lib/Sema/SemaType.cpp
    cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/a.h
    cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/b.h
    cfe/trunk/test/Modules/merge-template-pattern-visibility.cpp

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=279557&r1=279556&r2=279557&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Aug 23 14:41:39 2016
@@ -11274,9 +11274,8 @@ Sema::CheckForFunctionRedefinition(Funct
     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;
   }
 

Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=279557&r1=279556&r2=279557&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp Tue Aug 23 14:41:39 2016
@@ -483,15 +483,13 @@ bool Sema::DiagnoseUninstantiableTemplat
     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::DiagnoseUninstantiableTemplat
     // 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

Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=279557&r1=279556&r2=279557&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Tue Aug 23 14:41:39 2016
@@ -3554,23 +3554,38 @@ void Sema::InstantiateFunctionDefinition
   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
     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

Modified: cfe/trunk/lib/Sema/SemaType.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=279557&r1=279556&r2=279557&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaType.cpp (original)
+++ cfe/trunk/lib/Sema/SemaType.cpp Tue Aug 23 14:41:39 2016
@@ -6890,6 +6890,10 @@ bool Sema::hasVisibleDefinition(NamedDec
       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(NamedDec
   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);

Modified: cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/a.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/a.h?rev=279557&r1=279556&r2=279557&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/a.h (original)
+++ cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/a.h Tue Aug 23 14:41:39 2016
@@ -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) {}

Modified: cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/b.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/b.h?rev=279557&r1=279556&r2=279557&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/b.h (original)
+++ cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/b.h Tue Aug 23 14:41:39 2016
@@ -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);
 }

Modified: cfe/trunk/test/Modules/merge-template-pattern-visibility.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/merge-template-pattern-visibility.cpp?rev=279557&r1=279556&r2=279557&view=diff
==============================================================================
--- cfe/trunk/test/Modules/merge-template-pattern-visibility.cpp (original)
+++ cfe/trunk/test/Modules/merge-template-pattern-visibility.cpp Tue Aug 23 14:41:39 2016
@@ -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




More information about the cfe-commits mailing list