[PATCH] D154764: [ASTImporter] Fields are imported first and reordered for correct layout.

[Ding Fei via Phabricator via cfe-commits]

danix800 added inline comments.


================
Comment at: clang/unittests/AST/ASTImporterTest.cpp:8018
+        int m() {
+          return &((A *)0)->f1 - &((A *)0)->f2;
+        }
----------------
danix800 wrote:
> shafik wrote:
> > So is it the case that this caused `f2` to be imported first and then `f1`? Which is the opposite of the example in the comments:
> > 
> > ```
> >  struct declToImport {
> >           int a = c + b;
> >           int b = 1;
> >           int c = 2;
> >       };
> > ```
> > 
> > which causes the order to be c, b and then a.
> The issue caused by this case
> ```
> int m() {
>    return &((A *)0)->f1 - &((A *)0)->f2;
> }
> int f1;
> int f2;
> ```
> is that before fixing, method `m` is imported first, which triggers expr (UnaryOperator) dependency computation, further relies on the correct RecordLayout of `declToImport`, but `f1` & `f2` is not imported at this moment.
> 
> This is not the opposite to the case like `int a = c + b;`, but both are caused by circular refs.
> 
> Similar record layout issue will also be triggered like:
> ```
> class B;
> class A {
>   B* b;
>   int c;
> };
> class B {
>    A *f() { return &((B *)0)->a; }
>    A a;
> };
> ```
> https://reviews.llvm.org/D156201 removes the unnecessary record layout as another fix to this kind of circular refs
> (which cannot be fixed simply by re-adjusting import ordering).
> 
> 
Sorry for typos, they are both import ordering issues (not circular ref issues).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154764/new/

https://reviews.llvm.org/D154764