https://github.com/balazske created
https://github.com/llvm/llvm-project/pull/99281
After the last change in PR #87144 regressions appeared in some cases. The
problem was that if multiple anonymous enums are present in a class and are
imported as new the import of the second enum can fail because it is detected
as different from the first and causes ODR error.
Now in case of enums without name an existing similar enum is searched, if not
found the enum is imported. ODR error is not detected. This may be incorrect if
non-matching structures are imported, but this is the less important case
(import of matching classes is more important to work).
From a4886d59c3a8c7222d80a48206db22a521f5851d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com>
Date: Tue, 16 Jul 2024 18:19:10 +0200
Subject: [PATCH] [clang][ASTImporter] Fix import of anonymous enums if
multiple are present.
After the last change in PR #87144 regressions appeared in some cases.
The problem was that if multiple anonymous enums are present in a class
and are imported as new the import of the second enum can fail because
it is detected as different from the first and causes ODR error.
Now in case of enums without name an existing similar enum is searched,
if not found the enum is imported. ODR error is not detected. This may
be incorrect if non-matching structures are imported, but this is the
less important case (import of matching classes is more important to
work).
---
clang/lib/AST/ASTImporter.cpp | 9 ++-
clang/unittests/AST/ASTImporterTest.cpp | 95 +++++++++++++++++++++----
2 files changed, 89 insertions(+), 15 deletions(-)
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 4e1b3a5a94de7..43891d29ad4ea 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -2949,7 +2949,7 @@ ExpectedDecl ASTNodeImporter::VisitEnumDecl(EnumDecl *D) {
if (auto *FoundEnum = dyn_cast<EnumDecl>(FoundDecl)) {
if (!hasSameVisibilityContextAndLinkage(FoundEnum, D))
continue;
- if (IsStructuralMatch(D, FoundEnum)) {
+ if (IsStructuralMatch(D, FoundEnum, !SearchName.isEmpty())) {
EnumDecl *FoundDef = FoundEnum->getDefinition();
if (D->isThisDeclarationADefinition() && FoundDef)
return Importer.MapImported(D, FoundDef);
@@ -2960,7 +2960,12 @@ ExpectedDecl ASTNodeImporter::VisitEnumDecl(EnumDecl *D)
{
}
}
- if (!ConflictingDecls.empty()) {
+ // In case of unnamed enums, we try to find an existing similar one, if
none
+ // was found, perform the import always.
+ // Structural in-equivalence is not detected in this way here, but it may
+ // be found when the parent decl is imported (if the enum is part of a
+ // class). To make this totally exact a more difficult solution is needed.
+ if (SearchName && !ConflictingDecls.empty()) {
ExpectedName NameOrErr = Importer.HandleNameConflict(
SearchName, DC, IDNS, ConflictingDecls.data(),
ConflictingDecls.size());
diff --git a/clang/unittests/AST/ASTImporterTest.cpp
b/clang/unittests/AST/ASTImporterTest.cpp
index 92f9bae6cb064..6d987cc7e9ec6 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -9681,37 +9681,106 @@ AST_MATCHER_P(EnumDecl, hasEnumConstName, StringRef,
ConstName) {
return false;
}
-TEST_P(ASTImporterOptionSpecificTestBase, ImportAnonymousEnum) {
+TEST_P(ASTImporterOptionSpecificTestBase, ImportAnonymousEnums) {
+ const char *Code =
+ R"(
+ struct A {
+ enum { E1, E2 } x;
+ enum { E3, E4 } y;
+ };
+ )";
+ Decl *FromTU = getTuDecl(Code, Lang_CXX11);
+ auto *FromEnumE1 = FirstDeclMatcher<EnumDecl>().match(
+ FromTU, enumDecl(hasEnumConstName("E1")));
+ auto *ImportedEnumE1 = Import(FromEnumE1, Lang_CXX11);
+ EXPECT_TRUE(ImportedEnumE1);
+ auto *FromEnumE3 = FirstDeclMatcher<EnumDecl>().match(
+ FromTU, enumDecl(hasEnumConstName("E3")));
+ auto *ImportedEnumE3 = Import(FromEnumE3, Lang_CXX11);
+ EXPECT_TRUE(ImportedEnumE3);
+ EXPECT_NE(ImportedEnumE1, ImportedEnumE3);
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase, ImportFreeStandingAnonymousEnums) {
+ const char *Code =
+ R"(
+ struct A {
+ enum { E1, E2 };
+ enum { E3, E4 };
+ };
+ )";
+ Decl *FromTU = getTuDecl(Code, Lang_CXX11);
+ auto *FromEnumE1 = FirstDeclMatcher<EnumDecl>().match(
+ FromTU, enumDecl(hasEnumConstName("E1")));
+ auto *ImportedEnumE1 = Import(FromEnumE1, Lang_CXX11);
+ EXPECT_TRUE(ImportedEnumE1);
+ auto *FromEnumE3 = FirstDeclMatcher<EnumDecl>().match(
+ FromTU, enumDecl(hasEnumConstName("E3")));
+ auto *ImportedEnumE3 = Import(FromEnumE3, Lang_CXX11);
+ EXPECT_TRUE(ImportedEnumE3);
+ EXPECT_NE(ImportedEnumE1, ImportedEnumE3);
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase, ImportExistingAnonymousEnums) {
const char *ToCode =
R"(
struct A {
- enum { E1, E2} x;
- enum { E3, E4} y;
+ enum { E1, E2 } x;
+ enum { E3, E4 } y;
};
)";
Decl *ToTU = getToTuDecl(ToCode, Lang_CXX11);
- auto *ToE1 = FirstDeclMatcher<EnumDecl>().match(
+ auto *ToEnumE1 = FirstDeclMatcher<EnumDecl>().match(
ToTU, enumDecl(hasEnumConstName("E1")));
- auto *ToE3 = FirstDeclMatcher<EnumDecl>().match(
+ auto *ToEnumE3 = FirstDeclMatcher<EnumDecl>().match(
ToTU, enumDecl(hasEnumConstName("E3")));
const char *Code =
R"(
struct A {
- enum { E1, E2} x;
- enum { E3, E4} y;
+ enum { E1, E2 } x;
+ enum { E3, E4 } y;
};
)";
Decl *FromTU = getTuDecl(Code, Lang_CXX11);
- auto *FromE1 = FirstDeclMatcher<EnumDecl>().match(
+ auto *FromEnumE1 = FirstDeclMatcher<EnumDecl>().match(
FromTU, enumDecl(hasEnumConstName("E1")));
+ auto *ImportedEnumE1 = Import(FromEnumE1, Lang_CXX11);
+ ASSERT_TRUE(ImportedEnumE1);
+ EXPECT_EQ(ImportedEnumE1, ToEnumE1);
+ auto *FromEnumE3 = FirstDeclMatcher<EnumDecl>().match(
+ FromTU, enumDecl(hasEnumConstName("E3")));
+ auto *ImportedEnumE3 = Import(FromEnumE3, Lang_CXX11);
+ ASSERT_TRUE(ImportedEnumE3);
+ EXPECT_EQ(ImportedEnumE3, ToEnumE3);
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase, ImportExistingEmptyAnonymousEnums) {
+ const char *ToCode =
+ R"(
+ struct A {
+ enum {};
+ };
+ )";
+ Decl *ToTU = getToTuDecl(ToCode, Lang_CXX11);
+ auto *ToE1 = FirstDeclMatcher<EnumDecl>().match(ToTU, enumDecl());
+ const char *Code =
+ R"(
+ struct A {
+ enum {};
+ enum {};
+ };
+ )";
+ Decl *FromTU = getTuDecl(Code, Lang_CXX11);
+ auto *FromE1 = FirstDeclMatcher<EnumDecl>().match(FromTU, enumDecl());
auto *ImportedE1 = Import(FromE1, Lang_CXX11);
ASSERT_TRUE(ImportedE1);
EXPECT_EQ(ImportedE1, ToE1);
- auto *FromE3 = FirstDeclMatcher<EnumDecl>().match(
- FromTU, enumDecl(hasEnumConstName("E3")));
- auto *ImportedE3 = Import(FromE3, Lang_CXX11);
- ASSERT_TRUE(ImportedE3);
- EXPECT_EQ(ImportedE3, ToE3);
+ auto *FromE2 = LastDeclMatcher<EnumDecl>().match(FromTU, enumDecl());
+ ASSERT_NE(FromE1, FromE2);
+ auto *ImportedE2 = Import(FromE2, Lang_CXX11);
+ ASSERT_TRUE(ImportedE2);
+ // FIXME: These should not be equal, or the import should fail.
+ EXPECT_EQ(ImportedE2, ToE1);
}
INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ASTImporterLookupTableTest,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
