sdesmalen added inline comments.

================
Comment at: clang/include/clang/Basic/Attr.td:1541
 
+def ArmSveVectorBits128 : TypeAttr {
+  let Spellings = [];
----------------
aaron.ballman wrote:
> sdesmalen wrote:
> > nit: Can you add a comment saying why these are undocumented (and have no 
> > spellings)
> Also, I think these are all missing `let SemaHandler = 0;` and `let ASTNode = 
> 0;`
> 
> Is there a reason why we need N different type attributes instead of having a 
> single type attribute which encodes the N as an argument? I think this may 
> simplify the patch somewhat as you no longer need to switch over N as much.
> Is there a reason why we need N different type attributes instead of having a 
> single type attribute which encodes the N as an argument?
AIUI this was a workaround for getting the value of N from an AttributedType, 
because this only has `getAttrKind` to return the attribute kind, but no way to 
get the corresponding argument/value. This seemed like a simple way to do that 
without having to create a new subclass for Type and having to support that in 
various places. Is the latter the approach you were thinking of? (or is there 
perhaps a simpler way?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83551



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to