[PATCH] D83550: [PATCH 1/4][Sema][AArch64] Add parsing support for arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes marked an inline comment as done.
c-rhodes added a comment.

In D83550#2155837 <https://reviews.llvm.org/D83550#2155837>, @aaron.ballman wrote:

> LGTM aside from a small nit.




In D83550#2156063 <https://reviews.llvm.org/D83550#2156063>, @rsandifo-arm wrote:

> Thanks for doing this.  FWIW, the patch LGTM from a spec point of view.  I just saw one minor spelling nit.


Cheers!



================
Comment at: clang/include/clang/Basic/AttrDocs.td:4882
+to the SVE predicate type ``svbool_t``, this excludes tuple types such as
+``svint32x4_t``. The behaviour of the attribute is undefined unless
+``N==__ARM_FEATURE_SVE_BITS``, the implementation defined feature macro that is
----------------
rsandifo-arm wrote:
> nit: s/behaviour/behavior/, since I think the documentation uses US/international English.
> nit: s/behaviour/behavior/, since I think the documentation uses US/international English.

Good spot, I'll fix this before merging


================
Comment at: clang/lib/Sema/SemaType.cpp:7698
+                                    llvm::APSInt &Result) {
+  Expr *AttrExpr = static_cast<Expr *>(Attr.getArgAsExpr(0));
+  if (AttrExpr->isTypeDependent() || AttrExpr->isValueDependent() ||
----------------
aaron.ballman wrote:
> `const auto *` and you should use `cast<>` instead of `static_cast<>`.
> const auto * and you should use cast<> instead of static_cast<>.

I've added `const auto *`, I don't think the cast was necessary since `Attr.getArgAsExpr` returns an `Expr *` although I noticed a few places using `static_cast` when calling this


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

https://reviews.llvm.org/D83550