diff --git a/clang/include/clang/AST/ASTImporter.h b/clang/include/clang/AST/ASTImporter.h --- a/clang/include/clang/AST/ASTImporter.h +++ b/clang/include/clang/AST/ASTImporter.h @@ -491,12 +491,11 @@ /// /// \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 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -80,6 +80,7 @@ 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. @@ -2188,11 +2189,11 @@ } 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) + return NameOrErr.takeError(); } } @@ -2296,21 +2297,19 @@ // 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) + return NameOrErr.takeError(); } } @@ -2383,11 +2382,10 @@ } 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) + return NameOrErr.takeError(); } } @@ -2491,17 +2489,16 @@ 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) + return NameOrErr.takeError(); } } @@ -2626,17 +2623,15 @@ 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( + Name, DC, IDNS, ConflictingDecls.data(), ConflictingDecls.size()); + if (!NameOrErr) + return NameOrErr.takeError(); } } @@ -2795,17 +2790,15 @@ 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) + return NameOrErr.takeError(); } } @@ -3043,17 +3036,15 @@ << 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) + return NameOrErr.takeError(); } } @@ -3686,17 +3677,15 @@ << 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) + return NameOrErr.takeError(); } } @@ -5022,19 +5011,17 @@ FoundByLookup = FoundTemplate; break; } + 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) + return NameOrErr.takeError(); } - - if (!Name) - return make_error(ImportError::NameConflict); } CXXRecordDecl *FromTemplated = D->getTemplatedDecl(); @@ -5307,22 +5294,18 @@ 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) + 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. @@ -8640,12 +8623,12 @@ 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) { + return make_error(ImportError::NameConflict); } 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 --- a/clang/test/Analysis/Inputs/ctu-other.c +++ b/clang/test/Analysis/Inputs/ctu-other.c @@ -12,11 +12,11 @@ } // 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/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -1879,7 +1879,7 @@ 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")))); } @@ -2389,6 +2389,64 @@ functionDecl(hasName("f"), hasDescendant(declRefExpr())))))); } +struct ImportFunctionTemplates : ASTImporterOptionSpecificTestBase {}; + +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) { @@ -5207,6 +5265,9 @@ INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctions, DefaultTestValuesForRunOptions, ); +INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctionTemplates, + DefaultTestValuesForRunOptions, ); + INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFriendFunctionTemplates, DefaultTestValuesForRunOptions, );