From 488f861b836abcb08202a273f1839ca8c81cd14e Mon Sep 17 00:00:00 2001 From: Sean Callanan Date: Thu, 14 Jul 2016 19:53:44 +0000 Subject: [PATCH] When importing classes and structs with anonymous structs, it is critical that distinct anonymous structs remain distinct despite having similar layout. This is already ensured by distinguishing based on their placement in the parent struct, using the function `findAnonymousStructOrUnionIndex`. The problem is that this function only handles anonymous structs, like ``` class Foo { struct { int a; } } ``` and not untagged structs like ``` class Foo { struct { int a; } var; } ``` Both need to be handled, and this patch fixes that. The test case ensures that this functionality doesn't regress. Thanks to Manman Ren for review. https://reviews.llvm.org/D22270 llvm-svn: 275460 --- clang/lib/AST/ASTImporter.cpp | 34 ++++++++++++++----- .../ASTMerge/Inputs/anonymous-fields1.cpp | 5 +++ .../ASTMerge/Inputs/anonymous-fields2.cpp | 9 +++++ clang/test/ASTMerge/anonymous-fields.cpp | 4 +++ 4 files changed, 43 insertions(+), 9 deletions(-) create mode 100644 clang/test/ASTMerge/Inputs/anonymous-fields1.cpp create mode 100644 clang/test/ASTMerge/Inputs/anonymous-fields2.cpp create mode 100644 clang/test/ASTMerge/anonymous-fields.cpp diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index 2be4b9e938a7..bc1f9f96a06b 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -1029,7 +1029,7 @@ static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context, /// including the next assigned index (if none of them match). Returns an /// empty option if the context is not a record, i.e.. if the anonymous /// struct/union is at namespace or block scope. -static Optional findAnonymousStructOrUnionIndex(RecordDecl *Anon) { +static Optional findUntaggedStructOrUnionIndex(RecordDecl *Anon) { ASTContext &Context = Anon->getASTContext(); QualType AnonTy = Context.getRecordType(Anon); @@ -1040,13 +1040,29 @@ static Optional findAnonymousStructOrUnionIndex(RecordDecl *Anon) { unsigned Index = 0; for (const auto *D : Owner->noload_decls()) { const auto *F = dyn_cast(D); - if (!F || !F->isAnonymousStructOrUnion()) + if (!F) continue; - if (Context.hasSameType(F->getType(), AnonTy)) - break; + if (F->isAnonymousStructOrUnion()) { + if (Context.hasSameType(F->getType(), AnonTy)) + break; + ++Index; + continue; + } - ++Index; + // If the field looks like this: + // struct { ... } A; + QualType FieldType = F->getType(); + if (const auto *RecType = dyn_cast(FieldType)) { + const RecordDecl *RecDecl = RecType->getDecl(); + if (RecDecl->getDeclContext() == Owner && + !RecDecl->getIdentifier()) { + if (Context.hasSameType(FieldType, AnonTy)) + break; + ++Index; + continue; + } + } } return Index; @@ -1068,8 +1084,8 @@ static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context, if (D1->isAnonymousStructOrUnion() && D2->isAnonymousStructOrUnion()) { // If both anonymous structs/unions are in a record context, make sure // they occur in the same location in the context records. - if (Optional Index1 = findAnonymousStructOrUnionIndex(D1)) { - if (Optional Index2 = findAnonymousStructOrUnionIndex(D2)) { + if (Optional Index1 = findUntaggedStructOrUnionIndex(D1)) { + if (Optional Index2 = findUntaggedStructOrUnionIndex(D2)) { if (*Index1 != *Index2) return false; } @@ -2749,9 +2765,9 @@ Decl *ASTNodeImporter::VisitRecordDecl(RecordDecl *D) { // If both anonymous structs/unions are in a record context, make sure // they occur in the same location in the context records. if (Optional Index1 - = findAnonymousStructOrUnionIndex(D)) { + = findUntaggedStructOrUnionIndex(D)) { if (Optional Index2 = - findAnonymousStructOrUnionIndex(FoundRecord)) { + findUntaggedStructOrUnionIndex(FoundRecord)) { if (*Index1 != *Index2) continue; } diff --git a/clang/test/ASTMerge/Inputs/anonymous-fields1.cpp b/clang/test/ASTMerge/Inputs/anonymous-fields1.cpp new file mode 100644 index 000000000000..829bc0edd30a --- /dev/null +++ b/clang/test/ASTMerge/Inputs/anonymous-fields1.cpp @@ -0,0 +1,5 @@ +class A { +public: + struct { int foo; } f; + struct { int foo; } g; +}; diff --git a/clang/test/ASTMerge/Inputs/anonymous-fields2.cpp b/clang/test/ASTMerge/Inputs/anonymous-fields2.cpp new file mode 100644 index 000000000000..28ea46d98711 --- /dev/null +++ b/clang/test/ASTMerge/Inputs/anonymous-fields2.cpp @@ -0,0 +1,9 @@ +class A { +public: + struct { int foo; } f; + struct { int foo; } g; +}; + +inline int useA(A &a) { + return (a.f.foo + a.g.foo); +} diff --git a/clang/test/ASTMerge/anonymous-fields.cpp b/clang/test/ASTMerge/anonymous-fields.cpp new file mode 100644 index 000000000000..67afc29d07e5 --- /dev/null +++ b/clang/test/ASTMerge/anonymous-fields.cpp @@ -0,0 +1,4 @@ +// RUN: %clang_cc1 -emit-pch -o %t.1.ast %S/Inputs/anonymous-fields1.cpp +// RUN: %clang_cc1 -emit-pch -o %t.2.ast %S/Inputs/anonymous-fields2.cpp +// RUN: %clang_cc1 -emit-obj -o /dev/null -ast-merge %t.1.ast -ast-merge %t.2.ast %s +// expected-no-diagnostics