[PATCH] D145057: [clang][ASTImport] Add support for import of empty records

[Pavel Kosov via Phabricator via cfe-commits]

kpdev42 marked an inline comment as done.
kpdev42 added inline comments.


================
Comment at: clang/lib/AST/ASTImporter.cpp:3897
+  if (Err)
+    return std::move(Err);
   ToField->setAccess(D->getAccess());
----------------
balazske wrote:
> I am not familiar with this use case, is there a path where the attributes are read from a `FieldDecl` before return from `VisitFieldDecl`? Probably `ImportImpl` is overridden? Importing the attributes here should work but not totally sure if it does not cause problems. Problematic case is if the attribute has pointer to a `Decl` or `Type` that is imported here in a state when the field is already created but not initialized. Another problem is that attributes are added a second time in `Import(Decl *)`. Can it work if the `ImportAttrs` is made a protected function and called from (overridden) `ImportImpl` (still there can be a second import in `Import(Decl *)`?
The problem is that field attributes are required when we are adding a copy of a field to a structure in `VisitFieldDecl`. Attributes themselves are read in `CXXRecordDecl::addedMember` (see the call to `isZeroSize`). Adding them after `ImportImpl` is called is too late: record is already marked as non-empty. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145057