Skip to content
This repository was archived by the owner on Nov 1, 2021. It is now read-only.

Commit 5b36200

Browse files
zygoloidzygoloid
committed
Reinstate r284008 reverted in r284081, with two fixes:
1) Merge and demote variable definitions when we find a redefinition in MergeVarDecls, not only when we find one in AddInitializerToDecl (we only reach the second case if it's the addition of the initializer itself that converts an existing declaration into a definition). 2) When rebuilding a redeclaration chain for a variable, if we merge two definitions together, mark the definitions as merged so the retained definition is made visible whenever the demoted definition would have been. Original commit message (from r283882): [modules] PR28752: Do not instantiate variable declarations which are not visible. Original patch by Vassil Vassilev! Changes listed above are mine. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@284284 91177308-0d34-0410-b5e6-96231b3b80d8
1 parent 80ec660 commit 5b36200

File tree

16 files changed

+266
-72
lines changed

16 files changed

+266
-72
lines changed

Diff for: ‎include/clang/AST/Decl.h

+26-1
Original file line numberDiff line numberDiff line change
@@ -865,6 +865,11 @@ class VarDecl : public DeclaratorDecl, public Redeclarable<VarDecl> {
865865

866866
unsigned : NumVarDeclBits;
867867

868+
// FIXME: We need something similar to CXXRecordDecl::DefinitionData.
869+
/// \brief Whether this variable is a definition which was demoted due to
870+
/// module merge.
871+
unsigned IsThisDeclarationADemotedDefinition : 1;
872+
868873
/// \brief Whether this variable is the exception variable in a C++ catch
869874
/// or an Objective-C @catch statement.
870875
unsigned ExceptionVar : 1;
@@ -1198,12 +1203,28 @@ class VarDecl : public DeclaratorDecl, public Redeclarable<VarDecl> {
11981203
InitializationStyle getInitStyle() const {
11991204
return static_cast<InitializationStyle>(VarDeclBits.InitStyle);
12001205
}
1201-
12021206
/// \brief Whether the initializer is a direct-initializer (list or call).
12031207
bool isDirectInit() const {
12041208
return getInitStyle() != CInit;
12051209
}
12061210

1211+
/// \brief If this definition should pretend to be a declaration.
1212+
bool isThisDeclarationADemotedDefinition() const {
1213+
return isa<ParmVarDecl>(this) ? false :
1214+
NonParmVarDeclBits.IsThisDeclarationADemotedDefinition;
1215+
}
1216+
1217+
/// \brief This is a definition which should be demoted to a declaration.
1218+
///
1219+
/// In some cases (mostly module merging) we can end up with two visible
1220+
/// definitions one of which needs to be demoted to a declaration to keep
1221+
/// the AST invariants.
1222+
void demoteThisDefinitionToDeclaration() {
1223+
assert (isThisDeclarationADefinition() && "Not a definition!");
1224+
assert (!isa<ParmVarDecl>(this) && "Cannot demote ParmVarDecls!");
1225+
NonParmVarDeclBits.IsThisDeclarationADemotedDefinition = 1;
1226+
}
1227+
12071228
/// \brief Determine whether this variable is the exception variable in a
12081229
/// C++ catch statememt or an Objective-C \@catch statement.
12091230
bool isExceptionVariable() const {
@@ -1302,6 +1323,10 @@ class VarDecl : public DeclaratorDecl, public Redeclarable<VarDecl> {
13021323
NonParmVarDeclBits.PreviousDeclInSameBlockScope = Same;
13031324
}
13041325

1326+
/// \brief Retrieve the variable declaration from which this variable could
1327+
/// be instantiated, if it is an instantiation (rather than a non-template).
1328+
VarDecl *getTemplateInstantiationPattern() const;
1329+
13051330
/// \brief If this variable is an instantiated static data member of a
13061331
/// class template specialization, returns the templated static data member
13071332
/// from which it was instantiated.

Diff for: ‎include/clang/Sema/Sema.h

+1
Original file line numberDiff line numberDiff line change
@@ -2286,6 +2286,7 @@ class Sema {
22862286
void MergeVarDecl(VarDecl *New, LookupResult &Previous);
22872287
void MergeVarDeclTypes(VarDecl *New, VarDecl *Old, bool MergeTypeWithOld);
22882288
void MergeVarDeclExceptionSpecs(VarDecl *New, VarDecl *Old);
2289+
bool checkVarDeclRedefinition(VarDecl *OldDefn, VarDecl *NewDefn);
22892290
bool MergeCXXFunctionDecl(FunctionDecl *New, FunctionDecl *Old, Scope *S);
22902291

22912292
// AssignmentAction - This is used by all the assignment diagnostic functions

Diff for: ‎lib/AST/Decl.cpp

+53
Original file line numberDiff line numberDiff line change
@@ -1926,6 +1926,9 @@ VarDecl::isThisDeclarationADefinition(ASTContext &C) const {
19261926
//
19271927
// FIXME: How do you declare (but not define) a partial specialization of
19281928
// a static data member template outside the containing class?
1929+
if (isThisDeclarationADemotedDefinition())
1930+
return DeclarationOnly;
1931+
19291932
if (isStaticDataMember()) {
19301933
if (isOutOfLine() &&
19311934
!(getCanonicalDecl()->isInline() &&
@@ -2250,6 +2253,56 @@ bool VarDecl::checkInitIsICE() const {
22502253
return Eval->IsICE;
22512254
}
22522255

2256+
VarDecl *VarDecl::getTemplateInstantiationPattern() const {
2257+
// If it's a variable template specialization, find the template or partial
2258+
// specialization from which it was instantiated.
2259+
if (auto *VDTemplSpec = dyn_cast<VarTemplateSpecializationDecl>(this)) {
2260+
auto From = VDTemplSpec->getInstantiatedFrom();
2261+
if (auto *VTD = From.dyn_cast<VarTemplateDecl *>()) {
2262+
while (auto *NewVTD = VTD->getInstantiatedFromMemberTemplate()) {
2263+
if (NewVTD->isMemberSpecialization())
2264+
break;
2265+
VTD = NewVTD;
2266+
}
2267+
return VTD->getTemplatedDecl()->getDefinition();
2268+
}
2269+
if (auto *VTPSD =
2270+
From.dyn_cast<VarTemplatePartialSpecializationDecl *>()) {
2271+
while (auto *NewVTPSD = VTPSD->getInstantiatedFromMember()) {
2272+
if (NewVTPSD->isMemberSpecialization())
2273+
break;
2274+
VTPSD = NewVTPSD;
2275+
}
2276+
return VTPSD->getDefinition();
2277+
}
2278+
}
2279+
2280+
if (MemberSpecializationInfo *MSInfo = getMemberSpecializationInfo()) {
2281+
if (isTemplateInstantiation(MSInfo->getTemplateSpecializationKind())) {
2282+
VarDecl *VD = getInstantiatedFromStaticDataMember();
2283+
while (auto *NewVD = VD->getInstantiatedFromStaticDataMember())
2284+
VD = NewVD;
2285+
return VD->getDefinition();
2286+
}
2287+
}
2288+
2289+
if (VarTemplateDecl *VarTemplate = getDescribedVarTemplate()) {
2290+
2291+
while (VarTemplate->getInstantiatedFromMemberTemplate()) {
2292+
if (VarTemplate->isMemberSpecialization())
2293+
break;
2294+
VarTemplate = VarTemplate->getInstantiatedFromMemberTemplate();
2295+
}
2296+
2297+
assert((!VarTemplate->getTemplatedDecl() ||
2298+
!isTemplateInstantiation(getTemplateSpecializationKind())) &&
2299+
"couldn't find pattern for variable instantiation");
2300+
2301+
return VarTemplate->getTemplatedDecl();
2302+
}
2303+
return nullptr;
2304+
}
2305+
22532306
VarDecl *VarDecl::getInstantiatedFromStaticDataMember() const {
22542307
if (MemberSpecializationInfo *MSI = getMemberSpecializationInfo())
22552308
return cast<VarDecl>(MSI->getInstantiatedFrom());

Diff for: ‎lib/Sema/SemaDecl.cpp

+39-36
Original file line numberDiff line numberDiff line change
@@ -3675,29 +3675,16 @@ void Sema::MergeVarDecl(VarDecl *New, LookupResult &Previous) {
36753675
}
36763676

36773677
// C++ doesn't have tentative definitions, so go right ahead and check here.
3678-
VarDecl *Def;
36793678
if (getLangOpts().CPlusPlus &&
3680-
New->isThisDeclarationADefinition() == VarDecl::Definition &&
3681-
(Def = Old->getDefinition())) {
3682-
NamedDecl *Hidden = nullptr;
3683-
if (!hasVisibleDefinition(Def, &Hidden) &&
3684-
(New->getFormalLinkage() == InternalLinkage ||
3685-
New->getDescribedVarTemplate() ||
3686-
New->getNumTemplateParameterLists() ||
3687-
New->getDeclContext()->isDependentContext())) {
3688-
// The previous definition is hidden, and multiple definitions are
3689-
// permitted (in separate TUs). Form another definition of it.
3690-
} else if (Old->isStaticDataMember() &&
3691-
Old->getCanonicalDecl()->isInline() &&
3692-
Old->getCanonicalDecl()->isConstexpr()) {
3679+
New->isThisDeclarationADefinition() == VarDecl::Definition) {
3680+
if (Old->isStaticDataMember() && Old->getCanonicalDecl()->isInline() &&
3681+
Old->getCanonicalDecl()->isConstexpr()) {
36933682
// This definition won't be a definition any more once it's been merged.
36943683
Diag(New->getLocation(),
36953684
diag::warn_deprecated_redundant_constexpr_static_def);
3696-
} else {
3697-
Diag(New->getLocation(), diag::err_redefinition) << New;
3698-
Diag(Def->getLocation(), diag::note_previous_definition);
3699-
New->setInvalidDecl();
3700-
return;
3685+
} else if (VarDecl *Def = Old->getDefinition()) {
3686+
if (checkVarDeclRedefinition(Def, New))
3687+
return;
37013688
}
37023689
}
37033690

@@ -3726,6 +3713,32 @@ void Sema::MergeVarDecl(VarDecl *New, LookupResult &Previous) {
37263713
New->setImplicitlyInline();
37273714
}
37283715

3716+
/// We've just determined that \p Old and \p New both appear to be definitions
3717+
/// of the same variable. Either diagnose or fix the problem.
3718+
bool Sema::checkVarDeclRedefinition(VarDecl *Old, VarDecl *New) {
3719+
if (!hasVisibleDefinition(Old) &&
3720+
(New->getFormalLinkage() == InternalLinkage ||
3721+
New->isInline() ||
3722+
New->getDescribedVarTemplate() ||
3723+
New->getNumTemplateParameterLists() ||
3724+
New->getDeclContext()->isDependentContext())) {
3725+
// The previous definition is hidden, and multiple definitions are
3726+
// permitted (in separate TUs). Demote this to a declaration.
3727+
New->demoteThisDefinitionToDeclaration();
3728+
3729+
// Make the canonical definition visible.
3730+
if (auto *OldTD = Old->getDescribedVarTemplate())
3731+
makeMergedDefinitionVisible(OldTD, New->getLocation());
3732+
makeMergedDefinitionVisible(Old, New->getLocation());
3733+
return false;
3734+
} else {
3735+
Diag(New->getLocation(), diag::err_redefinition) << New;
3736+
Diag(Old->getLocation(), diag::note_previous_definition);
3737+
New->setInvalidDecl();
3738+
return true;
3739+
}
3740+
}
3741+
37293742
/// ParsedFreeStandingDeclSpec - This method is invoked when a declspec with
37303743
/// no declarator (e.g. "struct foo;") is parsed.
37313744
Decl *
@@ -9697,25 +9710,15 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init,
96979710
VDecl->setInvalidDecl();
96989711
}
96999712

9713+
// If adding the initializer will turn this declaration into a definition,
9714+
// and we already have a definition for this variable, diagnose or otherwise
9715+
// handle the situation.
97009716
VarDecl *Def;
97019717
if ((Def = VDecl->getDefinition()) && Def != VDecl &&
9702-
(!VDecl->isStaticDataMember() || VDecl->isOutOfLine())) {
9703-
NamedDecl *Hidden = nullptr;
9704-
if (!hasVisibleDefinition(Def, &Hidden) &&
9705-
(VDecl->getFormalLinkage() == InternalLinkage ||
9706-
VDecl->getDescribedVarTemplate() ||
9707-
VDecl->getNumTemplateParameterLists() ||
9708-
VDecl->getDeclContext()->isDependentContext())) {
9709-
// The previous definition is hidden, and multiple definitions are
9710-
// permitted (in separate TUs). Form another definition of it.
9711-
} else {
9712-
Diag(VDecl->getLocation(), diag::err_redefinition)
9713-
<< VDecl->getDeclName();
9714-
Diag(Def->getLocation(), diag::note_previous_definition);
9715-
VDecl->setInvalidDecl();
9716-
return;
9717-
}
9718-
}
9718+
(!VDecl->isStaticDataMember() || VDecl->isOutOfLine()) &&
9719+
!VDecl->isThisDeclarationADemotedDefinition() &&
9720+
checkVarDeclRedefinition(Def, VDecl))
9721+
return;
97199722

97209723
if (getLangOpts().CPlusPlus) {
97219724
// C++ [class.static.data]p4

Diff for: ‎lib/Sema/SemaTemplate.cpp

+34-13
Original file line numberDiff line numberDiff line change
@@ -466,10 +466,14 @@ bool Sema::DiagnoseUninstantiableTemplate(SourceLocation PointOfInstantiation,
466466
const NamedDecl *PatternDef,
467467
TemplateSpecializationKind TSK,
468468
bool Complain /*= true*/) {
469-
assert(isa<TagDecl>(Instantiation) || isa<FunctionDecl>(Instantiation));
469+
assert(isa<TagDecl>(Instantiation) || isa<FunctionDecl>(Instantiation) ||
470+
isa<VarDecl>(Instantiation));
470471

471-
if (PatternDef && (isa<FunctionDecl>(PatternDef)
472-
|| !cast<TagDecl>(PatternDef)->isBeingDefined())) {
472+
bool IsEntityBeingDefined = false;
473+
if (const TagDecl *TD = dyn_cast_or_null<TagDecl>(PatternDef))
474+
IsEntityBeingDefined = TD->isBeingDefined();
475+
476+
if (PatternDef && !IsEntityBeingDefined) {
473477
NamedDecl *SuggestedDef = nullptr;
474478
if (!hasVisibleDefinition(const_cast<NamedDecl*>(PatternDef), &SuggestedDef,
475479
/*OnlyNeedComplete*/false)) {
@@ -486,13 +490,14 @@ bool Sema::DiagnoseUninstantiableTemplate(SourceLocation PointOfInstantiation,
486490
if (!Complain || (PatternDef && PatternDef->isInvalidDecl()))
487491
return true;
488492

493+
llvm::Optional<unsigned> Note;
489494
QualType InstantiationTy;
490495
if (TagDecl *TD = dyn_cast<TagDecl>(Instantiation))
491496
InstantiationTy = Context.getTypeDeclType(TD);
492497
if (PatternDef) {
493498
Diag(PointOfInstantiation,
494499
diag::err_template_instantiate_within_definition)
495-
<< (TSK != TSK_ImplicitInstantiation)
500+
<< /*implicit|explicit*/(TSK != TSK_ImplicitInstantiation)
496501
<< InstantiationTy;
497502
// Not much point in noting the template declaration here, since
498503
// we're lexically inside it.
@@ -501,28 +506,44 @@ bool Sema::DiagnoseUninstantiableTemplate(SourceLocation PointOfInstantiation,
501506
if (isa<FunctionDecl>(Instantiation)) {
502507
Diag(PointOfInstantiation,
503508
diag::err_explicit_instantiation_undefined_member)
504-
<< 1 << Instantiation->getDeclName() << Instantiation->getDeclContext();
509+
<< /*member function*/ 1 << Instantiation->getDeclName()
510+
<< Instantiation->getDeclContext();
511+
Note = diag::note_explicit_instantiation_here;
505512
} else {
513+
assert(isa<TagDecl>(Instantiation) && "Must be a TagDecl!");
506514
Diag(PointOfInstantiation,
507515
diag::err_implicit_instantiate_member_undefined)
508516
<< InstantiationTy;
517+
Note = diag::note_member_declared_at;
509518
}
510-
Diag(Pattern->getLocation(), isa<FunctionDecl>(Instantiation)
511-
? diag::note_explicit_instantiation_here
512-
: diag::note_member_declared_at);
513519
} else {
514-
if (isa<FunctionDecl>(Instantiation))
520+
if (isa<FunctionDecl>(Instantiation)) {
515521
Diag(PointOfInstantiation,
516522
diag::err_explicit_instantiation_undefined_func_template)
517523
<< Pattern;
518-
else
524+
Note = diag::note_explicit_instantiation_here;
525+
} else if (isa<TagDecl>(Instantiation)) {
519526
Diag(PointOfInstantiation, diag::err_template_instantiate_undefined)
520527
<< (TSK != TSK_ImplicitInstantiation)
521528
<< InstantiationTy;
522-
Diag(Pattern->getLocation(), isa<FunctionDecl>(Instantiation)
523-
? diag::note_explicit_instantiation_here
524-
: diag::note_template_decl_here);
529+
Note = diag::note_template_decl_here;
530+
} else {
531+
assert(isa<VarDecl>(Instantiation) && "Must be a VarDecl!");
532+
if (isa<VarTemplateSpecializationDecl>(Instantiation)) {
533+
Diag(PointOfInstantiation,
534+
diag::err_explicit_instantiation_undefined_var_template)
535+
<< Instantiation;
536+
Instantiation->setInvalidDecl();
537+
} else
538+
Diag(PointOfInstantiation,
539+
diag::err_explicit_instantiation_undefined_member)
540+
<< /*static data member*/ 2 << Instantiation->getDeclName()
541+
<< Instantiation->getDeclContext();
542+
Note = diag::note_explicit_instantiation_here;
543+
}
525544
}
545+
if (Note) // Diagnostics were emitted.
546+
Diag(Pattern->getLocation(), Note.getValue());
526547

527548
// In general, Instantiation isn't marked invalid to get more than one
528549
// error for multiple undefined instantiations. But the code that does

Diff for: ‎lib/Sema/SemaTemplateInstantiateDecl.cpp

+18-22
Original file line numberDiff line numberDiff line change
@@ -4068,6 +4068,10 @@ void Sema::InstantiateVariableDefinition(SourceLocation PointOfInstantiation,
40684068
PrettyDeclStackTraceEntry CrashInfo(*this, Var, SourceLocation(),
40694069
"instantiating variable initializer");
40704070

4071+
// The instantiation is visible here, even if it was first declared in an
4072+
// unimported module.
4073+
Var->setHidden(false);
4074+
40714075
// If we're performing recursive template instantiation, create our own
40724076
// queue of pending implicit instantiations that we will instantiate
40734077
// later, while we're still within our own instantiation context.
@@ -4116,33 +4120,17 @@ void Sema::InstantiateVariableDefinition(SourceLocation PointOfInstantiation,
41164120
Def = PatternDecl->getDefinition();
41174121
}
41184122

4119-
// FIXME: Check that the definition is visible before trying to instantiate
4120-
// it. This requires us to track the instantiation stack in order to know
4121-
// which definitions should be visible.
4123+
TemplateSpecializationKind TSK = Var->getTemplateSpecializationKind();
41224124

41234125
// If we don't have a definition of the variable template, we won't perform
41244126
// any instantiation. Rather, we rely on the user to instantiate this
41254127
// definition (or provide a specialization for it) in another translation
41264128
// unit.
4127-
if (!Def) {
4128-
if (DefinitionRequired) {
4129-
if (VarSpec) {
4130-
Diag(PointOfInstantiation,
4131-
diag::err_explicit_instantiation_undefined_var_template) << Var;
4132-
Var->setInvalidDecl();
4133-
}
4134-
else
4135-
Diag(PointOfInstantiation,
4136-
diag::err_explicit_instantiation_undefined_member)
4137-
<< 2 << Var->getDeclName() << Var->getDeclContext();
4138-
Diag(PatternDecl->getLocation(),
4139-
diag::note_explicit_instantiation_here);
4140-
} else if (Var->getTemplateSpecializationKind()
4141-
== TSK_ExplicitInstantiationDefinition) {
4129+
if (!Def && !DefinitionRequired) {
4130+
if (TSK == TSK_ExplicitInstantiationDefinition) {
41424131
PendingInstantiations.push_back(
41434132
std::make_pair(Var, PointOfInstantiation));
4144-
} else if (Var->getTemplateSpecializationKind()
4145-
== TSK_ImplicitInstantiation) {
4133+
} else if (TSK == TSK_ImplicitInstantiation) {
41464134
// Warn about missing definition at the end of translation unit.
41474135
if (AtEndOfTU && !getDiagnostics().hasErrorOccurred()) {
41484136
Diag(PointOfInstantiation, diag::warn_var_template_missing)
@@ -4151,12 +4139,20 @@ void Sema::InstantiateVariableDefinition(SourceLocation PointOfInstantiation,
41514139
if (getLangOpts().CPlusPlus11)
41524140
Diag(PointOfInstantiation, diag::note_inst_declaration_hint) << Var;
41534141
}
4142+
return;
41544143
}
41554144

4156-
return;
41574145
}
41584146

4159-
TemplateSpecializationKind TSK = Var->getTemplateSpecializationKind();
4147+
// FIXME: We need to track the instantiation stack in order to know which
4148+
// definitions should be visible within this instantiation.
4149+
// FIXME: Produce diagnostics when Var->getInstantiatedFromStaticDataMember().
4150+
if (DiagnoseUninstantiableTemplate(PointOfInstantiation, Var,
4151+
/*InstantiatedFromMember*/false,
4152+
PatternDecl, Def, TSK,
4153+
/*Complain*/DefinitionRequired))
4154+
return;
4155+
41604156

41614157
// Never instantiate an explicit specialization.
41624158
if (TSK == TSK_ExplicitSpecialization)

0 commit comments

Comments
 (0)