Commit eb3f20e8 authored by Carlos Galvez's avatar Carlos Galvez
Browse files

[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
parent d4ed3eff
...@@ -76,8 +76,7 @@ void ProBoundsConstantArrayIndexCheck::check( ...@@ -76,8 +76,7 @@ void ProBoundsConstantArrayIndexCheck::check(
auto Diag = diag(Matched->getExprLoc(), auto Diag = diag(Matched->getExprLoc(),
"do not use array subscript when the index is " "do not use array subscript when the index is "
"not an integer constant expression; use gsl::at() " "not an integer constant expression");
"instead");
if (!GslHeader.empty()) { if (!GslHeader.empty()) {
Diag << FixItHint::CreateInsertion(BaseRange.getBegin(), "gsl::at(") Diag << FixItHint::CreateInsertion(BaseRange.getBegin(), "gsl::at(")
<< FixItHint::CreateReplacement( << FixItHint::CreateReplacement(
......
...@@ -159,6 +159,12 @@ Changes in existing checks ...@@ -159,6 +159,12 @@ Changes in existing checks
- Removed default setting ``cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors = "true"``, - Removed default setting ``cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors = "true"``,
to match the current state of the C++ Core Guidelines. 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 - Updated :doc:`google-readability-casting
<clang-tidy/checks/google-readability-casting>` to diagnose and fix functional <clang-tidy/checks/google-readability-casting>` to diagnose and fix functional
casts, to achieve feature parity with the corresponding ``cpplint.py`` check. casts, to achieve feature parity with the corresponding ``cpplint.py`` check.
......
...@@ -11,6 +11,8 @@ arrays, see the `-Warray-bounds` Clang diagnostic. ...@@ -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 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. https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Pro-bounds-arrayindex.
Optionally, this check can generate fixes using ``gsl::at`` for indexing.
Options Options
------- -------
......
...@@ -27,10 +27,10 @@ constexpr int const_index(int base) { ...@@ -27,10 +27,10 @@ constexpr int const_index(int base) {
void f(std::array<int, 10> a, int pos) { void f(std::array<int, 10> a, int pos) {
a [ pos / 2 /*comment*/] = 1; 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; // CHECK-FIXES: gsl::at(a, pos / 2 /*comment*/) = 1;
int j = a[pos - 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); // CHECK-FIXES: int j = gsl::at(a, pos - 1);
a.at(pos-1) = 2; // OK, at() instead of [] a.at(pos-1) = 2; // OK, at() instead of []
...@@ -54,7 +54,7 @@ void g() { ...@@ -54,7 +54,7 @@ void g() {
int a[10]; int a[10];
for (int i = 0; i < 10; ++i) { for (int i = 0; i < 10; ++i) {
a[i] = 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; // CHECK-FIXES: gsl::at(a, i) = i;
gsl::at(a, i) = i; // OK, gsl::at() instead of [] gsl::at(a, i) = i; // OK, gsl::at() instead of []
} }
......
...@@ -25,9 +25,9 @@ constexpr int const_index(int base) { ...@@ -25,9 +25,9 @@ constexpr int const_index(int base) {
void f(std::array<int, 10> a, int pos) { void f(std::array<int, 10> a, int pos) {
a [ pos / 2 /*comment*/] = 1; 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]; 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 [] a.at(pos-1) = 2; // OK, at() instead of []
gsl::at(a, pos-1) = 2; // OK, gsl::at() instead of [] gsl::at(a, pos-1) = 2; // OK, gsl::at() instead of []
...@@ -50,7 +50,7 @@ void g() { ...@@ -50,7 +50,7 @@ void g() {
int a[10]; int a[10];
for (int i = 0; i < 10; ++i) { for (int i = 0; i < 10; ++i) {
a[i] = 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; // CHECK-FIXES: gsl::at(a, i) = i;
gsl::at(a, i) = i; // OK, gsl::at() instead of [] gsl::at(a, i) = i; // OK, gsl::at() instead of []
} }
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment