Index: lib/AST/ASTImporter.cpp =================================================================== --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -107,9 +107,11 @@ } 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 updateFlags(const Decl *From, Decl *To) { @@ -280,10 +282,9 @@ (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 @@ 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; - // FIXME: Other bits to merge? + Expr *ToInit = Importer.Import(const_cast(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::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; @@ -3150,8 +3169,8 @@ // 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; @@ -3166,7 +3185,23 @@ 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(FoundDInit)); + + FoundByLookup = FoundVar; break; } @@ -3183,11 +3218,11 @@ return nullptr; FoundVar->setType(T); - MergeWithVar = FoundVar; + FoundByLookup = FoundVar; break; } else if (isa(TArray) && isa(FoundArray)) { - MergeWithVar = FoundVar; + FoundByLookup = FoundVar; break; } } @@ -3202,32 +3237,6 @@ 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 @@ 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; } @@ -4914,12 +4933,7 @@ 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 @@ // Notify subclasses. Imported(FromD, ToD); + updateFlags(FromD, ToD); return ToD; } Index: unittests/AST/ASTImporterTest.cpp =================================================================== --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1828,13 +1828,62 @@ { 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")); @@ -3244,6 +3293,94 @@ 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().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()); +} + struct DeclContextTest : ASTImporterTestBase {}; TEST_P(DeclContextTest, removeDeclOfClassTemplateSpecialization) { @@ -3623,5 +3760,11 @@ ImportFunctionTemplateSpecializations, DefaultTestValuesForRunOptions, ); +INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportImplicitMethods, + DefaultTestValuesForRunOptions, ); + +INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportVariables, + DefaultTestValuesForRunOptions, ); + } // end namespace ast_matchers } // end namespace clang