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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
