martong updated this revision to Diff 192263.
martong marked 2 inline comments as done.
martong added a comment.
- Add braces around else
- Remove falsely duplicated push_back
- Use Expected<T> in HandleNameConflict
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59692/new/
https://reviews.llvm.org/D59692
Files:
include/clang/AST/ASTImporter.h
lib/AST/ASTImporter.cpp
test/Analysis/Inputs/ctu-other.c
unittests/AST/ASTImporterTest.cpp
Index: unittests/AST/ASTImporterTest.cpp
===================================================================
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1964,7 +1964,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<RecordDecl>().match(ToTU, recordDecl(hasName("X"))));
}
@@ -2674,6 +2674,64 @@
functionDecl(hasName("f"), hasDescendant(declRefExpr()))))));
}
+struct ImportFunctionTemplates : ASTImporterOptionSpecificTestBase {};
+
+TEST_P(ImportFunctionTemplates,
+ ImportFunctionWhenThereIsAFunTemplateWithSameName) {
+ getToTuDecl(
+ R"(
+ template <typename T>
+ void foo(T) {}
+ void foo();
+ )",
+ Lang_CXX);
+ Decl *FromTU = getTuDecl("void foo();", Lang_CXX);
+ auto *FromD = FirstDeclMatcher<FunctionDecl>().match(
+ FromTU, functionDecl(hasName("foo")));
+ auto *ImportedD = Import(FromD, Lang_CXX);
+ EXPECT_TRUE(ImportedD);
+}
+
+TEST_P(ImportFunctionTemplates,
+ ImportConstructorWhenThereIsAFunTemplateWithSameName) {
+ auto Code =
+ R"(
+ struct Foo {
+ template <typename T>
+ Foo(T) {}
+ Foo();
+ };
+ )";
+ getToTuDecl(Code, Lang_CXX);
+ Decl *FromTU = getTuDecl(Code, Lang_CXX);
+ auto *FromD =
+ LastDeclMatcher<CXXConstructorDecl>().match(FromTU, cxxConstructorDecl());
+ auto *ImportedD = Import(FromD, Lang_CXX);
+ EXPECT_TRUE(ImportedD);
+}
+
+TEST_P(ImportFunctionTemplates,
+ ImportOperatorWhenThereIsAFunTemplateWithSameName) {
+ getToTuDecl(
+ R"(
+ template <typename T>
+ 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<FunctionDecl>().match(
+ FromTU, functionDecl(hasOverloadedOperatorName("<")));
+ auto *ImportedD = Import(FromD, Lang_CXX);
+ EXPECT_TRUE(ImportedD);
+}
+
struct ImportFriendFunctions : ImportFunctions {};
TEST_P(ImportFriendFunctions, ImportFriendFunctionRedeclChainProto) {
@@ -5534,6 +5592,9 @@
INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctions,
DefaultTestValuesForRunOptions, );
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctionTemplates,
+ DefaultTestValuesForRunOptions, );
+
INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFriendFunctionTemplates,
DefaultTestValuesForRunOptions, );
Index: test/Analysis/Inputs/ctu-other.c
===================================================================
--- test/Analysis/Inputs/ctu-other.c
+++ 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
Index: lib/AST/ASTImporter.cpp
===================================================================
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -79,6 +79,7 @@
using ExpectedExpr = llvm::Expected<Expr *>;
using ExpectedDecl = llvm::Expected<Decl *>;
using ExpectedSLoc = llvm::Expected<SourceLocation>;
+ using ExpectedName = llvm::Expected<DeclarationName>;
std::string ImportError::toString() const {
// FIXME: Improve error texts.
@@ -1944,15 +1945,6 @@
bool ASTNodeImporter::IsStructuralMatch(RecordDecl *FromRecord,
RecordDecl *ToRecord, bool Complain) {
- // Eliminate a potential failure point where we attempt to re-import
- // something we're trying to import while completing ToRecord.
- Decl *ToOrigin = Importer.GetOriginalDecl(ToRecord);
- if (ToOrigin) {
- auto *ToOriginRecord = dyn_cast<RecordDecl>(ToOrigin);
- if (ToOriginRecord)
- ToRecord = ToOriginRecord;
- }
-
StructuralEquivalenceContext Ctx(Importer.getFromContext(),
ToRecord->getASTContext(),
Importer.getNonEquivalentDecls(),
@@ -2154,11 +2146,11 @@
}
if (!ConflictingDecls.empty()) {
- Name = Importer.HandleNameConflict(Name, DC, Decl::IDNS_Namespace,
- ConflictingDecls.data(),
- ConflictingDecls.size());
- if (!Name)
- return make_error<ImportError>(ImportError::NameConflict);
+ ExpectedName NameOrErr = Importer.HandleNameConflict(
+ Name, DC, Decl::IDNS_Namespace, ConflictingDecls.data(),
+ ConflictingDecls.size());
+ if (!NameOrErr)
+ return NameOrErr.takeError();
}
}
@@ -2262,20 +2254,17 @@
// already have a complete underlying type then return with that.
if (!FromUT->isIncompleteType() && !FoundUT->isIncompleteType())
return Importer.MapImported(D, FoundTypedef);
+ } else {
+ ConflictingDecls.push_back(FoundDecl);
}
- // FIXME Handle redecl chain.
- break;
}
-
- ConflictingDecls.push_back(FoundDecl);
}
if (!ConflictingDecls.empty()) {
- Name = Importer.HandleNameConflict(Name, DC, IDNS,
- ConflictingDecls.data(),
- ConflictingDecls.size());
- if (!Name)
- return make_error<ImportError>(ImportError::NameConflict);
+ ExpectedName NameOrErr = Importer.HandleNameConflict(
+ Name, DC, IDNS, ConflictingDecls.data(), ConflictingDecls.size());
+ if (!NameOrErr)
+ return NameOrErr.takeError();
}
}
@@ -2348,11 +2337,10 @@
}
if (!ConflictingDecls.empty()) {
- Name = Importer.HandleNameConflict(Name, DC, IDNS,
- ConflictingDecls.data(),
- ConflictingDecls.size());
- if (!Name)
- return make_error<ImportError>(ImportError::NameConflict);
+ ExpectedName NameOrErr = Importer.HandleNameConflict(
+ Name, DC, IDNS, ConflictingDecls.data(), ConflictingDecls.size());
+ if (!NameOrErr)
+ return NameOrErr.takeError();
}
}
@@ -2454,17 +2442,15 @@
if (auto *FoundEnum = dyn_cast<EnumDecl>(FoundDecl)) {
if (IsStructuralMatch(D, FoundEnum))
return Importer.MapImported(D, FoundEnum);
+ 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>(ImportError::NameConflict);
+ ExpectedName NameOrErr = Importer.HandleNameConflict(
+ Name, DC, IDNS, ConflictingDecls.data(), ConflictingDecls.size());
+ if (!NameOrErr)
+ return NameOrErr.takeError();
}
}
@@ -2586,17 +2572,15 @@
PrevDecl = FoundRecord->getMostRecentDecl();
break;
}
- }
-
- ConflictingDecls.push_back(FoundDecl);
+ ConflictingDecls.push_back(FoundDecl);
+ } // kind is RecordDecl
} // for
if (!ConflictingDecls.empty() && SearchName) {
- Name = Importer.HandleNameConflict(Name, DC, IDNS,
- ConflictingDecls.data(),
- ConflictingDecls.size());
- if (!Name)
- return make_error<ImportError>(ImportError::NameConflict);
+ ExpectedName NameOrErr = Importer.HandleNameConflict(
+ Name, DC, IDNS, ConflictingDecls.data(), ConflictingDecls.size());
+ if (!NameOrErr)
+ return NameOrErr.takeError();
}
}
@@ -2755,17 +2739,15 @@
if (auto *FoundEnumConstant = dyn_cast<EnumConstantDecl>(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>(ImportError::NameConflict);
+ ExpectedName NameOrErr = Importer.HandleNameConflict(
+ Name, DC, IDNS, ConflictingDecls.data(), ConflictingDecls.size());
+ if (!NameOrErr)
+ return NameOrErr.takeError();
}
}
@@ -2984,17 +2966,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>(ImportError::NameConflict);
+ ExpectedName NameOrErr = Importer.HandleNameConflict(
+ Name, DC, IDNS, ConflictingDecls.data(), ConflictingDecls.size());
+ if (!NameOrErr)
+ return NameOrErr.takeError();
}
}
@@ -3606,17 +3586,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>(ImportError::NameConflict);
+ ExpectedName NameOrErr = Importer.HandleNameConflict(
+ Name, DC, IDNS, ConflictingDecls.data(), ConflictingDecls.size());
+ if (!NameOrErr)
+ return NameOrErr.takeError();
}
}
@@ -4942,19 +4920,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>(ImportError::NameConflict);
}
CXXRecordDecl *FromTemplated = D->getTemplatedDecl();
@@ -5221,22 +5197,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>(ImportError::NameConflict);
-
VarDecl *DTemplated = D->getTemplatedDecl();
// Import the type.
@@ -8545,12 +8517,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<DeclarationName> ASTImporter::HandleNameConflict(DeclarationName Name,
+ DeclContext *DC,
+ unsigned IDNS,
+ NamedDecl **Decls,
+ unsigned NumDecls) {
+ return make_error<ImportError>(ImportError::NameConflict);
}
Selector ASTImporter::Import(Selector From) {
llvm::Expected<Selector> To = Import_New(From);
Index: include/clang/AST/ASTImporter.h
===================================================================
--- include/clang/AST/ASTImporter.h
+++ include/clang/AST/ASTImporter.h
@@ -389,12 +389,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<DeclarationName>
+ 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; }
@@ -430,6 +429,7 @@
/// Store and assign the imported declaration to its counterpart.
Decl *MapImported(Decl *From, Decl *To);
+ /// Deprecated. FIXME use [[deprecated]] once Clang enables C++14.
/// Called by StructuralEquivalenceContext. If a RecordDecl is
/// being compared to another RecordDecl as part of import, completing the
/// other RecordDecl may trigger importation of the first RecordDecl. This
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
