llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tidy Author: Sebastian Wolf (sebwolf-de) <details> <summary>Changes</summary> The string based test to find out whether the check is applicable on the class is not ideal, but I did not find a more elegant way, yet. If there is more clang-query magic available, that I'm not aware of, I'm happy to adapt that. --- Full diff: https://github.com/llvm/llvm-project/pull/90043.diff 8 Files Affected: - (added) clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp (+81) - (added) clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h (+32) - (modified) clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt (+1) - (modified) clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp (+3) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5) - (added) clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst (+20) - (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+1) - (added) clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp (+66) ``````````diff diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp new file mode 100644 index 00000000000000..524c21b5bdb818 --- /dev/null +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp @@ -0,0 +1,81 @@ +//===--- AvoidBoundsErrorsCheck.cpp - clang-tidy --------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "AvoidBoundsErrorsCheck.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" + +#include <iostream> +using namespace clang::ast_matchers; + +namespace clang::tidy::cppcoreguidelines { + +bool isApplicable(const QualType &Type) { + const auto TypeStr = Type.getAsString(); + bool Result = false; + // Only check for containers in the std namespace + if (TypeStr.find("std::vector") != std::string::npos) { + Result = true; + } + if (TypeStr.find("std::array") != std::string::npos) { + Result = true; + } + if (TypeStr.find("std::deque") != std::string::npos) { + Result = true; + } + if (TypeStr.find("std::map") != std::string::npos) { + Result = true; + } + if (TypeStr.find("std::unordered_map") != std::string::npos) { + Result = true; + } + if (TypeStr.find("std::flat_map") != std::string::npos) { + Result = true; + } + // TODO Add std::span with C++26 + return Result; +} + +void AvoidBoundsErrorsCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + callExpr(callee(cxxMethodDecl(hasName("operator[]")).bind("f"))) + .bind("x"), + this); +} + +void AvoidBoundsErrorsCheck::check(const MatchFinder::MatchResult &Result) { + const ASTContext &Context = *Result.Context; + const SourceManager &Source = Context.getSourceManager(); + const auto *MatchedExpr = Result.Nodes.getNodeAs<CallExpr>("x"); + const auto *MatchedFunction = Result.Nodes.getNodeAs<CXXMethodDecl>("f"); + const auto Type = MatchedFunction->getThisType(); + if (!isApplicable(Type)) { + return; + } + + // Get original code. + const SourceLocation b(MatchedExpr->getBeginLoc()); + const SourceLocation e(MatchedExpr->getEndLoc()); + const std::string OriginalCode = + Lexer::getSourceText(CharSourceRange::getTokenRange(b, e), Source, + getLangOpts()) + .str(); + const auto Range = SourceRange(b, e); + + // Build replacement. + std::string NewCode = OriginalCode; + const auto BeginOpen = NewCode.find("["); + NewCode.replace(BeginOpen, 1, ".at("); + const auto BeginClose = NewCode.find("]"); + NewCode.replace(BeginClose, 1, ")"); + + diag(MatchedExpr->getBeginLoc(), "Do not use operator[], use at() instead.") + << FixItHint::CreateReplacement(Range, NewCode); +} + +} // namespace clang::tidy::cppcoreguidelines diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h new file mode 100644 index 00000000000000..f915729cd7bbee --- /dev/null +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h @@ -0,0 +1,32 @@ +//===--- AvoidBoundsErrorsCheck.h - clang-tidy ------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDBOUNDSERRORSCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDBOUNDSERRORSCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::cppcoreguidelines { + +/// Enforce CPP core guidelines SL.con.3 +/// +/// See +/// https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slcon3-avoid-bounds-errors +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.html +class AvoidBoundsErrorsCheck : public ClangTidyCheck { +public: + AvoidBoundsErrorsCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace clang::tidy::cppcoreguidelines + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDBOUNDSERRORSCHECK_H diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt index eb35bbc6a538fe..4972d20417928d 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt @@ -4,6 +4,7 @@ set(LLVM_LINK_COMPONENTS ) add_clang_library(clangTidyCppCoreGuidelinesModule + AvoidBoundsErrorsCheck.cpp AvoidCapturingLambdaCoroutinesCheck.cpp AvoidConstOrRefDataMembersCheck.cpp AvoidDoWhileCheck.cpp diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp index e9f0201615616f..525bbc7a42adaa 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp @@ -19,6 +19,7 @@ #include "../performance/NoexceptMoveConstructorCheck.h" #include "../performance/NoexceptSwapCheck.h" #include "../readability/MagicNumbersCheck.h" +#include "AvoidBoundsErrorsCheck.h" #include "AvoidCapturingLambdaCoroutinesCheck.h" #include "AvoidConstOrRefDataMembersCheck.h" #include "AvoidDoWhileCheck.h" @@ -57,6 +58,8 @@ namespace cppcoreguidelines { class CppCoreGuidelinesModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck<AvoidBoundsErrorsCheck>( + "cppcoreguidelines-avoid-bounds-errors"); CheckFactories.registerCheck<AvoidCapturingLambdaCoroutinesCheck>( "cppcoreguidelines-avoid-capturing-lambda-coroutines"); CheckFactories.registerCheck<modernize::AvoidCArraysCheck>( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 5b1feffb89ea06..1949f18a586ed8 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -131,6 +131,11 @@ New checks to reading out-of-bounds data due to inadequate or incorrect string null termination. +- New :doc:`cppcoreguidelines-avoid-bounds-errors + <clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors>` check. + + Flags the unsafe `operator[]` and replaces it with `at()`. + - New :doc:`modernize-use-designated-initializers <clang-tidy/checks/modernize/use-designated-initializers>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst new file mode 100644 index 00000000000000..8fb2e3bfde0981 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst @@ -0,0 +1,20 @@ +.. title:: clang-tidy - cppcoreguidelines-avoid-bounds-errors + +cppcoreguidelines-avoid-bounds-errors +===================================== + +This check enforces the `SL.con.3 <https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slcon3-avoid-bounds-errors>` guideline. +It flags all uses of `operator[]` on `std::vector`, `std::array`, `std::deque`, `std::map`, `std::unordered_map`, and `std::flat_map` and suggests to replace it with `at()`. +Note that `std::span` and `std::mdspan` do not support `at()` as of C++23, so the use of `operator[]` is not flagged. + +For example the code + +.. code-block:: c++ + std::array<int, 3> a; + int b = a[4]; + +will be replaced by + +.. code-block:: c++ + std::vector<int, 3> a; + int b = a.at(4); diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 5d9d487f75f9cb..8b472fca2d8ca1 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -174,6 +174,7 @@ Clang-Tidy Checks :doc:`cert-oop58-cpp <cert/oop58-cpp>`, :doc:`concurrency-mt-unsafe <concurrency/mt-unsafe>`, :doc:`concurrency-thread-canceltype-asynchronous <concurrency/thread-canceltype-asynchronous>`, + :doc:`cppcoreguidelines-avoid-bounds-errors <cppcoreguidelines/avoid-bounds-errors>`, "Yes" :doc:`cppcoreguidelines-avoid-capturing-lambda-coroutines <cppcoreguidelines/avoid-capturing-lambda-coroutines>`, :doc:`cppcoreguidelines-avoid-const-or-ref-data-members <cppcoreguidelines/avoid-const-or-ref-data-members>`, :doc:`cppcoreguidelines-avoid-do-while <cppcoreguidelines/avoid-do-while>`, diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp new file mode 100644 index 00000000000000..23453b1f2df218 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp @@ -0,0 +1,66 @@ +namespace std { + template<typename T, unsigned size> + struct array { + T operator[](unsigned i) { + return T{1}; + } + T at(unsigned i) { + return T{1}; + } + }; + + template<typename T> + struct unique_ptr { + T operator[](unsigned i) { + return T{1}; + } + }; + + template<typename T> + struct span { + T operator[](unsigned i) { + return T{1}; + } + }; +} // namespace std + +namespace json { + template<typename T> + struct node{ + T operator[](unsigned i) { + return T{1}; + } + }; +} // namespace json + + +// RUN: %check_clang_tidy %s cppcoreguidelines-avoid-bounds-errors %t +std::array<int, 3> a; + +auto b = a[0]; +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: Do not use operator[], use at() instead. [cppcoreguidelines-avoid-bounds-errors] +// CHECK-FIXES: auto b = a.at(0); +auto c = a[1+1]; +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: Do not use operator[], use at() instead. [cppcoreguidelines-avoid-bounds-errors] +// CHECK-FIXES: auto c = a.at(1+1); +constexpr int index = 1; +auto d = a[index]; +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: Do not use operator[], use at() instead. [cppcoreguidelines-avoid-bounds-errors] +// CHECK-FIXES: auto d = a.at(index); + +int e(int index) { + return a[index]; +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: Do not use operator[], use at() instead. [cppcoreguidelines-avoid-bounds-errors] +// CHECK-FIXES: return a.at(index); +} + +auto f = a.at(0); + +std::unique_ptr<int> p; +auto q = p[0]; + +std::span<int> s; +auto t = s[0]; + +json::node<int> n; +auto m = n[0]; `````````` </details> https://github.com/llvm/llvm-project/pull/90043 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
