[PATCH] D49796: [ASTImporter] Load external Decls when getting field index.
balazske created this revision.
Herald added subscribers: cfe-commits, martong.
Herald added a reviewer: a.sidorin.
At equality check of fields without name the index of fields is compared.
At determining the index of a field all fields of the parent context
should be loaded from external source to find the field at all.
Repository:
rC Clang
https://reviews.llvm.org/D49796
Files:
lib/AST/ASTImporter.cpp
test/ASTMerge/unnamed_fields/Inputs/il.cpp
test/ASTMerge/unnamed_fields/test.cpp
unittests/AST/ASTImporterTest.cpp
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -2612,6 +2612,45 @@
R1, recordDecl(has(fieldDecl(hasName("next"));
}
+TEST_P(ASTImporterTestBase, ImportUnnamedFieldsInCorrectOrder) {
+ Decl *FromTU = getTuDecl(
+ R"(
+ void f(int X, int Y, bool Z) {
+(void)[X, Y, Z] { (void)Z; };
+ }
+ )",
+ Lang_CXX11, "input0.cc");
+ auto *FromF = FirstDeclMatcher().match(
+ FromTU, functionDecl(hasName("f")));
+ auto *ToF = cast_or_null(Import(FromF, Lang_CXX11));
+ EXPECT_TRUE(ToF);
+
+ CXXRecordDecl *FromLambda =
+ cast(cast(cast(
+ FromF->getBody())->body_front())->getSubExpr())->getLambdaClass();
+
+ auto *ToLambda = cast_or_null(Import(FromLambda, Lang_CXX11));
+ EXPECT_TRUE(ToLambda);
+
+ // Check if the fields of the lambda class are imported in correct order.
+ unsigned FromIndex = 0u;
+ for (auto *FromField : FromLambda->fields()) {
+ASSERT_FALSE(FromField->getDeclName());
+auto *ToField = cast_or_null(Import(FromField, Lang_CXX11));
+EXPECT_TRUE(ToField);
+unsigned ToIndex = 0u;
+for (auto *F : ToLambda->fields()) {
+ if (F == ToField)
+break;
+ ++ToIndex;
+}
+EXPECT_EQ(ToIndex, FromIndex);
+++FromIndex;
+ }
+
+ EXPECT_EQ(FromIndex, 3u);
+}
+
struct DeclContextTest : ASTImporterTestBase {};
TEST_P(DeclContextTest, removeDeclOfClassTemplateSpecialization) {
Index: test/ASTMerge/unnamed_fields/test.cpp
===
--- /dev/null
+++ test/ASTMerge/unnamed_fields/test.cpp
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 -emit-pch -o %t.1.ast %S/Inputs/il.cpp
+// RUN: %clang_cc1 -ast-merge %t.1.ast -fsyntax-only %s 2>&1 | FileCheck
--allow-empty %s
+// CHECK-NOT: warning: field '' declared with incompatible types in different
translation units ('bool' vs. 'int')
Index: test/ASTMerge/unnamed_fields/Inputs/il.cpp
===
--- /dev/null
+++ test/ASTMerge/unnamed_fields/Inputs/il.cpp
@@ -0,0 +1,3 @@
+void f(int X, int Y, bool Z) {
+ auto x = [X, Y, Z] { (void)Z; };
+}
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2829,15 +2829,17 @@
return 0;
unsigned Index = 1;
- for (const auto *D : Owner->noload_decls()) {
+ for (const auto *D : Owner->decls()) {
if (D == F)
return Index;
if (isa(*D) || isa(*D))
++Index;
}
- return Index;
+ assert(false && "Field was not found in its parent context.");
+
+ return 0;
}
Decl *ASTNodeImporter::VisitFieldDecl(FieldDecl *D) {
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -2612,6 +2612,45 @@
R1, recordDecl(has(fieldDecl(hasName("next"));
}
+TEST_P(ASTImporterTestBase, ImportUnnamedFieldsInCorrectOrder) {
+ Decl *FromTU = getTuDecl(
+ R"(
+ void f(int X, int Y, bool Z) {
+(void)[X, Y, Z] { (void)Z; };
+ }
+ )",
+ Lang_CXX11, "input0.cc");
+ auto *FromF = FirstDeclMatcher().match(
+ FromTU, functionDecl(hasName("f")));
+ auto *ToF = cast_or_null(Import(FromF, Lang_CXX11));
+ EXPECT_TRUE(ToF);
+
+ CXXRecordDecl *FromLambda =
+ cast(cast(cast(
+ FromF->getBody())->body_front())->getSubExpr())->getLambdaClass();
+
+ auto *ToLambda = cast_or_null(Import(FromLambda, Lang_CXX11));
+ EXPECT_TRUE(ToLambda);
+
+ // Check if the fields of the lambda class are imported in correct order.
+ unsigned FromIndex = 0u;
+ for (auto *FromField : FromLambda->fields()) {
+ASSERT_FALSE(FromField->getDeclName());
+auto *ToField = cast_or_null(Import(FromField, Lang_CXX11));
+EXPECT_TRUE(ToField);
+unsigned ToIndex = 0u;
+for (auto *F : ToLambda->fields()) {
+ if (F == ToField)
+break;
+ ++ToIndex;
+}
+EXPECT_EQ(ToIndex, FromIndex);
+++FromIndex;
+ }
+
+ EXPECT_EQ(FromIndex, 3u);
+}
+
struct DeclContextTest : ASTImporterTestBase {};
TEST_P(DeclContextTest, removeDeclOfClassTemplateSpecialization) {
Index: test/ASTMerge/unnamed_fields/test.cpp
[PATCH] D49796: [ASTImporter] Load external Decls when getting field index.
a_sidorin added a comment.
Hi Balázs,
The approach is OK but I have some minor comments inline.
Comment at: lib/AST/ASTImporter.cpp:2840
- return Index;
+ assert(false && "Field was not found in its parent context.");
+
`llvm_unreachable`?
Comment at: unittests/AST/ASTImporterTest.cpp:2642
+unsigned ToIndex = 0u;
+for (auto *F : ToLambda->fields()) {
+ if (F == ToField)
I think we can make `getFieldIndex()` a static method of ASTImporter and remove
this loop.
Repository:
rC Clang
https://reviews.llvm.org/D49796
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49796: [ASTImporter] Load external Decls when getting field index.
balazske updated this revision to Diff 159467.
balazske added a comment.
- Added common getFieldIndex.
Repository:
rC Clang
https://reviews.llvm.org/D49796
Files:
include/clang/AST/ASTImporter.h
lib/AST/ASTImporter.cpp
test/ASTMerge/unnamed_fields/Inputs/il.cpp
test/ASTMerge/unnamed_fields/test.cpp
unittests/AST/ASTImporterTest.cpp
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -2612,6 +2612,40 @@
R1, recordDecl(has(fieldDecl(hasName("next"));
}
+TEST_P(ASTImporterTestBase, ImportUnnamedFieldsInCorrectOrder) {
+ Decl *FromTU = getTuDecl(
+ R"(
+ void f(int X, int Y, bool Z) {
+(void)[X, Y, Z] { (void)Z; };
+ }
+ )",
+ Lang_CXX11, "input0.cc");
+ auto *FromF = FirstDeclMatcher().match(
+ FromTU, functionDecl(hasName("f")));
+ auto *ToF = cast_or_null(Import(FromF, Lang_CXX11));
+ EXPECT_TRUE(ToF);
+
+ CXXRecordDecl *FromLambda =
+ cast(cast(cast(
+ FromF->getBody())->body_front())->getSubExpr())->getLambdaClass();
+
+ auto *ToLambda = cast_or_null(Import(FromLambda, Lang_CXX11));
+ EXPECT_TRUE(ToLambda);
+
+ // Check if the fields of the lambda class are imported in correct order.
+ unsigned FromIndex = 0u;
+ for (auto *FromField : FromLambda->fields()) {
+ASSERT_FALSE(FromField->getDeclName());
+auto *ToField = cast_or_null(Import(FromField, Lang_CXX11));
+EXPECT_TRUE(ToField);
+unsigned ToIndex = ASTImporter::getFieldIndex(ToField);
+EXPECT_EQ(ToIndex, FromIndex);
+++FromIndex;
+ }
+
+ EXPECT_EQ(FromIndex, 3u);
+}
+
struct DeclContextTest : ASTImporterTestBase {};
TEST_P(DeclContextTest, removeDeclOfClassTemplateSpecialization) {
Index: test/ASTMerge/unnamed_fields/test.cpp
===
--- /dev/null
+++ test/ASTMerge/unnamed_fields/test.cpp
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 -emit-pch -o %t.1.ast %S/Inputs/il.cpp
+// RUN: %clang_cc1 -ast-merge %t.1.ast -fsyntax-only %s 2>&1 | FileCheck --allow-empty %s
+// CHECK-NOT: warning: field '' declared with incompatible types in different translation units ('bool' vs. 'int')
Index: test/ASTMerge/unnamed_fields/Inputs/il.cpp
===
--- /dev/null
+++ test/ASTMerge/unnamed_fields/Inputs/il.cpp
@@ -0,0 +1,3 @@
+void f(int X, int Y, bool Z) {
+ auto x = [X, Y, Z] { (void)Z; };
+}
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -71,6 +71,28 @@
namespace clang {
+ unsigned ASTImporter::getFieldIndex(Decl *F) {
+assert(F && (isa(*F) || isa(*F)) &&
+"Try to get field index for non-field.");
+
+auto *Owner = dyn_cast(F->getDeclContext());
+if (!Owner)
+ return 0;
+
+unsigned Index = 1;
+for (const auto *D : Owner->decls()) {
+ if (D == F)
+return Index;
+
+ if (isa(*D) || isa(*D))
+++Index;
+}
+
+llvm_unreachable("Field was not found in its parent context.");
+
+return 0;
+ }
+
template
SmallVector
getCanonicalForwardRedeclChain(Redeclarable* D) {
@@ -2823,23 +2845,6 @@
return VisitCXXMethodDecl(D);
}
-static unsigned getFieldIndex(Decl *F) {
- auto *Owner = dyn_cast(F->getDeclContext());
- if (!Owner)
-return 0;
-
- unsigned Index = 1;
- for (const auto *D : Owner->noload_decls()) {
-if (D == F)
- return Index;
-
-if (isa(*D) || isa(*D))
- ++Index;
- }
-
- return Index;
-}
-
Decl *ASTNodeImporter::VisitFieldDecl(FieldDecl *D) {
// Import the major distinguishing characteristics of a variable.
DeclContext *DC, *LexicalDC;
@@ -2857,7 +2862,9 @@
for (auto *FoundDecl : FoundDecls) {
if (auto *FoundField = dyn_cast(FoundDecl)) {
// For anonymous fields, match up by index.
- if (!Name && getFieldIndex(D) != getFieldIndex(FoundField))
+ if (!Name &&
+ ASTImporter::getFieldIndex(D) !=
+ ASTImporter::getFieldIndex(FoundField))
continue;
if (Importer.IsStructurallyEquivalent(D->getType(),
@@ -2922,7 +2929,9 @@
for (unsigned I = 0, N = FoundDecls.size(); I != N; ++I) {
if (auto *FoundField = dyn_cast(FoundDecls[I])) {
// For anonymous indirect fields, match up by index.
- if (!Name && getFieldIndex(D) != getFieldIndex(FoundField))
+ if (!Name &&
+ ASTImporter::getFieldIndex(D) !=
+ ASTImporter::getFieldIndex(FoundField))
continue;
if (Importer.IsStructurallyEquivalent(D->getType(),
Index: include/clang/AST/ASTImporter.h
===
--- include/clang/AST/ASTImporter.h
+++ include/clang/AST/ASTImporter.h
@@ -333,6 +333,13 @@
/// equivalent.
bool IsStructurallyEquivalent(QualType From, QualT
[PATCH] D49796: [ASTImporter] Load external Decls when getting field index.
balazske updated this revision to Diff 159659.
balazske added a comment.
- Added common getFieldIndex.
- Corrected test ImportUnnamedFieldsInCorrectOrder.
Repository:
rC Clang
https://reviews.llvm.org/D49796
Files:
include/clang/AST/ASTImporter.h
lib/AST/ASTImporter.cpp
test/ASTMerge/unnamed_fields/Inputs/il.cpp
test/ASTMerge/unnamed_fields/test.cpp
unittests/AST/ASTImporterTest.cpp
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -2641,6 +2641,40 @@
R1, recordDecl(has(fieldDecl(hasName("next"));
}
+TEST_P(ASTImporterTestBase, ImportUnnamedFieldsInCorrectOrder) {
+ Decl *FromTU = getTuDecl(
+ R"(
+ void f(int X, int Y, bool Z) {
+(void)[X, Y, Z] { (void)Z; };
+ }
+ )",
+ Lang_CXX11, "input0.cc");
+ auto *FromF = FirstDeclMatcher().match(
+ FromTU, functionDecl(hasName("f")));
+ auto *ToF = cast_or_null(Import(FromF, Lang_CXX11));
+ EXPECT_TRUE(ToF);
+
+ CXXRecordDecl *FromLambda =
+ cast(cast(cast(
+ FromF->getBody())->body_front())->getSubExpr())->getLambdaClass();
+
+ auto *ToLambda = cast_or_null(Import(FromLambda, Lang_CXX11));
+ EXPECT_TRUE(ToLambda);
+
+ // Check if the fields of the lambda class are imported in correct order.
+ unsigned FromIndex = 0u;
+ for (auto *FromField : FromLambda->fields()) {
+ASSERT_FALSE(FromField->getDeclName());
+auto *ToField = cast_or_null(Import(FromField, Lang_CXX11));
+EXPECT_TRUE(ToField);
+unsigned ToIndex = ASTImporter::getFieldIndex(ToField);
+EXPECT_EQ(ToIndex, FromIndex + 1);
+++FromIndex;
+ }
+
+ EXPECT_EQ(FromIndex, 3u);
+}
+
struct DeclContextTest : ASTImporterTestBase {};
TEST_P(DeclContextTest, removeDeclOfClassTemplateSpecialization) {
Index: test/ASTMerge/unnamed_fields/test.cpp
===
--- /dev/null
+++ test/ASTMerge/unnamed_fields/test.cpp
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 -emit-pch -o %t.1.ast %S/Inputs/il.cpp
+// RUN: %clang_cc1 -ast-merge %t.1.ast -fsyntax-only %s 2>&1 | FileCheck --allow-empty %s
+// CHECK-NOT: warning: field '' declared with incompatible types in different translation units ('bool' vs. 'int')
Index: test/ASTMerge/unnamed_fields/Inputs/il.cpp
===
--- /dev/null
+++ test/ASTMerge/unnamed_fields/Inputs/il.cpp
@@ -0,0 +1,3 @@
+void f(int X, int Y, bool Z) {
+ auto x = [X, Y, Z] { (void)Z; };
+}
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -71,6 +71,28 @@
namespace clang {
+ unsigned ASTImporter::getFieldIndex(Decl *F) {
+assert(F && (isa(*F) || isa(*F)) &&
+"Try to get field index for non-field.");
+
+auto *Owner = dyn_cast(F->getDeclContext());
+if (!Owner)
+ return 0;
+
+unsigned Index = 1;
+for (const auto *D : Owner->decls()) {
+ if (D == F)
+return Index;
+
+ if (isa(*D) || isa(*D))
+++Index;
+}
+
+llvm_unreachable("Field was not found in its parent context.");
+
+return 0;
+ }
+
template
SmallVector
getCanonicalForwardRedeclChain(Redeclarable* D) {
@@ -2829,23 +2851,6 @@
return VisitCXXMethodDecl(D);
}
-static unsigned getFieldIndex(Decl *F) {
- auto *Owner = dyn_cast(F->getDeclContext());
- if (!Owner)
-return 0;
-
- unsigned Index = 1;
- for (const auto *D : Owner->noload_decls()) {
-if (D == F)
- return Index;
-
-if (isa(*D) || isa(*D))
- ++Index;
- }
-
- return Index;
-}
-
Decl *ASTNodeImporter::VisitFieldDecl(FieldDecl *D) {
// Import the major distinguishing characteristics of a variable.
DeclContext *DC, *LexicalDC;
@@ -2863,7 +2868,9 @@
for (auto *FoundDecl : FoundDecls) {
if (auto *FoundField = dyn_cast(FoundDecl)) {
// For anonymous fields, match up by index.
- if (!Name && getFieldIndex(D) != getFieldIndex(FoundField))
+ if (!Name &&
+ ASTImporter::getFieldIndex(D) !=
+ ASTImporter::getFieldIndex(FoundField))
continue;
if (Importer.IsStructurallyEquivalent(D->getType(),
@@ -2928,7 +2935,9 @@
for (unsigned I = 0, N = FoundDecls.size(); I != N; ++I) {
if (auto *FoundField = dyn_cast(FoundDecls[I])) {
// For anonymous indirect fields, match up by index.
- if (!Name && getFieldIndex(D) != getFieldIndex(FoundField))
+ if (!Name &&
+ ASTImporter::getFieldIndex(D) !=
+ ASTImporter::getFieldIndex(FoundField))
continue;
if (Importer.IsStructurallyEquivalent(D->getType(),
Index: include/clang/AST/ASTImporter.h
===
--- include/clang/AST/ASTImporter.h
+++ include/clang/AST/ASTImporter.h
@@ -333,6 +333,13 @@
/// equivalent.
[PATCH] D49796: [ASTImporter] Load external Decls when getting field index.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL339226: [ASTImporter] Load external Decls when getting field
index. (authored by balazske, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D49796
Files:
cfe/trunk/include/clang/AST/ASTImporter.h
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/test/ASTMerge/unnamed_fields/Inputs/il.cpp
cfe/trunk/test/ASTMerge/unnamed_fields/test.cpp
cfe/trunk/unittests/AST/ASTImporterTest.cpp
Index: cfe/trunk/include/clang/AST/ASTImporter.h
===
--- cfe/trunk/include/clang/AST/ASTImporter.h
+++ cfe/trunk/include/clang/AST/ASTImporter.h
@@ -333,6 +333,13 @@
/// equivalent.
bool IsStructurallyEquivalent(QualType From, QualType To,
bool Complain = true);
+
+/// Determine the index of a field in its parent record.
+/// F should be a field (or indirect field) declaration.
+/// \returns The index of the field in its parent context, starting from 1.
+/// 0 is returned on error (parent context is non-record).
+static unsigned getFieldIndex(Decl *F);
+
};
} // namespace clang
Index: cfe/trunk/test/ASTMerge/unnamed_fields/Inputs/il.cpp
===
--- cfe/trunk/test/ASTMerge/unnamed_fields/Inputs/il.cpp
+++ cfe/trunk/test/ASTMerge/unnamed_fields/Inputs/il.cpp
@@ -0,0 +1,3 @@
+void f(int X, int Y, bool Z) {
+ auto x = [X, Y, Z] { (void)Z; };
+}
Index: cfe/trunk/test/ASTMerge/unnamed_fields/test.cpp
===
--- cfe/trunk/test/ASTMerge/unnamed_fields/test.cpp
+++ cfe/trunk/test/ASTMerge/unnamed_fields/test.cpp
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 -emit-pch -o %t.1.ast %S/Inputs/il.cpp
+// RUN: %clang_cc1 -ast-merge %t.1.ast -fsyntax-only %s 2>&1 | FileCheck --allow-empty %s
+// CHECK-NOT: warning: field '' declared with incompatible types in different translation units ('bool' vs. 'int')
Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -71,6 +71,28 @@
namespace clang {
+ unsigned ASTImporter::getFieldIndex(Decl *F) {
+assert(F && (isa(*F) || isa(*F)) &&
+"Try to get field index for non-field.");
+
+auto *Owner = dyn_cast(F->getDeclContext());
+if (!Owner)
+ return 0;
+
+unsigned Index = 1;
+for (const auto *D : Owner->decls()) {
+ if (D == F)
+return Index;
+
+ if (isa(*D) || isa(*D))
+++Index;
+}
+
+llvm_unreachable("Field was not found in its parent context.");
+
+return 0;
+ }
+
template
SmallVector
getCanonicalForwardRedeclChain(Redeclarable* D) {
@@ -2829,23 +2851,6 @@
return VisitCXXMethodDecl(D);
}
-static unsigned getFieldIndex(Decl *F) {
- auto *Owner = dyn_cast(F->getDeclContext());
- if (!Owner)
-return 0;
-
- unsigned Index = 1;
- for (const auto *D : Owner->noload_decls()) {
-if (D == F)
- return Index;
-
-if (isa(*D) || isa(*D))
- ++Index;
- }
-
- return Index;
-}
-
Decl *ASTNodeImporter::VisitFieldDecl(FieldDecl *D) {
// Import the major distinguishing characteristics of a variable.
DeclContext *DC, *LexicalDC;
@@ -2863,7 +2868,9 @@
for (auto *FoundDecl : FoundDecls) {
if (auto *FoundField = dyn_cast(FoundDecl)) {
// For anonymous fields, match up by index.
- if (!Name && getFieldIndex(D) != getFieldIndex(FoundField))
+ if (!Name &&
+ ASTImporter::getFieldIndex(D) !=
+ ASTImporter::getFieldIndex(FoundField))
continue;
if (Importer.IsStructurallyEquivalent(D->getType(),
@@ -2928,7 +2935,9 @@
for (unsigned I = 0, N = FoundDecls.size(); I != N; ++I) {
if (auto *FoundField = dyn_cast(FoundDecls[I])) {
// For anonymous indirect fields, match up by index.
- if (!Name && getFieldIndex(D) != getFieldIndex(FoundField))
+ if (!Name &&
+ ASTImporter::getFieldIndex(D) !=
+ ASTImporter::getFieldIndex(FoundField))
continue;
if (Importer.IsStructurallyEquivalent(D->getType(),
Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp
===
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp
@@ -2641,6 +2641,40 @@
R1, recordDecl(has(fieldDecl(hasName("next"));
}
+TEST_P(ASTImporterTestBase, ImportUnnamedFieldsInCorrectOrder) {
+ Decl *FromTU = getTuDecl(
+ R"(
+ void f(int X, int Y, bool Z) {
+(void)[X, Y, Z] { (void)Z; };
+ }
+ )",
+ Lang_CXX11, "input0.cc");
+ auto *FromF = FirstDeclMatcher().match(
+ FromTU, functionDecl(hasName("f")));
+ auto *ToF = cast_or_null(Import(FromF, Lang_CXX11));
+ EXPECT
