[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
This commit is contained in:
Vassil Vassilev 2016-10-11 13:57:36 +00:00
parent 092cfc597f
commit 4b3e7388d1
15 changed files with 237 additions and 36 deletions

View File

@ -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<InitializationStyle>(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.

View File

@ -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<VarTemplateSpecializationDecl>(this)) {
auto From = VDTemplSpec->getInstantiatedFrom();
if (auto *VTD = From.dyn_cast<VarTemplateDecl *>()) {
while (auto *NewVTD = VTD->getInstantiatedFromMemberTemplate()) {
if (NewVTD->isMemberSpecialization())
break;
VTD = NewVTD;
}
return VTD->getTemplatedDecl()->getDefinition();
}
if (auto *VTPSD =
From.dyn_cast<VarTemplatePartialSpecializationDecl *>()) {
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<VarDecl>(MSI->getInstantiatedFrom());

View File

@ -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();

View File

@ -466,10 +466,14 @@ bool Sema::DiagnoseUninstantiableTemplate(SourceLocation PointOfInstantiation,
const NamedDecl *PatternDef,
TemplateSpecializationKind TSK,
bool Complain /*= true*/) {
assert(isa<TagDecl>(Instantiation) || isa<FunctionDecl>(Instantiation));
assert(isa<TagDecl>(Instantiation) || isa<FunctionDecl>(Instantiation) ||
isa<VarDecl>(Instantiation));
if (PatternDef && (isa<FunctionDecl>(PatternDef)
|| !cast<TagDecl>(PatternDef)->isBeingDefined())) {
bool IsEntityBeingDefined = false;
if (const TagDecl *TD = dyn_cast_or_null<TagDecl>(PatternDef))
IsEntityBeingDefined = TD->isBeingDefined();
if (PatternDef && !IsEntityBeingDefined) {
NamedDecl *SuggestedDef = nullptr;
if (!hasVisibleDefinition(const_cast<NamedDecl*>(PatternDef), &SuggestedDef,
/*OnlyNeedComplete*/false)) {
@ -486,13 +490,14 @@ bool Sema::DiagnoseUninstantiableTemplate(SourceLocation PointOfInstantiation,
if (!Complain || (PatternDef && PatternDef->isInvalidDecl()))
return true;
llvm::Optional<unsigned> Note;
QualType InstantiationTy;
if (TagDecl *TD = dyn_cast<TagDecl>(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<FunctionDecl>(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<TagDecl>(Instantiation) && "Must be a TagDecl!");
Diag(PointOfInstantiation,
diag::err_implicit_instantiate_member_undefined)
<< InstantiationTy;
Note = diag::note_member_declared_at;
}
Diag(Pattern->getLocation(), isa<FunctionDecl>(Instantiation)
? diag::note_explicit_instantiation_here
: diag::note_member_declared_at);
} else {
if (isa<FunctionDecl>(Instantiation))
if (isa<FunctionDecl>(Instantiation)) {
Diag(PointOfInstantiation,
diag::err_explicit_instantiation_undefined_func_template)
<< Pattern;
else
Note = diag::note_explicit_instantiation_here;
} else if (isa<TagDecl>(Instantiation)) {
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);
Note = diag::note_template_decl_here;
} else {
assert(isa<VarDecl>(Instantiation) && "Must be a VarDecl!");
if (isa<VarTemplateSpecializationDecl>(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

View File

@ -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)

View File

@ -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<VarDecl>(D)) {
if (auto *Pattern = VD->getTemplateInstantiationPattern())
VD = Pattern;
D = VD->getDefinition();
}
assert(D && "missing definition for pattern of instantiated definition");

View File

@ -1216,6 +1216,7 @@ ASTDeclReader::RedeclarableResult ASTDeclReader::VisitVarDeclImpl(VarDecl *VD) {
VD->VarDeclBits.TSCSpec = Record[Idx++];
VD->VarDeclBits.InitStyle = Record[Idx++];
if (!isa<ParmVarDecl>(VD)) {
VD->NonParmVarDeclBits.IsThisDeclarationADemotedDefinition = Record[Idx++];
VD->NonParmVarDeclBits.ExceptionVar = Record[Idx++];
VD->NonParmVarDeclBits.NRVOVariable = Record[Idx++];
VD->NonParmVarDeclBits.CXXForRangeDecl = Record[Idx++];
@ -3067,6 +3068,34 @@ void ASTDeclReader::attachPreviousDeclImpl(ASTReader &Reader,
}
namespace clang {
template<>
void ASTDeclReader::attachPreviousDeclImpl(ASTReader &Reader,
Redeclarable<VarDecl> *D,
Decl *Previous, Decl *Canon) {
VarDecl *VD = static_cast<VarDecl*>(D);
VarDecl *PrevVD = cast<VarDecl>(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<FunctionDecl> *D,

View File

@ -894,6 +894,7 @@ void ASTDeclWriter::VisitVarDecl(VarDecl *D) {
Record.push_back(D->getTSCSpec());
Record.push_back(D->getInitStyle());
if (!isa<ParmVarDecl>(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

View File

@ -0,0 +1 @@
#include <vector>

View File

@ -0,0 +1,5 @@
module b {
module "b.h" { header "b.h" export * }
module "c.h" { header "c.h" export * }
export *
}

View File

@ -0,0 +1 @@
#include <vector>

View File

@ -0,0 +1 @@
module a { header "a.h" export * }

View File

@ -0,0 +1,28 @@
#ifndef VECTOR
#define VECTOR
template <bool, typename> struct B;
template <typename _Tp> struct B<true, _Tp> { typedef _Tp type; };
namespace std {
template <typename> struct D {
template <typename _Alloc2> struct F {
static const bool value = 0;
};
template <typename _Alloc2>
typename B<F<_Alloc2>::value, _Alloc2>::type _S_select(_Alloc2);
template <typename _Alloc2>
static
typename B<!F<_Alloc2>::value, _Alloc2>::type _S_select(_Alloc2);
};
template <typename _Alloc>
template <typename _Alloc2>
const bool D<_Alloc>::F<_Alloc2>::value;
template <typename> class vector {
public:
vector(int);
vector(vector &) : vector(D<bool>::_S_select((bool)0)) {}
};
}
#endif // VECTOR

View File

@ -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 <vector>
class TClingClassInfo {
std::vector<int> fIterStack;
};
TClingClassInfo *a;
class TClingBaseClassInfo {
TClingBaseClassInfo() { new TClingClassInfo(*a); }
};
// expected-no-diagnostics