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

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin accepted this revision.
a_sidorin added a comment.
This revision is now accepted and ready to land.

Hi Gabor,
Thank you again for working on this patch. I think it can be committed after minor stylish issues are fixed.



================
Comment at: clang/lib/AST/ASTImporter.cpp:1677
+  // automatically load all the fields by calling
+  // LoadFieldsFromExternalStorage().  LoadFieldsFromExternalStorage() would
+  // call ASTImporter::Import(). This is because the ExternalASTSource
----------------
Two spaces after dot.


================
Comment at: clang/lib/AST/ASTImporter.cpp:1685
+    return ChildErrors;
+  auto ToDCOrErr = Importer.ImportContext(FromDC);
+  if (!ToDCOrErr) {
----------------
Could you please add a newline after `return`?


================
Comment at: clang/lib/AST/ASTImporter.cpp:1696
+    if (isa<FieldDecl>(D) || isa<FriendDecl>(D)) {
+      assert(D && "DC has a contained decl which is null?");
+      Decl *ToD = Importer.GetAlreadyImportedOrNull(D);
----------------
"DC contains a null decl"?


================
Comment at: clang/unittests/AST/ASTImporterTest.cpp:5244
+}
+
+
----------------
A redundant newline?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44100