[PATCH] D69759: [BPF] Add preserve_access_index attribute for record definition

Yonghong Song via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 14 11:11:16 PST 2019


yonghong-song marked 7 inline comments as done.
yonghong-song added a comment.

Sorry about the committing. I thought all the review questions are addressed.
Definitely will be more patient next time until getting review confirmation and
also adhering to llvm review policy.
Thanks for the detailed review! I have addressed all of your review comments in patch
https://reviews.llvm.org/D70257. I have added you as the reviewer.



================
Comment at: clang/include/clang/Basic/Attr.td:1584
+                             TargetSpecificAttr<TargetBPF>  {
+  let Spellings = [Clang<"preserve_access_index">];
+  let Subjects = SubjectList<[Record], ErrorDiag>;
----------------
aaron.ballman wrote:
> If this attribute is only supported in C mode, then this attribute should have: `let LangOpts = [COnly];` in it.
Yes, added.


================
Comment at: clang/lib/CodeGen/CGExpr.cpp:3440-3443
+    while (TypedefT) {
+      PointeeT = TypedefT->desugar().getTypePtr();
+      TypedefT = dyn_cast<TypedefType>(PointeeT);
+    }
----------------
aaron.ballman wrote:
> Is there a reason you only want to strip off typedefs, and not other forms of sugar? e.g., why not call `getUnqualifiedDesugaredType()`?
Actually, ElaboratedT is also a sugar type, the above getUnqualifiedDesugaredType() will remove it too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69759





More information about the cfe-commits mailing list