r342384 - [ASTImporter] Fix import of VarDecl init
Gabor Marton via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 17 05:04:52 PDT 2018
Author: martong
Date: Mon Sep 17 05:04:52 2018
New Revision: 342384
URL: http://llvm.org/viewvc/llvm-project?rev=342384&view=rev
Log:
[ASTImporter] Fix import of VarDecl init
Summary:
The init expression of a VarDecl is overwritten in the "To" context if we
import a VarDecl without an init expression (and with a definition). Please
refer to the added tests, especially InitAndDefinitionAreInDifferentTUs. This
patch fixes the malfunction by importing the whole Decl chain similarly as we
did that in case of FunctionDecls. We handle the init expression similarly to
a definition, alas only one init expression will be in the merged ast.
Reviewers: a_sidorin, xazax.hun, r.stahl, a.sidorin
Subscribers: rnkovacs, dkrupp, cfe-commits
Differential Revision: https://reviews.llvm.org/D51597
Modified:
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/unittests/AST/ASTImporterTest.cpp
Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=342384&r1=342383&r2=342384&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Mon Sep 17 05:04:52 2018
@@ -107,9 +107,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 updateFlags(const Decl *From, Decl *To) {
@@ -280,10 +282,9 @@ namespace clang {
(IDK == IDK_Default && !Importer.isMinimalImport());
}
+ bool ImportInitializer(VarDecl *From, VarDecl *To);
bool ImportDefinition(RecordDecl *From, RecordDecl *To,
ImportDefinitionKind Kind = IDK_Default);
- bool ImportDefinition(VarDecl *From, VarDecl *To,
- ImportDefinitionKind Kind = IDK_Default);
bool ImportDefinition(EnumDecl *From, EnumDecl *To,
ImportDefinitionKind Kind = IDK_Default);
bool ImportDefinition(ObjCInterfaceDecl *From, ObjCInterfaceDecl *To,
@@ -1424,18 +1425,26 @@ bool ASTNodeImporter::ImportDefinition(R
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;
- // FIXME: Other bits to merge?
+ Expr *ToInit = Importer.Import(const_cast<Expr *>(FromInit));
+ if (!ToInit)
+ return true;
+ To->setInit(ToInit);
+ if (From->isInitKnownICE()) {
+ EvaluatedStmt *Eval = To->ensureEvaluatedStmt();
+ Eval->CheckedICE = true;
+ Eval->IsICE = From->isInitICE();
+ }
+
+ // FIXME: Other bits to merge?
return false;
}
@@ -3138,6 +3147,16 @@ Decl *ASTNodeImporter::VisitObjCIvarDecl
}
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;
@@ -3150,8 +3169,8 @@ Decl *ASTNodeImporter::VisitVarDecl(VarD
// 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;
@@ -3166,7 +3185,23 @@ Decl *ASTNodeImporter::VisitVarDecl(VarD
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 have 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 have 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;
}
@@ -3183,11 +3218,11 @@ Decl *ASTNodeImporter::VisitVarDecl(VarD
return nullptr;
FoundVar->setType(T);
- MergeWithVar = FoundVar;
+ FoundByLookup = FoundVar;
break;
} else if (isa<IncompleteArrayType>(TArray) &&
isa<ConstantArrayType>(FoundArray)) {
- MergeWithVar = FoundVar;
+ FoundByLookup = FoundVar;
break;
}
}
@@ -3202,32 +3237,6 @@ Decl *ASTNodeImporter::VisitVarDecl(VarD
ConflictingDecls.push_back(FoundDecl);
}
- if (MergeWithVar) {
- // An equivalent variable with external linkage has been found. Link
- // the two declarations, then merge them.
- Importer.MapImported(D, MergeWithVar);
- updateFlags(D, MergeWithVar);
-
- if (VarDecl *DDef = D->getDefinition()) {
- if (VarDecl *ExistingDef = MergeWithVar->getDefinition()) {
- 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(),
@@ -3255,17 +3264,27 @@ Decl *ASTNodeImporter::VisitVarDecl(VarD
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;
}
@@ -4914,12 +4933,7 @@ Decl *ASTNodeImporter::VisitVarTemplateS
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;
@@ -7084,6 +7098,7 @@ Decl *ASTImporter::Import(Decl *FromD) {
// Notify subclasses.
Imported(FromD, ToD);
+ updateFlags(FromD, ToD);
return ToD;
}
Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=342384&r1=342383&r2=342384&view=diff
==============================================================================
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Mon Sep 17 05:04:52 2018
@@ -1828,13 +1828,62 @@ TEST_P(ASTImporterTestBase, ImportDoesUp
{
Decl *FromTU =
getTuDecl("extern int x; int f() { return x; }", Lang_CXX, "input2.cc");
- auto *FromD =
- FirstDeclMatcher<FunctionDecl>().match(FromTU, functionDecl());
+ auto *FromD = FirstDeclMatcher<FunctionDecl>().match(
+ FromTU, functionDecl(hasName("f")));
Import(FromD, Lang_CXX);
}
EXPECT_TRUE(Imported2->isUsed(false));
}
+TEST_P(ASTImporterTestBase, ImportDoesUpdateUsedFlag2) {
+ auto Pattern = varDecl(hasName("x"));
+ VarDecl *ExistingD;
+ {
+ Decl *ToTU = getToTuDecl("int x = 1;", Lang_CXX);
+ ExistingD = FirstDeclMatcher<VarDecl>().match(ToTU, Pattern);
+ }
+ EXPECT_FALSE(ExistingD->isUsed(false));
+ {
+ Decl *FromTU = getTuDecl(
+ "int x = 1; int f() { return x; }", Lang_CXX, "input1.cc");
+ auto *FromD = FirstDeclMatcher<FunctionDecl>().match(
+ FromTU, functionDecl(hasName("f")));
+ Import(FromD, Lang_CXX);
+ }
+ EXPECT_TRUE(ExistingD->isUsed(false));
+}
+
+TEST_P(ASTImporterTestBase, ImportDoesUpdateUsedFlag3) {
+ auto Pattern = varDecl(hasName("a"));
+ VarDecl *ExistingD;
+ {
+ Decl *ToTU = getToTuDecl(
+ R"(
+ struct A {
+ static const int a = 1;
+ };
+ )", Lang_CXX);
+ ExistingD = FirstDeclMatcher<VarDecl>().match(ToTU, Pattern);
+ }
+ EXPECT_FALSE(ExistingD->isUsed(false));
+ {
+ Decl *FromTU = getTuDecl(
+ R"(
+ struct A {
+ static const int a = 1;
+ };
+ const int *f() { return &A::a; } // requires storage,
+ // thus used flag will be set
+ )", Lang_CXX, "input1.cc");
+ auto *FromFunD = FirstDeclMatcher<FunctionDecl>().match(
+ FromTU, functionDecl(hasName("f")));
+ auto *FromD = FirstDeclMatcher<VarDecl>().match(FromTU, Pattern);
+ ASSERT_TRUE(FromD->isUsed(false));
+ Import(FromFunD, Lang_CXX);
+ }
+ EXPECT_TRUE(ExistingD->isUsed(false));
+}
+
TEST_P(ASTImporterTestBase, ReimportWithUsedFlag) {
auto Pattern = varDecl(hasName("x"));
@@ -3244,6 +3293,94 @@ TEST_P(ASTImporterTestBase, InitListExpr
EXPECT_TRUE(ToInitExpr->isGLValue());
}
+struct ImportVariables : ASTImporterTestBase {};
+
+TEST_P(ImportVariables, ImportOfOneDeclBringsInTheWholeChain) {
+ Decl *FromTU = getTuDecl(
+ R"(
+ struct A {
+ static const int a = 1 + 2;
+ };
+ const int A::a;
+ )", Lang_CXX, "input1.cc");
+
+ auto *FromDWithInit = FirstDeclMatcher<VarDecl>().match(
+ FromTU, varDecl(hasName("a"))); // Decl with init
+ auto *FromDWithDef = LastDeclMatcher<VarDecl>().match(
+ FromTU, varDecl(hasName("a"))); // Decl with definition
+ ASSERT_NE(FromDWithInit, FromDWithDef);
+ ASSERT_EQ(FromDWithDef->getPreviousDecl(), FromDWithInit);
+
+ auto *ToD0 = cast<VarDecl>(Import(FromDWithInit, Lang_CXX11));
+ auto *ToD1 = cast<VarDecl>(Import(FromDWithDef, Lang_CXX11));
+ ASSERT_TRUE(ToD0);
+ ASSERT_TRUE(ToD1);
+ EXPECT_NE(ToD0, ToD1);
+ EXPECT_EQ(ToD1->getPreviousDecl(), ToD0);
+}
+
+TEST_P(ImportVariables, InitAndDefinitionAreInDifferentTUs) {
+ auto StructA =
+ R"(
+ struct A {
+ static const int a = 1 + 2;
+ };
+ )";
+ Decl *ToTU = getToTuDecl(StructA, Lang_CXX);
+ Decl *FromTU = getTuDecl(std::string(StructA) + "const int A::a;", Lang_CXX,
+ "input1.cc");
+
+ auto *FromDWithInit = FirstDeclMatcher<VarDecl>().match(
+ FromTU, varDecl(hasName("a"))); // Decl with init
+ auto *FromDWithDef = LastDeclMatcher<VarDecl>().match(
+ FromTU, varDecl(hasName("a"))); // Decl with definition
+ ASSERT_EQ(FromDWithInit, FromDWithDef->getPreviousDecl());
+ ASSERT_TRUE(FromDWithInit->getInit());
+ ASSERT_FALSE(FromDWithInit->isThisDeclarationADefinition());
+ ASSERT_TRUE(FromDWithDef->isThisDeclarationADefinition());
+ ASSERT_FALSE(FromDWithDef->getInit());
+
+ auto *ToD = FirstDeclMatcher<VarDecl>().match(
+ ToTU, varDecl(hasName("a"))); // Decl with init
+ ASSERT_TRUE(ToD->getInit());
+ ASSERT_FALSE(ToD->getDefinition());
+
+ auto *ImportedD = cast<VarDecl>(Import(FromDWithDef, Lang_CXX11));
+ EXPECT_TRUE(ImportedD->getAnyInitializer());
+ EXPECT_TRUE(ImportedD->getDefinition());
+}
+
+TEST_P(ImportVariables, InitAndDefinitionAreInTheFromContext) {
+ auto StructA =
+ R"(
+ struct A {
+ static const int a;
+ };
+ )";
+ Decl *ToTU = getToTuDecl(StructA, Lang_CXX);
+ Decl *FromTU = getTuDecl(std::string(StructA) + "const int A::a = 1 + 2;",
+ Lang_CXX, "input1.cc");
+
+ auto *FromDDeclarationOnly = FirstDeclMatcher<VarDecl>().match(
+ FromTU, varDecl(hasName("a")));
+ auto *FromDWithDef = LastDeclMatcher<VarDecl>().match(
+ FromTU, varDecl(hasName("a"))); // Decl with definition and with init.
+ ASSERT_EQ(FromDDeclarationOnly, FromDWithDef->getPreviousDecl());
+ ASSERT_FALSE(FromDDeclarationOnly->getInit());
+ ASSERT_FALSE(FromDDeclarationOnly->isThisDeclarationADefinition());
+ ASSERT_TRUE(FromDWithDef->isThisDeclarationADefinition());
+ ASSERT_TRUE(FromDWithDef->getInit());
+
+ auto *ToD = FirstDeclMatcher<VarDecl>().match(
+ ToTU, varDecl(hasName("a")));
+ ASSERT_FALSE(ToD->getInit());
+ ASSERT_FALSE(ToD->getDefinition());
+
+ auto *ImportedD = cast<VarDecl>(Import(FromDWithDef, Lang_CXX11));
+ EXPECT_TRUE(ImportedD->getAnyInitializer());
+ EXPECT_TRUE(ImportedD->getDefinition());
+}
+
struct DeclContextTest : ASTImporterTestBase {};
TEST_P(DeclContextTest, removeDeclOfClassTemplateSpecialization) {
@@ -3623,5 +3760,11 @@ INSTANTIATE_TEST_CASE_P(ParameterizedTes
ImportFunctionTemplateSpecializations,
DefaultTestValuesForRunOptions, );
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportImplicitMethods,
+ DefaultTestValuesForRunOptions, );
+
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportVariables,
+ DefaultTestValuesForRunOptions, );
+
} // end namespace ast_matchers
} // end namespace clang
More information about the cfe-commits
mailing list