Index: include/clang/AST/ASTImporter.h =================================================================== --- include/clang/AST/ASTImporter.h +++ include/clang/AST/ASTImporter.h @@ -32,6 +32,7 @@ namespace clang { class ASTContext; +class Attr; class ASTImporterLookupTable; class CXXBaseSpecifier; class CXXCtorInitializer; @@ -42,8 +43,8 @@ class NamedDecl; class Stmt; class TagDecl; +class TranslationUnitDecl; class TypeSourceInfo; -class Attr; class ImportError : public llvm::ErrorInfo { public: @@ -115,6 +116,10 @@ /// context to the corresponding declarations in the "to" context. llvm::DenseMap ImportedDecls; + /// Mapping from the already-imported declarations in the "to" + /// context to the corresponding declarations in the "from" context. + llvm::DenseMap ImportedFromDecls; + /// Mapping from the already-imported statements in the "from" /// context to the corresponding statements in the "to" context. llvm::DenseMap ImportedStmts; @@ -226,9 +231,13 @@ /// Return the copy of the given declaration in the "to" context if /// it has already been imported from the "from" context. Otherwise return - /// NULL. + /// nullptr. Decl *GetAlreadyImportedOrNull(const Decl *FromD) const; + /// Return the translation unit from where the declaration was + /// imported. If it does not exist nullptr is returned. + TranslationUnitDecl *GetFromTU(Decl *ToD); + /// Import the given declaration context from the "from" /// AST context into the "to" AST context. /// Index: lib/AST/ASTImporter.cpp =================================================================== --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -437,6 +437,9 @@ Error ImportTemplateInformation(FunctionDecl *FromFD, FunctionDecl *ToFD); + template + bool hasSameVisibilityContext(T *Found, T *From); + bool IsStructuralMatch(Decl *From, Decl *To, bool Complain); bool IsStructuralMatch(RecordDecl *FromRecord, RecordDecl *ToRecord, bool Complain = true); @@ -2944,6 +2947,19 @@ return FoundSpec; } +template +bool ASTNodeImporter::hasSameVisibilityContext(T *Found, T *From) { + if (From->hasExternalFormalLinkage()) + return Found->hasExternalFormalLinkage(); + if (Importer.GetFromTU(Found) != From->getTranslationUnitDecl()) + return false; + if (From->isInAnonymousNamespace()) + return Found->isInAnonymousNamespace(); + else + return !Found->isInAnonymousNamespace() && + !Found->hasExternalFormalLinkage(); +} + ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) { SmallVector Redecls = getCanonicalForwardRedeclChain(D); @@ -2997,33 +3013,30 @@ continue; if (auto *FoundFunction = dyn_cast(FoundDecl)) { - if (FoundFunction->hasExternalFormalLinkage() && - D->hasExternalFormalLinkage()) { - if (IsStructuralMatch(D, FoundFunction)) { - const FunctionDecl *Definition = nullptr; - if (D->doesThisDeclarationHaveABody() && - FoundFunction->hasBody(Definition)) { - return Importer.MapImported( - D, const_cast(Definition)); - } - FoundByLookup = FoundFunction; - break; - } - - // FIXME: Check for overloading more carefully, e.g., by boosting - // Sema::IsOverload out to the AST library. + if (!hasSameVisibilityContext(FoundFunction, D)) + continue; + + if (IsStructuralMatch(D, FoundFunction)) { + const FunctionDecl *Definition = nullptr; + if (D->doesThisDeclarationHaveABody() && + FoundFunction->hasBody(Definition)) + return Importer.MapImported(D, + const_cast(Definition)); + FoundByLookup = FoundFunction; + break; + } + // FIXME: Check for overloading more carefully, e.g., by boosting + // Sema::IsOverload out to the AST library. - // Function overloading is okay in C++. - if (Importer.getToContext().getLangOpts().CPlusPlus) - continue; + // Function overloading is okay in C++. + if (Importer.getToContext().getLangOpts().CPlusPlus) + continue; - // Complain about inconsistent function types. - Importer.ToDiag(Loc, diag::err_odr_function_type_inconsistent) + // Complain about inconsistent function types. + Importer.ToDiag(Loc, diag::err_odr_function_type_inconsistent) << Name << D->getType() << FoundFunction->getType(); - Importer.ToDiag(FoundFunction->getLocation(), - diag::note_odr_value_here) + Importer.ToDiag(FoundFunction->getLocation(), diag::note_odr_value_here) << FoundFunction->getType(); - } } ConflictingDecls.push_back(FoundDecl); @@ -3579,58 +3592,56 @@ continue; if (auto *FoundVar = dyn_cast(FoundDecl)) { - // We have found a variable that we may need to merge with. Check it. - if (FoundVar->hasExternalFormalLinkage() && - D->hasExternalFormalLinkage()) { - if (Importer.IsStructurallyEquivalent(D->getType(), - FoundVar->getType())) { - - // 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)); + if (!hasSameVisibilityContext(FoundVar, D)) + continue; + if (Importer.IsStructurallyEquivalent(D->getType(), + FoundVar->getType())) { + + // 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; + } + + const ArrayType *FoundArray + = Importer.getToContext().getAsArrayType(FoundVar->getType()); + const ArrayType *TArray + = Importer.getToContext().getAsArrayType(D->getType()); + if (FoundArray && TArray) { + if (isa(FoundArray) && + isa(TArray)) { + // Import the type. + if (auto TyOrErr = import(D->getType())) + FoundVar->setType(*TyOrErr); + else + return TyOrErr.takeError(); FoundByLookup = FoundVar; break; + } else if (isa(TArray) && + isa(FoundArray)) { + FoundByLookup = FoundVar; + break; } - - const ArrayType *FoundArray - = Importer.getToContext().getAsArrayType(FoundVar->getType()); - const ArrayType *TArray - = Importer.getToContext().getAsArrayType(D->getType()); - if (FoundArray && TArray) { - if (isa(FoundArray) && - isa(TArray)) { - // Import the type. - if (auto TyOrErr = import(D->getType())) - FoundVar->setType(*TyOrErr); - else - return TyOrErr.takeError(); - - FoundByLookup = FoundVar; - break; - } else if (isa(TArray) && - isa(FoundArray)) { - FoundByLookup = FoundVar; - break; - } - } - - Importer.ToDiag(Loc, diag::err_odr_variable_type_inconsistent) - << Name << D->getType() << FoundVar->getType(); - Importer.ToDiag(FoundVar->getLocation(), diag::note_odr_value_here) - << FoundVar->getType(); } + + Importer.ToDiag(Loc, diag::err_odr_variable_type_inconsistent) + << Name << D->getType() << FoundVar->getType(); + Importer.ToDiag(FoundVar->getLocation(), diag::note_odr_value_here) + << FoundVar->getType(); } ConflictingDecls.push_back(FoundDecl); @@ -7761,6 +7772,13 @@ return nullptr; } +TranslationUnitDecl *ASTImporter::GetFromTU(Decl *ToD) { + auto FromDPos = ImportedFromDecls.find(ToD); + if (FromDPos == ImportedFromDecls.end()) + return nullptr; + return FromDPos->second->getTranslationUnitDecl(); +} + Expected ASTImporter::Import_New(Decl *FromD) { Decl *ToD = Import(FromD); if (!ToD && FromD) @@ -8511,6 +8529,9 @@ if (Pos != ImportedDecls.end()) return Pos->second; ImportedDecls[From] = To; + // This mapping should be maintained only in this function. Therefore do not + // check for additional consistency. + ImportedFromDecls[To] = From; return To; } Index: unittests/AST/ASTImporterTest.cpp =================================================================== --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -10,6 +10,10 @@ // //===----------------------------------------------------------------------===// +// Define this to have ::testing::Combine available. +// FIXME: Better solution for this? +#define GTEST_HAS_COMBINE 1 + #include "clang/AST/ASTImporter.h" #include "MatchVerifier.h" #include "clang/AST/ASTContext.h" @@ -461,6 +465,10 @@ return FromTU->import(*LookupTablePtr, ToAST.get(), From); } + template DeclT *Import(DeclT *From, Language Lang) { + return cast_or_null(Import(cast(From), Lang)); + } + QualType ImportType(QualType FromType, Decl *TUDecl, Language ToLang) { lazyInitToAST(ToLang, "", OutputFileName); TU *FromTU = findFromTU(TUDecl); @@ -2256,6 +2264,266 @@ }).match(ToTU, functionDecl())); } +//FIXME Move these tests to a separate test file. +namespace TypeAndValueParameterizedTests { + +// Type parameters for type-parameterized test fixtures. +struct GetFunPattern { + using DeclTy = FunctionDecl; + BindableMatcher operator()() { return functionDecl(hasName("f")); } +}; +struct GetVarPattern { + using DeclTy = VarDecl; + BindableMatcher operator()() { return varDecl(hasName("v")); } +}; + +// Values for the value-parameterized test fixtures. +// FunctionDecl: +auto *ExternF = "void f();"; +auto *StaticF = "static void f();"; +auto *AnonF = "namespace { void f(); }"; +// VarDecl: +auto *ExternV = "extern int v;"; +auto *StaticV = "static int v;"; +auto *AnonV = "namespace { extern int v; }"; + +// First value in tuple: Compile options. +// Second value in tuple: Source code to be used in the test. +using ImportVisibilityChainParams = + ::testing::WithParamInterface>; +// Fixture to test the redecl chain of Decls with the same visibility. Gtest +// makes it possible to have either value-parameterized or type-parameterized +// fixtures. However, we cannot have both value- and type-parameterized test +// fixtures. This is a value-parameterized test fixture in the gtest sense. We +// intend to mimic gtest's type-parameters via the PatternFactory template +// parameter. We manually instantiate the different tests with the each types. +template +class ImportVisibilityChain + : public ASTImporterTestBase, public ImportVisibilityChainParams { +protected: + using DeclTy = typename PatternFactory::DeclTy; + ArgVector getExtraArgs() const override { return std::get<0>(GetParam()); } + std::string getCode() const { return std::get<1>(GetParam()); } + BindableMatcher getPattern() const { return PatternFactory()(); } + + // Type-parameterized test. + void TypedTest_ImportChain() { + std::string Code = getCode() + getCode(); + auto Pattern = getPattern(); + + TranslationUnitDecl *FromTu = getTuDecl(Code, Lang_CXX, "input0.cc"); + + auto *FromF0 = FirstDeclMatcher().match(FromTu, Pattern); + auto *FromF1 = LastDeclMatcher().match(FromTu, Pattern); + + auto *ToF0 = Import(FromF0, Lang_CXX); + auto *ToF1 = Import(FromF1, Lang_CXX); + + EXPECT_TRUE(ToF0); + ASSERT_TRUE(ToF1); + EXPECT_NE(ToF0, ToF1); + EXPECT_EQ(ToF1->getPreviousDecl(), ToF0); + } +}; + +// Manual instantiation of the fixture with each type. +using ImportFunctionsVisibilityChain = ImportVisibilityChain; +using ImportVariablesVisibilityChain = ImportVisibilityChain; +// Value-parameterized test for the first type. +TEST_P(ImportFunctionsVisibilityChain, ImportChain) { + TypedTest_ImportChain(); +} +// Value-parameterized test for the second type. +TEST_P(ImportVariablesVisibilityChain, ImportChain) { + TypedTest_ImportChain(); +} + +// Automatic instantiation of the value-parameterized tests. +INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctionsVisibilityChain, + ::testing::Combine( + DefaultTestValuesForRunOptions, + ::testing::Values(ExternF, StaticF, AnonF)), ); +INSTANTIATE_TEST_CASE_P( + ParameterizedTests, ImportVariablesVisibilityChain, + ::testing::Combine( + DefaultTestValuesForRunOptions, + // There is no point to instantiate with StaticV, because in C++ we can + // forward declare a variable only with the 'extern' keyword. + // Consequently, each fwd declared variable has external linkage. This + // is different in the C language where any declaration without an + // initializer is a tentative definition, subsequent definitions may be + // provided but they must have the same linkage. See also the test + // ImportVariableChainInC which test for this special C Lang case. + ::testing::Values(ExternV, AnonV)), ); + +// First value in tuple: Compile options. +// Second value in tuple: Tuple with informations for the test. +// Code for first import (or initial code), code to import, whether the `f` +// functions are expected to be linked in a declaration chain. +// One value of this tuple is combined with every value of compile options. +// The test can have a single tuple as parameter only. +using ImportVisibilityParams = ::testing::WithParamInterface< + std::tuple>>; + +template +class ImportVisibility + : public ASTImporterTestBase, + public ImportVisibilityParams { +protected: + using DeclTy = typename PatternFactory::DeclTy; + ArgVector getExtraArgs() const override { return std::get<0>(GetParam()); } + std::string getCode0() const { return std::get<0>(std::get<1>(GetParam())); } + std::string getCode1() const { return std::get<1>(std::get<1>(GetParam())); } + bool shouldBeLinked() const { return std::get<2>(std::get<1>(GetParam())); } + BindableMatcher getPattern() const { return PatternFactory()(); } + + void TypedTest_ImportAfter() { + TranslationUnitDecl *ToTu = getToTuDecl(getCode0(), Lang_CXX); + TranslationUnitDecl *FromTu = getTuDecl(getCode1(), Lang_CXX, "input1.cc"); + + auto *ToF0 = FirstDeclMatcher().match(ToTu, getPattern()); + auto *FromF1 = FirstDeclMatcher().match(FromTu, getPattern()); + + auto *ToF1 = Import(FromF1, Lang_CXX); + + ASSERT_TRUE(ToF0); + ASSERT_TRUE(ToF1); + EXPECT_NE(ToF0, ToF1); + + if (shouldBeLinked()) + EXPECT_EQ(ToF1->getPreviousDecl(), ToF0); + else + EXPECT_FALSE(ToF1->getPreviousDecl()); + } + + void TypedTest_ImportAfterImport() { + TranslationUnitDecl *FromTu0 = getTuDecl(getCode0(), Lang_CXX, "input0.cc"); + TranslationUnitDecl *FromTu1 = getTuDecl(getCode1(), Lang_CXX, "input1.cc"); + auto *FromF0 = + FirstDeclMatcher().match(FromTu0, getPattern()); + auto *FromF1 = + FirstDeclMatcher().match(FromTu1, getPattern()); + auto *ToF0 = Import(FromF0, Lang_CXX); + auto *ToF1 = Import(FromF1, Lang_CXX); + ASSERT_TRUE(ToF0); + ASSERT_TRUE(ToF1); + EXPECT_NE(ToF0, ToF1); + if (shouldBeLinked()) + EXPECT_EQ(ToF1->getPreviousDecl(), ToF0); + else + EXPECT_FALSE(ToF1->getPreviousDecl()); + } +}; +using ImportFunctionsVisibility = ImportVisibility; +using ImportVariablesVisibility = ImportVisibility; + +// FunctionDecl. +TEST_P(ImportFunctionsVisibility, ImportAfter) { + TypedTest_ImportAfter(); +} +TEST_P(ImportFunctionsVisibility, ImportAfterImport) { + TypedTest_ImportAfterImport(); +} +// VarDecl. +TEST_P(ImportVariablesVisibility, ImportAfter) { + TypedTest_ImportAfter(); +} +TEST_P(ImportVariablesVisibility, ImportAfterImport) { + TypedTest_ImportAfterImport(); +} + +bool ExpectLink = true; +bool ExpectNotLink = false; + +INSTANTIATE_TEST_CASE_P( + ParameterizedTests, ImportFunctionsVisibility, + ::testing::Combine( + DefaultTestValuesForRunOptions, + ::testing::Values(std::make_tuple(ExternF, ExternF, ExpectLink), + std::make_tuple(ExternF, StaticF, ExpectNotLink), + std::make_tuple(ExternF, AnonF, ExpectNotLink), + std::make_tuple(StaticF, ExternF, ExpectNotLink), + std::make_tuple(StaticF, StaticF, ExpectNotLink), + std::make_tuple(StaticF, AnonF, ExpectNotLink), + std::make_tuple(AnonF, ExternF, ExpectNotLink), + std::make_tuple(AnonF, StaticF, ExpectNotLink), + std::make_tuple(AnonF, AnonF, ExpectNotLink))), ); +INSTANTIATE_TEST_CASE_P( + ParameterizedTests, ImportVariablesVisibility, + ::testing::Combine( + DefaultTestValuesForRunOptions, + ::testing::Values(std::make_tuple(ExternV, ExternV, ExpectLink), + std::make_tuple(ExternV, StaticV, ExpectNotLink), + std::make_tuple(ExternV, AnonV, ExpectNotLink), + std::make_tuple(StaticV, ExternV, ExpectNotLink), + std::make_tuple(StaticV, StaticV, ExpectNotLink), + std::make_tuple(StaticV, AnonV, ExpectNotLink), + std::make_tuple(AnonV, ExternV, ExpectNotLink), + std::make_tuple(AnonV, StaticV, ExpectNotLink), + std::make_tuple(AnonV, AnonV, ExpectNotLink))), ); + +} // namespace TypeAndValueParameterizedTests + +TEST_P(ASTImporterOptionSpecificTestBase, ImportVariableChainInC) { + std::string Code = "static int v; static int v = 0;"; + auto Pattern = varDecl(hasName("v")); + + TranslationUnitDecl *FromTu = getTuDecl(Code, Lang_C, "input0.c"); + + auto *From0 = FirstDeclMatcher().match(FromTu, Pattern); + auto *From1 = LastDeclMatcher().match(FromTu, Pattern); + + auto *To0 = Import(From0, Lang_C); + auto *To1 = Import(From1, Lang_C); + + EXPECT_TRUE(To0); + ASSERT_TRUE(To1); + EXPECT_NE(To0, To1); + EXPECT_EQ(To1->getPreviousDecl(), To0); +} + +TEST_P(ImportFunctions, ImportFromDifferentScopedAnonNamespace) { + TranslationUnitDecl *FromTu = getTuDecl( + "namespace NS0 { namespace { void f(); } }" + "namespace NS1 { namespace { void f(); } }", + Lang_CXX, "input0.cc"); + auto Pattern = functionDecl(hasName("f")); + + auto *FromF0 = FirstDeclMatcher().match(FromTu, Pattern); + auto *FromF1 = LastDeclMatcher().match(FromTu, Pattern); + + auto *ToF0 = Import(FromF0, Lang_CXX); + auto *ToF1 = Import(FromF1, Lang_CXX); + + EXPECT_TRUE(ToF0); + ASSERT_TRUE(ToF1); + EXPECT_NE(ToF0, ToF1); + EXPECT_FALSE(ToF1->getPreviousDecl()); +} + +TEST_P(ImportFunctions, ImportFunctionFromUnnamedNamespace) { + { + Decl *FromTU = getTuDecl("namespace { void f() {} } void g0() { f(); }", + Lang_CXX, "input0.cc"); + auto *FromD = FirstDeclMatcher().match( + FromTU, functionDecl(hasName("g0"))); + + Import(FromD, Lang_CXX); + } + { + Decl *FromTU = + getTuDecl("namespace { void f() { int a; } } void g1() { f(); }", + Lang_CXX, "input1.cc"); + auto *FromD = FirstDeclMatcher().match( + FromTU, functionDecl(hasName("g1"))); + Import(FromD, Lang_CXX); + } + + Decl *ToTU = ToAST->getASTContext().getTranslationUnitDecl(); + ASSERT_EQ(DeclCounter().match(ToTU, functionDecl(hasName("f"))), + 2u); +} + struct ImportFriendFunctions : ImportFunctions {}; TEST_P(ImportFriendFunctions, ImportFriendFunctionRedeclChainProto) {