balazske added inline comments.
================
Comment at: clang/lib/AST/ASTImporter.cpp:3897
+ if (Err)
+ return std::move(Err);
ToField->setAccess(D->getAccess());
----------------
kpdev42 wrote:
> 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.
I understand now. But a comment could be added here to explain why the
attribute import is needed here.
================
Comment at: clang/lib/AST/ASTImporter.cpp:8985
+Error ASTImporter::ImportAttrs(Decl *ToD, Decl *FromD) {
+ if (FromD->hasAttrs())
+ for (const Attr *FromAttr : FromD->getAttrs()) {
----------------
Should use braces in this case
(https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145057/new/
https://reviews.llvm.org/D145057
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
