martong created this revision.
martong added reviewers: a.sidorin, a_sidorin, balazske, gerazo.
Herald added subscribers: cfe-commits, dkrupp, rnkovacs.
https://reviews.llvm.org/D48773 simplified ASTImporter nicely, but it
introduced a new error: Unnamed
structs are not imported correctly, if they appear in a recursive context.
This patch provides a fix for structural equivalency.
Repository:
rC Clang
https://reviews.llvm.org/D49296
Files:
lib/AST/ASTStructuralEquivalence.cpp
unittests/AST/ASTImporterTest.cpp
unittests/AST/StructuralEquivalenceTest.cpp
Index: unittests/AST/StructuralEquivalenceTest.cpp
===================================================================
--- unittests/AST/StructuralEquivalenceTest.cpp
+++ unittests/AST/StructuralEquivalenceTest.cpp
@@ -42,6 +42,21 @@
return std::make_tuple(D0, D1);
}
+ std::tuple<TranslationUnitDecl *, TranslationUnitDecl *> makeTuDecls(
+ const std::string &SrcCode0, const std::string &SrcCode1, Language Lang) {
+ this->Code0 = SrcCode0;
+ this->Code1 = SrcCode1;
+ ArgVector Args = getBasicRunOptionsForLanguage(Lang);
+
+ const char *const InputFileName = "input.cc";
+
+ AST0 = tooling::buildASTFromCodeWithArgs(Code0, Args, InputFileName);
+ AST1 = tooling::buildASTFromCodeWithArgs(Code1, Args, InputFileName);
+
+ return std::make_tuple(AST0->getASTContext().getTranslationUnitDecl(),
+ AST1->getASTContext().getTranslationUnitDecl());
+ }
+
// Get a pair of node pointers into the synthesized AST from the given code
// snippets. The same matcher is used for both snippets.
template <typename NodeType, typename MatcherType>
@@ -62,15 +77,15 @@
return makeDecls<NamedDecl>(SrcCode0, SrcCode1, Lang, Matcher);
}
- bool testStructuralMatch(NamedDecl *D0, NamedDecl *D1) {
+ bool testStructuralMatch(Decl *D0, Decl *D1) {
llvm::DenseSet<std::pair<Decl *, Decl *>> NonEquivalentDecls;
StructuralEquivalenceContext Ctx(
D0->getASTContext(), D1->getASTContext(), NonEquivalentDecls,
StructuralEquivalenceKind::Default, false, false);
return Ctx.IsStructurallyEquivalent(D0, D1);
}
- bool testStructuralMatch(std::tuple<NamedDecl *, NamedDecl *> t) {
+ bool testStructuralMatch(std::tuple<Decl *, Decl *> t) {
return testStructuralMatch(get<0>(t), get<1>(t));
}
};
@@ -468,6 +483,10 @@
}
struct StructuralEquivalenceRecordTest : StructuralEquivalenceTest {
+ RecordDecl* getRecordDecl(FieldDecl *FD) {
+ auto *ET = cast<ElaboratedType>(FD->getType().getTypePtr());
+ return cast<RecordType>(ET->getNamedType().getTypePtr())->getDecl();
+ };
};
TEST_F(StructuralEquivalenceRecordTest, Name) {
@@ -535,6 +554,73 @@
EXPECT_TRUE(testStructuralMatch(t));
}
+TEST_F(StructuralEquivalenceRecordTest, UnnamedRecordsShouldBeInequivalent) {
+ auto t = makeTuDecls(
+ R"(
+ struct A {
+ struct {
+ struct A *next;
+ } entry0;
+ struct {
+ struct A *next;
+ } entry1;
+ };
+ )",
+ "", Lang_C);
+ auto *TU = get<0>(t);
+ auto *Entry0 =
+ FirstDeclMatcher<FieldDecl>().match(TU, fieldDecl(hasName("entry0")));
+ auto *Entry1 =
+ FirstDeclMatcher<FieldDecl>().match(TU, fieldDecl(hasName("entry1")));
+ auto *R0 = getRecordDecl(Entry0);
+ auto *R1 = getRecordDecl(Entry1);
+
+ ASSERT_TRUE(R0);
+ ASSERT_TRUE(R1);
+ ASSERT_NE(R0, R1);
+ EXPECT_TRUE(testStructuralMatch(R0, R0));
+ EXPECT_TRUE(testStructuralMatch(R1, R1));
+ EXPECT_FALSE(testStructuralMatch(R0, R1));
+}
+
+TEST_F(StructuralEquivalenceRecordTest,
+ UnnamedRecordsShouldBeInequivalentEvenIfTheSecondIsBeingDefined) {
+ auto Code =
+ R"(
+ struct A {
+ struct {
+ struct A *next;
+ } entry0;
+ struct {
+ struct A *next;
+ } entry1;
+ };
+ )";
+ auto t = makeTuDecls(Code, Code, Lang_C);
+
+ auto *FromTU = get<0>(t);
+ auto *Entry1 =
+ FirstDeclMatcher<FieldDecl>().match(FromTU, fieldDecl(hasName("entry1")));
+
+ auto *ToTU = get<1>(t);
+ auto *Entry0 =
+ FirstDeclMatcher<FieldDecl>().match(ToTU, fieldDecl(hasName("entry0")));
+ auto *A =
+ FirstDeclMatcher<RecordDecl>().match(ToTU, recordDecl(hasName("A")));
+ A->startDefinition(); // Set isBeingDefined, getDefinition() will return a
+ // nullptr. This may be the case during ASTImport.
+
+ auto *R0 = getRecordDecl(Entry0);
+ auto *R1 = getRecordDecl(Entry1);
+ ASSERT_TRUE(R0);
+ ASSERT_TRUE(R1);
+ ASSERT_NE(R0, R1);
+ EXPECT_TRUE(testStructuralMatch(R0, R0));
+ EXPECT_TRUE(testStructuralMatch(R1, R1));
+ EXPECT_FALSE(testStructuralMatch(R0, R1));
+}
+
+
TEST_F(StructuralEquivalenceTest, CompareSameDeclWithMultiple) {
auto t = makeNamedDecls(
"struct A{ }; struct B{ }; void foo(A a, A b);",
Index: unittests/AST/ASTImporterTest.cpp
===================================================================
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -2469,6 +2469,44 @@
EXPECT_NE(ToM1, ToM2);
}
+TEST_P(ASTImporterTestBase, ImportUnnamedStructsWithRecursingField) {
+ Decl *FromTU = getTuDecl(
+ R"(
+ struct A {
+ struct {
+ struct A *next;
+ } entry0;
+ struct {
+ struct A *next;
+ } entry1;
+ };
+ )",
+ Lang_C, "input0.cc");
+ auto *From =
+ FirstDeclMatcher<RecordDecl>().match(FromTU, recordDecl(hasName("A")));
+
+ Import(From, Lang_C);
+
+ auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+ auto *Entry0 =
+ FirstDeclMatcher<FieldDecl>().match(ToTU, fieldDecl(hasName("entry0")));
+ auto *Entry1 =
+ FirstDeclMatcher<FieldDecl>().match(ToTU, fieldDecl(hasName("entry1")));
+ auto getRecordDecl = [](FieldDecl *FD) {
+ auto *ET = cast<ElaboratedType>(FD->getType().getTypePtr());
+ return cast<RecordType>(ET->getNamedType().getTypePtr())->getDecl();
+ };
+ auto *R0 = getRecordDecl(Entry0);
+ auto *R1 = getRecordDecl(Entry1);
+ ASSERT_TRUE(R0);
+ ASSERT_TRUE(R1);
+ EXPECT_NE(R0, R1);
+ EXPECT_TRUE(MatchVerifier<RecordDecl>().match(
+ R0, recordDecl(has(fieldDecl(hasName("next"))))));
+ EXPECT_TRUE(MatchVerifier<RecordDecl>().match(
+ R1, recordDecl(has(fieldDecl(hasName("next"))))));
+}
+
struct DeclContextTest : ASTImporterTestBase {};
TEST_P(DeclContextTest, removeDeclOfClassTemplateSpecialization) {
Index: lib/AST/ASTStructuralEquivalence.cpp
===================================================================
--- lib/AST/ASTStructuralEquivalence.cpp
+++ lib/AST/ASTStructuralEquivalence.cpp
@@ -910,7 +910,7 @@
return false;
}
- if (D1->isAnonymousStructOrUnion() && D2->isAnonymousStructOrUnion()) {
+ if (!D1->getDeclName() && !D2->getDeclName()) {
// If both anonymous structs/unions are in a record context, make sure
// they occur in the same location in the context records.
if (Optional<unsigned> Index1 =
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
