Skip to content
This repository has been archived by the owner on Mar 30, 2021. It is now read-only.

Commit

Permalink
Merge pull request #482 from martong/fix_VarDecl
Browse files Browse the repository at this point in the history 
Fix VarDecl Import
  • Loading branch information
@martong
martong authored Aug 31, 2018
2 parents a3f59b8 + 5d2169a commit 4cf7f40
Show file tree
Hide file tree
Showing 2 changed files with 214 additions and 66 deletions.
136 changes: 72 additions & 64 deletions lib/AST/ASTImporter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,11 @@ namespace clang {
}

SmallVector<Decl*, 2> getCanonicalForwardRedeclChain(Decl* D) {
// Currently only FunctionDecl is supported
auto FD = cast<FunctionDecl>(D);
return getCanonicalForwardRedeclChain<FunctionDecl>(FD);
if (auto *FD = dyn_cast<FunctionDecl>(D))
return getCanonicalForwardRedeclChain<FunctionDecl>(FD);
if (auto *VD = dyn_cast<VarDecl>(D))
return getCanonicalForwardRedeclChain<VarDecl>(VD);
llvm_unreachable("Bad declaration kind");
}

void updateAttrAndFlags(const Decl *From, Decl *To) {
Expand Down Expand Up @@ -235,9 +237,8 @@ namespace clang {
(IDK == IDK_Default && !Importer.isMinimalImport());
}

bool ImportDefinition(RecordDecl *From, RecordDecl *To,
ImportDefinitionKind Kind = IDK_Default);
bool ImportDefinition(VarDecl *From, VarDecl *To,
bool ImportInitializer(VarDecl *From, VarDecl *To);
bool ImportDefinition(RecordDecl *From, RecordDecl *To,
ImportDefinitionKind Kind = IDK_Default);
bool ImportDefinition(EnumDecl *From, EnumDecl *To,
ImportDefinitionKind Kind = IDK_Default);
Expand Down Expand Up @@ -1416,18 +1417,26 @@ bool ASTNodeImporter::ImportDefinition(RecordDecl *From, RecordDecl *To,
return false;
}

bool ASTNodeImporter::ImportDefinition(VarDecl *From, VarDecl *To,
ImportDefinitionKind Kind) {
bool ASTNodeImporter::ImportInitializer(VarDecl *From, VarDecl *To) {
if (To->getAnyInitializer())
return false;

// FIXME: Can we really import any initializer? Alternatively, we could force
// ourselves to import every declaration of a variable and then only use
// getInit() here.
To->setInit(Importer.Import(const_cast<Expr *>(From->getAnyInitializer())));
Expr *FromInit = From->getInit();
if (!FromInit)
return false;

Expr *ToInit = Importer.Import(const_cast<Expr *>(FromInit));
if (!ToInit)
return true;

// FIXME: Other bits to merge?
To->setInit(ToInit);
if (From->isInitKnownICE()) {
EvaluatedStmt *Eval = To->ensureEvaluatedStmt();
Eval->CheckedICE = true;
Eval->IsICE = From->isInitICE();
}

// FIXME: Other bits to merge?
return false;
}

Expand Down Expand Up @@ -3166,6 +3175,16 @@ Decl *ASTNodeImporter::VisitObjCIvarDecl(ObjCIvarDecl *D) {
}

Decl *ASTNodeImporter::VisitVarDecl(VarDecl *D) {

SmallVector<Decl*, 2> Redecls = getCanonicalForwardRedeclChain(D);
auto RedeclIt = Redecls.begin();
// Import the first part of the decl chain. I.e. import all previous
// declarations starting from the canonical decl.
for (; RedeclIt != Redecls.end() && *RedeclIt != D; ++RedeclIt)
if (!Importer.Import(*RedeclIt))
return nullptr;
assert(*RedeclIt == D);

// Import the major distinguishing characteristics of a variable.
DeclContext *DC, *LexicalDC;
DeclarationName Name;
Expand All @@ -3178,8 +3197,8 @@ Decl *ASTNodeImporter::VisitVarDecl(VarDecl *D) {

// Try to find a variable in our own ("to") context with the same name and
// in the same context as the variable we're importing.
VarDecl *FoundByLookup = nullptr;
if (D->isFileVarDecl()) {
VarDecl *MergeWithVar = nullptr;
SmallVector<NamedDecl *, 4> ConflictingDecls;
unsigned IDNS = Decl::IDNS_Ordinary;
SmallVector<NamedDecl *, 2> FoundDecls;
Expand All @@ -3194,7 +3213,23 @@ Decl *ASTNodeImporter::VisitVarDecl(VarDecl *D) {
D->hasExternalFormalLinkage()) {
if (Importer.IsStructurallyEquivalent(D->getType(),
FoundVar->getType())) {
MergeWithVar = FoundVar;

// The VarDecl in the "From" context has a definition, but in the
// "To" context we already has a definition.
VarDecl *FoundDef = FoundVar->getDefinition();
if (D->isThisDeclarationADefinition() && FoundDef)
// FIXME Check for ODR error if the two definitions have
// different initializers?
return Importer.MapImported(D, FoundDef);

// The VarDecl in the "From" context has an initializer, but in the
// "To" context we already has an initializer.
const VarDecl *FoundDInit = nullptr;
if (D->getInit() && FoundVar->getAnyInitializer(FoundDInit))
// FIXME Diagnose ODR error if the two initializers are different?
return Importer.MapImported(D, const_cast<VarDecl*>(FoundDInit));

FoundByLookup = FoundVar;
break;
}

Expand All @@ -3211,11 +3246,11 @@ Decl *ASTNodeImporter::VisitVarDecl(VarDecl *D) {
return nullptr;

FoundVar->setType(T);
MergeWithVar = FoundVar;
FoundByLookup = FoundVar;
break;
} else if (isa<IncompleteArrayType>(TArray) &&
isa<ConstantArrayType>(FoundArray)) {
MergeWithVar = FoundVar;
FoundByLookup = FoundVar;
break;
}
}
Expand All @@ -3226,52 +3261,19 @@ Decl *ASTNodeImporter::VisitVarDecl(VarDecl *D) {
<< FoundVar->getType();
}
}

ConflictingDecls.push_back(FoundDecls[I]);
}

if (MergeWithVar) {
// An equivalent variable with external linkage has been found. Link
// the two declarations, then merge them.
Importer.MapImported(D, MergeWithVar);
updateAttrAndFlags(D, MergeWithVar);

if (VarDecl *DDef = D->getDefinition()) {
if (VarDecl *ExistingDef = MergeWithVar->getDefinition()) {
// TODO: Somehow the check bellow is required. I suspect that,
// the variable has multiple declarations, and while we import the
// declaration that is also a definition, we only add one of the
// declaration to the imported decls (which is not the definition).
if (Importer.Import(DDef->getLocation()) !=
ExistingDef->getLocation()) {
Importer.ToDiag(ExistingDef->getLocation(),
diag::err_odr_variable_multiple_def)
<< Name;
Importer.FromDiag(DDef->getLocation(), diag::note_odr_defined_here);
}
} else {
Expr *Init = Importer.Import(DDef->getInit());
MergeWithVar->setInit(Init);
if (DDef->isInitKnownICE()) {
EvaluatedStmt *Eval = MergeWithVar->ensureEvaluatedStmt();
Eval->CheckedICE = true;
Eval->IsICE = DDef->isInitICE();
}
}
}

return MergeWithVar;
}

if (!ConflictingDecls.empty()) {
Name = Importer.HandleNameConflict(Name, DC, IDNS,
ConflictingDecls.data(),
ConflictingDecls.data(),
ConflictingDecls.size());
if (!Name)
return nullptr;
}
}

// Import the type.
QualType T = Importer.Import(D->getType());
if (T.isNull())
Expand All @@ -3291,17 +3293,27 @@ Decl *ASTNodeImporter::VisitVarDecl(VarDecl *D) {
ToVar->setAccess(D->getAccess());
ToVar->setLexicalDeclContext(LexicalDC);

// Templated declarations should never appear in the enclosing DeclContext.
if (!D->getDescribedVarTemplate())
LexicalDC->addDeclInternal(ToVar);
if (FoundByLookup) {
auto *Recent = const_cast<VarDecl *>(FoundByLookup->getMostRecentDecl());
ToVar->setPreviousDecl(Recent);
}

// Merge the initializer.
if (ImportDefinition(D, ToVar))
if (ImportInitializer(D, ToVar))
return nullptr;

if (D->isConstexpr())
ToVar->setConstexpr(true);

if (D->getDeclContext()->containsDeclAndLoad(D))
DC->addDeclInternal(ToVar);
if (DC != LexicalDC && D->getLexicalDeclContext()->containsDeclAndLoad(D))
LexicalDC->addDeclInternal(ToVar);

// Import the rest of the chain. I.e. import all subsequent declarations.
for (++RedeclIt; RedeclIt != Redecls.end(); ++RedeclIt)
if (!Importer.Import(*RedeclIt))
return nullptr;

return ToVar;
}

Expand Down Expand Up @@ -4958,12 +4970,7 @@ Decl *ASTNodeImporter::VisitVarTemplateSpecializationDecl(
D2->setAccess(D->getAccess());
}

// NOTE: isThisDeclarationADefinition() can return DeclarationOnly even if
// declaration has initializer. Should this be fixed in the AST?.. Anyway,
// we have to check the declaration for initializer - otherwise, it won't be
// imported.
if ((D->isThisDeclarationADefinition() || D->hasInit()) &&
ImportDefinition(D, D2))
if (ImportInitializer(D, D2))
return nullptr;

return D2;
Expand Down Expand Up @@ -7224,6 +7231,7 @@ Decl *ASTImporter::Import(Decl *FromD) {
// Notify subclasses.
Imported(FromD, ToD);

updateAttrAndFlags(FromD, ToD);
return ToD;
}

Expand Down
Loading

0 comments on commit 4cf7f40

Please sign in to comment.