Thanks. Fixed and reapplied as r279557. On Mon, Aug 22, 2016 at 7:08 PM, Chandler Carruth <chandl...@gmail.com> wrote:
> 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->getInstantiatedFromMemberFunct >> ion(), >> - 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