Reverted this per Richard's request in r279500. On Mon, Aug 22, 2016 at 3:33 PM Richard Smith via cfe-commits < cfe-commits@lists.llvm.org> wrote:
> Author: rsmith > Date: Mon Aug 22 17:25:03 2016 > New Revision: 279486 > > URL: http://llvm.org/viewvc/llvm-project?rev=279486&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. > > 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=279486&r1=279485&r2=279486&view=diff > > ============================================================================== > --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) > +++ cfe/trunk/lib/Sema/SemaDecl.cpp Mon Aug 22 17:25:03 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=279486&r1=279485&r2=279486&view=diff > > ============================================================================== > --- cfe/trunk/lib/Sema/SemaTemplate.cpp (original) > +++ cfe/trunk/lib/Sema/SemaTemplate.cpp Mon Aug 22 17:25:03 2016 > @@ -487,8 +487,6 @@ bool Sema::DiagnoseUninstantiableTemplat > 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) { > @@ -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=279486&r1=279485&r2=279486&view=diff > > ============================================================================== > --- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original) > +++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Mon Aug 22 17:25:03 > 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=279486&r1=279485&r2=279486&view=diff > > ============================================================================== > --- cfe/trunk/lib/Sema/SemaType.cpp (original) > +++ cfe/trunk/lib/Sema/SemaType.cpp Mon Aug 22 17:25:03 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=279486&r1=279485&r2=279486&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 > Mon Aug 22 17:25:03 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=279486&r1=279485&r2=279486&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 > Mon Aug 22 17:25:03 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=279486&r1=279485&r2=279486&view=diff > > ============================================================================== > --- cfe/trunk/test/Modules/merge-template-pattern-visibility.cpp (original) > +++ cfe/trunk/test/Modules/merge-template-pattern-visibility.cpp Mon Aug > 22 17:25:03 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 > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
