[clang-tools-extra] eb3f20e - [clang-tidy] Remove gsl::at suggestion from cppcoreguidelines-pro-bounds-constant-array-index

[Carlos Galvez via cfe-commits]

Author: Carlos Galvez
Date: 2022-01-23T15:52:42Z
New Revision: eb3f20e8fa4b76e0103f15623a2fc3b27fb03f85

URL: https://github.com/llvm/llvm-project/commit/eb3f20e8fa4b76e0103f15623a2fc3b27fb03f85
DIFF: https://github.com/llvm/llvm-project/commit/eb3f20e8fa4b76e0103f15623a2fc3b27fb03f85.diff

LOG: [clang-tidy] Remove gsl::at suggestion from cppcoreguidelines-pro-bounds-constant-array-index

Currently the fix hint is hardcoded to gsl::at(). This poses
a problem for people who, for a number of reasons, don't want
or cannot use the GSL library (introducing a new third-party
dependency into a project is not a minor task).

In these situations, the fix hint does more harm than good
as it creates confusion as to what the fix should be. People
can even misinterpret the fix "gsl::at" as e.g. "std::array::at",
which can lead to even more trouble (e.g. when having guidelines
that disallow exceptions).

Furthermore, this is not a requirement from the C++ Core Guidelines.
simply that array indexing needs to be safe. Each project should
be able to decide upon a strategy for safe indexing.

The fix-it is kept for people who want to use the GSL library.

Differential Revision: https://reviews.llvm.org/D117857

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst
    clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp
    clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-constant-array-index.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
index 9cbe1fa65c139..59886ee4a3ebc 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
@@ -76,8 +76,7 @@ void ProBoundsConstantArrayIndexCheck::check(
 
     auto Diag = diag(Matched->getExprLoc(),
                      "do not use array subscript when the index is "
-                     "not an integer constant expression; use gsl::at() "
-                     "instead");
+                     "not an integer constant expression");
     if (!GslHeader.empty()) {
       Diag << FixItHint::CreateInsertion(BaseRange.getBegin(), "gsl::at(")
            << FixItHint::CreateReplacement(

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index e3c1c4b9411bb..1f7228d5732bd 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -159,6 +159,12 @@ Changes in existing checks
 - Removed default setting ``cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors = "true"``,
   to match the current state of the C++ Core Guidelines.
 
+- Removed suggestion ``use gsl::at`` from warning message in the
+  ``cppcoreguidelines-pro-bounds-constant-array-index`` check, since that is not
+  a requirement from the C++ Core Guidelines. This allows people to choose
+  their own safe indexing strategy. The fix-it is kept for those who want to
+  use the GSL library.
+
 - Updated :doc:`google-readability-casting
   <clang-tidy/checks/google-readability-casting>` to diagnose and fix functional
   casts, to achieve feature parity with the corresponding ``cpplint.py`` check.

diff  --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst
index 4528f2ac6bef6..2a598f2592019 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst
@@ -11,6 +11,8 @@ arrays, see the `-Warray-bounds` Clang diagnostic.
 This rule is part of the "Bounds safety" profile of the C++ Core Guidelines, see
 https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Pro-bounds-arrayindex.
 
+Optionally, this check can generate fixes using ``gsl::at`` for indexing.
+
 Options
 -------
 

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp
index 71b957d84740b..87550cbe84335 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-constant-array-index-gslheader.cpp
@@ -27,10 +27,10 @@ constexpr int const_index(int base) {
 
 void f(std::array<int, 10> a, int pos) {
   a [ pos / 2 /*comment*/] = 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead [cppcoreguidelines-pro-bounds-constant-array-index]
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use array subscript when the index is not an integer constant expression [cppcoreguidelines-pro-bounds-constant-array-index]
   // CHECK-FIXES: gsl::at(a,  pos / 2 /*comment*/) = 1;
   int j = a[pos - 1];
-  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not use array subscript when the index is not an integer constant expression
   // CHECK-FIXES: int j = gsl::at(a, pos - 1);
 
   a.at(pos-1) = 2; // OK, at() instead of []
@@ -54,7 +54,7 @@ void g() {
   int a[10];
   for (int i = 0; i < 10; ++i) {
     a[i] = i;
-    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use array subscript when the index is not an integer constant expression
     // CHECK-FIXES: gsl::at(a, i) = i;
     gsl::at(a, i) = i; // OK, gsl::at() instead of []
   }

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-constant-array-index.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-constant-array-index.cpp
index 351941099343a..7d5390ddecfb9 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-constant-array-index.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-constant-array-index.cpp
@@ -25,9 +25,9 @@ constexpr int const_index(int base) {
 
 void f(std::array<int, 10> a, int pos) {
   a [ pos / 2 /*comment*/] = 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead [cppcoreguidelines-pro-bounds-constant-array-index]
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use array subscript when the index is not an integer constant expression [cppcoreguidelines-pro-bounds-constant-array-index]
   int j = a[pos - 1];
-  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not use array subscript when the index is not an integer constant expression
 
   a.at(pos-1) = 2; // OK, at() instead of []
   gsl::at(a, pos-1) = 2; // OK, gsl::at() instead of []
@@ -50,7 +50,7 @@ void g() {
   int a[10];
   for (int i = 0; i < 10; ++i) {
     a[i] = i;
-    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use array subscript when the index is not an integer constant expression
     // CHECK-FIXES: gsl::at(a, i) = i;
     gsl::at(a, i) = i; // OK, gsl::at() instead of []
   }