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

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.


================
Comment at: lib/AST/ASTImporter.cpp:1672
       // FIXME: Handle this case somehow better.
-      consumeError(ImportedOrErr.takeError());
+      else
+        consumeError(ImportedOrErr.takeError());
----------------
this else is redundant.


================
Comment at: lib/AST/ASTImporter.cpp:1696
+    if (isa<FieldDecl>(D) || isa<FriendDecl>(D)) {
+      Decl *ToD = Importer.GetAlreadyImportedOrNull(D);
+      assert(ToRD == ToD->getDeclContext() && ToRD->containsDecl(ToD));
----------------
Can you add a comment explaining why this function named ...OrNull never returns a nullptr?


================
Comment at: lib/AST/ASTImporter.cpp:1703
+  if (!ToRD->hasExternalLexicalStorage())
+    assert(ToRD->field_empty());
+
----------------
` assert(ToRD->hasExternalLexicalStorage() || ToRD->field_empty());`


================
Comment at: lib/AST/ASTImporter.cpp:1708
+      Decl *ToD = Importer.GetAlreadyImportedOrNull(D);
+      assert(ToD);
+      assert(ToRD == ToD->getDeclContext());
----------------
This is redundant with the next assertion.


================
Comment at: lib/AST/ASTImporter.cpp:1712
+      if (!ToRD->hasExternalLexicalStorage())
+        assert(!ToRD->containsDecl(ToD));
+
----------------
ditto


Repository:
  rC Clang

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

https://reviews.llvm.org/D44100