sbenza added a comment.
Thanks for the patch.
A few comments below.
================
Comment at: clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp:23
@@ +22,3 @@
+
+
Finder->addMatcher(arraySubscriptExpr(hasBase(ignoringImpCasts(hasType(constantArrayType().bind("type")))),
+ hasIndex(expr().bind("index"))
----------------
All this code needs to be reformatted to 80 cols.
http://llvm.org/docs/CodingStandards.html#source-code-width
Have you tried clang-format?
It does a great job here.
================
Comment at: clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp:28
@@ +27,3 @@
+ Finder->addMatcher(cxxOperatorCallExpr(hasOverloadedOperatorName("[]"),
+
hasArgument(0,hasType(qualType(hasDeclaration(classTemplateSpecializationDecl(hasName("::std::array")).bind("type"))))),
+ hasArgument(1, expr().bind("index"))
----------------
hasType(qualType(hasDeclaration(classTemplateSpecializationDecl(hasName("::std::array"))))
can be written as
hasType(cxxRecordDecl(hasName("::std::array")))
================
Comment at: clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp:36
@@ +35,3 @@
+ llvm::APInt ArraySize;
+ const auto *Matched = Result.Nodes.getNodeAs<Expr>("expr");
+ if (const auto *ConstArrayType =
Result.Nodes.getNodeAs<ConstantArrayType>("type")) {
----------------
Move this declaration closer to its first use.
================
Comment at: clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp:55
@@ +54,3 @@
+ if (!IndexExpr->isIntegerConstantExpr(Index, *Result.Context, nullptr,
true)) {
+ //Fixit would need gsl::at()
+ diag(Matched->getExprLoc(), "do not use array subscript when the index is
not a compile-time constant; use gsl::at() instead");
----------------
Space after //
================
Comment at: clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp:55
@@ +54,3 @@
+ if (!IndexExpr->isIntegerConstantExpr(Index, *Result.Context, nullptr,
true)) {
+ //Fixit would need gsl::at()
+ diag(Matched->getExprLoc(), "do not use array subscript when the index is
not a compile-time constant; use gsl::at() instead");
----------------
sbenza wrote:
> Space after //
If this check is enabled, it is very likely that the library is available.
Maybe we should suggest the fix.
You can use clang::tidy::IncludeInserter to add the include if needed.
We could also add a configuration option to enable/disable the automated fix
for this case.
================
Comment at: clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.h:18
@@ +17,3 @@
+
+/// This checks that all array subscriptions on static arrays and std::arrays
have a constant index and are within bounds
+///
----------------
reflow to 80 cols
http://reviews.llvm.org/D13746
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
