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 [] } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
