[PATCH] D13746: [clang-tidy] add check cppcoreguidelines-pro-bounds-constant-array-index

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

Sorry for the delay. A few more comments.


================
Comment at: clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp:86
@@ +85,3 @@
+  if (!IndexExpr->isIntegerConstantExpr(Index, *Result.Context, nullptr,
+                                        true)) {
+    SourceRange BaseRange;
----------------
nit: What is `true` here? Please add an argument comment.

================
Comment at: clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp:102
@@ +101,3 @@
+           << FixItHint::CreateReplacement(
+                  {BaseRange.getEnd().getLocWithOffset(1),
+                   IndexRange.getBegin().getLocWithOffset(-1)},
----------------
I'm not sure initializer lists are supported by all toolchains this code needs to compile on. Please explicitly specify `SourceRange` here.

================
Comment at: clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp:124
@@ +123,3 @@
+  // Get uint64_t values, because different bitwidths would lead to an assertion
+  // in APInt::uge
+  if (Index.getZExtValue() >= ArraySize.getZExtValue()) {
----------------
nit: please add trailing period.

================
Comment at: docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst:12
@@ +11,2 @@
+This rule is part of the "Bounds safety" profile of the C++ Core Guidelines, see
+https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#-bounds2-only-index-into-arrays-using-constant-expressions
----------------
nit: please add trailing period.

================
Comment at: test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp:51
@@ +50,3 @@
+
+  a[-1] = 3;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: array index -1 is before the beginning of the array
----------------
So what's the difference between this warning and -Warray-bounds?


http://reviews.llvm.org/D13746