[PATCH] D39886: [ASTImporter] Fix wrong conflict detections for unnamed structures

[Takafumi Kubota via Phabricator via cfe-commits]

tk1012 added a comment.

Hello Aleksei,

Unfortunately, I find the related problem with the unnamed structs/unions, even if I apply https://reviews.llvm.org/D39886.

For example, in PostgreSQL, there is a part of code like below.

  typedef struct { int  a; } b;
  
  struct { const char *x; } y;

then, original AST becomse like below.

  -RecordDecl 0x7ac3900 ... struct definition
  | `-FieldDecl 0x7b18690 ... a 'int'
  |-TypedefDecl 0x7b18730 ... b 'struct b':'b'
  | `-ElaboratedType 0x7b186e0 'struct b' sugar
  |   `-RecordType 0x7ac3990 'b'
  |     `-Record 0x7ac3900 ''
  |-RecordDecl 0x7b187a0 ... struct definition
  | `-FieldDecl 0x7b18868 ... x 'const char *'
  `-VarDecl 0x7b18900 ... y 'struct (anonymous struct at ...)':'struct (anonymous at ...)'

However, ASTImporter imports the AST incorrectly.
The result AST becomes

  |-RecordDecl 0x7f696c07ee50 ... struct definition
  | `-FieldDecl 0x7f696c177470 ... a 'int'
  |-TypedefDecl 0x7f696c177510 ...  b 'struct b':'b'
  | `-ElaboratedType 0x7f696c1774c0 'struct b' sugar
  |   `-RecordType 0x7f696c07eee0 'b'
  |     `-Record 0x7f696c07ee50 ''
  |-RecordDecl 0x7f696c177568 prev 0x7f696c07ee50 ...  struct
  `-VarDecl 0x7f696c177660 ...  y 'struct b':'b'

The variable `y`'s type becomes `struct b` mistakenly.
This is because `FoundDecl` is set into `PrevDecl` at L1676.
In this case, `FoundDecl` is `struct { int a; }`.
Then, ASTImporter set  `PrevDecl` as a previous RecordDecl of the imported RecordDecl at L1772.

To avoid this, I think there are two possible solutions.

1. Like this patch, skipping conflict resolution part for the unnamed structs/unions.
2. Add a condition for setting the previous decl at L1676.

What do you think?
( I guess the first is unexpectedly dependable. )

p.s.
Should I also share this in https://reviews.llvm.org/D39886?



================
Comment at: lib/AST/ASTImporter.cpp:1676
 
         PrevDecl = FoundRecord;
 
----------------
highlight


================
Comment at: lib/AST/ASTImporter.cpp:1772
       // FIXME: do this for all Redeclarables, not just RecordDecls.
       D2->setPreviousDecl(PrevDecl);
     }
----------------
highlight


https://reviews.llvm.org/D39886