tk1012 updated this revision to Diff 123493.
tk1012 added a comment.
Herald added a subscriber: rnkovacs.
Hello,
I update the diff to solve the below thing.
> 1. Are import conflicts for anonymous structures resolved correctly?
Before I describe the updates, I want to detail the difference between unnamed
structs and anonymous structs in clang.
According to the comment of RecordDecl::isAnonymousStructOrUnion in
include/AST/Decl.h,
there are unnamed structures that are not anonymous.
For example,
union { int i; float f; } obj;
struct { int a } A;
both of them are not anonymous but unnamed.
Originally, the anonymous structs/unions are checked by making sure that they
occur in the same location.
However, the unnamed structures that are not anonymous are only checked by
their name.
This results in the different unnamed structures are identified as the same one
because their names are the null string.
So, this update makes the skip of the confliction check more strictly.
It also adds a test for importing the anonymous unions
> Are equal structures present in both TUs imported correctly, without
> duplication?
I think this is a more complicated problem because this does not depend on the
structures are unnamed or not.
For example, I try the below test and "make check-clang" fails due to "the
Multiple declarations were found!".
TEST(ImportDecl, ImportSameRecordDecl) {
MatchVerifier<Decl> Verifier;
EXPECT_TRUE(
testImport(
"struct A {"
" int a;"
"};",
Lang_C,
"struct A {"
" int a;"
"};",
Lang_C, Verifier,
recordDecl()));
}
struct A is not anonymous and also not unnamed.
To avoid this, in my application, I check the same structure already exists in
the "To" ASTContext before the import.
If the same struct is found, I map the "From" RecordDecl to the "To" one in
advance.
However, I'm not sure I should request the review of this solution. ( I mean,
should ASTImporter support this ?)
Even if I should, I think this feature must be in a different patch.
Best regards
https://reviews.llvm.org/D39886
Files:
lib/AST/ASTImporter.cpp
unittests/AST/ASTImporterTest.cpp
Index: unittests/AST/ASTImporterTest.cpp
===================================================================
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -485,6 +485,65 @@
has(atomicType()))))))))));
}
+TEST(ImportDecl, ImportUnnamedRecordDecl) {
+ MatchVerifier<Decl> Verifier;
+ EXPECT_TRUE(
+ testImport(
+ "void declToImport() {"
+ " struct Root {"
+ " struct { int a; } A;"
+ " struct { float b; } B;"
+ " } root;"
+ "}",
+ Lang_C, "", Lang_C, Verifier,
+ functionDecl(
+ hasBody(
+ compoundStmt(
+ has(
+ declStmt(
+ has(
+ recordDecl(
+ has(
+ recordDecl(
+ has(
+ fieldDecl(
+ hasType(asString("int")))))),
+ has(
+ recordDecl(
+ has(
+ fieldDecl(
+ hasType(asString("float"))))))
+ )))))))));
+}
+
+TEST(ImportDecl, ImportAnonymousRecordDecl) {
+ MatchVerifier<Decl> Verifier;
+ EXPECT_TRUE(
+ testImport(
+ "void declToImport() {"
+ " struct Root {"
+ " union { char a;};"
+ " union { int b;};"
+ " } root;"
+ "}",
+ Lang_C, "", Lang_C, Verifier,
+ functionDecl(
+ hasBody(
+ compoundStmt(
+ has(
+ declStmt(
+ has(
+ recordDecl(
+ has(
+ recordDecl(
+ has(
+ fieldDecl(hasType(asString("char")))))),
+ has(
+ recordDecl(
+ has(
+ fieldDecl(hasType(asString("int"))))))
+ )))))))));
+}
} // end namespace ast_matchers
} // end namespace clang
Index: lib/AST/ASTImporter.cpp
===================================================================
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -1631,7 +1631,12 @@
// We may already have a record of the same name; try to find and match it.
RecordDecl *AdoptDecl = nullptr;
RecordDecl *PrevDecl = nullptr;
- if (!DC->isFunctionOrMethod()) {
+
+ // There are unnamed structures that are not anonymous.
+ // They cause wrong conflict detections due to the null string name.
+ bool isNotAnonymousButUnnamed = (!D->isAnonymousStructOrUnion() &&
SearchName.getAsString() == "");
+
+ if (!DC->isFunctionOrMethod() && !isNotAnonymousButUnnamed) {
SmallVector<NamedDecl *, 4> ConflictingDecls;
SmallVector<NamedDecl *, 2> FoundDecls;
DC->getRedeclContext()->localUncachedLookup(SearchName, FoundDecls);
Index: unittests/AST/ASTImporterTest.cpp
===================================================================
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -485,6 +485,65 @@
has(atomicType()))))))))));
}
+TEST(ImportDecl, ImportUnnamedRecordDecl) {
+ MatchVerifier<Decl> Verifier;
+ EXPECT_TRUE(
+ testImport(
+ "void declToImport() {"
+ " struct Root {"
+ " struct { int a; } A;"
+ " struct { float b; } B;"
+ " } root;"
+ "}",
+ Lang_C, "", Lang_C, Verifier,
+ functionDecl(
+ hasBody(
+ compoundStmt(
+ has(
+ declStmt(
+ has(
+ recordDecl(
+ has(
+ recordDecl(
+ has(
+ fieldDecl(
+ hasType(asString("int")))))),
+ has(
+ recordDecl(
+ has(
+ fieldDecl(
+ hasType(asString("float"))))))
+ )))))))));
+}
+
+TEST(ImportDecl, ImportAnonymousRecordDecl) {
+ MatchVerifier<Decl> Verifier;
+ EXPECT_TRUE(
+ testImport(
+ "void declToImport() {"
+ " struct Root {"
+ " union { char a;};"
+ " union { int b;};"
+ " } root;"
+ "}",
+ Lang_C, "", Lang_C, Verifier,
+ functionDecl(
+ hasBody(
+ compoundStmt(
+ has(
+ declStmt(
+ has(
+ recordDecl(
+ has(
+ recordDecl(
+ has(
+ fieldDecl(hasType(asString("char")))))),
+ has(
+ recordDecl(
+ has(
+ fieldDecl(hasType(asString("int"))))))
+ )))))))));
+}
} // end namespace ast_matchers
} // end namespace clang
Index: lib/AST/ASTImporter.cpp
===================================================================
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -1631,7 +1631,12 @@
// We may already have a record of the same name; try to find and match it.
RecordDecl *AdoptDecl = nullptr;
RecordDecl *PrevDecl = nullptr;
- if (!DC->isFunctionOrMethod()) {
+
+ // There are unnamed structures that are not anonymous.
+ // They cause wrong conflict detections due to the null string name.
+ bool isNotAnonymousButUnnamed = (!D->isAnonymousStructOrUnion() && SearchName.getAsString() == "");
+
+ if (!DC->isFunctionOrMethod() && !isNotAnonymousButUnnamed) {
SmallVector<NamedDecl *, 4> ConflictingDecls;
SmallVector<NamedDecl *, 2> FoundDecls;
DC->getRedeclContext()->localUncachedLookup(SearchName, FoundDecls);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
