[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
Thu Apr 25 04:58:49 PDT 2024
https://github.com/sebwolf-de created https://github.com/llvm/llvm-project/pull/90043
The string based test to find out whether the check is applicable on the class is not ideal, but I did not find a more elegant way, yet. If there is more clang-query magic available, that I'm not aware of, I'm happy to adapt that.
>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] 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 00000000000000..524c21b5bdb818
--- /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 00000000000000..f915729cd7bbee
--- /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 eb35bbc6a538fe..4972d20417928d 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 e9f0201615616f..525bbc7a42adaa 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 5b1feffb89ea06..1949f18a586ed8 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 00000000000000..8fb2e3bfde0981
--- /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 5d9d487f75f9cb..8b472fca2d8ca1 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 00000000000000..23453b1f2df218
--- /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];
More information about the cfe-commits
mailing list