carlosgalvezp updated this revision to Diff 399674.
carlosgalvezp marked an inline comment as done.
carlosgalvezp added a comment.
Single backticks for gsl::at
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D117205/new/
https://reviews.llvm.org/D117205
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.h
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-fixhint.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-constant-array-index-fixhint.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-constant-array-index-fixhint.cpp
@@ -0,0 +1,78 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-pro-bounds-constant-array-index %t -- \
+// RUN: -config='{CheckOptions: [{key: cppcoreguidelines-pro-bounds-constant-array-index.FixHint, value: "foo::at()"}]}'
+
+typedef __SIZE_TYPE__ size_t;
+
+namespace std {
+template <typename T, size_t N>
+struct array {
+ T &operator[](size_t n);
+ T &at(size_t n);
+};
+} // namespace std
+
+namespace gsl {
+template <class T, size_t N>
+T &at(T (&a)[N], size_t index);
+
+template <class T, size_t N>
+T &at(std::array<T, N> &a, size_t index);
+} // namespace gsl
+
+constexpr int const_index(int base) {
+ return base + 3;
+}
+
+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 foo::at() instead [cppcoreguidelines-pro-bounds-constant-array-index]
+ // CHECK-FIXES-NOT: 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 foo::at() instead
+ // CHECK-FIXES-NOT: int j = gsl::at(a, pos - 1);
+
+ a.at(pos - 1) = 2; // OK, at() instead of []
+ gsl::at(a, pos - 1) = 2; // OK, gsl::at() instead of []
+
+ a[-1] = 3;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: std::array<> index -1 is negative [cppcoreguidelines-pro-bounds-constant-array-index]
+ a[10] = 4;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: std::array<> index 10 is past the end of the array (which contains 10 elements) [cppcoreguidelines-pro-bounds-constant-array-index]
+
+ a[const_index(7)] = 3;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: std::array<> index 10 is past the end of the array (which contains 10 elements)
+
+ a[0] = 3; // OK, constant index and inside bounds
+ a[1] = 3; // OK, constant index and inside bounds
+ a[9] = 3; // OK, constant index and inside bounds
+ a[const_index(6)] = 3; // OK, constant index and inside bounds
+}
+
+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 foo::at() instead
+ // CHECK-FIXES-NOT: gsl::at(a, i) = i;
+ gsl::at(a, i) = i + 1; // OK, gsl::at() instead of []
+ }
+
+ a[-1] = 3; // flagged by clang-diagnostic-array-bounds
+ a[10] = 4; // flagged by clang-diagnostic-array-bounds
+ a[const_index(7)] = 3; // flagged by clang-diagnostic-array-bounds
+
+ a[0] = 3; // OK, constant index and inside bounds
+ a[1] = 3; // OK, constant index and inside bounds
+ a[9] = 3; // OK, constant index and inside bounds
+ a[const_index(6)] = 3; // OK, constant index and inside bounds
+}
+
+struct S {
+ int &operator[](int i);
+};
+
+void customOperator() {
+ S s;
+ int i = 0;
+ s[i] = 3; // OK, custom operator
+}
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst
+++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst
@@ -23,3 +23,12 @@
A string specifying which include-style is used, `llvm` or `google`. Default
is `llvm`.
+
+.. option:: FixHint
+
+ A string specifying which hint to suggest as a fix, in case users do not want
+ or cannot use GSL. Default is `gsl::at()`. If a non-default fix hint is
+ used, the check will not output fix-it hints, since they could potentially
+ lead to invalid code if the `FixHint` option is not a direct or valid
+ replacement. For example, passing `"MyCustomSafeArrayWrapper::at()"` would be
+ helpful and understandable by the users, but not automatable as a fix-it.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -153,6 +153,9 @@
Changes in existing checks
^^^^^^^^^^^^^^^^^^^^^^^^^^
+- Add new option ``cppcoreguidelines-pro-bounds-constant-array-index.FixHint``
+ to allow users to specify a different fix hint than ``gsl::at``.
+
- Removed default setting ``cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors = "true"``,
to match the current state of the C++ Core Guidelines.
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.h
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.h
@@ -24,6 +24,7 @@
class ProBoundsConstantArrayIndexCheck : public ClangTidyCheck {
const std::string GslHeader;
utils::IncludeInserter Inserter;
+ const std::string FixHint;
public:
ProBoundsConstantArrayIndexCheck(StringRef Name, ClangTidyContext *Context);
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
@@ -22,12 +22,14 @@
StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context), GslHeader(Options.get("GslHeader", "")),
Inserter(Options.getLocalOrGlobal("IncludeStyle",
- utils::IncludeSorter::IS_LLVM)) {}
+ utils::IncludeSorter::IS_LLVM)),
+ FixHint(Options.get("FixHint", "gsl::at()")) {}
void ProBoundsConstantArrayIndexCheck::storeOptions(
ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "GslHeader", GslHeader);
Options.store(Opts, "IncludeStyle", Inserter.getStyle());
+ Options.store(Opts, "FixHint", FixHint);
}
void ProBoundsConstantArrayIndexCheck::registerPPCallbacks(
@@ -75,10 +77,11 @@
SourceRange IndexRange = IndexExpr->getSourceRange();
auto Diag = diag(Matched->getExprLoc(),
- "do not use array subscript when the index is "
- "not an integer constant expression; use gsl::at() "
- "instead");
- if (!GslHeader.empty()) {
+ std::string("do not use array subscript when the index is "
+ "not an integer constant expression; use ") +
+ FixHint + " instead");
+
+ if (!GslHeader.empty() && (FixHint == "gsl::at()")) {
Diag << FixItHint::CreateInsertion(BaseRange.getBegin(), "gsl::at(")
<< FixItHint::CreateReplacement(
SourceRange(BaseRange.getEnd().getLocWithOffset(1),
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
