aaron.ballman added a comment.
This is missing a bunch of C++ tests that show what happens in the case of
inheritance, template instantiations, etc.
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:10068
"__builtin_preserve_field_info argument %0 not a constant">;
+def err_preserve_access_index_wrong_type: Error<"%0 attribute only applies to
%1">;
----------------
This is unnecessary.
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5706
+ // Add preserve_access_index attribute to all fields and inner records.
+ for (DeclContext::decl_iterator D = RD->decls_begin(), DEnd =
RD->decls_end();
+ D != DEnd; ++D) {
----------------
Any reason not to use a range-based for?
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5712
+
+ RecordDecl *Rec = dyn_cast<RecordDecl>(*D);
+ if (Rec) {
----------------
Can use `auto` here
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5713-5718
+ if (Rec) {
+ Rec->addAttr(::new (S.Context) BPFPreserveAccessIndexAttr(S.Context,
AL));
+ handleBPFPreserveAIRecord(S, Rec, AL);
+ } else {
+ D->addAttr(::new (S.Context) BPFPreserveAccessIndexAttr(S.Context, AL));
+ }
----------------
These should be implicit attributes, not explicit ones, right?
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5725
+ // Add preserve_access_index attribute to all fields and inner records.
+ for (DeclContext::decl_iterator D = RD->decls_begin(), DEnd =
RD->decls_end();
+ D != DEnd; ++D) {
----------------
Similar here.
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5727
+ D != DEnd; ++D) {
+ RecordDecl *Rec = dyn_cast<RecordDecl>(*D);
+ if (Rec) {
----------------
Can use `auto` here as well.
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5730-5736
+ if (!Rec->hasAttr<BPFPreserveAccessIndexAttr>()) {
+ Rec->addAttr(::new (S.Context) BPFPreserveAccessIndexAttr(S.Context,
AL));
+ handleBPFPreserveAIRecord(S, Rec, AL);
+ }
+ } else {
+ D->addAttr(::new (S.Context) BPFPreserveAccessIndexAttr(S.Context, AL));
+ }
----------------
Implicit attributes?
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5742-5747
+ RecordDecl *Rec = dyn_cast<RecordDecl>(D);
+ if (!Rec) {
+ S.Diag(D->getLocation(), diag::err_preserve_access_index_wrong_type)
+ << "preserve_addess_index" << "struct or union type";
+ return;
+ }
----------------
This should be handled in Attr.td with an explicit subject.
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5749
+
+ if (!checkAttributeNumArgs(S, AL, 0))
+ return;
----------------
This is not needed.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69759/new/
https://reviews.llvm.org/D69759
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
