From f035b75d8f079a7b3c8d5c163e2ab0596ac59d17 Mon Sep 17 00:00:00 2001 From: Gabor Marton Date: Tue, 27 Aug 2019 11:36:10 +0000 Subject: [PATCH] [ASTImporter] Fix name conflict handling with different strategies There are numorous flaws about the name conflict handling, this patch attempts fixes them. Changes in details: * HandleNameConflict return with a false DeclarationName Hitherto we effectively never returned with a NameConflict error, even if the preceding StructuralMatch indicated a conflict. Because we just simply returned with the parameter `Name` in HandleNameConflict and that name is almost always `true` when converted to `bool`. * Add tests which indicate wrong NameConflict handling * Add to ConflictingDecls only if decl kind is different Note, we might not indicate an ODR error when there is an existing record decl and a enum is imported with same name. But there are other cases. E.g. think about the case when we import a FunctionTemplateDecl with name f and we found a simple FunctionDecl with name f. They overload. Or in case of a ClassTemplateDecl and CXXRecordDecl, the CXXRecordDecl could be the 'templated' class, so it would be false to report error. So I think we should report a name conflict error only when we are 100% sure of that. That is why I think it should be a general pattern to report the error only if the kind is the same. * Fix failing ctu test with EnumConstandDecl In ctu-main.c we have the enum class 'A' which brings in the enum constant 'x' with value 0 into the global namespace. In ctu-other.c we had the enum class 'B' which brought in the same name ('x') as an enum constant but with a different enum value (42). This is clearly an ODR violation in the global namespace. The solution was to rename the second enum constant. * Introduce ODR handling strategies Reviewers: a_sidorin, shafik Differential Revision: https://reviews.llvm.org/D59692 llvm-svn: 370045 --- clang/include/clang/AST/ASTImporter.h | 17 +- clang/lib/AST/ASTImporter.cpp | 171 +++++++------ clang/test/Analysis/Inputs/ctu-other.c | 8 +- clang/unittests/AST/ASTImporterFixtures.cpp | 32 ++- clang/unittests/AST/ASTImporterFixtures.h | 63 ++++- clang/unittests/AST/ASTImporterTest.cpp | 269 ++++++++++++++++++-- lldb/include/lldb/Symbol/ClangASTImporter.h | 4 +- 7 files changed, 450 insertions(+), 114 deletions(-) diff --git a/clang/include/clang/AST/ASTImporter.h b/clang/include/clang/AST/ASTImporter.h index 6f30fa47d841..c82dcab35db5 100644 --- a/clang/include/clang/AST/ASTImporter.h +++ b/clang/include/clang/AST/ASTImporter.h @@ -90,6 +90,8 @@ class TypeSourceInfo; using FileIDImportHandlerType = std::function; + enum class ODRHandlingType { Conservative, Liberal }; + // An ImportPath is the list of the AST nodes which we visit during an // Import call. // If node `A` depends on node `B` then the path contains an `A`->`B` edge. @@ -236,6 +238,8 @@ class TypeSourceInfo; /// Whether to perform a minimal import. bool Minimal; + ODRHandlingType ODRHandling; + /// Whether the last diagnostic came from the "from" context. bool LastDiagFromFrom = false; @@ -326,6 +330,8 @@ class TypeSourceInfo; /// to-be-completed forward declarations when possible. bool isMinimalImport() const { return Minimal; } + void setODRHandling(ODRHandlingType T) { ODRHandling = T; } + /// \brief Import the given object, returns the result. /// /// \param To Import the object into this variable. @@ -517,12 +523,11 @@ class TypeSourceInfo; /// /// \param NumDecls the number of conflicting declarations in \p Decls. /// - /// \returns the name that the newly-imported declaration should have. - virtual DeclarationName HandleNameConflict(DeclarationName Name, - DeclContext *DC, - unsigned IDNS, - NamedDecl **Decls, - unsigned NumDecls); + /// \returns the name that the newly-imported declaration should have. Or + /// an error if we can't handle the name conflict. + virtual Expected + HandleNameConflict(DeclarationName Name, DeclContext *DC, unsigned IDNS, + NamedDecl **Decls, unsigned NumDecls); /// Retrieve the context that AST nodes are being imported into. ASTContext &getToContext() const { return ToContext; } diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index 8a442bbef666..19321fd0747d 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -80,6 +80,7 @@ namespace clang { using ExpectedExpr = llvm::Expected; using ExpectedDecl = llvm::Expected; using ExpectedSLoc = llvm::Expected; + using ExpectedName = llvm::Expected; std::string ImportError::toString() const { // FIXME: Improve error texts. @@ -2247,11 +2248,13 @@ ExpectedDecl ASTNodeImporter::VisitNamespaceDecl(NamespaceDecl *D) { } if (!ConflictingDecls.empty()) { - Name = Importer.HandleNameConflict(Name, DC, Decl::IDNS_Namespace, - ConflictingDecls.data(), - ConflictingDecls.size()); - if (!Name) - return make_error(ImportError::NameConflict); + ExpectedName NameOrErr = Importer.HandleNameConflict( + Name, DC, Decl::IDNS_Namespace, ConflictingDecls.data(), + ConflictingDecls.size()); + if (NameOrErr) + Name = NameOrErr.get(); + else + return NameOrErr.takeError(); } } @@ -2355,21 +2358,21 @@ ASTNodeImporter::VisitTypedefNameDecl(TypedefNameDecl *D, bool IsAlias) { // already have a complete underlying type then return with that. if (!FromUT->isIncompleteType() && !FoundUT->isIncompleteType()) return Importer.MapImported(D, FoundTypedef); + // FIXME Handle redecl chain. When you do that make consistent changes + // in ASTImporterLookupTable too. + } else { + ConflictingDecls.push_back(FoundDecl); } - // FIXME Handle redecl chain. When you do that make consistent changes - // in ASTImporterLookupTable too. - break; } - - ConflictingDecls.push_back(FoundDecl); } if (!ConflictingDecls.empty()) { - Name = Importer.HandleNameConflict(Name, DC, IDNS, - ConflictingDecls.data(), - ConflictingDecls.size()); - if (!Name) - return make_error(ImportError::NameConflict); + ExpectedName NameOrErr = Importer.HandleNameConflict( + Name, DC, IDNS, ConflictingDecls.data(), ConflictingDecls.size()); + if (NameOrErr) + Name = NameOrErr.get(); + else + return NameOrErr.takeError(); } } @@ -2442,11 +2445,12 @@ ASTNodeImporter::VisitTypeAliasTemplateDecl(TypeAliasTemplateDecl *D) { } if (!ConflictingDecls.empty()) { - Name = Importer.HandleNameConflict(Name, DC, IDNS, - ConflictingDecls.data(), - ConflictingDecls.size()); - if (!Name) - return make_error(ImportError::NameConflict); + ExpectedName NameOrErr = Importer.HandleNameConflict( + Name, DC, IDNS, ConflictingDecls.data(), ConflictingDecls.size()); + if (NameOrErr) + Name = NameOrErr.get(); + else + return NameOrErr.takeError(); } } @@ -2550,17 +2554,18 @@ ExpectedDecl ASTNodeImporter::VisitEnumDecl(EnumDecl *D) { continue; if (IsStructuralMatch(D, FoundEnum)) return Importer.MapImported(D, FoundEnum); + ConflictingDecls.push_back(FoundDecl); } - - ConflictingDecls.push_back(FoundDecl); } if (!ConflictingDecls.empty()) { - Name = Importer.HandleNameConflict(SearchName, DC, IDNS, - ConflictingDecls.data(), - ConflictingDecls.size()); - if (!Name) - return make_error(ImportError::NameConflict); + ExpectedName NameOrErr = Importer.HandleNameConflict( + SearchName, DC, IDNS, ConflictingDecls.data(), + ConflictingDecls.size()); + if (NameOrErr) + Name = NameOrErr.get(); + else + return NameOrErr.takeError(); } } @@ -2685,17 +2690,18 @@ ExpectedDecl ASTNodeImporter::VisitRecordDecl(RecordDecl *D) { PrevDecl = FoundRecord->getMostRecentDecl(); break; } - } - - ConflictingDecls.push_back(FoundDecl); + ConflictingDecls.push_back(FoundDecl); + } // kind is RecordDecl } // for if (!ConflictingDecls.empty() && SearchName) { - Name = Importer.HandleNameConflict(SearchName, DC, IDNS, - ConflictingDecls.data(), - ConflictingDecls.size()); - if (!Name) - return make_error(ImportError::NameConflict); + ExpectedName NameOrErr = Importer.HandleNameConflict( + SearchName, DC, IDNS, ConflictingDecls.data(), + ConflictingDecls.size()); + if (NameOrErr) + Name = NameOrErr.get(); + else + return NameOrErr.takeError(); } } @@ -2854,17 +2860,17 @@ ExpectedDecl ASTNodeImporter::VisitEnumConstantDecl(EnumConstantDecl *D) { if (auto *FoundEnumConstant = dyn_cast(FoundDecl)) { if (IsStructuralMatch(D, FoundEnumConstant)) return Importer.MapImported(D, FoundEnumConstant); + ConflictingDecls.push_back(FoundDecl); } - - ConflictingDecls.push_back(FoundDecl); } if (!ConflictingDecls.empty()) { - Name = Importer.HandleNameConflict(Name, DC, IDNS, - ConflictingDecls.data(), - ConflictingDecls.size()); - if (!Name) - return make_error(ImportError::NameConflict); + ExpectedName NameOrErr = Importer.HandleNameConflict( + Name, DC, IDNS, ConflictingDecls.data(), ConflictingDecls.size()); + if (NameOrErr) + Name = NameOrErr.get(); + else + return NameOrErr.takeError(); } } @@ -3102,17 +3108,17 @@ ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) { << Name << D->getType() << FoundFunction->getType(); Importer.ToDiag(FoundFunction->getLocation(), diag::note_odr_value_here) << FoundFunction->getType(); + ConflictingDecls.push_back(FoundDecl); } - - ConflictingDecls.push_back(FoundDecl); } if (!ConflictingDecls.empty()) { - Name = Importer.HandleNameConflict(Name, DC, IDNS, - ConflictingDecls.data(), - ConflictingDecls.size()); - if (!Name) - return make_error(ImportError::NameConflict); + ExpectedName NameOrErr = Importer.HandleNameConflict( + Name, DC, IDNS, ConflictingDecls.data(), ConflictingDecls.size()); + if (NameOrErr) + Name = NameOrErr.get(); + else + return NameOrErr.takeError(); } } @@ -3758,17 +3764,17 @@ ExpectedDecl ASTNodeImporter::VisitVarDecl(VarDecl *D) { << Name << D->getType() << FoundVar->getType(); Importer.ToDiag(FoundVar->getLocation(), diag::note_odr_value_here) << FoundVar->getType(); + ConflictingDecls.push_back(FoundDecl); } - - ConflictingDecls.push_back(FoundDecl); } if (!ConflictingDecls.empty()) { - Name = Importer.HandleNameConflict(Name, DC, IDNS, - ConflictingDecls.data(), - ConflictingDecls.size()); - if (!Name) - return make_error(ImportError::NameConflict); + ExpectedName NameOrErr = Importer.HandleNameConflict( + Name, DC, IDNS, ConflictingDecls.data(), ConflictingDecls.size()); + if (NameOrErr) + Name = NameOrErr.get(); + else + return NameOrErr.takeError(); } } @@ -5106,19 +5112,19 @@ ExpectedDecl ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) { // see ASTTests test ImportExistingFriendClassTemplateDef. continue; } + ConflictingDecls.push_back(FoundDecl); } - - ConflictingDecls.push_back(FoundDecl); } if (!ConflictingDecls.empty()) { - Name = Importer.HandleNameConflict(Name, DC, Decl::IDNS_Ordinary, - ConflictingDecls.data(), - ConflictingDecls.size()); + ExpectedName NameOrErr = Importer.HandleNameConflict( + Name, DC, Decl::IDNS_Ordinary, ConflictingDecls.data(), + ConflictingDecls.size()); + if (NameOrErr) + Name = NameOrErr.get(); + else + return NameOrErr.takeError(); } - - if (!Name) - return make_error(ImportError::NameConflict); } CXXRecordDecl *FromTemplated = D->getTemplatedDecl(); @@ -5391,22 +5397,20 @@ ExpectedDecl ASTNodeImporter::VisitVarTemplateDecl(VarTemplateDecl *D) { FoundTemplate->getTemplatedDecl()); return Importer.MapImported(D, FoundTemplate); } + ConflictingDecls.push_back(FoundDecl); } - - ConflictingDecls.push_back(FoundDecl); } if (!ConflictingDecls.empty()) { - Name = Importer.HandleNameConflict(Name, DC, Decl::IDNS_Ordinary, - ConflictingDecls.data(), - ConflictingDecls.size()); + ExpectedName NameOrErr = Importer.HandleNameConflict( + Name, DC, Decl::IDNS_Ordinary, ConflictingDecls.data(), + ConflictingDecls.size()); + if (NameOrErr) + Name = NameOrErr.get(); + else + return NameOrErr.takeError(); } - if (!Name) - // FIXME: Is it possible to get other error than name conflict? - // (Put this `if` into the previous `if`?) - return make_error(ImportError::NameConflict); - VarDecl *DTemplated = D->getTemplatedDecl(); // Import the type. @@ -7817,7 +7821,7 @@ ASTImporter::ASTImporter(ASTContext &ToContext, FileManager &ToFileManager, std::shared_ptr SharedState) : SharedState(SharedState), ToContext(ToContext), FromContext(FromContext), ToFileManager(ToFileManager), FromFileManager(FromFileManager), - Minimal(MinimalImport) { + Minimal(MinimalImport), ODRHandling(ODRHandlingType::Conservative) { // Create a default state without the lookup table: LLDB case. if (!SharedState) { @@ -8744,12 +8748,17 @@ Expected ASTImporter::Import(Selector FromSel) { return ToContext.Selectors.getSelector(FromSel.getNumArgs(), Idents.data()); } -DeclarationName ASTImporter::HandleNameConflict(DeclarationName Name, - DeclContext *DC, - unsigned IDNS, - NamedDecl **Decls, - unsigned NumDecls) { - return Name; +Expected ASTImporter::HandleNameConflict(DeclarationName Name, + DeclContext *DC, + unsigned IDNS, + NamedDecl **Decls, + unsigned NumDecls) { + if (ODRHandling == ODRHandlingType::Conservative) + // Report error at any name conflict. + return make_error(ImportError::NameConflict); + else + // Allow to create the new Decl with the same name. + return Name; } DiagnosticBuilder ASTImporter::ToDiag(SourceLocation Loc, unsigned DiagID) { diff --git a/clang/test/Analysis/Inputs/ctu-other.c b/clang/test/Analysis/Inputs/ctu-other.c index 9a952061102f..82d29c6edb30 100644 --- a/clang/test/Analysis/Inputs/ctu-other.c +++ b/clang/test/Analysis/Inputs/ctu-other.c @@ -12,11 +12,11 @@ int f(int i) { } // Test enums. -enum B { x = 42, - l, - s }; +enum B { x2 = 42, + y2, + z2 }; int enumCheck(void) { - return x; + return x2; } // Test reporting an error in macro definition diff --git a/clang/unittests/AST/ASTImporterFixtures.cpp b/clang/unittests/AST/ASTImporterFixtures.cpp index 0be4e7893750..80fcd1f1fc15 100644 --- a/clang/unittests/AST/ASTImporterFixtures.cpp +++ b/clang/unittests/AST/ASTImporterFixtures.cpp @@ -39,10 +39,12 @@ void createVirtualFileIfNeeded(ASTUnit *ToAST, StringRef FileName, } ASTImporterTestBase::TU::TU(StringRef Code, StringRef FileName, ArgVector Args, - ImporterConstructor C) + ImporterConstructor C, + ASTImporter::ODRHandlingType ODRHandling) : Code(Code), FileName(FileName), Unit(tooling::buildASTFromCodeWithArgs(this->Code, Args, this->FileName)), - TUDecl(Unit->getASTContext().getTranslationUnitDecl()), Creator(C) { + TUDecl(Unit->getASTContext().getTranslationUnitDecl()), Creator(C), + ODRHandling(ODRHandling) { Unit->enableSourceFileDiagnostics(); // If the test doesn't need a specific ASTImporter, we just create a @@ -63,10 +65,12 @@ void ASTImporterTestBase::TU::lazyInitImporter( const std::shared_ptr &SharedState, ASTUnit *ToAST) { assert(ToAST); - if (!Importer) + if (!Importer) { Importer.reset(Creator(ToAST->getASTContext(), ToAST->getFileManager(), Unit->getASTContext(), Unit->getFileManager(), false, SharedState)); + Importer->setODRHandling(ODRHandling); + } assert(&ToAST->getASTContext() == &Importer->getToContext()); createVirtualFileIfNeeded(ToAST, FileName, Code); } @@ -83,6 +87,13 @@ Decl *ASTImporterTestBase::TU::import( } } +llvm::Expected ASTImporterTestBase::TU::importOrError( + const std::shared_ptr &SharedState, ASTUnit *ToAST, + Decl *FromDecl) { + lazyInitImporter(SharedState, ToAST); + return Importer->Import(FromDecl); +} + QualType ASTImporterTestBase::TU::import( const std::shared_ptr &SharedState, ASTUnit *ToAST, QualType FromType) { @@ -132,7 +143,8 @@ ASTImporterTestBase::getImportedDecl(StringRef FromSrcCode, Language FromLang, ArgVector FromArgs = getArgVectorForLanguage(FromLang), ToArgs = getArgVectorForLanguage(ToLang); - FromTUs.emplace_back(FromSrcCode, InputFileName, FromArgs, Creator); + FromTUs.emplace_back(FromSrcCode, InputFileName, FromArgs, Creator, + ODRHandling); TU &FromTU = FromTUs.back(); assert(!ToAST); @@ -165,7 +177,7 @@ TranslationUnitDecl *ASTImporterTestBase::getTuDecl(StringRef SrcCode, }) == FromTUs.end()); ArgVector Args = getArgVectorForLanguage(Lang); - FromTUs.emplace_back(SrcCode, FileName, Args, Creator); + FromTUs.emplace_back(SrcCode, FileName, Args, Creator, ODRHandling); TU &Tu = FromTUs.back(); return Tu.TUDecl; @@ -187,6 +199,16 @@ Decl *ASTImporterTestBase::Import(Decl *From, Language ToLang) { return To; } +llvm::Expected ASTImporterTestBase::importOrError(Decl *From, + Language ToLang) { + lazyInitToAST(ToLang, "", OutputFileName); + TU *FromTU = findFromTU(From); + assert(SharedStatePtr); + llvm::Expected To = + FromTU->importOrError(SharedStatePtr, ToAST.get(), From); + return To; +} + QualType ASTImporterTestBase::ImportType(QualType FromType, Decl *TUDecl, Language ToLang) { lazyInitToAST(ToLang, "", OutputFileName); diff --git a/clang/unittests/AST/ASTImporterFixtures.h b/clang/unittests/AST/ASTImporterFixtures.h index 4b67a9435f1f..34cef16712d7 100644 --- a/clang/unittests/AST/ASTImporterFixtures.h +++ b/clang/unittests/AST/ASTImporterFixtures.h @@ -17,12 +17,16 @@ #include "gmock/gmock.h" #include "clang/AST/ASTImporter.h" -#include "clang/Frontend/ASTUnit.h" #include "clang/AST/ASTImporterSharedState.h" +#include "clang/Frontend/ASTUnit.h" +#include "llvm/Support/Error.h" +#include "llvm/Support/ErrorHandling.h" #include "DeclMatcher.h" #include "Language.h" +#include + namespace clang { class ASTImporter; @@ -82,6 +86,9 @@ public: const std::shared_ptr &SharedState)> ImporterConstructor; + // ODR handling type for the AST importer. + ASTImporter::ODRHandlingType ODRHandling; + // The lambda that constructs the ASTImporter we use in this test. ImporterConstructor Creator; @@ -99,9 +106,12 @@ private: TranslationUnitDecl *TUDecl = nullptr; std::unique_ptr Importer; ImporterConstructor Creator; + ASTImporter::ODRHandlingType ODRHandling; TU(StringRef Code, StringRef FileName, ArgVector Args, - ImporterConstructor C = ImporterConstructor()); + ImporterConstructor C = ImporterConstructor(), + ASTImporter::ODRHandlingType ODRHandling = + ASTImporter::ODRHandlingType::Conservative); ~TU(); void @@ -109,6 +119,9 @@ private: ASTUnit *ToAST); Decl *import(const std::shared_ptr &SharedState, ASTUnit *ToAST, Decl *FromDecl); + llvm::Expected + importOrError(const std::shared_ptr &SharedState, + ASTUnit *ToAST, Decl *FromDecl); QualType import(const std::shared_ptr &SharedState, ASTUnit *ToAST, QualType FromType); }; @@ -162,8 +175,14 @@ public: return cast_or_null(Import(cast(From), Lang)); } + // Import the given Decl into the ToCtx. + // Same as Import but returns the result of the import which can be an error. + llvm::Expected importOrError(Decl *From, Language ToLang); + QualType ImportType(QualType FromType, Decl *TUDecl, Language ToLang); + ASTImporterTestBase() + : ODRHandling(ASTImporter::ODRHandlingType::Conservative) {} ~ASTImporterTestBase(); }; @@ -174,6 +193,46 @@ protected: ArgVector getExtraArgs() const override { return GetParam(); } }; +template +::testing::AssertionResult isSuccess(llvm::Expected &ValOrErr) { + if (ValOrErr) + return ::testing::AssertionSuccess() << "Expected<> contains no error."; + else + return ::testing::AssertionFailure() + << "Expected<> contains error: " << toString(ValOrErr.takeError()); +} + +template +::testing::AssertionResult isImportError(llvm::Expected &ValOrErr, + ImportError::ErrorKind Kind) { + if (ValOrErr) { + return ::testing::AssertionFailure() << "Expected<> is expected to contain " + "error but does contain value \"" + << (*ValOrErr) << "\""; + } else { + std::ostringstream OS; + bool Result = false; + auto Err = llvm::handleErrors( + ValOrErr.takeError(), [&OS, &Result, Kind](clang::ImportError &IE) { + if (IE.Error == Kind) { + Result = true; + OS << "Expected<> contains an ImportError " << IE.toString(); + } else { + OS << "Expected<> contains an ImportError " << IE.toString() + << " instead of kind " << Kind; + } + }); + if (Err) { + OS << "Expected<> contains unexpected error: " + << toString(std::move(Err)); + } + if (Result) + return ::testing::AssertionSuccess() << OS.str(); + else + return ::testing::AssertionFailure() << OS.str(); + } +} + } // end namespace ast_matchers } // end namespace clang diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp index 78b67d836b9c..2038e3a28a3e 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -1879,7 +1879,7 @@ TEST_P(ASTImporterOptionSpecificTestBase, AnonymousRecordsReversed) { auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl(); // We expect one (ODR) warning during the import. EXPECT_EQ(1u, ToTU->getASTContext().getDiagnostics().getNumWarnings()); - EXPECT_EQ(2u, + EXPECT_EQ(1u, DeclCounter().match(ToTU, recordDecl(hasName("X")))); } @@ -2432,6 +2432,62 @@ TEST_P(ImportFunctionTemplates, EXPECT_EQ(ToD1, ToD2); } +TEST_P(ImportFunctionTemplates, + ImportFunctionWhenThereIsAFunTemplateWithSameName) { + getToTuDecl( + R"( + template + void foo(T) {} + void foo(); + )", + Lang_CXX); + Decl *FromTU = getTuDecl("void foo();", Lang_CXX); + auto *FromD = FirstDeclMatcher().match( + FromTU, functionDecl(hasName("foo"))); + auto *ImportedD = Import(FromD, Lang_CXX); + EXPECT_TRUE(ImportedD); +} + +TEST_P(ImportFunctionTemplates, + ImportConstructorWhenThereIsAFunTemplateWithSameName) { + auto Code = + R"( + struct Foo { + template + Foo(T) {} + Foo(); + }; + )"; + getToTuDecl(Code, Lang_CXX); + Decl *FromTU = getTuDecl(Code, Lang_CXX); + auto *FromD = + LastDeclMatcher().match(FromTU, cxxConstructorDecl()); + auto *ImportedD = Import(FromD, Lang_CXX); + EXPECT_TRUE(ImportedD); +} + +TEST_P(ImportFunctionTemplates, + ImportOperatorWhenThereIsAFunTemplateWithSameName) { + getToTuDecl( + R"( + template + void operator<(T,T) {} + struct X{}; + void operator<(X, X); + )", + Lang_CXX); + Decl *FromTU = getTuDecl( + R"( + struct X{}; + void operator<(X, X); + )", + Lang_CXX); + auto *FromD = LastDeclMatcher().match( + FromTU, functionDecl(hasOverloadedOperatorName("<"))); + auto *ImportedD = Import(FromD, Lang_CXX); + EXPECT_TRUE(ImportedD); +} + struct ImportFriendFunctions : ImportFunctions {}; TEST_P(ImportFriendFunctions, ImportFriendFunctionRedeclChainProto) { @@ -5127,15 +5183,6 @@ TEST_P(ErrorHandlingTest, } } -INSTANTIATE_TEST_CASE_P(ParameterizedTests, ErrorHandlingTest, - DefaultTestValuesForRunOptions, ); - -INSTANTIATE_TEST_CASE_P(ParameterizedTests, DeclContextTest, - ::testing::Values(ArgVector()), ); - -INSTANTIATE_TEST_CASE_P(ParameterizedTests, CanonicalRedeclChain, - ::testing::Values(ArgVector()), ); - TEST_P(ASTImporterOptionSpecificTestBase, LambdaInFunctionBody) { Decl *FromTU = getTuDecl( R"( @@ -5233,8 +5280,8 @@ TEST_P(ASTImporterOptionSpecificTestBase, // prototype (inside 'friend') for it comes first in the AST and is not // linked to the definition. EXPECT_EQ(ImportedDef, ToClassDef); -} - +} + struct LLDBLookupTest : ASTImporterOptionSpecificTestBase { LLDBLookupTest() { Creator = [](ASTContext &ToContext, FileManager &ToFileManager, @@ -5362,10 +5409,197 @@ TEST_P(ASTImporterOptionSpecificTestBase, EXPECT_EQ(ImportedX->isAggregate(), FromX->isAggregate()); } +struct ConflictingDeclsWithLiberalStrategy : ASTImporterOptionSpecificTestBase { + ConflictingDeclsWithLiberalStrategy() { + this->ODRHandling = ASTImporter::ODRHandlingType::Liberal; + } +}; + +// Check that a Decl has been successfully imported into a standalone redecl +// chain. +template +static void CheckImportedAsNew(llvm::Expected &Result, Decl *ToTU, + PatternTy Pattern) { + ASSERT_TRUE(isSuccess(Result)); + Decl *ImportedD = *Result; + ASSERT_TRUE(ImportedD); + auto *ToD = FirstDeclMatcher().match(ToTU, Pattern); + EXPECT_NE(ImportedD, ToD); + EXPECT_FALSE(ImportedD->getPreviousDecl()); + EXPECT_EQ(DeclCounter().match(ToTU, Pattern), 2u); +} + +TEST_P(ConflictingDeclsWithLiberalStrategy, Typedef) { + Decl *ToTU = getToTuDecl( + R"( + typedef int X; + )", + Lang_CXX11); + Decl *FromTU = getTuDecl( + R"( + typedef double X; + )", + Lang_CXX11); + auto Pattern = typedefNameDecl(hasName("X")); + auto *FromX = FirstDeclMatcher().match(FromTU, Pattern); + + Expected Result = importOrError(FromX, Lang_CXX11); + CheckImportedAsNew(Result, ToTU, Pattern); +} + +TEST_P(ConflictingDeclsWithLiberalStrategy, TypeAlias) { + Decl *ToTU = getToTuDecl( + R"( + using X = int; + )", + Lang_CXX11); + Decl *FromTU = getTuDecl( + R"( + using X = double; + )", + Lang_CXX11); + auto Pattern = typedefNameDecl(hasName("X")); + auto *FromX = FirstDeclMatcher().match(FromTU, Pattern); + Expected Result = importOrError(FromX, Lang_CXX11); + CheckImportedAsNew(Result, ToTU, Pattern); +} + +TEST_P(ConflictingDeclsWithLiberalStrategy, EnumDecl) { + Decl *ToTU = getToTuDecl( + R"( + enum X { a, b }; + )", + Lang_CXX11); + Decl *FromTU = getTuDecl( + R"( + enum X { a, b, c }; + )", + Lang_CXX11); + auto Pattern = enumDecl(hasName("X")); + auto *FromX = FirstDeclMatcher().match(FromTU, Pattern); + Expected Result = importOrError(FromX, Lang_CXX11); + CheckImportedAsNew(Result, ToTU, Pattern); +} + +TEST_P(ConflictingDeclsWithLiberalStrategy, EnumConstantDecl) { + Decl *ToTU = getToTuDecl( + R"( + enum E { X = 0 }; + )", + Lang_CXX11); + Decl *FromTU = getTuDecl( + R"( + enum E { X = 1 }; + )", + Lang_CXX11); + auto Pattern = enumConstantDecl(hasName("X")); + auto *FromX = FirstDeclMatcher().match(FromTU, Pattern); + Expected Result = importOrError(FromX, Lang_CXX11); + CheckImportedAsNew(Result, ToTU, Pattern); +} + +TEST_P(ConflictingDeclsWithLiberalStrategy, RecordDecl) { + Decl *ToTU = getToTuDecl( + R"( + class X { int a; }; + )", + Lang_CXX11); + Decl *FromTU = getTuDecl( + R"( + class X { int b; }; + )", + Lang_CXX11); + auto Pattern = cxxRecordDecl(hasName("X"), unless(isImplicit())); + auto *FromX = FirstDeclMatcher().match(FromTU, Pattern); + Expected Result = importOrError(FromX, Lang_CXX11); + CheckImportedAsNew(Result, ToTU, Pattern); +} + +TEST_P(ConflictingDeclsWithLiberalStrategy, VarDecl) { + Decl *ToTU = getToTuDecl( + R"( + int X; + )", + Lang_CXX11); + Decl *FromTU = getTuDecl( + R"( + double X; + )", + Lang_CXX11); + auto Pattern = varDecl(hasName("X")); + auto *FromX = FirstDeclMatcher().match(FromTU, Pattern); + Expected Result = importOrError(FromX, Lang_CXX11); + CheckImportedAsNew(Result, ToTU, Pattern); +} + +TEST_P(ConflictingDeclsWithLiberalStrategy, FunctionDecl) { + Decl *ToTU = getToTuDecl( + R"( + void X(int); + )", + Lang_C); // C, no overloading! + Decl *FromTU = getTuDecl( + R"( + void X(double); + )", + Lang_C); + auto Pattern = functionDecl(hasName("X")); + auto *FromX = FirstDeclMatcher().match(FromTU, Pattern); + Expected Result = importOrError(FromX, Lang_CXX11); + CheckImportedAsNew(Result, ToTU, Pattern); +} + +TEST_P(ConflictingDeclsWithLiberalStrategy, ClassTemplateDecl) { + Decl *ToTU = getToTuDecl( + R"( + template + struct X; + )", + Lang_CXX11); + Decl *FromTU = getTuDecl( + R"( + template + struct X; + )", + Lang_CXX11); + auto Pattern = classTemplateDecl(hasName("X")); + auto *FromX = FirstDeclMatcher().match(FromTU, Pattern); + Expected Result = importOrError(FromX, Lang_CXX11); + CheckImportedAsNew(Result, ToTU, Pattern); +} + +TEST_P(ConflictingDeclsWithLiberalStrategy, DISABLED_VarTemplateDecl) { + const internal::VariadicDynCastAllOfMatcher + varTemplateDecl; + Decl *ToTU = getToTuDecl( + R"( + template + constexpr T X; + )", + Lang_CXX14); + Decl *FromTU = getTuDecl( + R"( + template + constexpr int X = 0; + )", + Lang_CXX14); + auto Pattern = varTemplateDecl(hasName("X")); + auto *FromX = FirstDeclMatcher().match( + FromTU, varTemplateDecl(hasName("X"))); + Expected Result = importOrError(FromX, Lang_CXX11); + CheckImportedAsNew(Result, ToTU, Pattern); +} + INSTANTIATE_TEST_CASE_P(ParameterizedTests, SVEBuiltins, ::testing::Values(ArgVector{"-target", "aarch64-linux-gnu"}), ); +INSTANTIATE_TEST_CASE_P(ParameterizedTests, DeclContextTest, + ::testing::Values(ArgVector()), ); + +INSTANTIATE_TEST_CASE_P(ParameterizedTests, CanonicalRedeclChain, + ::testing::Values(ArgVector()), ); + INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterLookupTableTest, DefaultTestValuesForRunOptions, ); @@ -5384,21 +5618,24 @@ INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportDecl, INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterOptionSpecificTestBase, DefaultTestValuesForRunOptions, ); +INSTANTIATE_TEST_CASE_P(ParameterizedTests, ErrorHandlingTest, + DefaultTestValuesForRunOptions, ); + INSTANTIATE_TEST_CASE_P(ParameterizedTests, RedirectingImporterTest, DefaultTestValuesForRunOptions, ); INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctions, DefaultTestValuesForRunOptions, ); +INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctionTemplates, + DefaultTestValuesForRunOptions, ); + INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFriendFunctionTemplates, DefaultTestValuesForRunOptions, ); INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportClasses, DefaultTestValuesForRunOptions, ); -INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctionTemplates, - DefaultTestValuesForRunOptions, ); - INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFriendFunctions, DefaultTestValuesForRunOptions, ); @@ -5418,6 +5655,8 @@ INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportVariables, INSTANTIATE_TEST_CASE_P(ParameterizedTests, LLDBLookupTest, DefaultTestValuesForRunOptions, ); +INSTANTIATE_TEST_CASE_P(ParameterizedTests, ConflictingDeclsWithLiberalStrategy, + DefaultTestValuesForRunOptions, ); } // end namespace ast_matchers } // end namespace clang diff --git a/lldb/include/lldb/Symbol/ClangASTImporter.h b/lldb/include/lldb/Symbol/ClangASTImporter.h index 353b1232d940..eb3689485cfb 100644 --- a/lldb/include/lldb/Symbol/ClangASTImporter.h +++ b/lldb/include/lldb/Symbol/ClangASTImporter.h @@ -252,7 +252,9 @@ private: : clang::ASTImporter(*target_ctx, master.m_file_manager, *source_ctx, master.m_file_manager, true /*minimal*/), m_decls_to_deport(nullptr), m_decls_already_deported(nullptr), - m_master(master), m_source_ctx(source_ctx) {} + m_master(master), m_source_ctx(source_ctx) { + setODRHandling(clang::ASTImporter::ODRHandlingType::Liberal); + } /// Scope guard that attaches a CxxModuleHandler to an ASTImporterDelegate /// and deattaches it at the end of the scope. Supports being used multiple