From: Richard Smith Date: Mon, 22 Aug 2016 22:25:03 +0000 (+0000) Subject: Fix regression introduced by r279164: only pass definitions as the PatternDef X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e7c9a6c34ce335bd20aeb550763fa51e958f111f;p=clang 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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@279486 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index ca4de55fb5..9122030e16 100644 --- 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(Definition), - FD->getLocation()); + makeMergedDefinitionVisible(const_cast(Definition), + FD->getLocation()); return; } diff --git a/lib/Sema/SemaTemplate.cpp b/lib/Sema/SemaTemplate.cpp index ede1c5bcc6..6241f524af 100644 --- a/lib/Sema/SemaTemplate.cpp +++ b/lib/Sema/SemaTemplate.cpp @@ -487,8 +487,6 @@ bool Sema::DiagnoseUninstantiableTemplate(SourceLocation PointOfInstantiation, QualType InstantiationTy; if (TagDecl *TD = dyn_cast(Instantiation)) InstantiationTy = Context.getTypeDeclType(TD); - else - InstantiationTy = cast(Instantiation)->getType(); if (!Complain || (PatternDef && PatternDef->isInvalidDecl())) { // Say nothing } else if (PatternDef) { @@ -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(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(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(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(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 3f4a0f6f39..75d7f70a63 100644 --- 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 091a4e34e1..8a49f6120c 100644 --- 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(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 7f9b6497e7..e659202761 100644 --- 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 struct B; template struct A {}; template struct B : A {}; +template 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 5ed18e7e7c..6db3c2c6c9 100644 --- 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 struct B; template struct A {}; template struct B : A {}; +template inline auto C(T) {} inline void f() { B bi; + C(0); } diff --git a/test/Modules/merge-template-pattern-visibility.cpp b/test/Modules/merge-template-pattern-visibility.cpp index db759b5a46..b97f9f113e 100644 --- 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