https://github.com/PBHDK updated https://github.com/llvm/llvm-project/pull/95220
From 37292995de0c5aa87408586749795a97468d4725 Mon Sep 17 00:00:00 2001 From: Sebastian Wolf <wolf.sebast...@in.tum.de> Date: Wed, 17 Apr 2024 16:16:35 +0200 Subject: [PATCH 01/23] 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 6bf70c5cf4f8a..a0cd0189fbb1c 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -160,6 +160,11 @@ New checks Replaces nested ``std::min`` and ``std::max`` calls with an initializer list where applicable. +- 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 a698cecc0825c..d9c72bedfb95b 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 953e75296861e4ee359b3fc086be4ea2fcf9dd2e Mon Sep 17 00:00:00 2001 From: Sebastian Wolf <wolf.sebast...@in.tum.de> Date: Fri, 26 Apr 2024 09:06:02 +0200 Subject: [PATCH 02/23] 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 a0cd0189fbb1c..0106b6ea732db 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -163,7 +163,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 ae98aa1d46bb71111f35b1af4b7a01dc5cb11281 Mon Sep 17 00:00:00 2001 From: Sebastian Wolf <wolf.sebast...@in.tum.de> Date: Fri, 24 May 2024 12:09:11 +0200 Subject: [PATCH 03/23] 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]; From ff305986c5c43284e462dcd1f4c301fe8ae293bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Heidekr=C3=BCger?= <paul.heidekrue...@tum.de> Date: Mon, 10 Jun 2024 20:56:39 +0200 Subject: [PATCH 04/23] Rename AvoidBoundsErrorsCheck to PreferAtOverSubscriptOperatorCheck Co-authored-by: Manuel Pietsch <manuelpiet...@outlook.de> --- .../clang-tidy/cppcoreguidelines/CMakeLists.txt | 2 +- .../CppCoreGuidelinesTidyModule.cpp | 6 +++--- ...pp => PreferAtOverSubscriptOperatorCheck.cpp} | 9 +++++---- ...ck.h => PreferAtOverSubscriptOperatorCheck.h} | 16 ++++++++-------- clang-tools-extra/docs/ReleaseNotes.rst | 6 +++--- ...rst => prefer-at-over-subscript-operator.rst} | 4 ++-- .../docs/clang-tidy/checks/list.rst | 2 +- ...cpp => prefer-at-over-subscript-operator.cpp} | 12 ++++++------ 8 files changed, 29 insertions(+), 28 deletions(-) rename clang-tools-extra/clang-tidy/cppcoreguidelines/{AvoidBoundsErrorsCheck.cpp => PreferAtOverSubscriptOperatorCheck.cpp} (90%) rename clang-tools-extra/clang-tidy/cppcoreguidelines/{AvoidBoundsErrorsCheck.h => PreferAtOverSubscriptOperatorCheck.h} (60%) rename clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/{avoid-bounds-errors.rst => prefer-at-over-subscript-operator.rst} (83%) rename clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/{avoid-bounds-errors.cpp => prefer-at-over-subscript-operator.cpp} (67%) diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt index 4972d20417928..fd436859ad04a 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt @@ -4,7 +4,6 @@ set(LLVM_LINK_COMPONENTS ) add_clang_library(clangTidyCppCoreGuidelinesModule - AvoidBoundsErrorsCheck.cpp AvoidCapturingLambdaCoroutinesCheck.cpp AvoidConstOrRefDataMembersCheck.cpp AvoidDoWhileCheck.cpp @@ -21,6 +20,7 @@ add_clang_library(clangTidyCppCoreGuidelinesModule NoMallocCheck.cpp NoSuspendWithLockCheck.cpp OwningMemoryCheck.cpp + PreferAtOverSubscriptOperatorCheck.cpp PreferMemberInitializerCheck.cpp ProBoundsArrayToPointerDecayCheck.cpp ProBoundsConstantArrayIndexCheck.cpp diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp index 525bbc7a42ada..565a99a865519 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp @@ -19,7 +19,6 @@ #include "../performance/NoexceptMoveConstructorCheck.h" #include "../performance/NoexceptSwapCheck.h" #include "../readability/MagicNumbersCheck.h" -#include "AvoidBoundsErrorsCheck.h" #include "AvoidCapturingLambdaCoroutinesCheck.h" #include "AvoidConstOrRefDataMembersCheck.h" #include "AvoidDoWhileCheck.h" @@ -35,6 +34,7 @@ #include "NoMallocCheck.h" #include "NoSuspendWithLockCheck.h" #include "OwningMemoryCheck.h" +#include "PreferAtOverSubscriptOperatorCheck.h" #include "PreferMemberInitializerCheck.h" #include "ProBoundsArrayToPointerDecayCheck.h" #include "ProBoundsConstantArrayIndexCheck.h" @@ -58,8 +58,6 @@ 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>( @@ -105,6 +103,8 @@ class CppCoreGuidelinesModule : public ClangTidyModule { "cppcoreguidelines-non-private-member-variables-in-classes"); CheckFactories.registerCheck<OwningMemoryCheck>( "cppcoreguidelines-owning-memory"); + CheckFactories.registerCheck<PreferAtOverSubscriptOperatorCheck>( + "cppcoreguidelines-prefer-at-over-subscript-operator"); CheckFactories.registerCheck<PreferMemberInitializerCheck>( "cppcoreguidelines-prefer-member-initializer"); CheckFactories.registerCheck<ProBoundsArrayToPointerDecayCheck>( diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.cpp similarity index 90% rename from clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp rename to clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.cpp index f10b97820f4c7..64eaf8a1d4ebd 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.cpp @@ -1,4 +1,4 @@ -//===--- AvoidBoundsErrorsCheck.cpp - clang-tidy --------------------------===// +//===--- PreferAtOverSubscriptOperatorCheck.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. @@ -6,7 +6,7 @@ // //===----------------------------------------------------------------------===// -#include "AvoidBoundsErrorsCheck.h" +#include "PreferAtOverSubscriptOperatorCheck.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Lex/Lexer.h" @@ -44,7 +44,7 @@ const CXXMethodDecl *findAlternative(const CXXRecordDecl *MatchedParent, return static_cast<CXXMethodDecl *>(nullptr); } -void AvoidBoundsErrorsCheck::registerMatchers(MatchFinder *Finder) { +void PreferAtOverSubscriptOperatorCheck::registerMatchers(MatchFinder *Finder) { // Need a callExpr here to match CXXOperatorCallExpr ``(&a)->operator[](0)`` // and CXXMemberCallExpr ``a[0]``. Finder->addMatcher( @@ -57,7 +57,8 @@ void AvoidBoundsErrorsCheck::registerMatchers(MatchFinder *Finder) { this); } -void AvoidBoundsErrorsCheck::check(const MatchFinder::MatchResult &Result) { +void PreferAtOverSubscriptOperatorCheck::check( + const MatchFinder::MatchResult &Result) { const ASTContext &Context = *Result.Context; const SourceManager &Source = Context.getSourceManager(); const CallExpr *MatchedExpr = Result.Nodes.getNodeAs<CallExpr>("caller"); diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.h similarity index 60% rename from clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h rename to clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.h index 12c7852877123..eb6e3a021e1b6 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.h @@ -1,4 +1,5 @@ -//===--- AvoidBoundsErrorsCheck.h - clang-tidy ------------------*- C++ -*-===// +//===--- PreferAtOverSubscriptOperatorCheck.h - clang-tidy ------*- C++ -*-===// +//===--- PreferMemberInitializerCheck.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. @@ -6,8 +7,8 @@ // //===----------------------------------------------------------------------===// -#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDBOUNDSERRORSCHECK_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDBOUNDSERRORSCHECK_H +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PREFERATOVERSUBSCRIPTOPERATORCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PREFERATOVERSUBSCRIPTOPERATORCHECK_H #include "../ClangTidyCheck.h" @@ -18,11 +19,10 @@ namespace clang::tidy::cppcoreguidelines { /// 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 { +/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/prefer-at-over-subscript-operator.html +class PreferAtOverSubscriptOperatorCheck : public ClangTidyCheck { public: - AvoidBoundsErrorsCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + PreferAtOverSubscriptOperatorCheck(StringRef Name, ClangTidyContext *Context); bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { return LangOpts.CPlusPlus; } @@ -32,4 +32,4 @@ class AvoidBoundsErrorsCheck : public ClangTidyCheck { } // namespace clang::tidy::cppcoreguidelines -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDBOUNDSERRORSCHECK_H +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PREFERATOVERSUBSCRIPTOPERATORCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 0106b6ea732db..c88e59e453265 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -160,10 +160,10 @@ New checks Replaces nested ``std::min`` and ``std::max`` calls with an initializer list where applicable. -- New :doc:`cppcoreguidelines-avoid-bounds-errors - <clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors>` check. +- New :doc:`cppcoreguidelines-prefer-at-over-subscript-operator + <clang-tidy/checks/cppcoreguidelines/prefer-at-over-subscript-operator>` check. - Flags the unsafe ``operator[]`` and replaces it with ``at()``. + Flags the unsafe ``operator[]`` and suggests replacing 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/prefer-at-over-subscript-operator.rst similarity index 83% rename from clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst rename to clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-at-over-subscript-operator.rst index 13683ee9b5a46..183175b596668 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-at-over-subscript-operator.rst @@ -1,6 +1,6 @@ -.. title:: clang-tidy - cppcoreguidelines-avoid-bounds-errors +.. title:: clang-tidy - cppcoreguidelines-prefer-at-over-subscript-operator -cppcoreguidelines-avoid-bounds-errors +cppcoreguidelines-prefer-at-over-subscript-operator ===================================== 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()``. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index d9c72bedfb95b..d83dad31b3547 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -174,7 +174,6 @@ 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>`, @@ -190,6 +189,7 @@ Clang-Tidy Checks :doc:`cppcoreguidelines-no-malloc <cppcoreguidelines/no-malloc>`, :doc:`cppcoreguidelines-no-suspend-with-lock <cppcoreguidelines/no-suspend-with-lock>`, :doc:`cppcoreguidelines-owning-memory <cppcoreguidelines/owning-memory>`, + :doc:`cppcoreguidelines-prefer-at-over-subscript-operator <cppcoreguidelines/prefer-at-over-subscript-operator>`, :doc:`cppcoreguidelines-prefer-member-initializer <cppcoreguidelines/prefer-member-initializer>`, "Yes" :doc:`cppcoreguidelines-pro-bounds-array-to-pointer-decay <cppcoreguidelines/pro-bounds-array-to-pointer-decay>`, :doc:`cppcoreguidelines-pro-bounds-constant-array-index <cppcoreguidelines/pro-bounds-constant-array-index>`, "Yes" diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-at-over-subscript-operator.cpp similarity index 67% rename from clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp rename to clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-at-over-subscript-operator.cpp index 5e12e7d71790d..45071f8f9f207 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-at-over-subscript-operator.cpp @@ -34,24 +34,24 @@ namespace json { } // namespace json -// RUN: %check_clang_tidy %s cppcoreguidelines-avoid-bounds-errors %t +// RUN: %check_clang_tidy %s cppcoreguidelines-prefer-at-over-subscript-operator %t std::array<int, 3> a; auto b = a[0]; -// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-avoid-bounds-errors] +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-prefer-at-over-subscript-operator] auto c = a[1+1]; -// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-avoid-bounds-errors] +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-prefer-at-over-subscript-operator] constexpr int index = 1; auto d = a[index]; -// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-avoid-bounds-errors] +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-prefer-at-over-subscript-operator] int e(int index) { return a[index]; -// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-avoid-bounds-errors] +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-prefer-at-over-subscript-operator] } auto f = (&a)->operator[](1); -// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-avoid-bounds-errors] +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-prefer-at-over-subscript-operator] auto g = a.at(0); From ce18fe6d17385d2ad5b4d3e05661af660e6575cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Heidekr=C3=BCger?= <paul.heidekrue...@tum.de> Date: Mon, 10 Jun 2024 22:14:54 +0200 Subject: [PATCH 05/23] Add a user-customizable mechanism for excluding classes from the analysis Co-authored-by: Manuel Pietsch <manuelpiet...@outlook.de> --- .../PreferAtOverSubscriptOperatorCheck.cpp | 49 +++++++++++++++++-- .../PreferAtOverSubscriptOperatorCheck.h | 5 ++ .../prefer-at-over-subscript-operator.cpp | 36 +++++++++++++- 3 files changed, 86 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.cpp index 64eaf8a1d4ebd..d7cdbc59d3941 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.cpp @@ -7,13 +7,52 @@ //===----------------------------------------------------------------------===// #include "PreferAtOverSubscriptOperatorCheck.h" +#include "../utils/OptionsUtils.h" #include "clang/ASTMatchers/ASTMatchFinder.h" -#include "clang/Lex/Lexer.h" +#include "llvm/ADT/StringRef.h" +#include <algorithm> +#include <numeric> using namespace clang::ast_matchers; namespace clang::tidy::cppcoreguidelines { +static constexpr std::array<llvm::StringRef, 3> DefaultExclusions = { + llvm::StringRef("std::map"), llvm::StringRef("std::unordered_map"), + llvm::StringRef("std::flat_map")}; + +PreferAtOverSubscriptOperatorCheck::PreferAtOverSubscriptOperatorCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) { + + ExcludedClasses = clang::tidy::utils::options::parseStringList( + Options.get("ExcludeClasses", "")); + ExcludedClasses.insert(ExcludedClasses.end(), DefaultExclusions.begin(), + DefaultExclusions.end()); +} + +void PreferAtOverSubscriptOperatorCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + + if (ExcludedClasses.size() == DefaultExclusions.size()) { + Options.store(Opts, "ExcludeClasses", ""); + return; + } + + // Sum up the sizes of the defaults ( + semicolons), so we can remove them + // from the saved options + size_t DefaultsStringLength = + std::transform_reduce(DefaultExclusions.begin(), DefaultExclusions.end(), + DefaultExclusions.size(), std::plus<>(), + [](llvm::StringRef Name) { return Name.size(); }); + + std::string Serialized = + clang::tidy::utils::options::serializeStringList(ExcludedClasses); + + Options.store(Opts, "ExcludeClasses", + Serialized.substr(0, Serialized.size() - DefaultsStringLength)); +} + const CXXMethodDecl *findAlternative(const CXXRecordDecl *MatchedParent, const CXXMethodDecl *MatchedOperator) { for (const CXXMethodDecl *Method : MatchedParent->methods()) { @@ -67,13 +106,17 @@ void PreferAtOverSubscriptOperatorCheck::check( const CXXRecordDecl *MatchedParent = Result.Nodes.getNodeAs<CXXRecordDecl>("parent"); + std::string ClassIdentifier = MatchedParent->getQualifiedNameAsString(); + + if (std::find(ExcludedClasses.begin(), ExcludedClasses.end(), + ClassIdentifier) != ExcludedClasses.end()) + return; + const CXXMethodDecl *Alternative = findAlternative(MatchedParent, MatchedOperator); if (!Alternative) return; - const SourceLocation AlternativeSource(Alternative->getBeginLoc()); - diag(MatchedExpr->getBeginLoc(), "found possibly unsafe operator[], consider using at() instead"); diag(Alternative->getBeginLoc(), "alternative at() defined here", diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.h index eb6e3a021e1b6..f2450a7ab3470 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.h +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.h @@ -28,6 +28,11 @@ class PreferAtOverSubscriptOperatorCheck : public ClangTidyCheck { } void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + +private: + // A list of class names that are excluded from the warning + std::vector<llvm::StringRef> ExcludedClasses; }; } // namespace clang::tidy::cppcoreguidelines diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-at-over-subscript-operator.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-at-over-subscript-operator.cpp index 45071f8f9f207..7da6c31556969 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-at-over-subscript-operator.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-at-over-subscript-operator.cpp @@ -34,7 +34,30 @@ namespace json { } // namespace json -// RUN: %check_clang_tidy %s cppcoreguidelines-prefer-at-over-subscript-operator %t +class ExcludedClass1 { + public: + int operator[](unsigned i) { + return 1; + } + int at(unsigned i) { + return 1; + } +}; + +class ExcludedClass2 { + public: + int operator[](unsigned i) { + return 1; + } + int at(unsigned i) { + return 1; + } +}; + + +// RUN: %check_clang_tidy %s \ +// RUN: cppcoreguidelines-prefer-at-over-subscript-operator %t -- \ +// RUN: -config='{CheckOptions: {cppcoreguidelines-prefer-at-over-subscript-operator.ExcludeClasses: "ExcludedClass1;ExcludedClass2"}}' std::array<int, 3> a; auto b = a[0]; @@ -63,3 +86,14 @@ auto t = s[0]; json::node<int> n; auto m = n[0]; + +//explicitely excluded classes / struct / template +ExcludedClass1 E1; +auto x1 = E1[0]; + +ExcludedClass2 E2; +auto x2 = E1[0]; + +std::map<int,int> TestMap; +auto y = TestMap[0]; + From 28d26b558d2ef9f29c8a1afbf52d32c2d0258d43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Heidekr=C3=BCger?= <paul.heidekrue...@tum.de> Date: Mon, 10 Jun 2024 22:16:49 +0200 Subject: [PATCH 06/23] Update the documentation Co-authored-by: Manuel Pietsch <manuelpiet...@outlook.de> --- .../prefer-at-over-subscript-operator.rst | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-at-over-subscript-operator.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-at-over-subscript-operator.rst index 183175b596668..42a2100f32582 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-at-over-subscript-operator.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-at-over-subscript-operator.rst @@ -3,8 +3,7 @@ cppcoreguidelines-prefer-at-over-subscript-operator ===================================== -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. +This check flags all uses of ``operator[]`` where an equivalent (same parameter and return types) ``at()`` method exists and suggest using that instead. For example the code @@ -12,10 +11,21 @@ For example the code std::array<int, 3> a; int b = a[4]; -will be replaced by +will generate a warning but .. code-block:: c++ - std::vector<int, 3> a; - int b = a.at(4); + std::unique_ptr<int> a; + int b = a[0]; -This check enforces the `SL.con.3 <https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slcon3-avoid-bounds-errors>` guideline. +will not. + +The classes ``std::map``, ``std::unordered_map`` and ``std::flat_map`` are excluded from this check, because for them the subscript operator has a defined behaviour when a key does not exist (inserting a new element). + +Options +------- + +.. option:: ExcludeClasses + + Semicolon-delimited list of class names that should additionally be excluded from this check. By default empty. + +This check enforces part of the `SL.con.3 <https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slcon3-avoid-bounds-errors>` guideline. From e7ea897c0a7cec86a103f890f459d287a5c00868 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Heidekr=C3=BCger?= <paul.heidekrue...@tum.de> Date: Mon, 10 Jun 2024 21:43:23 +0200 Subject: [PATCH 07/23] Add remaining tests requested by @PiotrZSL Co-authored-by: Manuel Pietsch <manuelpiet...@outlook.de> --- .../prefer-at-over-subscript-operator.cpp | 53 +++++++++++++++++-- 1 file changed, 48 insertions(+), 5 deletions(-) diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-at-over-subscript-operator.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-at-over-subscript-operator.cpp index 7da6c31556969..cc7088bffeda9 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-at-over-subscript-operator.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-at-over-subscript-operator.cpp @@ -9,6 +9,16 @@ namespace std { } }; + template<typename T, typename V> + struct map { + T operator[](unsigned i) { + return T{1}; + } + T at(unsigned i) { + return T{1}; + } + }; + template<typename T> struct unique_ptr { T operator[](unsigned i) { @@ -33,6 +43,7 @@ namespace json { }; } // namespace json +struct SubClass : std::array<int, 3> {}; class ExcludedClass1 { public: @@ -62,15 +73,17 @@ std::array<int, 3> a; auto b = a[0]; // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-prefer-at-over-subscript-operator] + auto c = a[1+1]; // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-prefer-at-over-subscript-operator] -constexpr int index = 1; -auto d = a[index]; -// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-prefer-at-over-subscript-operator] -int e(int index) { - return a[index]; +constexpr int Index = 1; +auto d = a[Index]; // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-prefer-at-over-subscript-operator] + +int e(int Ind) { + return a[Ind]; + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-prefer-at-over-subscript-operator] } auto f = (&a)->operator[](1); @@ -87,6 +100,24 @@ auto t = s[0]; json::node<int> n; auto m = n[0]; +SubClass Sub; +auto r = Sub[0]; +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-prefer-at-over-subscript-operator] + +typedef std::array<int, 3> ar; +ar BehindDef; +auto u = BehindDef[0]; +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-prefer-at-over-subscript-operator] + +template<typename T> int TestTemplate(T t){ + return t[0]; + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-prefer-at-over-subscript-operator] + +} + +auto v = TestTemplate<>(a); +auto w = TestTemplate<>(p); + //explicitely excluded classes / struct / template ExcludedClass1 E1; auto x1 = E1[0]; @@ -97,3 +128,15 @@ auto x2 = E1[0]; std::map<int,int> TestMap; auto y = TestMap[0]; +#define SUBSCRIPT_BEHIND_MARCO(x) a[x] +#define ARG_BEHIND_MACRO 0 +#define OBJECT_BEHIND_MACRO a + +auto m1 = SUBSCRIPT_BEHIND_MARCO(0); +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-prefer-at-over-subscript-operator] + +auto m2 = a[ARG_BEHIND_MACRO]; +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-prefer-at-over-subscript-operator] + +auto m3 = OBJECT_BEHIND_MACRO[0]; +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-prefer-at-over-subscript-operator] From 19793edbc3d96af77d49d06037fabb043af5b370 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Heidekr=C3=BCger?= <paul.heidekrue...@tum.de> Date: Mon, 10 Jun 2024 22:22:21 +0200 Subject: [PATCH 08/23] Remove unused and rename variables Co-authored-by: Manuel Pietsch <manuelpiet...@outlook.de> --- .../PreferAtOverSubscriptOperatorCheck.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.cpp index d7cdbc59d3941..dc036e23e2af1 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.cpp @@ -70,10 +70,10 @@ const CXXMethodDecl *findAlternative(const CXXRecordDecl *MatchedParent, if (!SameNumberOfArguments) continue; - for (unsigned a = 0; a < Method->getNumParams(); a++) { + for (unsigned ArgInd = 0; ArgInd < Method->getNumParams(); ArgInd++) { const bool SameArgType = - Method->parameters()[a]->getOriginalType() == - MatchedOperator->parameters()[a]->getOriginalType(); + Method->parameters()[ArgInd]->getOriginalType() == + MatchedOperator->parameters()[ArgInd]->getOriginalType(); if (!SameArgType) continue; } @@ -98,8 +98,6 @@ void PreferAtOverSubscriptOperatorCheck::registerMatchers(MatchFinder *Finder) { void PreferAtOverSubscriptOperatorCheck::check( const MatchFinder::MatchResult &Result) { - const ASTContext &Context = *Result.Context; - const SourceManager &Source = Context.getSourceManager(); const CallExpr *MatchedExpr = Result.Nodes.getNodeAs<CallExpr>("caller"); const CXXMethodDecl *MatchedOperator = Result.Nodes.getNodeAs<CXXMethodDecl>("operator"); From 514f86783699534d6e078a44356d862b59484c61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Heidekr=C3=BCger?= <paul.heidekrue...@tum.de> Date: Wed, 12 Jun 2024 16:56:40 +0200 Subject: [PATCH 09/23] Update types in PreferAtOverSubscriptOperatorCheck::check() Co-authored-by: Manuel Pietsch <manuelpiet...@outlook.de> --- .../PreferAtOverSubscriptOperatorCheck.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.cpp index dc036e23e2af1..0924fda164b4a 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.cpp @@ -98,11 +98,10 @@ void PreferAtOverSubscriptOperatorCheck::registerMatchers(MatchFinder *Finder) { void PreferAtOverSubscriptOperatorCheck::check( const MatchFinder::MatchResult &Result) { - const CallExpr *MatchedExpr = Result.Nodes.getNodeAs<CallExpr>("caller"); - const CXXMethodDecl *MatchedOperator = + const auto *MatchedExpr = Result.Nodes.getNodeAs<CallExpr>("caller"); + const auto *MatchedOperator = Result.Nodes.getNodeAs<CXXMethodDecl>("operator"); - const CXXRecordDecl *MatchedParent = - Result.Nodes.getNodeAs<CXXRecordDecl>("parent"); + const auto *MatchedParent = Result.Nodes.getNodeAs<CXXRecordDecl>("parent"); std::string ClassIdentifier = MatchedParent->getQualifiedNameAsString(); From e5317255c9affc6e78f228561996b6e9789b5c6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Heidekr=C3=BCger?= <paul.heidekrue...@tum.de> Date: Wed, 12 Jun 2024 17:00:00 +0200 Subject: [PATCH 10/23] Fix length of underline in prefer-at-over-subscript-operator.rst Co-authored-by: Manuel Pietsch <manuelpiet...@outlook.de> --- .../cppcoreguidelines/prefer-at-over-subscript-operator.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-at-over-subscript-operator.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-at-over-subscript-operator.rst index 42a2100f32582..96c71931ff2a3 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-at-over-subscript-operator.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-at-over-subscript-operator.rst @@ -1,7 +1,7 @@ .. title:: clang-tidy - cppcoreguidelines-prefer-at-over-subscript-operator cppcoreguidelines-prefer-at-over-subscript-operator -===================================== +=================================================== This check flags all uses of ``operator[]`` where an equivalent (same parameter and return types) ``at()`` method exists and suggest using that instead. From 872ebecd627a72596e62867ab85ba27e319cfe99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Heidekr=C3=BCger?= <paul.heidekrue...@tum.de> Date: Wed, 12 Jun 2024 17:02:33 +0200 Subject: [PATCH 11/23] Enforce 80 char column width in prefer-at-over-subscript-operator.rst Co-authored-by: Manuel Pietsch <manuelpiet...@outlook.de> --- .../prefer-at-over-subscript-operator.rst | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-at-over-subscript-operator.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-at-over-subscript-operator.rst index 96c71931ff2a3..873564d12dd6b 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-at-over-subscript-operator.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-at-over-subscript-operator.rst @@ -3,7 +3,8 @@ cppcoreguidelines-prefer-at-over-subscript-operator =================================================== -This check flags all uses of ``operator[]`` where an equivalent (same parameter and return types) ``at()`` method exists and suggest using that instead. +This check flags all uses of ``operator[]`` where an equivalent (same parameter +and return types) ``at()`` method exists and suggest using that instead. For example the code @@ -19,13 +20,18 @@ will generate a warning but will not. -The classes ``std::map``, ``std::unordered_map`` and ``std::flat_map`` are excluded from this check, because for them the subscript operator has a defined behaviour when a key does not exist (inserting a new element). +The classes ``std::map``, ``std::unordered_map`` and ``std::flat_map`` are +excluded from this check, because for them the subscript operator has a defined +behaviour when a key does not exist (inserting a new element). Options ------- .. option:: ExcludeClasses - Semicolon-delimited list of class names that should additionally be excluded from this check. By default empty. + Semicolon-delimited list of class names that should additionally be + excluded from this check. By default empty. -This check enforces part of the `SL.con.3 <https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slcon3-avoid-bounds-errors>` guideline. +This check enforces part of the `SL.con.3 +<https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slcon3-avoid-bounds-errors>` +guideline. From 27ce22d0ec43b9e11167286db7b15b5c936e5de8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Heidekr=C3=BCger?= <paul.heidekrue...@tum.de> Date: Thu, 13 Jun 2024 10:18:58 +0200 Subject: [PATCH 12/23] Synchronise prefer-at-over-subscript-operator.rst with ReleaseNotes.rst Co-authored-by: Manuel Pietsch <manuelpiet...@outlook.de> --- .../cppcoreguidelines/prefer-at-over-subscript-operator.rst | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-at-over-subscript-operator.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-at-over-subscript-operator.rst index 873564d12dd6b..7999cd4baeb7c 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-at-over-subscript-operator.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-at-over-subscript-operator.rst @@ -3,8 +3,7 @@ cppcoreguidelines-prefer-at-over-subscript-operator =================================================== -This check flags all uses of ``operator[]`` where an equivalent (same parameter -and return types) ``at()`` method exists and suggest using that instead. +Flags the unsafe ``operator[]`` and suggests replacing it with ``at()``. For example the code From e905c98deb907d879e2a58a89a1d97737b92865c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Heidekr=C3=BCger?= <paul.heidekrue...@tum.de> Date: Mon, 17 Jun 2024 12:03:57 +0200 Subject: [PATCH 13/23] Move RUN lines to the top of prefer-at-over-subscript-operator.cpp Co-authored-by: Manuel Pietsch <manuelpiet...@outlook.de> --- .../prefer-at-over-subscript-operator.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-at-over-subscript-operator.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-at-over-subscript-operator.cpp index cc7088bffeda9..76b84bf860cb6 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-at-over-subscript-operator.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-at-over-subscript-operator.cpp @@ -1,3 +1,7 @@ +// RUN: %check_clang_tidy %s \ +// RUN: cppcoreguidelines-prefer-at-over-subscript-operator %t -- \ +// RUN: -config='{CheckOptions: {cppcoreguidelines-prefer-at-over-subscript-operator.ExcludeClasses: "ExcludedClass1;ExcludedClass2"}}' + namespace std { template<typename T, unsigned size> struct array { @@ -65,10 +69,6 @@ class ExcludedClass2 { } }; - -// RUN: %check_clang_tidy %s \ -// RUN: cppcoreguidelines-prefer-at-over-subscript-operator %t -- \ -// RUN: -config='{CheckOptions: {cppcoreguidelines-prefer-at-over-subscript-operator.ExcludeClasses: "ExcludedClass1;ExcludedClass2"}}' std::array<int, 3> a; auto b = a[0]; From 111f71fa069ff474cc31229f5a8cb1dfd3d663cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Heidekr=C3=BCger?= <paul.heidekrue...@tum.de> Date: Mon, 17 Jun 2024 12:05:10 +0200 Subject: [PATCH 14/23] Add an empty lien after each `code-block` in prefer-at-over-subscript-operator.rst Co-authored-by: Manuel Pietsch <manuelpiet...@outlook.de> --- .../cppcoreguidelines/prefer-at-over-subscript-operator.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-at-over-subscript-operator.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-at-over-subscript-operator.rst index 7999cd4baeb7c..f3577cb5b15f0 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-at-over-subscript-operator.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-at-over-subscript-operator.rst @@ -8,12 +8,14 @@ Flags the unsafe ``operator[]`` and suggests replacing it with ``at()``. For example the code .. code-block:: c++ + std::array<int, 3> a; int b = a[4]; will generate a warning but .. code-block:: c++ + std::unique_ptr<int> a; int b = a[0]; From 4da1da0eeadeb51f8d435932dccc2b28b688f239 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Heidekr=C3=BCger?= <paul.heidekrue...@tum.de> Date: Mon, 17 Jun 2024 12:08:56 +0200 Subject: [PATCH 15/23] Use ofClass() instead of hasParent() in PreferAtOverSubscriptOperatorCheck.cpp Co-authored-by: Manuel Pietsch <manuelpiet...@outlook.de> --- .../cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.cpp index 0924fda164b4a..d047c0b2332d1 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.cpp @@ -90,8 +90,8 @@ void PreferAtOverSubscriptOperatorCheck::registerMatchers(MatchFinder *Finder) { callExpr( callee( cxxMethodDecl(hasOverloadedOperatorName("[]")).bind("operator")), - callee(cxxMethodDecl(hasParent( - cxxRecordDecl(hasMethod(hasName("at"))).bind("parent"))))) + callee(cxxMethodDecl( + ofClass(cxxRecordDecl(hasMethod(hasName("at"))).bind("parent"))))) .bind("caller"), this); } From 5098f65dc68e33ef854335e243b9570c4b0cf696 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Heidekr=C3=BCger?= <paul.heidekrue...@tum.de> Date: Mon, 17 Jun 2024 12:42:33 +0200 Subject: [PATCH 16/23] Use matchesAnyListedName() in matcher instead of explicit std::find() check Co-authored-by: Manuel Pietsch <manuelpiet...@outlook.de> --- .../PreferAtOverSubscriptOperatorCheck.cpp | 15 +++++---------- .../prefer-at-over-subscript-operator.cpp | 2 +- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.cpp index d047c0b2332d1..82a957241042a 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.cpp @@ -7,10 +7,10 @@ //===----------------------------------------------------------------------===// #include "PreferAtOverSubscriptOperatorCheck.h" +#include "../utils/Matchers.h" #include "../utils/OptionsUtils.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "llvm/ADT/StringRef.h" -#include <algorithm> #include <numeric> using namespace clang::ast_matchers; @@ -18,8 +18,8 @@ using namespace clang::ast_matchers; namespace clang::tidy::cppcoreguidelines { static constexpr std::array<llvm::StringRef, 3> DefaultExclusions = { - llvm::StringRef("std::map"), llvm::StringRef("std::unordered_map"), - llvm::StringRef("std::flat_map")}; + llvm::StringRef("::std::map"), llvm::StringRef("::std::unordered_map"), + llvm::StringRef("::std::flat_map")}; PreferAtOverSubscriptOperatorCheck::PreferAtOverSubscriptOperatorCheck( StringRef Name, ClangTidyContext *Context) @@ -91,7 +91,8 @@ void PreferAtOverSubscriptOperatorCheck::registerMatchers(MatchFinder *Finder) { callee( cxxMethodDecl(hasOverloadedOperatorName("[]")).bind("operator")), callee(cxxMethodDecl( - ofClass(cxxRecordDecl(hasMethod(hasName("at"))).bind("parent"))))) + ofClass(cxxRecordDecl(hasMethod(hasName("at"))).bind("parent")), + unless(matchers::matchesAnyListedName(ExcludedClasses))))) .bind("caller"), this); } @@ -103,12 +104,6 @@ void PreferAtOverSubscriptOperatorCheck::check( Result.Nodes.getNodeAs<CXXMethodDecl>("operator"); const auto *MatchedParent = Result.Nodes.getNodeAs<CXXRecordDecl>("parent"); - std::string ClassIdentifier = MatchedParent->getQualifiedNameAsString(); - - if (std::find(ExcludedClasses.begin(), ExcludedClasses.end(), - ClassIdentifier) != ExcludedClasses.end()) - return; - const CXXMethodDecl *Alternative = findAlternative(MatchedParent, MatchedOperator); if (!Alternative) diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-at-over-subscript-operator.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-at-over-subscript-operator.cpp index 76b84bf860cb6..4ae17688cb9e2 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-at-over-subscript-operator.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-at-over-subscript-operator.cpp @@ -1,6 +1,6 @@ // RUN: %check_clang_tidy %s \ // RUN: cppcoreguidelines-prefer-at-over-subscript-operator %t -- \ -// RUN: -config='{CheckOptions: {cppcoreguidelines-prefer-at-over-subscript-operator.ExcludeClasses: "ExcludedClass1;ExcludedClass2"}}' +// RUN: -config='{CheckOptions: {cppcoreguidelines-prefer-at-over-subscript-operator.ExcludeClasses: "::ExcludedClass1;::ExcludedClass2"}}' namespace std { template<typename T, unsigned size> From da428089d6cfdfaf02b563a325ab8d900b62de56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Heidekr=C3=BCger?= <paul.heidekrue...@tum.de> Date: Mon, 17 Jun 2024 14:23:35 +0200 Subject: [PATCH 17/23] Match source range of expression in warnings emitted by PreferAtOverSubscriptOperatorCheck Co-authored-by: Manuel Pietsch <manuelpiet...@outlook.de> --- .../PreferAtOverSubscriptOperatorCheck.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.cpp index 82a957241042a..5a5704deae14f 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.cpp @@ -110,9 +110,11 @@ void PreferAtOverSubscriptOperatorCheck::check( return; diag(MatchedExpr->getBeginLoc(), - "found possibly unsafe operator[], consider using at() instead"); + "found possibly unsafe operator[], consider using at() instead") + << MatchedExpr->getSourceRange(); diag(Alternative->getBeginLoc(), "alternative at() defined here", - DiagnosticIDs::Note); + DiagnosticIDs::Note) + << Alternative->getSourceRange(); } } // namespace clang::tidy::cppcoreguidelines From e4a4a694f0068d57b417adacd5c9a4595269c752 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Heidekr=C3=BCger?= <paul.heidekrue...@tum.de> Date: Fri, 21 Jun 2024 10:42:45 +0200 Subject: [PATCH 18/23] PreferAtOverSubscriptOpterator -> ProBoundsAvoidUncheckedContainerAccesses Co-authored-by: Manuel Pietsch <manuelpiet...@outlook.de> --- .../cppcoreguidelines/CMakeLists.txt | 2 +- .../CppCoreGuidelinesTidyModule.cpp | 6 ++--- ...BoundsAvoidUncheckedContainerAccesses.cpp} | 16 +++++++----- ...roBoundsAvoidUncheckedContainerAccesses.h} | 15 ++++++----- clang-tools-extra/docs/ReleaseNotes.rst | 4 +-- ...ds-avoid-unchecked-container-accesses.rst} | 4 +-- .../docs/clang-tidy/checks/list.rst | 2 +- ...ds-avoid-unchecked-container-accesses.cpp} | 26 +++++++++---------- 8 files changed, 39 insertions(+), 36 deletions(-) rename clang-tools-extra/clang-tidy/cppcoreguidelines/{PreferAtOverSubscriptOperatorCheck.cpp => ProBoundsAvoidUncheckedContainerAccesses.cpp} (88%) rename clang-tools-extra/clang-tidy/cppcoreguidelines/{PreferAtOverSubscriptOperatorCheck.h => ProBoundsAvoidUncheckedContainerAccesses.h} (67%) rename clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/{prefer-at-over-subscript-operator.rst => pro-bounds-avoid-unchecked-container-accesses.rst} (84%) rename clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/{prefer-at-over-subscript-operator.cpp => pro-bounds-avoid-unchecked-container-accesses.cpp} (82%) diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt index fd436859ad04a..aa7bdd86bc795 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt @@ -20,9 +20,9 @@ add_clang_library(clangTidyCppCoreGuidelinesModule NoMallocCheck.cpp NoSuspendWithLockCheck.cpp OwningMemoryCheck.cpp - PreferAtOverSubscriptOperatorCheck.cpp PreferMemberInitializerCheck.cpp ProBoundsArrayToPointerDecayCheck.cpp + ProBoundsAvoidUncheckedContainerAccesses.cpp ProBoundsConstantArrayIndexCheck.cpp ProBoundsPointerArithmeticCheck.cpp ProTypeConstCastCheck.cpp diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp index 565a99a865519..c154f4eec9373 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp @@ -34,9 +34,9 @@ #include "NoMallocCheck.h" #include "NoSuspendWithLockCheck.h" #include "OwningMemoryCheck.h" -#include "PreferAtOverSubscriptOperatorCheck.h" #include "PreferMemberInitializerCheck.h" #include "ProBoundsArrayToPointerDecayCheck.h" +#include "ProBoundsAvoidUncheckedContainerAccesses.h" #include "ProBoundsConstantArrayIndexCheck.h" #include "ProBoundsPointerArithmeticCheck.h" #include "ProTypeConstCastCheck.h" @@ -103,12 +103,12 @@ class CppCoreGuidelinesModule : public ClangTidyModule { "cppcoreguidelines-non-private-member-variables-in-classes"); CheckFactories.registerCheck<OwningMemoryCheck>( "cppcoreguidelines-owning-memory"); - CheckFactories.registerCheck<PreferAtOverSubscriptOperatorCheck>( - "cppcoreguidelines-prefer-at-over-subscript-operator"); CheckFactories.registerCheck<PreferMemberInitializerCheck>( "cppcoreguidelines-prefer-member-initializer"); CheckFactories.registerCheck<ProBoundsArrayToPointerDecayCheck>( "cppcoreguidelines-pro-bounds-array-to-pointer-decay"); + CheckFactories.registerCheck<ProBoundsAvoidUncheckedContainerAccesses>( + "cppcoreguidelines-pro-bounds-avoid-unchecked-container-accesses"); CheckFactories.registerCheck<ProBoundsConstantArrayIndexCheck>( "cppcoreguidelines-pro-bounds-constant-array-index"); CheckFactories.registerCheck<ProBoundsPointerArithmeticCheck>( diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsAvoidUncheckedContainerAccesses.cpp similarity index 88% rename from clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.cpp rename to clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsAvoidUncheckedContainerAccesses.cpp index 5a5704deae14f..19a52d0e565fa 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsAvoidUncheckedContainerAccesses.cpp @@ -1,4 +1,4 @@ -//===--- PreferAtOverSubscriptOperatorCheck.cpp - clang-tidy --------------===// +//===--- ProBoundsAvoidUncheckedContainerAccesses.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. @@ -6,7 +6,7 @@ // //===----------------------------------------------------------------------===// -#include "PreferAtOverSubscriptOperatorCheck.h" +#include "ProBoundsAvoidUncheckedContainerAccesses.h" #include "../utils/Matchers.h" #include "../utils/OptionsUtils.h" #include "clang/ASTMatchers/ASTMatchFinder.h" @@ -21,8 +21,9 @@ static constexpr std::array<llvm::StringRef, 3> DefaultExclusions = { llvm::StringRef("::std::map"), llvm::StringRef("::std::unordered_map"), llvm::StringRef("::std::flat_map")}; -PreferAtOverSubscriptOperatorCheck::PreferAtOverSubscriptOperatorCheck( - StringRef Name, ClangTidyContext *Context) +ProBoundsAvoidUncheckedContainerAccesses:: + ProBoundsAvoidUncheckedContainerAccesses(StringRef Name, + ClangTidyContext *Context) : ClangTidyCheck(Name, Context) { ExcludedClasses = clang::tidy::utils::options::parseStringList( @@ -31,7 +32,7 @@ PreferAtOverSubscriptOperatorCheck::PreferAtOverSubscriptOperatorCheck( DefaultExclusions.end()); } -void PreferAtOverSubscriptOperatorCheck::storeOptions( +void ProBoundsAvoidUncheckedContainerAccesses::storeOptions( ClangTidyOptions::OptionMap &Opts) { if (ExcludedClasses.size() == DefaultExclusions.size()) { @@ -83,7 +84,8 @@ const CXXMethodDecl *findAlternative(const CXXRecordDecl *MatchedParent, return static_cast<CXXMethodDecl *>(nullptr); } -void PreferAtOverSubscriptOperatorCheck::registerMatchers(MatchFinder *Finder) { +void ProBoundsAvoidUncheckedContainerAccesses::registerMatchers( + MatchFinder *Finder) { // Need a callExpr here to match CXXOperatorCallExpr ``(&a)->operator[](0)`` // and CXXMemberCallExpr ``a[0]``. Finder->addMatcher( @@ -97,7 +99,7 @@ void PreferAtOverSubscriptOperatorCheck::registerMatchers(MatchFinder *Finder) { this); } -void PreferAtOverSubscriptOperatorCheck::check( +void ProBoundsAvoidUncheckedContainerAccesses::check( const MatchFinder::MatchResult &Result) { const auto *MatchedExpr = Result.Nodes.getNodeAs<CallExpr>("caller"); const auto *MatchedOperator = diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsAvoidUncheckedContainerAccesses.h similarity index 67% rename from clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.h rename to clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsAvoidUncheckedContainerAccesses.h index f2450a7ab3470..33abbef5e8243 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferAtOverSubscriptOperatorCheck.h +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsAvoidUncheckedContainerAccesses.h @@ -1,4 +1,4 @@ -//===--- PreferAtOverSubscriptOperatorCheck.h - clang-tidy ------*- C++ -*-===// +//===--- ProBoundsAvoidUncheckedContainerAccesses.h - clang-tidy *- C++ -*-===// //===--- PreferMemberInitializerCheck.h - clang-tidy ------------*- C++ -*-===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. @@ -7,8 +7,8 @@ // //===----------------------------------------------------------------------===// -#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PREFERATOVERSUBSCRIPTOPERATORCHECK_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PREFERATOVERSUBSCRIPTOPERATORCHECK_H +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_BOUNDS_AVOID_UNCHECKED_CONTAINER_ACCESSES_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_BOUNDS_AVOID_UNCHECKED_CONTAINER_ACCESSES_H #include "../ClangTidyCheck.h" @@ -19,10 +19,11 @@ namespace clang::tidy::cppcoreguidelines { /// 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/prefer-at-over-subscript-operator.html -class PreferAtOverSubscriptOperatorCheck : public ClangTidyCheck { +/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/pro-bounds-avoid-unchecked-container-accesses.html +class ProBoundsAvoidUncheckedContainerAccesses : public ClangTidyCheck { public: - PreferAtOverSubscriptOperatorCheck(StringRef Name, ClangTidyContext *Context); + ProBoundsAvoidUncheckedContainerAccesses(StringRef Name, + ClangTidyContext *Context); bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { return LangOpts.CPlusPlus; } @@ -37,4 +38,4 @@ class PreferAtOverSubscriptOperatorCheck : public ClangTidyCheck { } // namespace clang::tidy::cppcoreguidelines -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PREFERATOVERSUBSCRIPTOPERATORCHECK_H +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_BOUNDS_AVOID_UNCHECKED_CONTAINER_ACCESSES_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index c88e59e453265..cb43840a54806 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -160,8 +160,8 @@ New checks Replaces nested ``std::min`` and ``std::max`` calls with an initializer list where applicable. -- New :doc:`cppcoreguidelines-prefer-at-over-subscript-operator - <clang-tidy/checks/cppcoreguidelines/prefer-at-over-subscript-operator>` check. +- New :doc:`cppcoreguidelines-pro-bounds-avoid-unchecked-container-accesses + <clang-tidy/checks/cppcoreguidelines/pro-bounds-avoid-unchecked-container-accesses>` check. Flags the unsafe ``operator[]`` and suggests replacing it with ``at()``. diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-at-over-subscript-operator.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-bounds-avoid-unchecked-container-accesses.rst similarity index 84% rename from clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-at-over-subscript-operator.rst rename to clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-bounds-avoid-unchecked-container-accesses.rst index f3577cb5b15f0..616882738653d 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-at-over-subscript-operator.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-bounds-avoid-unchecked-container-accesses.rst @@ -1,6 +1,6 @@ -.. title:: clang-tidy - cppcoreguidelines-prefer-at-over-subscript-operator +.. title:: clang-tidy - cppcoreguidelines-pro-bounds-avoid-unchecked-container-accesses -cppcoreguidelines-prefer-at-over-subscript-operator +cppcoreguidelines-pro-bounds-avoid-unchecked-container-accesses =================================================== Flags the unsafe ``operator[]`` and suggests replacing it with ``at()``. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index d83dad31b3547..06445e03b2022 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -189,9 +189,9 @@ Clang-Tidy Checks :doc:`cppcoreguidelines-no-malloc <cppcoreguidelines/no-malloc>`, :doc:`cppcoreguidelines-no-suspend-with-lock <cppcoreguidelines/no-suspend-with-lock>`, :doc:`cppcoreguidelines-owning-memory <cppcoreguidelines/owning-memory>`, - :doc:`cppcoreguidelines-prefer-at-over-subscript-operator <cppcoreguidelines/prefer-at-over-subscript-operator>`, :doc:`cppcoreguidelines-prefer-member-initializer <cppcoreguidelines/prefer-member-initializer>`, "Yes" :doc:`cppcoreguidelines-pro-bounds-array-to-pointer-decay <cppcoreguidelines/pro-bounds-array-to-pointer-decay>`, + :doc:`cppcoreguidelines-pro-bounds-avoid-unchecked-container-accesses <cppcoreguidelines/pro-bounds-avoid-unchecked-container-accesses>`, :doc:`cppcoreguidelines-pro-bounds-constant-array-index <cppcoreguidelines/pro-bounds-constant-array-index>`, "Yes" :doc:`cppcoreguidelines-pro-bounds-pointer-arithmetic <cppcoreguidelines/pro-bounds-pointer-arithmetic>`, :doc:`cppcoreguidelines-pro-type-const-cast <cppcoreguidelines/pro-type-const-cast>`, diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-at-over-subscript-operator.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-avoid-unchecked-container-accesses.cpp similarity index 82% rename from clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-at-over-subscript-operator.cpp rename to clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-avoid-unchecked-container-accesses.cpp index 4ae17688cb9e2..1a7a7fba7d8a2 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-at-over-subscript-operator.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-avoid-unchecked-container-accesses.cpp @@ -1,6 +1,6 @@ // RUN: %check_clang_tidy %s \ -// RUN: cppcoreguidelines-prefer-at-over-subscript-operator %t -- \ -// RUN: -config='{CheckOptions: {cppcoreguidelines-prefer-at-over-subscript-operator.ExcludeClasses: "::ExcludedClass1;::ExcludedClass2"}}' +// RUN: cppcoreguidelines-pro-bounds-avoid-unchecked-container-accesses %t -- \ +// RUN: -config='{CheckOptions: {cppcoreguidelines-pro-bounds-avoid-unchecked-container-accesses.ExcludeClasses: "::ExcludedClass1;::ExcludedClass2"}}' namespace std { template<typename T, unsigned size> @@ -72,22 +72,22 @@ class ExcludedClass2 { std::array<int, 3> a; auto b = a[0]; -// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-prefer-at-over-subscript-operator] +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-pro-bounds-avoid-unchecked-container-accesses] auto c = a[1+1]; -// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-prefer-at-over-subscript-operator] +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-pro-bounds-avoid-unchecked-container-accesses] constexpr int Index = 1; auto d = a[Index]; -// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-prefer-at-over-subscript-operator] +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-pro-bounds-avoid-unchecked-container-accesses] int e(int Ind) { return a[Ind]; - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-prefer-at-over-subscript-operator] + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-pro-bounds-avoid-unchecked-container-accesses] } auto f = (&a)->operator[](1); -// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-prefer-at-over-subscript-operator] +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-pro-bounds-avoid-unchecked-container-accesses] auto g = a.at(0); @@ -102,16 +102,16 @@ auto m = n[0]; SubClass Sub; auto r = Sub[0]; -// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-prefer-at-over-subscript-operator] +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-pro-bounds-avoid-unchecked-container-accesses] typedef std::array<int, 3> ar; ar BehindDef; auto u = BehindDef[0]; -// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-prefer-at-over-subscript-operator] +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-pro-bounds-avoid-unchecked-container-accesses] template<typename T> int TestTemplate(T t){ return t[0]; - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-prefer-at-over-subscript-operator] + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-pro-bounds-avoid-unchecked-container-accesses] } @@ -133,10 +133,10 @@ auto y = TestMap[0]; #define OBJECT_BEHIND_MACRO a auto m1 = SUBSCRIPT_BEHIND_MARCO(0); -// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-prefer-at-over-subscript-operator] +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-pro-bounds-avoid-unchecked-container-accesses] auto m2 = a[ARG_BEHIND_MACRO]; -// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-prefer-at-over-subscript-operator] +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-pro-bounds-avoid-unchecked-container-accesses] auto m3 = OBJECT_BEHIND_MACRO[0]; -// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-prefer-at-over-subscript-operator] +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-pro-bounds-avoid-unchecked-container-accesses] From 659b87f223f21bacbcc7766292c054ca5edeb491 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Heidekr=C3=BCger?= <paul.heidekrue...@tum.de> Date: Fri, 21 Jun 2024 10:44:39 +0200 Subject: [PATCH 19/23] Remove extra comment Co-authored-by: Manuel Pietsch <manuelpiet...@outlook.de> --- .../cppcoreguidelines/ProBoundsAvoidUncheckedContainerAccesses.h | 1 - 1 file changed, 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsAvoidUncheckedContainerAccesses.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsAvoidUncheckedContainerAccesses.h index 33abbef5e8243..8fdfd5e9eb518 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsAvoidUncheckedContainerAccesses.h +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsAvoidUncheckedContainerAccesses.h @@ -1,5 +1,4 @@ //===--- ProBoundsAvoidUncheckedContainerAccesses.h - clang-tidy *- C++ -*-===// -//===--- PreferMemberInitializerCheck.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. From da1cd78e4e5d2f501d67c2313d22f10423fc06f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Heidekr=C3=BCger?= <paul.heidekrue...@tum.de> Date: Fri, 21 Jun 2024 10:52:42 +0200 Subject: [PATCH 20/23] Mention bounds safety profile explicitly Co-authored-by: Manuel Pietsch <manuelpiet...@outlook.de> --- .../pro-bounds-avoid-unchecked-container-accesses.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-bounds-avoid-unchecked-container-accesses.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-bounds-avoid-unchecked-container-accesses.rst index 616882738653d..f39c5171110a0 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-bounds-avoid-unchecked-container-accesses.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-bounds-avoid-unchecked-container-accesses.rst @@ -35,4 +35,6 @@ Options This check enforces part of the `SL.con.3 <https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slcon3-avoid-bounds-errors>` -guideline. +guideline and is part of the `Bounds Safety (Bounds 4) +<https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Pro-bounds-arrayindex>` profile +profile from the C++ Core Guidelines. From 1188fb56d078bb9948d7d29814daea7c14433125 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Heidekr=C3=BCger?= <paul.heidekrue...@tum.de> Date: Fri, 21 Jun 2024 10:55:53 +0200 Subject: [PATCH 21/23] Move paragraph about which rule/profile is being implemented above "Options" Co-authored-by: Manuel Pietsch <manuelpiet...@outlook.de> --- ...pro-bounds-avoid-unchecked-container-accesses.rst | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-bounds-avoid-unchecked-container-accesses.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-bounds-avoid-unchecked-container-accesses.rst index f39c5171110a0..b62ec4fc7b6b5 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-bounds-avoid-unchecked-container-accesses.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-bounds-avoid-unchecked-container-accesses.rst @@ -25,6 +25,12 @@ The classes ``std::map``, ``std::unordered_map`` and ``std::flat_map`` are excluded from this check, because for them the subscript operator has a defined behaviour when a key does not exist (inserting a new element). +This check enforces part of the `SL.con.3 +<https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slcon3-avoid-bounds-errors>` +guideline and is part of the `Bounds Safety (Bounds 4) +<https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Pro-bounds-arrayindex>` profile +profile from the C++ Core Guidelines. + Options ------- @@ -32,9 +38,3 @@ Options Semicolon-delimited list of class names that should additionally be excluded from this check. By default empty. - -This check enforces part of the `SL.con.3 -<https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slcon3-avoid-bounds-errors>` -guideline and is part of the `Bounds Safety (Bounds 4) -<https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Pro-bounds-arrayindex>` profile -profile from the C++ Core Guidelines. From 93a504adc278e01ed44698286ad275ed538d153a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Heidekr=C3=BCger?= <paul.heidekrue...@tum.de> Date: Fri, 21 Jun 2024 11:09:05 +0200 Subject: [PATCH 22/23] Rename variables related to class exclusion mechanism Co-authored-by: Manuel Pietsch <manuelpiet...@outlook.de> --- ...oBoundsAvoidUncheckedContainerAccesses.cpp | 26 ++++++++++--------- ...ProBoundsAvoidUncheckedContainerAccesses.h | 2 +- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsAvoidUncheckedContainerAccesses.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsAvoidUncheckedContainerAccesses.cpp index 19a52d0e565fa..b68583a77ac7f 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsAvoidUncheckedContainerAccesses.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsAvoidUncheckedContainerAccesses.cpp @@ -17,7 +17,7 @@ using namespace clang::ast_matchers; namespace clang::tidy::cppcoreguidelines { -static constexpr std::array<llvm::StringRef, 3> DefaultExclusions = { +static constexpr std::array<llvm::StringRef, 3> SubscriptDefaultExclusions = { llvm::StringRef("::std::map"), llvm::StringRef("::std::unordered_map"), llvm::StringRef("::std::flat_map")}; @@ -26,29 +26,30 @@ ProBoundsAvoidUncheckedContainerAccesses:: ClangTidyContext *Context) : ClangTidyCheck(Name, Context) { - ExcludedClasses = clang::tidy::utils::options::parseStringList( + SubscriptExcludedClasses = clang::tidy::utils::options::parseStringList( Options.get("ExcludeClasses", "")); - ExcludedClasses.insert(ExcludedClasses.end(), DefaultExclusions.begin(), - DefaultExclusions.end()); + SubscriptExcludedClasses.insert(SubscriptExcludedClasses.end(), + SubscriptDefaultExclusions.begin(), + SubscriptDefaultExclusions.end()); } void ProBoundsAvoidUncheckedContainerAccesses::storeOptions( ClangTidyOptions::OptionMap &Opts) { - if (ExcludedClasses.size() == DefaultExclusions.size()) { + if (SubscriptExcludedClasses.size() == SubscriptDefaultExclusions.size()) { Options.store(Opts, "ExcludeClasses", ""); return; } // Sum up the sizes of the defaults ( + semicolons), so we can remove them // from the saved options - size_t DefaultsStringLength = - std::transform_reduce(DefaultExclusions.begin(), DefaultExclusions.end(), - DefaultExclusions.size(), std::plus<>(), - [](llvm::StringRef Name) { return Name.size(); }); + size_t DefaultsStringLength = std::transform_reduce( + SubscriptDefaultExclusions.begin(), SubscriptDefaultExclusions.end(), + SubscriptDefaultExclusions.size(), std::plus<>(), + [](llvm::StringRef Name) { return Name.size(); }); - std::string Serialized = - clang::tidy::utils::options::serializeStringList(ExcludedClasses); + std::string Serialized = clang::tidy::utils::options::serializeStringList( + SubscriptExcludedClasses); Options.store(Opts, "ExcludeClasses", Serialized.substr(0, Serialized.size() - DefaultsStringLength)); @@ -94,7 +95,8 @@ void ProBoundsAvoidUncheckedContainerAccesses::registerMatchers( cxxMethodDecl(hasOverloadedOperatorName("[]")).bind("operator")), callee(cxxMethodDecl( ofClass(cxxRecordDecl(hasMethod(hasName("at"))).bind("parent")), - unless(matchers::matchesAnyListedName(ExcludedClasses))))) + unless( + matchers::matchesAnyListedName(SubscriptExcludedClasses))))) .bind("caller"), this); } diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsAvoidUncheckedContainerAccesses.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsAvoidUncheckedContainerAccesses.h index 8fdfd5e9eb518..3776869e6d42a 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsAvoidUncheckedContainerAccesses.h +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsAvoidUncheckedContainerAccesses.h @@ -32,7 +32,7 @@ class ProBoundsAvoidUncheckedContainerAccesses : public ClangTidyCheck { private: // A list of class names that are excluded from the warning - std::vector<llvm::StringRef> ExcludedClasses; + std::vector<llvm::StringRef> SubscriptExcludedClasses; }; } // namespace clang::tidy::cppcoreguidelines From 5969c6381f7fd624d33010e849d94f5b62ea0925 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Heidekr=C3=BCger?= <paul.heidekrue...@tum.de> Date: Sun, 30 Jun 2024 14:57:07 +0200 Subject: [PATCH 23/23] Remove redundant nullptr cast Co-authored-by: Manuel Pietsch <manuelpiet...@outlook.de> --- .../ProBoundsAvoidUncheckedContainerAccesses.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsAvoidUncheckedContainerAccesses.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsAvoidUncheckedContainerAccesses.cpp index b68583a77ac7f..560ccf3c7ca86 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsAvoidUncheckedContainerAccesses.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsAvoidUncheckedContainerAccesses.cpp @@ -82,7 +82,7 @@ const CXXMethodDecl *findAlternative(const CXXRecordDecl *MatchedParent, return Method; } - return static_cast<CXXMethodDecl *>(nullptr); + return nullptr; } void ProBoundsAvoidUncheckedContainerAccesses::registerMatchers( _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
