shafik added a comment.
Done with first round.
================
Comment at: lib/AST/ASTImporter.cpp:1643
+ if (!ImportedOrErr) {
+ // For RecordDecls, failed import of a field will break the layout of the
+ // structure. Handle it as an error.
----------------
What cases are failed import of a field ok? Is that because we will get the
field later on by another path?
================
Comment at: lib/AST/ASTImporter.cpp:1655
+ // Reorder declarations in RecordDecls because they may have another
+ // order. Keeping field order is vitable because it determines structure
+ // layout.
----------------
vitable?
================
Comment at: lib/AST/ASTImporter.cpp:1663
+
+
+ // NOTE: Here and below, we cannot call field_begin() method and its callers
----------------
Maybe a comment here explaining the purpose of the loops below. IIUC removing
fields since they may have been imported in the wrong order and then adding
them back in what should be the correct order now.
================
Comment at: lib/AST/ASTImporter.cpp:1674
+ Decl *ToD = Importer.GetAlreadyImportedOrNull(D);
+ assert(ToRD == ToD->getDeclContext() && ToRD->containsDecl(ToD));
+ ToRD->removeDecl(ToD);
----------------
Are we sure `ToD` is never a `nullptr` b/c you are unconditionally derefenecing
it here.
================
Comment at: lib/AST/ASTImporter.cpp:1679
+
+ if (!ToRD->hasExternalLexicalStorage())
+ assert(ToRD->field_empty());
----------------
Can you add a comment explaining why if the Decl has ExternalLexicalStorage the
fields might not be empty even though we just removed them?
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D44100/new/
https://reviews.llvm.org/D44100
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
