[PATCH] D44100: [ASTImporter] Reorder fields after structure import is finished

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin marked 2 inline comments as done.
a_sidorin added inline comments.


================
Comment at: lib/AST/ASTImporter.cpp:1029
+
+  RecordDecl *ToRD = cast<RecordDecl>(Importer.Import(cast<Decl>(FromDC)));
+
----------------
martong wrote:
> Can't we just import the `FromRD`, why we need that cast at the end? 
> `auto *ToRD = cast<RecordDecl>(Importer.Import(FromRD)));` 
We need to cast it to DeclContext anyway, so I don't think that a narrower type will be worse


================
Comment at: lib/AST/ASTImporter.cpp:1032
+  for (auto *FromField : FromRD->fields()) {
+    Decl *ToField = Importer.GetAlreadyImportedOrNull(FromField);
+    assert(ToRD == ToField->getDeclContext() && ToRD->containsDecl(ToField));
----------------
martong wrote:
> I think `ToField` can be a nullptr, and if so, then `ToField->getDeclContext()` is UB.
> Same could happen at line 1040.
> Perhaps we should have and explicit check about the nullptr value:
> `if (!ToField) continue;`
> 
I have added a check for the return result into the import loop. So, after the import is finished, all nested decls should be non-null.


Repository:
  rC Clang

https://reviews.llvm.org/D44100