[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;
+        }
----------------
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).




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154764