From: Vassil Vassilev Date: Tue, 11 Oct 2016 13:57:36 +0000 (+0000) Subject: [modules] PR28752: Do not instantiate variable declarations which are not visible. X-Git-Tag: llvmorg-4.0.0-rc1~7512 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=4b3e7388d1a90f3e20a90d5624fe54dccca2610f;p=platform%2Fupstream%2Fllvm.git [modules] PR28752: Do not instantiate variable declarations which are not visible. https://reviews.llvm.org/D24508 Patch developed in collaboration with Richard Smith! llvm-svn: 283882 --- diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h index 90d8727..e4e3388 100644 --- a/clang/include/clang/AST/Decl.h +++ b/clang/include/clang/AST/Decl.h @@ -865,6 +865,11 @@ protected: unsigned : NumVarDeclBits; + // FIXME: We need something similar to CXXRecordDecl::DefinitionData. + /// \brief Whether this variable is a definition which was demoted due to + /// module merge. + unsigned IsThisDeclarationADemotedDefinition : 1; + /// \brief Whether this variable is the exception variable in a C++ catch /// or an Objective-C @catch statement. unsigned ExceptionVar : 1; @@ -1198,12 +1203,27 @@ public: InitializationStyle getInitStyle() const { return static_cast(VarDeclBits.InitStyle); } - /// \brief Whether the initializer is a direct-initializer (list or call). bool isDirectInit() const { return getInitStyle() != CInit; } + /// \brief If this definition should pretend to be a declaration. + bool isThisDeclarationADemotedDefinition() const { + return NonParmVarDeclBits.IsThisDeclarationADemotedDefinition; + } + + /// \brief This is a definition which should be demoted to a declaration. + /// + /// In some cases (mostly module merging) we can end up with two visible + /// definitions one of which needs to be demoted to a declaration to keep + /// the AST invariants. + void demoteThisDefinitionToDeclaration() { + assert (!isThisDeclarationADemotedDefinition() && "Aleady demoted!"); + assert (isThisDeclarationADefinition() && "Not a definition!"); + NonParmVarDeclBits.IsThisDeclarationADemotedDefinition = 1; + } + /// \brief Determine whether this variable is the exception variable in a /// C++ catch statememt or an Objective-C \@catch statement. bool isExceptionVariable() const { @@ -1302,6 +1322,10 @@ public: NonParmVarDeclBits.PreviousDeclInSameBlockScope = Same; } + /// \brief Retrieve the variable declaration from which this variable could + /// be instantiated, if it is an instantiation (rather than a non-template). + VarDecl *getTemplateInstantiationPattern() const; + /// \brief If this variable is an instantiated static data member of a /// class template specialization, returns the templated static data member /// from which it was instantiated. diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index e330e31..3d601ee 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -1926,6 +1926,9 @@ VarDecl::isThisDeclarationADefinition(ASTContext &C) const { // // FIXME: How do you declare (but not define) a partial specialization of // a static data member template outside the containing class? + if (isThisDeclarationADemotedDefinition()) + return DeclarationOnly; + if (isStaticDataMember()) { if (isOutOfLine() && !(getCanonicalDecl()->isInline() && @@ -2250,6 +2253,56 @@ bool VarDecl::checkInitIsICE() const { return Eval->IsICE; } +VarDecl *VarDecl::getTemplateInstantiationPattern() const { + // If it's a variable template specialization, find the template or partial + // specialization from which it was instantiated. + if (auto *VDTemplSpec = dyn_cast(this)) { + auto From = VDTemplSpec->getInstantiatedFrom(); + if (auto *VTD = From.dyn_cast()) { + while (auto *NewVTD = VTD->getInstantiatedFromMemberTemplate()) { + if (NewVTD->isMemberSpecialization()) + break; + VTD = NewVTD; + } + return VTD->getTemplatedDecl()->getDefinition(); + } + if (auto *VTPSD = + From.dyn_cast()) { + while (auto *NewVTPSD = VTPSD->getInstantiatedFromMember()) { + if (NewVTPSD->isMemberSpecialization()) + break; + VTPSD = NewVTPSD; + } + return VTPSD->getDefinition(); + } + } + + if (MemberSpecializationInfo *MSInfo = getMemberSpecializationInfo()) { + if (isTemplateInstantiation(MSInfo->getTemplateSpecializationKind())) { + VarDecl *VD = getInstantiatedFromStaticDataMember(); + while (auto *NewVD = VD->getInstantiatedFromStaticDataMember()) + VD = NewVD; + return VD->getDefinition(); + } + } + + if (VarTemplateDecl *VarTemplate = getDescribedVarTemplate()) { + + while (VarTemplate->getInstantiatedFromMemberTemplate()) { + if (VarTemplate->isMemberSpecialization()) + break; + VarTemplate = VarTemplate->getInstantiatedFromMemberTemplate(); + } + + assert((!VarTemplate->getTemplatedDecl() || + !isTemplateInstantiation(getTemplateSpecializationKind())) && + "couldn't find pattern for variable instantiation"); + + return VarTemplate->getTemplatedDecl(); + } + return nullptr; +} + VarDecl *VarDecl::getInstantiatedFromStaticDataMember() const { if (MemberSpecializationInfo *MSI = getMemberSpecializationInfo()) return cast(MSI->getInstantiatedFrom()); diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 7baa85a..c9a45b2 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -9708,6 +9708,21 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, VDecl->getDeclContext()->isDependentContext())) { // The previous definition is hidden, and multiple definitions are // permitted (in separate TUs). Form another definition of it. + + // Demote the newly parsed definition to a fake declaration. + if (!VDecl->isThisDeclarationADemotedDefinition()) + VDecl->demoteThisDefinitionToDeclaration(); + + // Make the definition visible from the point of the demotion on. + assert (!Hidden || Def == Hidden && + "We were suggested another hidden definition!"); + makeMergedDefinitionVisible(Def, VDecl->getLocation()); + + // If this is a variable template definition, make its enclosing template + // visible. + if (VarDecl *VarPattern = Def->getTemplateInstantiationPattern()) + if (VarPattern->isThisDeclarationADefinition()) + makeMergedDefinitionVisible(VarPattern, VDecl->getLocation()); } else { Diag(VDecl->getLocation(), diag::err_redefinition) << VDecl->getDeclName(); diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp index 0a03f8e..71b2153 100644 --- a/clang/lib/Sema/SemaTemplate.cpp +++ b/clang/lib/Sema/SemaTemplate.cpp @@ -466,10 +466,14 @@ bool Sema::DiagnoseUninstantiableTemplate(SourceLocation PointOfInstantiation, const NamedDecl *PatternDef, TemplateSpecializationKind TSK, bool Complain /*= true*/) { - assert(isa(Instantiation) || isa(Instantiation)); + assert(isa(Instantiation) || isa(Instantiation) || + isa(Instantiation)); - if (PatternDef && (isa(PatternDef) - || !cast(PatternDef)->isBeingDefined())) { + bool IsEntityBeingDefined = false; + if (const TagDecl *TD = dyn_cast_or_null(PatternDef)) + IsEntityBeingDefined = TD->isBeingDefined(); + + if (PatternDef && !IsEntityBeingDefined) { NamedDecl *SuggestedDef = nullptr; if (!hasVisibleDefinition(const_cast(PatternDef), &SuggestedDef, /*OnlyNeedComplete*/false)) { @@ -486,13 +490,14 @@ bool Sema::DiagnoseUninstantiableTemplate(SourceLocation PointOfInstantiation, if (!Complain || (PatternDef && PatternDef->isInvalidDecl())) return true; + llvm::Optional Note; QualType InstantiationTy; if (TagDecl *TD = dyn_cast(Instantiation)) InstantiationTy = Context.getTypeDeclType(TD); if (PatternDef) { Diag(PointOfInstantiation, diag::err_template_instantiate_within_definition) - << (TSK != TSK_ImplicitInstantiation) + << /*implicit|explicit*/(TSK != TSK_ImplicitInstantiation) << InstantiationTy; // Not much point in noting the template declaration here, since // we're lexically inside it. @@ -501,28 +506,44 @@ bool Sema::DiagnoseUninstantiableTemplate(SourceLocation PointOfInstantiation, if (isa(Instantiation)) { Diag(PointOfInstantiation, diag::err_explicit_instantiation_undefined_member) - << 1 << Instantiation->getDeclName() << Instantiation->getDeclContext(); + << /*member function*/ 1 << Instantiation->getDeclName() + << Instantiation->getDeclContext(); + Note = diag::note_explicit_instantiation_here; } else { + assert(isa(Instantiation) && "Must be a TagDecl!"); Diag(PointOfInstantiation, diag::err_implicit_instantiate_member_undefined) << InstantiationTy; + Note = diag::note_member_declared_at; } - Diag(Pattern->getLocation(), isa(Instantiation) - ? diag::note_explicit_instantiation_here - : diag::note_member_declared_at); } else { - if (isa(Instantiation)) + if (isa(Instantiation)) { Diag(PointOfInstantiation, diag::err_explicit_instantiation_undefined_func_template) << Pattern; - else + Note = diag::note_explicit_instantiation_here; + } else if (isa(Instantiation)) { 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); + Note = diag::note_template_decl_here; + } else { + assert(isa(Instantiation) && "Must be a VarDecl!"); + if (isa(Instantiation)) { + Diag(PointOfInstantiation, + diag::err_explicit_instantiation_undefined_var_template) + << Instantiation; + Instantiation->setInvalidDecl(); + } else + Diag(PointOfInstantiation, + diag::err_explicit_instantiation_undefined_member) + << /*static data member*/ 2 << Instantiation->getDeclName() + << Instantiation->getDeclContext(); + Note = diag::note_explicit_instantiation_here; + } } + if (Note) // Diagnostics were emitted. + Diag(Pattern->getLocation(), Note.getValue()); // In general, Instantiation isn't marked invalid to get more than one // error for multiple undefined instantiations. But the code that does diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp index 09068d7..911bc51 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -4068,6 +4068,10 @@ void Sema::InstantiateVariableDefinition(SourceLocation PointOfInstantiation, PrettyDeclStackTraceEntry CrashInfo(*this, Var, SourceLocation(), "instantiating variable initializer"); + // The instantiation is visible here, even if it was first declared in an + // unimported module. + Var->setHidden(false); + // If we're performing recursive template instantiation, create our own // queue of pending implicit instantiations that we will instantiate // later, while we're still within our own instantiation context. @@ -4116,33 +4120,17 @@ void Sema::InstantiateVariableDefinition(SourceLocation PointOfInstantiation, Def = PatternDecl->getDefinition(); } - // FIXME: Check that the definition is visible before trying to instantiate - // it. This requires us to track the instantiation stack in order to know - // which definitions should be visible. + TemplateSpecializationKind TSK = Var->getTemplateSpecializationKind(); // If we don't have a definition of the variable template, we won't perform // any instantiation. Rather, we rely on the user to instantiate this // definition (or provide a specialization for it) in another translation // unit. - if (!Def) { - if (DefinitionRequired) { - if (VarSpec) { - Diag(PointOfInstantiation, - diag::err_explicit_instantiation_undefined_var_template) << Var; - Var->setInvalidDecl(); - } - else - Diag(PointOfInstantiation, - diag::err_explicit_instantiation_undefined_member) - << 2 << Var->getDeclName() << Var->getDeclContext(); - Diag(PatternDecl->getLocation(), - diag::note_explicit_instantiation_here); - } else if (Var->getTemplateSpecializationKind() - == TSK_ExplicitInstantiationDefinition) { + if (!Def && !DefinitionRequired) { + if (TSK == TSK_ExplicitInstantiationDefinition) { PendingInstantiations.push_back( std::make_pair(Var, PointOfInstantiation)); - } else if (Var->getTemplateSpecializationKind() - == TSK_ImplicitInstantiation) { + } else if (TSK == TSK_ImplicitInstantiation) { // Warn about missing definition at the end of translation unit. if (AtEndOfTU && !getDiagnostics().hasErrorOccurred()) { Diag(PointOfInstantiation, diag::warn_var_template_missing) @@ -4151,12 +4139,20 @@ void Sema::InstantiateVariableDefinition(SourceLocation PointOfInstantiation, if (getLangOpts().CPlusPlus11) Diag(PointOfInstantiation, diag::note_inst_declaration_hint) << Var; } + return; } - return; } - TemplateSpecializationKind TSK = Var->getTemplateSpecializationKind(); + // FIXME: We need to track the instantiation stack in order to know which + // definitions should be visible within this instantiation. + // FIXME: Produce diagnostics when Var->getInstantiatedFromStaticDataMember(). + if (DiagnoseUninstantiableTemplate(PointOfInstantiation, Var, + /*InstantiatedFromMember*/false, + PatternDecl, Def, TSK, + /*Complain*/DefinitionRequired)) + return; + // Never instantiate an explicit specialization. if (TSK == TSK_ExplicitSpecialization) diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp index b0f18fd..97eb3a1 100644 --- a/clang/lib/Sema/SemaType.cpp +++ b/clang/lib/Sema/SemaType.cpp @@ -6900,6 +6900,10 @@ bool Sema::hasVisibleDefinition(NamedDecl *D, NamedDecl **Suggested, if (auto *Pattern = FD->getTemplateInstantiationPattern()) FD = Pattern; D = FD->getDefinition(); + } else if (auto *VD = dyn_cast(D)) { + if (auto *Pattern = VD->getTemplateInstantiationPattern()) + VD = Pattern; + D = VD->getDefinition(); } assert(D && "missing definition for pattern of instantiated definition"); diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index 19113da..864eaf0 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -1216,6 +1216,7 @@ ASTDeclReader::RedeclarableResult ASTDeclReader::VisitVarDeclImpl(VarDecl *VD) { VD->VarDeclBits.TSCSpec = Record[Idx++]; VD->VarDeclBits.InitStyle = Record[Idx++]; if (!isa(VD)) { + VD->NonParmVarDeclBits.IsThisDeclarationADemotedDefinition = Record[Idx++]; VD->NonParmVarDeclBits.ExceptionVar = Record[Idx++]; VD->NonParmVarDeclBits.NRVOVariable = Record[Idx++]; VD->NonParmVarDeclBits.CXXForRangeDecl = Record[Idx++]; @@ -3069,6 +3070,34 @@ void ASTDeclReader::attachPreviousDeclImpl(ASTReader &Reader, namespace clang { template<> void ASTDeclReader::attachPreviousDeclImpl(ASTReader &Reader, + Redeclarable *D, + Decl *Previous, Decl *Canon) { + VarDecl *VD = static_cast(D); + VarDecl *PrevVD = cast(Previous); + D->RedeclLink.setPrevious(PrevVD); + D->First = PrevVD->First; + + + // We should keep at most one definition on the chain. + if (VD->isThisDeclarationADefinition()) { + for (VarDecl *CurD = PrevVD; CurD; CurD = CurD->getPreviousDecl()) { + // If we find an already demoted definition, this we already visited this + // part of the chain. Reduces the loop from quadratic-time to linear-time. + if (CurD->isThisDeclarationADemotedDefinition()) { + VD->demoteThisDefinitionToDeclaration(); + break; + } + if (CurD->isThisDeclarationADefinition()) { + // If we found another definition on the chain, demote the current one. + VD->demoteThisDefinitionToDeclaration(); + break; + } + } + } +} + +template<> +void ASTDeclReader::attachPreviousDeclImpl(ASTReader &Reader, Redeclarable *D, Decl *Previous, Decl *Canon) { FunctionDecl *FD = static_cast(D); diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp index a0fc155..2146861 100644 --- a/clang/lib/Serialization/ASTWriterDecl.cpp +++ b/clang/lib/Serialization/ASTWriterDecl.cpp @@ -894,6 +894,7 @@ void ASTDeclWriter::VisitVarDecl(VarDecl *D) { Record.push_back(D->getTSCSpec()); Record.push_back(D->getInitStyle()); if (!isa(D)) { + Record.push_back(D->isThisDeclarationADemotedDefinition()); Record.push_back(D->isExceptionVariable()); Record.push_back(D->isNRVOVariable()); Record.push_back(D->isCXXForRangeDecl()); @@ -998,6 +999,8 @@ void ASTDeclWriter::VisitParmVarDecl(ParmVarDecl *D) { // Check things we know are true of *every* PARM_VAR_DECL, which is more than // just us assuming it. assert(!D->getTSCSpec() && "PARM_VAR_DECL can't use TLS"); + assert(!D->isThisDeclarationADemotedDefinition() + && "PARM_VAR_DECL can't be demoted definition."); assert(D->getAccess() == AS_none && "PARM_VAR_DECL can't be public/private"); assert(!D->isExceptionVariable() && "PARM_VAR_DECL can't be exception var"); assert(D->getPreviousDecl() == nullptr && "PARM_VAR_DECL can't be redecl"); @@ -1957,6 +1960,7 @@ void ASTWriter::WriteDeclAbbrevs() { Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); // SClass Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 2)); // TSCSpec Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 2)); // InitStyle + Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // IsThisDeclarationADemotedDefinition Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isExceptionVariable Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isNRVOVariable Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isCXXForRangeDecl diff --git a/clang/test/Modules/Inputs/PR28752/Subdir1/b.h b/clang/test/Modules/Inputs/PR28752/Subdir1/b.h new file mode 100644 index 0000000..3b3a25f --- /dev/null +++ b/clang/test/Modules/Inputs/PR28752/Subdir1/b.h @@ -0,0 +1 @@ +#include diff --git a/clang/test/Modules/Inputs/PR28752/Subdir1/c.h b/clang/test/Modules/Inputs/PR28752/Subdir1/c.h new file mode 100644 index 0000000..e69de29 diff --git a/clang/test/Modules/Inputs/PR28752/Subdir1/module.modulemap b/clang/test/Modules/Inputs/PR28752/Subdir1/module.modulemap new file mode 100644 index 0000000..8d3bfe9 --- /dev/null +++ b/clang/test/Modules/Inputs/PR28752/Subdir1/module.modulemap @@ -0,0 +1,5 @@ +module b { + module "b.h" { header "b.h" export * } + module "c.h" { header "c.h" export * } + export * +} diff --git a/clang/test/Modules/Inputs/PR28752/a.h b/clang/test/Modules/Inputs/PR28752/a.h new file mode 100644 index 0000000..3b3a25f --- /dev/null +++ b/clang/test/Modules/Inputs/PR28752/a.h @@ -0,0 +1 @@ +#include diff --git a/clang/test/Modules/Inputs/PR28752/module.modulemap b/clang/test/Modules/Inputs/PR28752/module.modulemap new file mode 100644 index 0000000..caf888f --- /dev/null +++ b/clang/test/Modules/Inputs/PR28752/module.modulemap @@ -0,0 +1 @@ +module a { header "a.h" export * } diff --git a/clang/test/Modules/Inputs/PR28752/vector b/clang/test/Modules/Inputs/PR28752/vector new file mode 100644 index 0000000..fc5dafa --- /dev/null +++ b/clang/test/Modules/Inputs/PR28752/vector @@ -0,0 +1,28 @@ +#ifndef VECTOR +#define VECTOR +template struct B; +template struct B { typedef _Tp type; }; +namespace std { +template struct D { + + template struct F { + static const bool value = 0; + }; + + template + typename B::value, _Alloc2>::type _S_select(_Alloc2); + template + static + typename B::value, _Alloc2>::type _S_select(_Alloc2); +}; +template +template +const bool D<_Alloc>::F<_Alloc2>::value; + +template class vector { +public: + vector(int); + vector(vector &) : vector(D::_S_select((bool)0)) {} +}; +} +#endif // VECTOR \ No newline at end of file diff --git a/clang/test/Modules/pr28752.cpp b/clang/test/Modules/pr28752.cpp new file mode 100644 index 0000000..e73a54b --- /dev/null +++ b/clang/test/Modules/pr28752.cpp @@ -0,0 +1,19 @@ +// RUN: rm -rf %t +// RUN: %clang_cc1 -std=c++11 -nostdsysteminc -I%S/Inputs/PR28752 -verify %s +// RUN: %clang_cc1 -std=c++11 -nostdsysteminc -fmodules -fmodule-map-file=%S/Inputs/PR28752/Subdir1/module.modulemap -fmodule-map-file=%S/Inputs/PR28752/module.modulemap -fmodules-cache-path=%t -I%S/Inputs/PR28752 -I%S/Inputs/PR28752/Subdir1 -verify %s + +#include "a.h" +#include "Subdir1/c.h" +#include + +class TClingClassInfo { + std::vector fIterStack; +}; + +TClingClassInfo *a; +class TClingBaseClassInfo { + TClingBaseClassInfo() { new TClingClassInfo(*a); } +}; + +// expected-no-diagnostics +