[clang-tools-extra] eb3f20e - [clang-tidy] Remove gsl::at suggestion from cppcoreguidelines-pro-bounds-constant-array-index
Carlos Galvez via cfe-commits
cfe-commits at lists.llvm.org
Sun Jan 23 08:07:21 PST 2022
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 []
}
More information about the cfe-commits
mailing list