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