[PATCH] D48773: [ASTImporter] Fix import of objects with anonymous types

[Gabor Marton via Phabricator via cfe-commits]

martong marked an inline comment as done.
martong added inline comments.


================
Comment at: lib/AST/ASTImporter.cpp:2085
             }
+          } else {
+            if (!IsStructuralMatch(D, FoundRecord, false))
----------------
balazske wrote:
> martong wrote:
> > a_sidorin wrote:
> > > Is it possible to use the added code for the entire condition `if (auto *FoundRecord = dyn_cast<RecordDecl>(Found))`, replacing its body? Our structural matching already knows how to handle unnamed structures, and the upper code partially duplicates `IsStructurallyEquivalent(StructuralEquivalenceContext &Context,RecordDecl *D1, RecordDecl *D2)`. Can we change structural matching to handle this stuff instead of doing it in ASTNodeImporter?
> > Yes, this is a good point, updated the patch accordingly.
> My idea was to remove this whole `if (!SearchName)` branch, but some restructuring of the next conditions may be needed. Setting of `PrevDecl` in the `if` branch does not look correct (if `!SearchName` is false it is never set, unlike in the old code).
I tried what you mentioned Balazs.
It turned out, we can't just skip the whole `if (!SearchName)` branch. Because in the below case we would match the first unnamed union with the second, therefore we will break functionality (and some lit tests). The `continue` at line 2078 is very important in this edge case. (I agree that this code is very messy and would be good if we could refactor that to be cleaner, particularly if we could remove the `SearchName` variable would make it way better.)
```
// Matches
struct Unnamed {
  union {
    struct {
      int i;
    } S;
    struct {
      float i;
    } R;
  } U;
} x14;

```

The failing lit test and it's output:
```
/home/egbomrt/WORK/llvm3/git/llvm/tools/clang/test/ASTMerge/struct/Inputs/struct1.c:84:5: warning: type 'struct Unnamed::(anonymous at /home/egbomrt/WORK/llvm3/git/llvm/tools/clang/test/ASTMerge/struct/Inputs/struct1.c:84:5)' has incompatible definitions in different translation units
    struct {
    ^
/home/egbomrt/WORK/llvm3/git/llvm/tools/clang/test/ASTMerge/struct/Inputs/struct1.c:85:11: note: field 'i' has type 'int' here
      int i;
          ^
/home/egbomrt/WORK/llvm3/git/llvm/tools/clang/test/ASTMerge/struct/Inputs/struct1.c:88:13: note: field 'i' has type 'float' here
      float i;
            ^

```


Repository:
  rC Clang

https://reviews.llvm.org/D48773