balazske added inline comments.
================ Comment at: clang/lib/AST/ASTImporter.cpp:3897 + if (Err) + return std::move(Err); ToField->setAccess(D->getAccess()); ---------------- 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 *)`? ================ Comment at: clang/unittests/AST/ASTImporterTest.cpp:8157 + + CXXRecordDecl *ToD = cast<CXXRecordDecl>(Import(FromD, Lang_CXX20)); + EXPECT_EQ(true, ToD->isEmpty()); ---------------- ================ Comment at: clang/unittests/AST/ASTImporterTest.cpp:8160 + for (auto *FD : ToD->fields()) + EXPECT_EQ(true, FD->hasAttr<NoUniqueAddressAttr>()); +} ---------------- ================ Comment at: clang/unittests/AST/ASTImporterTest.cpp:8161 + EXPECT_EQ(true, FD->hasAttr<NoUniqueAddressAttr>()); +} + ---------------- Does this test fail without the changes applied? And does it not fail after (is the "Empty" value copied at import)? 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
