diff --git a/lib/AST/ASTImporter.cpp b/lib/AST/ASTImporter.cpp index 6bd32ea05fe0..55b76243e9a3 100644 --- a/lib/AST/ASTImporter.cpp +++ b/lib/AST/ASTImporter.cpp @@ -64,9 +64,11 @@ namespace clang { } SmallVector getCanonicalForwardRedeclChain(Decl* D) { - // Currently only FunctionDecl is supported - auto FD = cast(D); - return getCanonicalForwardRedeclChain(FD); + if (auto *FD = dyn_cast(D)) + return getCanonicalForwardRedeclChain(FD); + if (auto *VD = dyn_cast(D)) + return getCanonicalForwardRedeclChain(VD); + llvm_unreachable("Bad declaration kind"); } void updateAttrAndFlags(const Decl *From, Decl *To) { @@ -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); @@ -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(From->getAnyInitializer()))); + Expr *FromInit = From->getInit(); + if (!FromInit) + return false; + + Expr *ToInit = Importer.Import(const_cast(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; } @@ -3166,6 +3175,16 @@ Decl *ASTNodeImporter::VisitObjCIvarDecl(ObjCIvarDecl *D) { } Decl *ASTNodeImporter::VisitVarDecl(VarDecl *D) { + + SmallVector 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; @@ -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 ConflictingDecls; unsigned IDNS = Decl::IDNS_Ordinary; SmallVector FoundDecls; @@ -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(FoundDInit)); + + FoundByLookup = FoundVar; break; } @@ -3211,11 +3246,11 @@ Decl *ASTNodeImporter::VisitVarDecl(VarDecl *D) { return nullptr; FoundVar->setType(T); - MergeWithVar = FoundVar; + FoundByLookup = FoundVar; break; } else if (isa(TArray) && isa(FoundArray)) { - MergeWithVar = FoundVar; + FoundByLookup = FoundVar; break; } } @@ -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()) @@ -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(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; } @@ -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; @@ -7224,6 +7231,7 @@ Decl *ASTImporter::Import(Decl *FromD) { // Notify subclasses. Imported(FromD, ToD); + updateAttrAndFlags(FromD, ToD); return ToD; } diff --git a/unittests/AST/ASTImporterTest.cpp b/unittests/AST/ASTImporterTest.cpp index f27098c16a8d..0d8615a10180 100644 --- a/unittests/AST/ASTImporterTest.cpp +++ b/unittests/AST/ASTImporterTest.cpp @@ -3093,13 +3093,62 @@ TEST_P(ASTImporterTestBase, ImportDoesUpdateUsedFlag) { { Decl *FromTU = getTuDecl( "extern int x; int f() { return x; }", Lang_CXX, "input2.cc"); - auto *FromD = - FirstDeclMatcher().match(FromTU, functionDecl()); + auto *FromD = FirstDeclMatcher().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().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().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().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().match( + FromTU, functionDecl(hasName("f"))); + auto *FromD = FirstDeclMatcher().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")); @@ -3983,6 +4032,94 @@ TEST_P(DeclContextTest, removeDeclOfClassTemplateSpecialization) { EXPECT_FALSE(NS->containsDecl(Spec)); } +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().match( + FromTU, varDecl(hasName("a"))); // Decl with init + auto *FromDWithDef = LastDeclMatcher().match( + FromTU, varDecl(hasName("a"))); // Decl with definition + ASSERT_NE(FromDWithInit, FromDWithDef); + ASSERT_EQ(FromDWithDef->getPreviousDecl() ,FromDWithInit); + + auto *ToD0 = cast(Import(FromDWithInit, Lang_CXX11)); + auto *ToD1 = cast(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().match( + FromTU, varDecl(hasName("a"))); // Decl with init + auto *FromDWithDef = LastDeclMatcher().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().match( + ToTU, varDecl(hasName("a"))); // Decl with init + ASSERT_TRUE(ToD->getInit()); + ASSERT_FALSE(ToD->getDefinition()); + + auto *ImportedD = cast(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().match( + FromTU, varDecl(hasName("a"))); + auto *FromDWithDef = LastDeclMatcher().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().match( + ToTU, varDecl(hasName("a"))); + ASSERT_FALSE(ToD->getInit()); + ASSERT_FALSE(ToD->getDefinition()); + + auto *ImportedD = cast(Import(FromDWithDef, Lang_CXX11)); + EXPECT_TRUE(ImportedD->getAnyInitializer()); + EXPECT_TRUE(ImportedD->getDefinition()); +} + INSTANTIATE_TEST_CASE_P(ParameterizedTests, DeclContextTest, ::testing::Values(ArgVector()), ); @@ -4019,5 +4156,8 @@ INSTANTIATE_TEST_CASE_P(ParameterizedTests, CanonicalRedeclChain, INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportImplicitMethods, DefaultTestValuesForRunOptions, ); +INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportVariables, + DefaultTestValuesForRunOptions, ); + } // end namespace ast_matchers } // end namespace clang