martong updated this revision to Diff 217135.
martong added a comment.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
- Use resulting Name from HandleNameConflict if set
- Add ODRHandling strategies
- Refactor tests, to avoid some code repetition
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59692/new/
https://reviews.llvm.org/D59692
Files:
clang/include/clang/AST/ASTImporter.h
clang/lib/AST/ASTImporter.cpp
clang/test/Analysis/Inputs/ctu-other.c
clang/unittests/AST/ASTImporterFixtures.cpp
clang/unittests/AST/ASTImporterFixtures.h
clang/unittests/AST/ASTImporterTest.cpp
lldb/include/lldb/Symbol/ClangASTImporter.h
Index: lldb/include/lldb/Symbol/ClangASTImporter.h
===================================================================
--- lldb/include/lldb/Symbol/ClangASTImporter.h
+++ lldb/include/lldb/Symbol/ClangASTImporter.h
@@ -252,7 +252,9 @@
: 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
Index: clang/unittests/AST/ASTImporterTest.cpp
===================================================================
--- clang/unittests/AST/ASTImporterTest.cpp
+++ 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<RecordDecl>().match(ToTU, recordDecl(hasName("X"))));
}
@@ -2432,6 +2432,62 @@
EXPECT_EQ(ToD1, ToD2);
}
+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) {
@@ -5127,15 +5183,6 @@
}
}
-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 @@
// 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,388 @@
EXPECT_EQ(ImportedX->isAggregate(), FromX->isAggregate());
}
+// FIXME Put ODR handling strategy related tests into their own test file. And
+// create type parameterized tests for them like we do in
+// ASTImporterGenericRedeclTest.cpp
+struct ConflictingDeclsWithConservativeStrategy
+ : ASTImporterOptionSpecificTestBase {};
+
+struct ConflictingDeclsWithLiberalStrategy : ASTImporterOptionSpecificTestBase {
+ ConflictingDeclsWithLiberalStrategy() {
+ this->ODRHandling = ASTImporter::ODRHandlingType::Liberal;
+ }
+};
+
+// Check that a Decl has been successfully imported into a standalone redecl
+// chain.
+template <typename DeclTy, typename PatternTy>
+static void CheckImportedAsNew(llvm::Expected<Decl *> &Result, Decl *ToTU,
+ PatternTy Pattern) {
+ ASSERT_TRUE(isSuccess(Result));
+ Decl *ImportedD = *Result;
+ ASSERT_TRUE(ImportedD);
+ auto *ToD = FirstDeclMatcher<DeclTy>().match(ToTU, Pattern);
+ EXPECT_NE(ImportedD, ToD);
+ EXPECT_FALSE(ImportedD->getPreviousDecl());
+ EXPECT_EQ(DeclCounter<DeclTy>().match(ToTU, Pattern), 2u);
+}
+
+// Check that a Decl was not imported because of NameConflict.
+template <typename DeclTy, typename PatternTy>
+static void CheckImportNameConflict(llvm::Expected<Decl *> &Result, Decl *ToTU,
+ PatternTy Pattern) {
+ EXPECT_TRUE(isImportError(Result, ImportError::NameConflict));
+ EXPECT_EQ(DeclCounter<DeclTy>().match(ToTU, Pattern), 1u);
+}
+
+TEST_P(ConflictingDeclsWithConservativeStrategy, 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<TypedefNameDecl>().match(
+ FromTU, Pattern);
+ Expected<Decl*> Result = importOrError(FromX, Lang_CXX11);
+ CheckImportNameConflict<TypedefNameDecl>(Result, ToTU, Pattern);
+}
+
+TEST_P(ConflictingDeclsWithConservativeStrategy, 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<TypedefNameDecl>().match(
+ FromTU, Pattern);
+ Expected<Decl*> Result = importOrError(FromX, Lang_CXX11);
+ CheckImportNameConflict<TypedefNameDecl>(Result, ToTU, Pattern);
+}
+
+TEST_P(ConflictingDeclsWithConservativeStrategy, 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<EnumDecl>().match(
+ FromTU, Pattern);
+ Expected<Decl*> Result = importOrError(FromX, Lang_CXX11);
+ CheckImportNameConflict<EnumDecl>(Result, ToTU, Pattern);
+}
+
+TEST_P(ConflictingDeclsWithConservativeStrategy, 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<EnumConstantDecl>().match(
+ FromTU, Pattern);
+ Expected<Decl*> Result = importOrError(FromX, Lang_CXX11);
+ CheckImportNameConflict<EnumConstantDecl>(Result, ToTU, Pattern);
+}
+
+TEST_P(ConflictingDeclsWithConservativeStrategy, 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<RecordDecl>().match(
+ FromTU, Pattern);
+ Expected<Decl*> Result = importOrError(FromX, Lang_CXX11);
+ CheckImportNameConflict<RecordDecl>(Result, ToTU, Pattern);
+}
+
+TEST_P(ConflictingDeclsWithConservativeStrategy, 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<VarDecl>().match(
+ FromTU, Pattern);
+ Expected<Decl*> Result = importOrError(FromX, Lang_CXX11);
+ CheckImportNameConflict<VarDecl>(Result, ToTU, Pattern);
+}
+
+TEST_P(ConflictingDeclsWithConservativeStrategy, 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<FunctionDecl>().match(
+ FromTU, Pattern);
+ Expected<Decl*> Result = importOrError(FromX, Lang_CXX11);
+ CheckImportNameConflict<FunctionDecl>(Result, ToTU, Pattern);
+}
+
+TEST_P(ConflictingDeclsWithConservativeStrategy, ClassTemplateDecl) {
+ Decl * ToTU = getToTuDecl(
+ R"(
+ template <class>
+ struct X;
+ )",
+ Lang_CXX11);
+ Decl *FromTU = getTuDecl(
+ R"(
+ template <int>
+ struct X;
+ )",
+ Lang_CXX11);
+ auto Pattern = classTemplateDecl(hasName("X"));
+ auto *FromX = FirstDeclMatcher<ClassTemplateDecl>().match(
+ FromTU, Pattern);
+ Expected<Decl*> Result = importOrError(FromX, Lang_CXX11);
+ CheckImportNameConflict<ClassTemplateDecl>(Result, ToTU, Pattern);
+}
+
+// FIXME We don't have proper structural equivalency check for VarTemplateDecl
+TEST_P(ConflictingDeclsWithConservativeStrategy, DISABLED_VarTemplateDecl) {
+ const internal::VariadicDynCastAllOfMatcher<Decl, VarTemplateDecl>
+ varTemplateDecl;
+ Decl *ToTU =getToTuDecl(
+ R"(
+ template <class T>
+ constexpr T X;
+ )",
+ Lang_CXX14);
+ Decl *FromTU = getTuDecl(
+ R"(
+ template <int>
+ constexpr int X = 0;
+ )",
+ Lang_CXX14);
+ auto Pattern = varTemplateDecl(hasName("X"));
+ auto *FromX = FirstDeclMatcher<VarTemplateDecl>().match(
+ FromTU, varTemplateDecl(hasName("X")));
+ Expected<Decl*> Result = importOrError(FromX, Lang_CXX11);
+ CheckImportNameConflict<VarTemplateDecl>(Result, ToTU, Pattern);
+}
+
+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<TypedefNameDecl>().match(
+ FromTU, Pattern);
+
+ Expected<Decl*> Result = importOrError(FromX, Lang_CXX11);
+ CheckImportedAsNew<TypedefNameDecl>(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<TypedefNameDecl>().match(
+ FromTU, Pattern);
+ Expected<Decl*> Result = importOrError(FromX, Lang_CXX11);
+ CheckImportedAsNew<TypedefNameDecl>(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<EnumDecl>().match(
+ FromTU, Pattern);
+ Expected<Decl*> Result = importOrError(FromX, Lang_CXX11);
+ CheckImportedAsNew<EnumDecl>(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<EnumConstantDecl>().match(
+ FromTU, Pattern);
+ Expected<Decl*> Result = importOrError(FromX, Lang_CXX11);
+ CheckImportedAsNew<EnumConstantDecl>(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<RecordDecl>().match(
+ FromTU, Pattern);
+ Expected<Decl*> Result = importOrError(FromX, Lang_CXX11);
+ CheckImportedAsNew<RecordDecl>(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<VarDecl>().match(
+ FromTU, Pattern);
+ Expected<Decl*> Result = importOrError(FromX, Lang_CXX11);
+ CheckImportedAsNew<VarDecl>(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<FunctionDecl>().match(
+ FromTU, Pattern);
+ Expected<Decl*> Result = importOrError(FromX, Lang_CXX11);
+ CheckImportedAsNew<FunctionDecl>(Result, ToTU, Pattern);
+}
+
+TEST_P(ConflictingDeclsWithLiberalStrategy, ClassTemplateDecl) {
+ Decl * ToTU = getToTuDecl(
+ R"(
+ template <class>
+ struct X;
+ )",
+ Lang_CXX11);
+ Decl *FromTU = getTuDecl(
+ R"(
+ template <int>
+ struct X;
+ )",
+ Lang_CXX11);
+ auto Pattern = classTemplateDecl(hasName("X"));
+ auto *FromX = FirstDeclMatcher<ClassTemplateDecl>().match(
+ FromTU, Pattern);
+ Expected<Decl*> Result = importOrError(FromX, Lang_CXX11);
+ CheckImportedAsNew<ClassTemplateDecl>(Result, ToTU, Pattern);
+}
+
+TEST_P(ConflictingDeclsWithLiberalStrategy, DISABLED_VarTemplateDecl) {
+ const internal::VariadicDynCastAllOfMatcher<Decl, VarTemplateDecl>
+ varTemplateDecl;
+ Decl *ToTU =getToTuDecl(
+ R"(
+ template <class T>
+ constexpr T X;
+ )",
+ Lang_CXX14);
+ Decl *FromTU = getTuDecl(
+ R"(
+ template <int>
+ constexpr int X = 0;
+ )",
+ Lang_CXX14);
+ auto Pattern = varTemplateDecl(hasName("X"));
+ auto *FromX = FirstDeclMatcher<VarTemplateDecl>().match(
+ FromTU, varTemplateDecl(hasName("X")));
+ Expected<Decl*> Result = importOrError(FromX, Lang_CXX11);
+ CheckImportedAsNew<VarTemplateDecl>(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,19 +5809,22 @@
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, ImportFriendFunctionTemplates,
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctionTemplates,
DefaultTestValuesForRunOptions, );
-INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportClasses,
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFriendFunctionTemplates,
DefaultTestValuesForRunOptions, );
-INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctionTemplates,
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportClasses,
DefaultTestValuesForRunOptions, );
INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFriendFunctions,
@@ -5418,6 +5846,12 @@
INSTANTIATE_TEST_CASE_P(ParameterizedTests, LLDBLookupTest,
DefaultTestValuesForRunOptions, );
+INSTANTIATE_TEST_CASE_P(ParameterizedTests,
+ ConflictingDeclsWithConservativeStrategy,
+ DefaultTestValuesForRunOptions, );
+
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, ConflictingDeclsWithLiberalStrategy,
+ DefaultTestValuesForRunOptions, );
} // end namespace ast_matchers
} // end namespace clang
Index: clang/unittests/AST/ASTImporterFixtures.h
===================================================================
--- clang/unittests/AST/ASTImporterFixtures.h
+++ 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 <sstream>
+
namespace clang {
class ASTImporter;
@@ -82,6 +86,9 @@
const std::shared_ptr<ASTImporterSharedState> &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 @@
TranslationUnitDecl *TUDecl = nullptr;
std::unique_ptr<ASTImporter> 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 @@
ASTUnit *ToAST);
Decl *import(const std::shared_ptr<ASTImporterSharedState> &SharedState,
ASTUnit *ToAST, Decl *FromDecl);
+ llvm::Expected<Decl *>
+ importOrError(const std::shared_ptr<ASTImporterSharedState> &SharedState,
+ ASTUnit *ToAST, Decl *FromDecl);
QualType import(const std::shared_ptr<ASTImporterSharedState> &SharedState,
ASTUnit *ToAST, QualType FromType);
};
@@ -162,8 +175,14 @@
return cast_or_null<DeclT>(Import(cast<Decl>(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<Decl *> importOrError(Decl *From, Language ToLang);
+
QualType ImportType(QualType FromType, Decl *TUDecl, Language ToLang);
+ ASTImporterTestBase()
+ : ODRHandling(ASTImporter::ODRHandlingType::Conservative) {}
~ASTImporterTestBase();
};
@@ -174,6 +193,46 @@
ArgVector getExtraArgs() const override { return GetParam(); }
};
+template <class T>
+::testing::AssertionResult isSuccess(llvm::Expected<T> &ValOrErr) {
+ if (ValOrErr)
+ return ::testing::AssertionSuccess() << "Expected<> contains no error.";
+ else
+ return ::testing::AssertionFailure()
+ << "Expected<> contains error: " << toString(ValOrErr.takeError());
+}
+
+template <class T>
+::testing::AssertionResult isImportError(llvm::Expected<T> &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
Index: clang/unittests/AST/ASTImporterFixtures.cpp
===================================================================
--- clang/unittests/AST/ASTImporterFixtures.cpp
+++ clang/unittests/AST/ASTImporterFixtures.cpp
@@ -39,10 +39,12 @@
}
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 @@
const std::shared_ptr<ASTImporterSharedState> &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 @@
}
}
+llvm::Expected<Decl *> ASTImporterTestBase::TU::importOrError(
+ const std::shared_ptr<ASTImporterSharedState> &SharedState, ASTUnit *ToAST,
+ Decl *FromDecl) {
+ lazyInitImporter(SharedState, ToAST);
+ return Importer->Import(FromDecl);
+}
+
QualType ASTImporterTestBase::TU::import(
const std::shared_ptr<ASTImporterSharedState> &SharedState, ASTUnit *ToAST,
QualType FromType) {
@@ -132,7 +143,8 @@
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 @@
}) == 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 @@
return To;
}
+llvm::Expected<Decl *> ASTImporterTestBase::importOrError(Decl *From,
+ Language ToLang) {
+ lazyInitToAST(ToLang, "", OutputFileName);
+ TU *FromTU = findFromTU(From);
+ assert(SharedStatePtr);
+ llvm::Expected<Decl *> To =
+ FromTU->importOrError(SharedStatePtr, ToAST.get(), From);
+ return To;
+}
+
QualType ASTImporterTestBase::ImportType(QualType FromType, Decl *TUDecl,
Language ToLang) {
lazyInitToAST(ToLang, "", OutputFileName);
Index: clang/test/Analysis/Inputs/ctu-other.c
===================================================================
--- clang/test/Analysis/Inputs/ctu-other.c
+++ 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
Index: clang/lib/AST/ASTImporter.cpp
===================================================================
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -80,6 +80,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.
@@ -2247,11 +2248,13 @@
}
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)
+ Name = NameOrErr.get();
+ else
+ return NameOrErr.takeError();
}
}
@@ -2355,21 +2358,21 @@
// 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>(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 @@
}
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)
+ Name = NameOrErr.get();
+ else
+ return NameOrErr.takeError();
}
}
@@ -2550,17 +2554,18 @@
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>(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 @@
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>(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 @@
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)
+ Name = NameOrErr.get();
+ else
+ return NameOrErr.takeError();
}
}
@@ -3102,17 +3108,17 @@
<< 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)
+ Name = NameOrErr.get();
+ else
+ return NameOrErr.takeError();
}
}
@@ -3405,6 +3411,7 @@
// Determine whether we've already imported this field.
auto FoundDecls = Importer.findDeclsInToCtx(DC, Name);
+ SmallVector<NamedDecl *, 4> ConflictingDecls;
for (auto *FoundDecl : FoundDecls) {
if (FieldDecl *FoundField = dyn_cast<FieldDecl>(FoundDecl)) {
// For anonymous fields, match up by index.
@@ -3438,16 +3445,24 @@
return FoundField;
}
- // FIXME: Why is this case not handled with calling HandleNameConflict?
Importer.ToDiag(Loc, diag::warn_odr_field_type_inconsistent)
<< Name << D->getType() << FoundField->getType();
Importer.ToDiag(FoundField->getLocation(), diag::note_odr_value_here)
<< FoundField->getType();
-
- return make_error<ImportError>(ImportError::NameConflict);
+ ConflictingDecls.push_back(FoundField);
}
}
+ if (!ConflictingDecls.empty()) {
+ ExpectedName NameOrErr = Importer.HandleNameConflict(
+ Name, DC, Decl::IDNS_Member, ConflictingDecls.data(),
+ ConflictingDecls.size());
+ if (NameOrErr)
+ Name = NameOrErr.get();
+ else
+ return NameOrErr.takeError();
+ }
+
QualType ToType;
TypeSourceInfo *ToTInfo;
Expr *ToBitWidth;
@@ -3490,6 +3505,7 @@
// Determine whether we've already imported this field.
auto FoundDecls = Importer.findDeclsInToCtx(DC, Name);
+ SmallVector<NamedDecl *, 4> ConflictingDecls;
for (unsigned I = 0, N = FoundDecls.size(); I != N; ++I) {
if (auto *FoundField = dyn_cast<IndirectFieldDecl>(FoundDecls[I])) {
// For anonymous indirect fields, match up by index.
@@ -3509,16 +3525,24 @@
if (!Name && I < N-1)
continue;
- // FIXME: Why is this case not handled with calling HandleNameConflict?
Importer.ToDiag(Loc, diag::warn_odr_field_type_inconsistent)
<< Name << D->getType() << FoundField->getType();
Importer.ToDiag(FoundField->getLocation(), diag::note_odr_value_here)
<< FoundField->getType();
-
- return make_error<ImportError>(ImportError::NameConflict);
+ ConflictingDecls.push_back(FoundField);
}
}
+ if (!ConflictingDecls.empty()) {
+ ExpectedName NameOrErr = Importer.HandleNameConflict(
+ Name, DC, Decl::IDNS_Member, ConflictingDecls.data(),
+ ConflictingDecls.size());
+ if (NameOrErr)
+ Name = NameOrErr.get();
+ else
+ return NameOrErr.takeError();
+ }
+
// Import the type.
auto TypeOrErr = import(D->getType());
if (!TypeOrErr)
@@ -3758,17 +3782,17 @@
<< 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)
+ Name = NameOrErr.get();
+ else
+ return NameOrErr.takeError();
}
}
@@ -5106,19 +5130,19 @@
// 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>(ImportError::NameConflict);
}
CXXRecordDecl *FromTemplated = D->getTemplatedDecl();
@@ -5391,22 +5415,20 @@
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>(ImportError::NameConflict);
-
VarDecl *DTemplated = D->getTemplatedDecl();
// Import the type.
@@ -7817,7 +7839,7 @@
std::shared_ptr<ASTImporterSharedState> 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) {
@@ -8743,12 +8765,17 @@
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) {
+ if (ODRHandling == ODRHandlingType::Conservative)
+ // Report error at any name conflict.
+ return make_error<ImportError>(ImportError::NameConflict);
+ else
+ // Allow to create the new Decl with the same name.
+ return Name;
}
DiagnosticBuilder ASTImporter::ToDiag(SourceLocation Loc, unsigned DiagID) {
Index: clang/include/clang/AST/ASTImporter.h
===================================================================
--- clang/include/clang/AST/ASTImporter.h
+++ clang/include/clang/AST/ASTImporter.h
@@ -90,6 +90,8 @@
using FileIDImportHandlerType =
std::function<void(FileID /*ToID*/, FileID /*FromID*/)>;
+ 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 @@
/// 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 @@
/// 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 @@
///
/// \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; }
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
