[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