[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() (PR #90043)
Sebastian Wolf via cfe-commits
cfe-commits at lists.llvm.org
Fri May 24 03:10:19 PDT 2024
https://github.com/sebwolf-de updated https://github.com/llvm/llvm-project/pull/90043
>From 8eb5863305e8f9a1311a540faf35f24fc6f55c6c Mon Sep 17 00:00:00 2001
From: Sebastian Wolf <wolf.sebastian at in.tum.de>
Date: Wed, 17 Apr 2024 16:16:35 +0200
Subject: [PATCH 1/3] Enforce SL.con.3: Add check to replace operator[] with
at() on std containers
---
.../AvoidBoundsErrorsCheck.cpp | 81 +++++++++++++++++++
.../AvoidBoundsErrorsCheck.h | 32 ++++++++
.../cppcoreguidelines/CMakeLists.txt | 1 +
.../CppCoreGuidelinesTidyModule.cpp | 3 +
clang-tools-extra/docs/ReleaseNotes.rst | 5 ++
.../cppcoreguidelines/avoid-bounds-errors.rst | 20 +++++
.../docs/clang-tidy/checks/list.rst | 1 +
.../cppcoreguidelines/avoid-bounds-errors.cpp | 66 +++++++++++++++
8 files changed, 209 insertions(+)
create mode 100644 clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
create mode 100644 clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
create mode 100644 clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
new file mode 100644
index 0000000000000..524c21b5bdb81
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
@@ -0,0 +1,81 @@
+//===--- AvoidBoundsErrorsCheck.cpp - clang-tidy --------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "AvoidBoundsErrorsCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+#include <iostream>
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::cppcoreguidelines {
+
+bool isApplicable(const QualType &Type) {
+ const auto TypeStr = Type.getAsString();
+ bool Result = false;
+ // Only check for containers in the std namespace
+ if (TypeStr.find("std::vector") != std::string::npos) {
+ Result = true;
+ }
+ if (TypeStr.find("std::array") != std::string::npos) {
+ Result = true;
+ }
+ if (TypeStr.find("std::deque") != std::string::npos) {
+ Result = true;
+ }
+ if (TypeStr.find("std::map") != std::string::npos) {
+ Result = true;
+ }
+ if (TypeStr.find("std::unordered_map") != std::string::npos) {
+ Result = true;
+ }
+ if (TypeStr.find("std::flat_map") != std::string::npos) {
+ Result = true;
+ }
+ // TODO Add std::span with C++26
+ return Result;
+}
+
+void AvoidBoundsErrorsCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ callExpr(callee(cxxMethodDecl(hasName("operator[]")).bind("f")))
+ .bind("x"),
+ this);
+}
+
+void AvoidBoundsErrorsCheck::check(const MatchFinder::MatchResult &Result) {
+ const ASTContext &Context = *Result.Context;
+ const SourceManager &Source = Context.getSourceManager();
+ const auto *MatchedExpr = Result.Nodes.getNodeAs<CallExpr>("x");
+ const auto *MatchedFunction = Result.Nodes.getNodeAs<CXXMethodDecl>("f");
+ const auto Type = MatchedFunction->getThisType();
+ if (!isApplicable(Type)) {
+ return;
+ }
+
+ // Get original code.
+ const SourceLocation b(MatchedExpr->getBeginLoc());
+ const SourceLocation e(MatchedExpr->getEndLoc());
+ const std::string OriginalCode =
+ Lexer::getSourceText(CharSourceRange::getTokenRange(b, e), Source,
+ getLangOpts())
+ .str();
+ const auto Range = SourceRange(b, e);
+
+ // Build replacement.
+ std::string NewCode = OriginalCode;
+ const auto BeginOpen = NewCode.find("[");
+ NewCode.replace(BeginOpen, 1, ".at(");
+ const auto BeginClose = NewCode.find("]");
+ NewCode.replace(BeginClose, 1, ")");
+
+ diag(MatchedExpr->getBeginLoc(), "Do not use operator[], use at() instead.")
+ << FixItHint::CreateReplacement(Range, NewCode);
+}
+
+} // namespace clang::tidy::cppcoreguidelines
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
new file mode 100644
index 0000000000000..f915729cd7bbe
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
@@ -0,0 +1,32 @@
+//===--- AvoidBoundsErrorsCheck.h - clang-tidy ------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDBOUNDSERRORSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDBOUNDSERRORSCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::cppcoreguidelines {
+
+/// Enforce CPP core guidelines SL.con.3
+///
+/// See
+/// https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slcon3-avoid-bounds-errors
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.html
+class AvoidBoundsErrorsCheck : public ClangTidyCheck {
+public:
+ AvoidBoundsErrorsCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace clang::tidy::cppcoreguidelines
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDBOUNDSERRORSCHECK_H
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
index eb35bbc6a538f..4972d20417928 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
@@ -4,6 +4,7 @@ set(LLVM_LINK_COMPONENTS
)
add_clang_library(clangTidyCppCoreGuidelinesModule
+ AvoidBoundsErrorsCheck.cpp
AvoidCapturingLambdaCoroutinesCheck.cpp
AvoidConstOrRefDataMembersCheck.cpp
AvoidDoWhileCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
index e9f0201615616..525bbc7a42ada 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -19,6 +19,7 @@
#include "../performance/NoexceptMoveConstructorCheck.h"
#include "../performance/NoexceptSwapCheck.h"
#include "../readability/MagicNumbersCheck.h"
+#include "AvoidBoundsErrorsCheck.h"
#include "AvoidCapturingLambdaCoroutinesCheck.h"
#include "AvoidConstOrRefDataMembersCheck.h"
#include "AvoidDoWhileCheck.h"
@@ -57,6 +58,8 @@ namespace cppcoreguidelines {
class CppCoreGuidelinesModule : public ClangTidyModule {
public:
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+ CheckFactories.registerCheck<AvoidBoundsErrorsCheck>(
+ "cppcoreguidelines-avoid-bounds-errors");
CheckFactories.registerCheck<AvoidCapturingLambdaCoroutinesCheck>(
"cppcoreguidelines-avoid-capturing-lambda-coroutines");
CheckFactories.registerCheck<modernize::AvoidCArraysCheck>(
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 5b1feffb89ea0..1949f18a586ed 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -131,6 +131,11 @@ New checks
to reading out-of-bounds data due to inadequate or incorrect string null
termination.
+- New :doc:`cppcoreguidelines-avoid-bounds-errors
+ <clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors>` check.
+
+ Flags the unsafe `operator[]` and replaces it with `at()`.
+
- New :doc:`modernize-use-designated-initializers
<clang-tidy/checks/modernize/use-designated-initializers>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst
new file mode 100644
index 0000000000000..8fb2e3bfde098
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst
@@ -0,0 +1,20 @@
+.. title:: clang-tidy - cppcoreguidelines-avoid-bounds-errors
+
+cppcoreguidelines-avoid-bounds-errors
+=====================================
+
+This check enforces the `SL.con.3 <https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slcon3-avoid-bounds-errors>` guideline.
+It flags all uses of `operator[]` on `std::vector`, `std::array`, `std::deque`, `std::map`, `std::unordered_map`, and `std::flat_map` and suggests to replace it with `at()`.
+Note that `std::span` and `std::mdspan` do not support `at()` as of C++23, so the use of `operator[]` is not flagged.
+
+For example the code
+
+.. code-block:: c++
+ std::array<int, 3> a;
+ int b = a[4];
+
+will be replaced by
+
+.. code-block:: c++
+ std::vector<int, 3> a;
+ int b = a.at(4);
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 5d9d487f75f9c..8b472fca2d8ca 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -174,6 +174,7 @@ Clang-Tidy Checks
:doc:`cert-oop58-cpp <cert/oop58-cpp>`,
:doc:`concurrency-mt-unsafe <concurrency/mt-unsafe>`,
:doc:`concurrency-thread-canceltype-asynchronous <concurrency/thread-canceltype-asynchronous>`,
+ :doc:`cppcoreguidelines-avoid-bounds-errors <cppcoreguidelines/avoid-bounds-errors>`, "Yes"
:doc:`cppcoreguidelines-avoid-capturing-lambda-coroutines <cppcoreguidelines/avoid-capturing-lambda-coroutines>`,
:doc:`cppcoreguidelines-avoid-const-or-ref-data-members <cppcoreguidelines/avoid-const-or-ref-data-members>`,
:doc:`cppcoreguidelines-avoid-do-while <cppcoreguidelines/avoid-do-while>`,
diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp
new file mode 100644
index 0000000000000..23453b1f2df21
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp
@@ -0,0 +1,66 @@
+namespace std {
+ template<typename T, unsigned size>
+ struct array {
+ T operator[](unsigned i) {
+ return T{1};
+ }
+ T at(unsigned i) {
+ return T{1};
+ }
+ };
+
+ template<typename T>
+ struct unique_ptr {
+ T operator[](unsigned i) {
+ return T{1};
+ }
+ };
+
+ template<typename T>
+ struct span {
+ T operator[](unsigned i) {
+ return T{1};
+ }
+ };
+} // namespace std
+
+namespace json {
+ template<typename T>
+ struct node{
+ T operator[](unsigned i) {
+ return T{1};
+ }
+ };
+} // namespace json
+
+
+// RUN: %check_clang_tidy %s cppcoreguidelines-avoid-bounds-errors %t
+std::array<int, 3> a;
+
+auto b = a[0];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: Do not use operator[], use at() instead. [cppcoreguidelines-avoid-bounds-errors]
+// CHECK-FIXES: auto b = a.at(0);
+auto c = a[1+1];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: Do not use operator[], use at() instead. [cppcoreguidelines-avoid-bounds-errors]
+// CHECK-FIXES: auto c = a.at(1+1);
+constexpr int index = 1;
+auto d = a[index];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: Do not use operator[], use at() instead. [cppcoreguidelines-avoid-bounds-errors]
+// CHECK-FIXES: auto d = a.at(index);
+
+int e(int index) {
+ return a[index];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: Do not use operator[], use at() instead. [cppcoreguidelines-avoid-bounds-errors]
+// CHECK-FIXES: return a.at(index);
+}
+
+auto f = a.at(0);
+
+std::unique_ptr<int> p;
+auto q = p[0];
+
+std::span<int> s;
+auto t = s[0];
+
+json::node<int> n;
+auto m = n[0];
>From 416a8ad214b8e2c8da4bcd88cc7fb33379b59401 Mon Sep 17 00:00:00 2001
From: Sebastian Wolf <wolf.sebastian at in.tum.de>
Date: Fri, 26 Apr 2024 09:06:02 +0200
Subject: [PATCH 2/3] EugeneZelenko's comments
---
.../cppcoreguidelines/AvoidBoundsErrorsCheck.cpp | 10 +++++-----
.../cppcoreguidelines/AvoidBoundsErrorsCheck.h | 3 +++
clang-tools-extra/docs/ReleaseNotes.rst | 2 +-
.../checks/cppcoreguidelines/avoid-bounds-errors.rst | 7 ++++---
4 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
index 524c21b5bdb81..2c0a12e21d726 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
@@ -9,13 +9,13 @@
#include "AvoidBoundsErrorsCheck.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Lex/Lexer.h"
-
#include <iostream>
+
using namespace clang::ast_matchers;
namespace clang::tidy::cppcoreguidelines {
-bool isApplicable(const QualType &Type) {
+static bool isApplicable(const QualType &Type) {
const auto TypeStr = Type.getAsString();
bool Result = false;
// Only check for containers in the std namespace
@@ -51,9 +51,9 @@ void AvoidBoundsErrorsCheck::registerMatchers(MatchFinder *Finder) {
void AvoidBoundsErrorsCheck::check(const MatchFinder::MatchResult &Result) {
const ASTContext &Context = *Result.Context;
const SourceManager &Source = Context.getSourceManager();
- const auto *MatchedExpr = Result.Nodes.getNodeAs<CallExpr>("x");
- const auto *MatchedFunction = Result.Nodes.getNodeAs<CXXMethodDecl>("f");
- const auto Type = MatchedFunction->getThisType();
+ const CallExpr *MatchedExpr = Result.Nodes.getNodeAs<CallExpr>("x");
+ const CXXMethodDecl *MatchedFunction = Result.Nodes.getNodeAs<CXXMethodDecl>("f");
+ const QualType Type = MatchedFunction->getThisType();
if (!isApplicable(Type)) {
return;
}
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
index f915729cd7bbe..12c7852877123 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
@@ -23,6 +23,9 @@ class AvoidBoundsErrorsCheck : public ClangTidyCheck {
public:
AvoidBoundsErrorsCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context) {}
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus;
+ }
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
};
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 1949f18a586ed..d2bb59b5f89c4 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -134,7 +134,7 @@ New checks
- New :doc:`cppcoreguidelines-avoid-bounds-errors
<clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors>` check.
- Flags the unsafe `operator[]` and replaces it with `at()`.
+ Flags the unsafe ``operator[]`` and replaces it with ``at()``.
- New :doc:`modernize-use-designated-initializers
<clang-tidy/checks/modernize/use-designated-initializers>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst
index 8fb2e3bfde098..13683ee9b5a46 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst
@@ -3,9 +3,8 @@
cppcoreguidelines-avoid-bounds-errors
=====================================
-This check enforces the `SL.con.3 <https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slcon3-avoid-bounds-errors>` guideline.
-It flags all uses of `operator[]` on `std::vector`, `std::array`, `std::deque`, `std::map`, `std::unordered_map`, and `std::flat_map` and suggests to replace it with `at()`.
-Note that `std::span` and `std::mdspan` do not support `at()` as of C++23, so the use of `operator[]` is not flagged.
+This check flags all uses of ``operator[]`` on ``std::vector``, ``std::array``, ``std::deque``, ``std::map``, ``std::unordered_map``, and ``std::flat_map`` and suggests to replace it with ``at()``.
+Note that ``std::span`` and ``std::mdspan`` do not support ``at()`` as of C++23, so the use of ``operator[]`` is not flagged.
For example the code
@@ -18,3 +17,5 @@ will be replaced by
.. code-block:: c++
std::vector<int, 3> a;
int b = a.at(4);
+
+This check enforces the `SL.con.3 <https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slcon3-avoid-bounds-errors>` guideline.
>From cd94647c31cfcffea5eeea5b55eedc9e1f864bf5 Mon Sep 17 00:00:00 2001
From: Sebastian Wolf <wolf.sebastian at in.tum.de>
Date: Fri, 24 May 2024 12:09:11 +0200
Subject: [PATCH 3/3] Refactoring, but not finished yet
---
.../AvoidBoundsErrorsCheck.cpp | 97 ++++++++++---------
.../cppcoreguidelines/avoid-bounds-errors.cpp | 17 ++--
2 files changed, 57 insertions(+), 57 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
index 2c0a12e21d726..f10b97820f4c7 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
@@ -9,73 +9,74 @@
#include "AvoidBoundsErrorsCheck.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Lex/Lexer.h"
-#include <iostream>
using namespace clang::ast_matchers;
namespace clang::tidy::cppcoreguidelines {
-static bool isApplicable(const QualType &Type) {
- const auto TypeStr = Type.getAsString();
- bool Result = false;
- // Only check for containers in the std namespace
- if (TypeStr.find("std::vector") != std::string::npos) {
- Result = true;
- }
- if (TypeStr.find("std::array") != std::string::npos) {
- Result = true;
- }
- if (TypeStr.find("std::deque") != std::string::npos) {
- Result = true;
- }
- if (TypeStr.find("std::map") != std::string::npos) {
- Result = true;
- }
- if (TypeStr.find("std::unordered_map") != std::string::npos) {
- Result = true;
- }
- if (TypeStr.find("std::flat_map") != std::string::npos) {
- Result = true;
+const CXXMethodDecl *findAlternative(const CXXRecordDecl *MatchedParent,
+ const CXXMethodDecl *MatchedOperator) {
+ for (const CXXMethodDecl *Method : MatchedParent->methods()) {
+ const bool CorrectName = Method->getNameInfo().getAsString() == "at";
+ if (!CorrectName)
+ continue;
+
+ const bool SameReturnType =
+ Method->getReturnType() == MatchedOperator->getReturnType();
+ if (!SameReturnType)
+ continue;
+
+ const bool SameNumberOfArguments =
+ Method->getNumParams() == MatchedOperator->getNumParams();
+ if (!SameNumberOfArguments)
+ continue;
+
+ for (unsigned a = 0; a < Method->getNumParams(); a++) {
+ const bool SameArgType =
+ Method->parameters()[a]->getOriginalType() ==
+ MatchedOperator->parameters()[a]->getOriginalType();
+ if (!SameArgType)
+ continue;
+ }
+
+ return Method;
}
- // TODO Add std::span with C++26
- return Result;
+ return static_cast<CXXMethodDecl *>(nullptr);
}
void AvoidBoundsErrorsCheck::registerMatchers(MatchFinder *Finder) {
+ // Need a callExpr here to match CXXOperatorCallExpr ``(&a)->operator[](0)``
+ // and CXXMemberCallExpr ``a[0]``.
Finder->addMatcher(
- callExpr(callee(cxxMethodDecl(hasName("operator[]")).bind("f")))
- .bind("x"),
+ callExpr(
+ callee(
+ cxxMethodDecl(hasOverloadedOperatorName("[]")).bind("operator")),
+ callee(cxxMethodDecl(hasParent(
+ cxxRecordDecl(hasMethod(hasName("at"))).bind("parent")))))
+ .bind("caller"),
this);
}
void AvoidBoundsErrorsCheck::check(const MatchFinder::MatchResult &Result) {
const ASTContext &Context = *Result.Context;
const SourceManager &Source = Context.getSourceManager();
- const CallExpr *MatchedExpr = Result.Nodes.getNodeAs<CallExpr>("x");
- const CXXMethodDecl *MatchedFunction = Result.Nodes.getNodeAs<CXXMethodDecl>("f");
- const QualType Type = MatchedFunction->getThisType();
- if (!isApplicable(Type)) {
- return;
- }
+ const CallExpr *MatchedExpr = Result.Nodes.getNodeAs<CallExpr>("caller");
+ const CXXMethodDecl *MatchedOperator =
+ Result.Nodes.getNodeAs<CXXMethodDecl>("operator");
+ const CXXRecordDecl *MatchedParent =
+ Result.Nodes.getNodeAs<CXXRecordDecl>("parent");
- // Get original code.
- const SourceLocation b(MatchedExpr->getBeginLoc());
- const SourceLocation e(MatchedExpr->getEndLoc());
- const std::string OriginalCode =
- Lexer::getSourceText(CharSourceRange::getTokenRange(b, e), Source,
- getLangOpts())
- .str();
- const auto Range = SourceRange(b, e);
+ const CXXMethodDecl *Alternative =
+ findAlternative(MatchedParent, MatchedOperator);
+ if (!Alternative)
+ return;
- // Build replacement.
- std::string NewCode = OriginalCode;
- const auto BeginOpen = NewCode.find("[");
- NewCode.replace(BeginOpen, 1, ".at(");
- const auto BeginClose = NewCode.find("]");
- NewCode.replace(BeginClose, 1, ")");
+ const SourceLocation AlternativeSource(Alternative->getBeginLoc());
- diag(MatchedExpr->getBeginLoc(), "Do not use operator[], use at() instead.")
- << FixItHint::CreateReplacement(Range, NewCode);
+ diag(MatchedExpr->getBeginLoc(),
+ "found possibly unsafe operator[], consider using at() instead");
+ diag(Alternative->getBeginLoc(), "alternative at() defined here",
+ DiagnosticIDs::Note);
}
} // namespace clang::tidy::cppcoreguidelines
diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp
index 23453b1f2df21..5e12e7d71790d 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp
@@ -38,23 +38,22 @@ namespace json {
std::array<int, 3> a;
auto b = a[0];
-// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: Do not use operator[], use at() instead. [cppcoreguidelines-avoid-bounds-errors]
-// CHECK-FIXES: auto b = a.at(0);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-avoid-bounds-errors]
auto c = a[1+1];
-// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: Do not use operator[], use at() instead. [cppcoreguidelines-avoid-bounds-errors]
-// CHECK-FIXES: auto c = a.at(1+1);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-avoid-bounds-errors]
constexpr int index = 1;
auto d = a[index];
-// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: Do not use operator[], use at() instead. [cppcoreguidelines-avoid-bounds-errors]
-// CHECK-FIXES: auto d = a.at(index);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-avoid-bounds-errors]
int e(int index) {
return a[index];
-// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: Do not use operator[], use at() instead. [cppcoreguidelines-avoid-bounds-errors]
-// CHECK-FIXES: return a.at(index);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-avoid-bounds-errors]
}
-auto f = a.at(0);
+auto f = (&a)->operator[](1);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-avoid-bounds-errors]
+
+auto g = a.at(0);
std::unique_ptr<int> p;
auto q = p[0];
More information about the cfe-commits
mailing list