From 1a36fbf8c89b30b6c7c807b0da219d0b81553173 Mon Sep 17 00:00:00 2001 From: Giovanni Martins Date: Wed, 6 Dec 2023 11:26:53 -0300 Subject: [PATCH 01/15] replace memcpy with std::copy on clang-tidy removed typo on files sort imports removed some typo solve linter reports update modernize-replace-memcpy-with-stdcopy.rst --- .../clang-tidy/modernize/CMakeLists.txt | 1 + .../modernize/ModernizeTidyModule.cpp | 3 + .../modernize/ReplaceMemcpyWithStdCopy.cpp | 119 ++++++++++++++++++ .../modernize/ReplaceMemcpyWithStdCopy.h | 49 ++++++++ clang-tools-extra/docs/ReleaseNotes.rst | 5 + .../docs/clang-tidy/checks/list.rst | 1 + .../modernize-replace-memcpy-with-stdcopy.rst | 47 +++++++ 7 files changed, 225 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyWithStdCopy.cpp create mode 100644 clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyWithStdCopy.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/modernize/modernize-replace-memcpy-with-stdcopy.rst diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt index bab1167fb15ff..e67c583a5f173 100644 --- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt @@ -23,6 +23,7 @@ add_clang_library(clangTidyModernizeModule STATIC RedundantVoidArgCheck.cpp ReplaceAutoPtrCheck.cpp ReplaceDisallowCopyAndAssignMacroCheck.cpp + ReplaceMemcpyWithStdCopy.cpp ReplaceRandomShuffleCheck.cpp ReturnBracedInitListCheck.cpp ShrinkToFitCheck.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp index fc46c72982fdc..413b85c89ae4c 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -24,6 +24,7 @@ #include "RedundantVoidArgCheck.h" #include "ReplaceAutoPtrCheck.h" #include "ReplaceDisallowCopyAndAssignMacroCheck.h" +#include "ReplaceMemcpyWithStdCopy.h" #include "ReplaceRandomShuffleCheck.h" #include "ReturnBracedInitListCheck.h" #include "ShrinkToFitCheck.h" @@ -94,6 +95,8 @@ class ModernizeModule : public ClangTidyModule { "modernize-replace-auto-ptr"); CheckFactories.registerCheck( "modernize-replace-disallow-copy-and-assign-macro"); + CheckFactories.registerCheck( + "modernize-replace-memcpy-by-stdcopy"); CheckFactories.registerCheck( "modernize-replace-random-shuffle"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyWithStdCopy.cpp b/clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyWithStdCopy.cpp new file mode 100644 index 0000000000000..af6b365c16251 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyWithStdCopy.cpp @@ -0,0 +1,119 @@ +//===--- ReplaceMemcpyWithStdCopy.cpp - 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 +// +//===----------------------------------------------------------------------===// + +#include "ReplaceMemcpyWithStdCopy.h" +#include "../utils/OptionsUtils.h" +#include + +using namespace clang; +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace modernize { + +ReplaceMemcpyWithStdCopy::ReplaceMemcpyWithStdCopy(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + IncludeInserter(Options.getLocalOrGlobal("IncludeStyle", + utils::IncludeSorter::IS_LLVM)) { +} + +void ReplaceMemcpyWithStdCopy::registerMatchers(MatchFinder *Finder) { + assert(Finder != nullptr); + + if (!getLangOpts().CPlusPlus) + return; + + auto MemcpyMatcher = + callExpr(hasDeclaration(functionDecl(hasName("memcpy"), + isExpansionInSystemHeader())), + isExpansionInMainFile()) + .bind("memcpy_function"); + + Finder->addMatcher(MemcpyMatcher, this); +} + +void ReplaceMemcpyWithStdCopy::registerPPCallbacks( + const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { + if (!getLangOpts().CPlusPlus) + return; + + Inserter = + std::make_unique(SM, getLangOpts(), + IncludeStyle); + PP->addPPCallbacks(Inserter->CreatePPCallbacks()); +} + +void ReplaceMemcpyWithStdCopy::check(const MatchFinder::MatchResult &Result) { + const auto *MemcpyNode = Result.Nodes.getNodeAs("memcpy_function"); + assert(MemcpyNode != nullptr); + + DiagnosticBuilder Diag = + diag(MemcpyNode->getExprLoc(), "use std::copy instead of memcpy"); + + renameFunction(Diag, MemcpyNode); + reorderArgs(Diag, MemcpyNode); + insertHeader(Diag, MemcpyNode, Result.SourceManager); +} + +void ReplaceMemcpyWithStdCopy::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "IncludeStyle", + utils::IncludeSorter::toString(IncludeStyle)); +} + +void ReplaceMemcpyWithStdCopy::renameFunction(DiagnosticBuilder &Diag, + const CallExpr *MemcpyNode) { + const CharSourceRange FunctionNameSourceRange = CharSourceRange::getCharRange( + MemcpyNode->getBeginLoc(), MemcpyNode->getArg(0)->getBeginLoc()); + + Diag << FixItHint::CreateReplacement(FunctionNameSourceRange, "std::copy("); +} + +void ReplaceMemcpyWithStdCopy::reorderArgs(DiagnosticBuilder &Diag, + const CallExpr *MemcpyNode) { + std::array arg; + + LangOptions LangOpts; + LangOpts.CPlusPlus = true; + PrintingPolicy Policy(LangOpts); + + // Retrieve all the arguments + for (uint8_t i = 0; i < arg.size(); i++) { + llvm::raw_string_ostream s(arg[i]); + MemcpyNode->getArg(i)->printPretty(s, nullptr, Policy); + } + + // Create lambda that return SourceRange of an argument + auto getSourceRange = [MemcpyNode](uint8_t ArgCount) -> SourceRange { + return SourceRange(MemcpyNode->getArg(ArgCount)->getBeginLoc(), + MemcpyNode->getArg(ArgCount)->getEndLoc()); + }; + + // Reorder the arguments + Diag << FixItHint::CreateReplacement(getSourceRange(0), arg[1]); + + arg[2] = arg[1] + " + ((" + arg[2] + ") / sizeof(*(" + arg[1] + ")))"; + Diag << FixItHint::CreateReplacement(getSourceRange(1), arg[2]); + + Diag << FixItHint::CreateReplacement(getSourceRange(2), arg[0]); +} + +void ReplaceMemcpyWithStdCopy::insertHeader(DiagnosticBuilder &Diag, + const CallExpr *MemcpyNode, + SourceManager *const SM) { + Optional FixInclude = Inserter->CreateIncludeInsertion( + /*FileID=*/SM->getMainFileID(), /*Header=*/"algorithm", + /*IsAngled=*/true); + if (FixInclude) + Diag << *FixInclude; +} + +} // namespace modernize +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyWithStdCopy.h b/clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyWithStdCopy.h new file mode 100644 index 0000000000000..0f262bf839af2 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyWithStdCopy.h @@ -0,0 +1,49 @@ +//===--- ReplaceMemcpyWithStdCopy.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_MODERNIZE_REPLACE_MEMCPY_WITH_STDCOPY_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_WITH_STDCOPY_H + +#include "../ClangTidyCheck.h" +#include "../utils/IncludeInserter.h" +#include +#include +#include + +namespace clang { +namespace tidy { +namespace modernize { + +// Replace the C memcpy function with std::copy +class ReplaceMemcpyWithStdCopy : public ClangTidyCheck { +public: + ReplaceMemcpyWithStdCopy(StringRef Name, ClangTidyContext *Context); + ~ReplaceMemcpyWithStdCopy() override = default; + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap &Options) override; + +private: + void renameFunction(DiagnosticBuilder &Diag, const CallExpr *MemcpyNode); + void reorderArgs(DiagnosticBuilder &Diag, const CallExpr *MemcpyNode); + void insertHeader(DiagnosticBuilder &Diag, const CallExpr *MemcpyNode, + SourceManager *const SM); + +private: + std::unique_ptr Inserter; + utils::IncludeInserter IncludeInserter; + const utils::IncludeSorter::IncludeStyle IncludeStyle; +}; + +} // namespace modernize +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_WITH_STDCOPY_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 35cb3e387e4e6..552044356e404 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -319,6 +319,11 @@ Changes in existing checks member function calls too and to only expand macros starting with ``PRI`` and ``__PRI`` from ```` in the format string. +- New :doc:`modernize-replace-memcpy-with-stdcopy + ` check. + + Replaces all occurrences of the C ``memcpy`` function by ``std::copy``. + - Improved :doc:`modernize-use-std-print ` check to support replacing member function calls too and to only expand macros starting with ``PRI`` diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index e8f9b4e829634..8af5fc8fa3cfa 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -287,6 +287,7 @@ Clang-Tidy Checks :doc:`modernize-raw-string-literal `, "Yes" :doc:`modernize-redundant-void-arg `, "Yes" :doc:`modernize-replace-auto-ptr `, "Yes" + :doc:`modernize-replace-memcpy-with-std-copy `, "Yes" :doc:`modernize-replace-disallow-copy-and-assign-macro `, "Yes" :doc:`modernize-replace-random-shuffle `, "Yes" :doc:`modernize-return-braced-init-list `, "Yes" diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/modernize-replace-memcpy-with-stdcopy.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/modernize-replace-memcpy-with-stdcopy.rst new file mode 100644 index 0000000000000..922a7f36e7e07 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/modernize-replace-memcpy-with-stdcopy.rst @@ -0,0 +1,47 @@ +.. title:: clang-tidy - modernize-replace-memcpy-with-stdcopy + +modernize-replace-memcpy-with-stdcopy +=================================== + +Replaces all occurrences of the C ``memcpy`` function with ``std::copy`` + +Example: + +.. code-block:: c++ + + /*! + * \param destination Pointer to the destination array where the content is to be copied + * \param source Pointer to the source of data to be copied + * \param num Number of bytes to copy + */ + memcpy(destination, source, num); + +becomes + +.. code-block:: c++ + + /*! + * \param destination Pointer to the destination array where the content is to be copied + * \param source Pointer to the source of data to be copied + * \param num Number of bytes to copy + */ + std::copy(source, source + (num / sizeof *source), destination); + +Bytes to iterator conversion +---------------------------- + +Unlike ``std::copy`` that take an iterator on the last element of the source array, ``memcpy`` request the number of bytes to copy. +In order to make the check working, it will convert the size parameter to an iterator by replacing it by ``source + (num / sizeof *source)`` + +Header inclusion +---------------- + +``std::copy`` being provided by the ``algorithm`` header file, this check will include it if needed. + +Options +------- + +.. option:: IncludeStyle + + A string specifying which include-style is used, `llvm` or `google`. Default + is `llvm`. From 3cb944c4989b135d1d2c3f488174dcd4f338c80a Mon Sep 17 00:00:00 2001 From: kidp330 Date: Sat, 19 Oct 2024 16:49:27 +0200 Subject: [PATCH 02/15] Add unit tests --- .../replace-memcpy-with-std-copy.cpp | 146 ++++++++++++++++++ 1 file changed, 146 insertions(+) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/replace-memcpy-with-std-copy.cpp diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-memcpy-with-std-copy.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-memcpy-with-std-copy.cpp new file mode 100644 index 0000000000000..ce07fc5801da6 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-memcpy-with-std-copy.cpp @@ -0,0 +1,146 @@ +// RUN: %check_clang_tidy %s modernize-replace-memcpy-with-std-copy %t + +// CHECK-FIXES: #include + +namespace { + +using size_t = decltype(sizeof(int)); + +namespace std { +typedef long long int64_t; +typedef short int16_t; +typedef char int8_t; + +void *memcpy(void *__restrict dest, const void *__restrict src, size_t n); + +template struct vector { + vector(size_t); + + T *data(); + size_t size() const; + void resize(size_t); + using value_type = T; +}; + +size_t size(void *); + +size_t strlen(const char *); +} // namespace std + +void *memcpy(void *__restrict dest, const void *__restrict src, size_t n); +} // namespace + +void notSupportedEx() { + char source[] = "once upon a daydream...", dest[4]; + + auto *primitiveDest = new std::int8_t; + std::memcpy(primitiveDest, source, sizeof primitiveDest); + + auto *primitiveDest2 = new std::int16_t; + std::memcpy(primitiveDest2, source, sizeof primitiveDest); + std::memcpy(primitiveDest2, source, sizeof primitiveDest2); + + double d = 0.1; + std::int64_t n; + // don't warn on calls over non-sequences + std::memcpy(&n, &d, sizeof d); + + // object creation in destination buffer + struct S { + int x{42}; + void *operator new(size_t, void *) noexcept { return nullptr; } + } s; + alignas(S) char buf[sizeof(S)]; + S *ps = new (buf) S; // placement new + // // don't warn on calls over non-sequences + std::memcpy(ps, &s, sizeof s); + + const char *pSource = "once upon a daydream..."; + char *pDest = new char[4]; + std::memcpy(dest, pSource, sizeof dest); + std::memcpy(pDest, source, 4); +} + +void noFixItEx() { + char source[] = "once upon a daydream...", dest[4]; + + // no FixIt when return value is used + auto *ptr = std::memcpy(dest, source, sizeof dest); + // CHECK-MESSAGES: [[@LINE-1]]:20: warning: prefer std::copy_n to memcpy + + std::vector vec_i16(4); + // not a supported type, should be a sequence of bytes, otherwise it is difficult to compute the n in copy_n + std::memcpy(vec_i16.data(), source, + vec_i16.size() * sizeof(decltype(vec_i16)::value_type)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy +} + +void sequenceOfBytesEx() { + // the check should support memcpy conversion for the following types: + // T[] + // std::vector + // std::span + // std::deque + // std::array + // std::string + // std::string_view + // where T is byte-like + + char source[] = "once upon a daydream...", dest[4]; + std::memcpy(dest, source, sizeof dest); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy + // CHECK-FIXES: std::copy_n(std::begin(source), std::size(dest), std::begin(dest)); + + // __jm__ warn on global call as well + memcpy(dest, source, sizeof dest); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: prefer std::copy_n to memcpy + // CHECK-FIXES: std::copy_n(std::begin(source), std::size(dest), std::begin(dest)); + + std::vector vec_i8(4); + std::memcpy(vec_i8.data(), source, vec_i8.size()); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy + // CHECK-FIXES: std::copy_n(std::begin(source), std::size(vec_i8), std::begin(vec_i8)); + + // __jm__ make configurable whether stl containers should use members or free fns. + // __jm__ for now use free fns. only + + std::memcpy(dest, vec_i8.data(), vec_i8.size()); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy + // CHECK-FIXES: std::copy_n(std::begin(vec_i8), std::size(vec_i8), std::begin(dest)); + std::memcpy(dest, vec_i8.data(), sizeof(dest)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy + // CHECK-FIXES: std::copy_n(vec_i8.data(), std::size(dest), std::begin(dest)); + std::memcpy(dest, vec_i8.data(), std::size(dest)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy + // CHECK-FIXES: std::copy_n(std::begin(source), std::size(vec_i8), std::begin(vec_i8)); + + std::memcpy(dest, vec_i8.data(), 1 + vec_i8.size() / 2); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy + // CHECK-FIXES: std::copy_n(std::begin(vec_i8), 1 + std::size(vec_i8) / 2, std::begin(dest)); + std::memcpy(dest, vec_i8.data(), 1 + sizeof(dest) / 2); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy + // CHECK-FIXES: std::copy_n(vec_i8.data(), 1 + std::size(dest) / 2, std::begin(dest)); + std::memcpy(dest, vec_i8.data(), 1 + std::size(dest) / 2); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy + // CHECK-FIXES: std::copy_n(std::begin(source), 1 + std::size(dest) / 2, std::begin(vec_i8)); +} + +// void uninitialized_copy_ex() { +// std::vector v = {"This", "is", "an", "example"}; + +// std::string* p; +// std::size_t sz; +// std::tie(p, sz) = std::get_temporary_buffer(v.size()); +// sz = std::min(sz, v.size()); + +// std::uninitialized_copy_n(v.begin(), sz, p); + +// for (std::string* i = p; i != p + sz; ++i) +// { +// std::cout << *i << ' '; +// i->~basic_string(); +// } +// std::cout << '\n'; + +// std::return_temporary_buffer(p); +// } \ No newline at end of file From 416739af6767f40693e78c9af029851c3bbfbea1 Mon Sep 17 00:00:00 2001 From: kidp330 Date: Mon, 23 Dec 2024 22:02:46 +0100 Subject: [PATCH 03/15] Implement dest/src arg matchers (commit serves as proof of work for course credit :)) ) [skip ci] --- .../clang-tidy/modernize/CMakeLists.txt | 2 +- .../modernize/ModernizeTidyModule.cpp | 12 +- .../modernize/ReplaceMemcpyWithStdCopy.cpp | 119 ------ .../modernize/ReplaceMemcpyWithStdCopy.h | 49 --- .../modernize/ReplaceWithStdCopyCheck.cpp | 393 ++++++++++++++++++ .../modernize/ReplaceWithStdCopyCheck.h | 82 ++++ clang-tools-extra/docs/ReleaseNotes.rst | 4 +- .../docs/clang-tidy/checks/list.rst | 2 +- .../modernize-replace-memcpy-with-stdcopy.rst | 4 +- .../modernize/replace-memcpy-with-stdcopy.rst | 47 +++ .../replace-memcpy-with-std-copy.cpp | 146 ------- .../modernize/replace-with-std-copy.cpp | 320 ++++++++++++++ 12 files changed, 854 insertions(+), 326 deletions(-) delete mode 100644 clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyWithStdCopy.cpp delete mode 100644 clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyWithStdCopy.h create mode 100644 clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/modernize/replace-memcpy-with-stdcopy.rst delete mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/replace-memcpy-with-std-copy.cpp create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/replace-with-std-copy.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt index e67c583a5f173..67eaf01eb9155 100644 --- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt @@ -23,7 +23,7 @@ add_clang_library(clangTidyModernizeModule STATIC RedundantVoidArgCheck.cpp ReplaceAutoPtrCheck.cpp ReplaceDisallowCopyAndAssignMacroCheck.cpp - ReplaceMemcpyWithStdCopy.cpp + ReplaceWithStdCopyCheck.cpp ReplaceRandomShuffleCheck.cpp ReturnBracedInitListCheck.cpp ShrinkToFitCheck.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp index 413b85c89ae4c..0c2c351cc71eb 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -24,8 +24,8 @@ #include "RedundantVoidArgCheck.h" #include "ReplaceAutoPtrCheck.h" #include "ReplaceDisallowCopyAndAssignMacroCheck.h" -#include "ReplaceMemcpyWithStdCopy.h" #include "ReplaceRandomShuffleCheck.h" +#include "ReplaceWithStdCopyCheck.h" #include "ReturnBracedInitListCheck.h" #include "ShrinkToFitCheck.h" #include "TypeTraitsCheck.h" @@ -95,8 +95,8 @@ class ModernizeModule : public ClangTidyModule { "modernize-replace-auto-ptr"); CheckFactories.registerCheck( "modernize-replace-disallow-copy-and-assign-macro"); - CheckFactories.registerCheck( - "modernize-replace-memcpy-by-stdcopy"); + CheckFactories.registerCheck( + "modernize-replace-with-stdcopy"); CheckFactories.registerCheck( "modernize-replace-random-shuffle"); CheckFactories.registerCheck( @@ -113,11 +113,11 @@ class ModernizeModule : public ClangTidyModule { CheckFactories.registerCheck( "modernize-use-default-member-init"); CheckFactories.registerCheck("modernize-use-emplace"); - CheckFactories.registerCheck("modernize-use-equals-default"); + CheckFactories.registerCheck( + "modernize-use-equals-default"); CheckFactories.registerCheck( "modernize-use-equals-delete"); - CheckFactories.registerCheck( - "modernize-use-nodiscard"); + CheckFactories.registerCheck("modernize-use-nodiscard"); CheckFactories.registerCheck("modernize-use-noexcept"); CheckFactories.registerCheck("modernize-use-nullptr"); CheckFactories.registerCheck("modernize-use-override"); diff --git a/clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyWithStdCopy.cpp b/clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyWithStdCopy.cpp deleted file mode 100644 index af6b365c16251..0000000000000 --- a/clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyWithStdCopy.cpp +++ /dev/null @@ -1,119 +0,0 @@ -//===--- ReplaceMemcpyWithStdCopy.cpp - 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 -// -//===----------------------------------------------------------------------===// - -#include "ReplaceMemcpyWithStdCopy.h" -#include "../utils/OptionsUtils.h" -#include - -using namespace clang; -using namespace clang::ast_matchers; - -namespace clang { -namespace tidy { -namespace modernize { - -ReplaceMemcpyWithStdCopy::ReplaceMemcpyWithStdCopy(StringRef Name, - ClangTidyContext *Context) - : ClangTidyCheck(Name, Context), - IncludeInserter(Options.getLocalOrGlobal("IncludeStyle", - utils::IncludeSorter::IS_LLVM)) { -} - -void ReplaceMemcpyWithStdCopy::registerMatchers(MatchFinder *Finder) { - assert(Finder != nullptr); - - if (!getLangOpts().CPlusPlus) - return; - - auto MemcpyMatcher = - callExpr(hasDeclaration(functionDecl(hasName("memcpy"), - isExpansionInSystemHeader())), - isExpansionInMainFile()) - .bind("memcpy_function"); - - Finder->addMatcher(MemcpyMatcher, this); -} - -void ReplaceMemcpyWithStdCopy::registerPPCallbacks( - const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { - if (!getLangOpts().CPlusPlus) - return; - - Inserter = - std::make_unique(SM, getLangOpts(), - IncludeStyle); - PP->addPPCallbacks(Inserter->CreatePPCallbacks()); -} - -void ReplaceMemcpyWithStdCopy::check(const MatchFinder::MatchResult &Result) { - const auto *MemcpyNode = Result.Nodes.getNodeAs("memcpy_function"); - assert(MemcpyNode != nullptr); - - DiagnosticBuilder Diag = - diag(MemcpyNode->getExprLoc(), "use std::copy instead of memcpy"); - - renameFunction(Diag, MemcpyNode); - reorderArgs(Diag, MemcpyNode); - insertHeader(Diag, MemcpyNode, Result.SourceManager); -} - -void ReplaceMemcpyWithStdCopy::storeOptions(ClangTidyOptions::OptionMap &Opts) { - Options.store(Opts, "IncludeStyle", - utils::IncludeSorter::toString(IncludeStyle)); -} - -void ReplaceMemcpyWithStdCopy::renameFunction(DiagnosticBuilder &Diag, - const CallExpr *MemcpyNode) { - const CharSourceRange FunctionNameSourceRange = CharSourceRange::getCharRange( - MemcpyNode->getBeginLoc(), MemcpyNode->getArg(0)->getBeginLoc()); - - Diag << FixItHint::CreateReplacement(FunctionNameSourceRange, "std::copy("); -} - -void ReplaceMemcpyWithStdCopy::reorderArgs(DiagnosticBuilder &Diag, - const CallExpr *MemcpyNode) { - std::array arg; - - LangOptions LangOpts; - LangOpts.CPlusPlus = true; - PrintingPolicy Policy(LangOpts); - - // Retrieve all the arguments - for (uint8_t i = 0; i < arg.size(); i++) { - llvm::raw_string_ostream s(arg[i]); - MemcpyNode->getArg(i)->printPretty(s, nullptr, Policy); - } - - // Create lambda that return SourceRange of an argument - auto getSourceRange = [MemcpyNode](uint8_t ArgCount) -> SourceRange { - return SourceRange(MemcpyNode->getArg(ArgCount)->getBeginLoc(), - MemcpyNode->getArg(ArgCount)->getEndLoc()); - }; - - // Reorder the arguments - Diag << FixItHint::CreateReplacement(getSourceRange(0), arg[1]); - - arg[2] = arg[1] + " + ((" + arg[2] + ") / sizeof(*(" + arg[1] + ")))"; - Diag << FixItHint::CreateReplacement(getSourceRange(1), arg[2]); - - Diag << FixItHint::CreateReplacement(getSourceRange(2), arg[0]); -} - -void ReplaceMemcpyWithStdCopy::insertHeader(DiagnosticBuilder &Diag, - const CallExpr *MemcpyNode, - SourceManager *const SM) { - Optional FixInclude = Inserter->CreateIncludeInsertion( - /*FileID=*/SM->getMainFileID(), /*Header=*/"algorithm", - /*IsAngled=*/true); - if (FixInclude) - Diag << *FixInclude; -} - -} // namespace modernize -} // namespace tidy -} // namespace clang diff --git a/clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyWithStdCopy.h b/clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyWithStdCopy.h deleted file mode 100644 index 0f262bf839af2..0000000000000 --- a/clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyWithStdCopy.h +++ /dev/null @@ -1,49 +0,0 @@ -//===--- ReplaceMemcpyWithStdCopy.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_MODERNIZE_REPLACE_MEMCPY_WITH_STDCOPY_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_WITH_STDCOPY_H - -#include "../ClangTidyCheck.h" -#include "../utils/IncludeInserter.h" -#include -#include -#include - -namespace clang { -namespace tidy { -namespace modernize { - -// Replace the C memcpy function with std::copy -class ReplaceMemcpyWithStdCopy : public ClangTidyCheck { -public: - ReplaceMemcpyWithStdCopy(StringRef Name, ClangTidyContext *Context); - ~ReplaceMemcpyWithStdCopy() override = default; - void registerMatchers(ast_matchers::MatchFinder *Finder) override; - void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, - Preprocessor *ModuleExpanderPP) override; - void check(const ast_matchers::MatchFinder::MatchResult &Result) override; - void storeOptions(ClangTidyOptions::OptionMap &Options) override; - -private: - void renameFunction(DiagnosticBuilder &Diag, const CallExpr *MemcpyNode); - void reorderArgs(DiagnosticBuilder &Diag, const CallExpr *MemcpyNode); - void insertHeader(DiagnosticBuilder &Diag, const CallExpr *MemcpyNode, - SourceManager *const SM); - -private: - std::unique_ptr Inserter; - utils::IncludeInserter IncludeInserter; - const utils::IncludeSorter::IncludeStyle IncludeStyle; -}; - -} // namespace modernize -} // namespace tidy -} // namespace clang - -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_WITH_STDCOPY_H diff --git a/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.cpp b/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.cpp new file mode 100644 index 0000000000000..beba705f5a141 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.cpp @@ -0,0 +1,393 @@ +//===--- ReplaceWithStdCopyCheck.cpp - 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 +// +//===----------------------------------------------------------------------===// + +#include "ReplaceWithStdCopyCheck.h" +#include "clang/AST/DeclTemplate.h" +#include "clang/AST/Type.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "llvm/ADT/StringRef.h" +#include + +using namespace clang; +using namespace clang::ast_matchers; + +namespace clang::tidy { + +template <> +struct OptionEnumMapping< + enum modernize::ReplaceWithStdCopyCheck::FlaggableCallees> { + static llvm::ArrayRef> + getEnumMapping() { + static constexpr std::pair< + modernize::ReplaceWithStdCopyCheck::FlaggableCallees, StringRef> + Mapping[] = { + {modernize::ReplaceWithStdCopyCheck::FlaggableCallees:: + MemmoveAndMemcpy, + "MemmoveAndMemcpy"}, + {modernize::ReplaceWithStdCopyCheck::FlaggableCallees::OnlyMemmove, + "OnlyMemmove"}, + }; + return {Mapping}; + } +}; + +template <> +struct OptionEnumMapping< + enum modernize::ReplaceWithStdCopyCheck::StdCallsReplacementStyle> { + static llvm::ArrayRef> + getEnumMapping() { + static constexpr std::pair< + modernize::ReplaceWithStdCopyCheck::StdCallsReplacementStyle, StringRef> + Mapping[] = { + {modernize::ReplaceWithStdCopyCheck::StdCallsReplacementStyle:: + FreeFunc, + "FreeFunc"}, + {modernize::ReplaceWithStdCopyCheck::StdCallsReplacementStyle:: + MemberCall, + "MemberCall"}, + }; + return {Mapping}; + } +}; + +namespace modernize { +namespace { +namespace arg { +namespace tag { +struct VariantPtrArgRef { + llvm::StringLiteral AsContainer; + llvm::StringLiteral AsCArray; + llvm::StringLiteral Fallback; +}; + +struct PtrArgRefs { + VariantPtrArgRef VariantRefs; + llvm::StringLiteral CharType; +}; + +struct DestArg { + static constexpr PtrArgRefs Refs = { + {"DestArg::AsContainer", "DestArg::AsCArray", "DestArg::Fallback"}, + "DestArg::CharType"}; +}; +struct SourceArg { + static constexpr PtrArgRefs Refs = { + {"SourceArg::AsContainer", "SourceArg::AsCArray", "SourceArg::Fallback"}, + "SourceArg::CharType" + + }; +}; +struct SizeArg {}; +} // namespace tag + +template auto createPtrArgMatcher(tag::PtrArgRefs Refs) { + auto [VariantRefs, ValueTypeRef] = Refs; + auto [AsContainer, AsCArray] = VariantRefs; + + auto AllowedContainerNamesM = []() { + if constexpr (IsConst) { + return hasAnyName("::std::deque", "::std::forward_list", "::std::list", + "::std::vector", "::std::basic_string"); + } else { + return hasAnyName("::std::deque", "::std::forward_list", "::std::list", + "::std::vector", "::std::basic_string", + "::std::basic_string_view", "::std::span"); + } + }(); + + auto ValueTypeM = type().bind(ValueTypeRef); + + auto AllowedContainerTypeM = hasUnqualifiedDesugaredType( + recordType(hasDeclaration(recordDecl(classTemplateSpecializationDecl( + AllowedContainerNamesM, + hasTemplateArgument( + 0, templateArgument(refersToType( + hasUnqualifiedDesugaredType(ValueTypeM))))))))); + + auto VariantContainerM = + expr(hasType(AllowedContainerTypeM)).bind(AsContainer); + auto VariantCArrayM = + expr(hasType(arrayType(hasElementType(ValueTypeM)))).bind(AsCArray); + + auto StdDataReturnM = returns(pointerType(pointee( + hasUnqualifiedDesugaredType(equalsBoundNode(ValueTypeRef.str()))))); + + auto StdDataMemberDeclM = + cxxMethodDecl(hasName("data"), parameterCountIs(0), StdDataReturnM); + // ,unless(isConst()) // __jm__ ONLY difference between Dest + // and Source calls, but maybe don't include it? + + auto StdDataFreeDeclM = functionDecl( + hasName("::std::data"), parameterCountIs(1), + StdDataReturnM); // __jm__ possibly elaborate on argument type here? + + auto StdDataMemberCallM = cxxMemberCallExpr( + callee(StdDataMemberDeclM), argumentCountIs(0), + on(expr(hasType(AllowedContainerTypeM)).bind(AsContainer))); + + auto ArrayOrContainerM = expr(anyOf(VariantCArrayM, VariantContainerM)); + + auto StdDataFreeCallM = callExpr(callee(StdDataFreeDeclM), argumentCountIs(1), + hasArgument(0, ArrayOrContainerM)); + + return expr(anyOf(StdDataMemberCallM, StdDataFreeCallM, VariantCArrayM)); +} + +template class MatcherLinker : public TAG { +public: + static auto createMatcher(); + static void handleResult(const MatchFinder::MatchResult &Result); + +private: + using AllowedTAG = std::enable_if_t || + std::is_same_v || + std::is_same_v>; +}; + +template <> auto MatcherLinker::createMatcher() { + return createPtrArgMatcher(Refs); +} + +template <> auto MatcherLinker::createMatcher() { + return createPtrArgMatcher(Refs); +} + +template <> auto MatcherLinker::createMatcher() { + // cases: + // 1. call to std::size + // 2. member call to .size() + // 3. sizeof node or sizeof(node) (c-array) + // 4. N * sizeof(type) (hints at sequence-related use, however none singled out) + + // need a robust way of exchanging information between matchers that will help in sequence identification + // I see one possiblity + // size "thinks" dest is rawptr with pointee sequence + // N * sizeof + // size "thinks" src is rawptr with pointee sequence + // size agrees dest is container/c-array + // size agrees src is container/c-array + + return anything(); // __jm__ TODO +} + +using Dest = MatcherLinker; +using Source = MatcherLinker; +using Size = MatcherLinker; +} // namespace arg + +constexpr llvm::StringLiteral ExpressionRef = "::std::memcpy"; +constexpr llvm::StringLiteral StdCopyHeader = ""; +constexpr llvm::StringLiteral ReturnValueDiscardedRef = + "ReturnValueDiscardedRef"; + +// Helper Matcher which applies the given QualType Matcher either directly or by +// resolving a pointer type to its pointee. Used to match v.push_back() as well +// as p->push_back(). +auto hasTypeOrPointeeType( + const ast_matchers::internal::Matcher &TypeMatcher) { + return anyOf(hasType(TypeMatcher), + hasType(pointerType(pointee(TypeMatcher)))); +} + +} // namespace + +ReplaceWithStdCopyCheck::ReplaceWithStdCopyCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + Inserter(Options.getLocalOrGlobal("IncludeStyle", + utils::IncludeSorter::IS_LLVM), + areDiagsSelfContained()), + FlaggableCallees_(Options.getLocalOrGlobal("FlaggableCallees", + FlaggableCalleesDefault)), + StdCallsReplacementStyle_(Options.getLocalOrGlobal( + "StdCallsReplacementStyle", StdCallsReplacementStyleDefault)) {} + +void ReplaceWithStdCopyCheck::registerMatchers(MatchFinder *Finder) { + const auto ReturnValueUsedM = + hasParent(compoundStmt().bind(ReturnValueDiscardedRef)); + + const auto OffendingDeclM = + functionDecl(parameterCountIs(3), hasAnyName(getFlaggableCallees())); + + // cases: + // 1. One of the arguments is definitely a sequence and the other a pointer - + // match + // 2. Both source and dest are pointers, but size is of the form ((N := + // expr()) * sizeof(bytelike())) - match (false positive if N \in {0, 1}) + + using namespace arg; + const auto Expression = + callExpr(callee(OffendingDeclM), + anyOf(optionally(ReturnValueUsedM), + allOf(hasArgument(0, Dest::createMatcher()), + hasArgument(1, Source::createMatcher()), + hasArgument(2, Size::createMatcher())))); + Finder->addMatcher(Expression.bind(ExpressionRef), this); +} + +void ReplaceWithStdCopyCheck::registerPPCallbacks( + const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { + Inserter.registerPreprocessor(PP); +} + +void ReplaceWithStdCopyCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "IncludeStyle", Inserter.getStyle()); + Options.store(Opts, "FlaggableCallees", FlaggableCallees_); + Options.store(Opts, "StdCallsReplacementStyle", StdCallsReplacementStyle_); +} + +#include "llvm/Support/Debug.h" +#define DEBUG_TYPE "clang-tidy" + +void ReplaceWithStdCopyCheck::check(const MatchFinder::MatchResult &Result) { + const auto &CallNode = *Result.Nodes.getNodeAs(ExpressionRef); + + auto Diag = diag(CallNode.getExprLoc(), "prefer std::copy_n to %0") + << cast(CallNode.getCalleeDecl()); + + tryIssueFixIt(Result, Diag, CallNode); + + { + // using namespace std::literals; + // Diag << FixItHint::CreateReplacement( + // DestArg->getSourceRange(), + // ("std::begin(" + + // tooling::fixit::getText(SrcArg->getSourceRange(), *Result.Context) + + // ")") + // .str()); + // Diag << FixItHint::CreateReplacement( + // SrcArg->getSourceRange(), + // tooling::fixit::getText(SizeArg->getSourceRange(), *Result.Context)); + // Diag << FixItHint::CreateReplacement( + // SizeArg->getSourceRange(), + // ("std::begin(" + + // tooling::fixit::getText(DestArg->getSourceRange(), *Result.Context) + // + + // ")") + // .str()); + } + // dest source size -> source size dest +} + +// void ReplaceWithStdCopyCheck::handleMemcpy(const CallExpr &Node) { + +// // FixIt +// const CharSourceRange FunctionNameSourceRange = +// CharSourceRange::getCharRange( +// Node.getBeginLoc(), Node.getArg(0)->getBeginLoc()); + +// Diag << FixItHint::CreateReplacement(FunctionNameSourceRange, +// "std::copy_n("); +// } + +// bool ReplaceWithStdCopyCheck::fixItPossible( +// const MatchFinder::MatchResult &Result) {} + +void ReplaceWithStdCopyCheck::tryIssueFixIt( + const MatchFinder::MatchResult &Result, const DiagnosticBuilder &Diag, + const CallExpr &CallNode) { + // don't issue a fixit if the result of the call is used + if (bool IsReturnValueUsed = + Result.Nodes.getNodeAs(ReturnValueDiscardedRef) == nullptr; + IsReturnValueUsed) + return; + + // to make sure we can issue a FixIt, need to be pretty sure we're dealing + // with a sequence and it is a byte sequence + // 1. For now we are sure both source and dest are sequences, but not + // necessarily of bytes + // 2. We can relax conditions to where just one arg is a sequence, and the + // other can then be a sequence or raw pointer + // 3. when both dest and source are pointers (no expectations on the argument + // as everything required is enforced by the type system) we can use a + // heuristic using the form of the 3rd argument expression + // + + // delete the memmove/memcpy + // insert an std::copy_n + struct CArrayTag {}; + struct ContainerTag {}; + struct RawPtrTag {}; + using PtrArgVariant = std::variant; + struct PtrArg { + PtrArgVariant Tag; + const Expr *Node; + }; + + const auto MakePtrArg = [&](arg::tag::VariantPtrArgRef Refs) -> PtrArg { + if (const auto *Node = + Result.Nodes.getNodeAs(arg::Dest::Refs.VariantRefs.AsCArray); + Node != nullptr) { + // freestanding std::begin + return {CArrayTag{}, Node}; + } + if (const auto *Node = Result.Nodes.getNodeAs( + arg::Dest::Refs.VariantRefs.AsContainer); + Node != nullptr) { + return {ContainerTag{}, Node}; + } + const auto *Node = + Result.Nodes.getNodeAs(arg::Dest::Refs.VariantRefs.Fallback); + return {RawPtrTag{}, Node}; + }; + + auto Dest = MakePtrArg(arg::Dest::Refs.VariantRefs); + auto Source = MakePtrArg(arg::Source::Refs.VariantRefs); + + if (std::holds_alternative(Dest.Tag) and + std::holds_alternative(Source.Tag) and CheckSizeArgPermitsFix()) { + + } else { + + } + + Diag << Inserter.createIncludeInsertion( + Result.SourceManager->getFileID(CallNode.getBeginLoc()), StdCopyHeader); +} + +void ReplaceWithStdCopyCheck::reorderArgs(DiagnosticBuilder &Diag, + const CallExpr *MemcpyNode) { + std::array Arg; + + LangOptions LangOpts; + LangOpts.CPlusPlus = true; + PrintingPolicy Policy(LangOpts); + + // Retrieve all the arguments + for (uint8_t I = 0; I < Arg.size(); I++) { + llvm::raw_string_ostream S(Arg[I]); + MemcpyNode->getArg(I)->printPretty(S, nullptr, Policy); + } + + // Create lambda that return SourceRange of an argument + auto GetSourceRange = [MemcpyNode](uint8_t ArgCount) -> SourceRange { + return SourceRange(MemcpyNode->getArg(ArgCount)->getBeginLoc(), + MemcpyNode->getArg(ArgCount)->getEndLoc()); + }; + + // Reorder the arguments + Diag << FixItHint::CreateReplacement(GetSourceRange(0), Arg[1]); + + Arg[2] = Arg[1] + " + ((" + Arg[2] + ") / sizeof(*(" + Arg[1] + ")))"; + Diag << FixItHint::CreateReplacement(GetSourceRange(1), Arg[2]); + + Diag << FixItHint::CreateReplacement(GetSourceRange(2), Arg[0]); +} + +llvm::ArrayRef ReplaceWithStdCopyCheck::getFlaggableCallees() const { + switch (FlaggableCallees_) { + case FlaggableCallees::OnlyMemmove: + return {"::memmove", "::std::memmove"}; + case FlaggableCallees::MemmoveAndMemcpy: + return {"::memmove", "::std::memmove", "::memcpy", "::std::memcpy"}; + } +} +} // namespace modernize +} // namespace clang::tidy \ No newline at end of file diff --git a/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.h b/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.h new file mode 100644 index 0000000000000..16510174c1ee5 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.h @@ -0,0 +1,82 @@ +//===--- ReplaceMemcpyWithStdCopy.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_MODERNIZE_REPLACE_WITH_STDCOPY_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_WITH_STDCOPY_H + +#include "../ClangTidyCheck.h" +#include "../utils/IncludeInserter.h" +#include "clang/AST/ASTTypeTraits.h" +#include "clang/Basic/LangOptions.h" +#include "llvm/ADT/SmallVector.h" + +namespace clang::tidy::modernize { + +// Replace C-style calls to functions like memmove and memcpy with analogous +// calls to std::copy or std::copy_n, depending on the context +class ReplaceWithStdCopyCheck : public ClangTidyCheck { +public: + enum class FlaggableCallees { + OnlyMemmove, + MemmoveAndMemcpy, + }; + + enum class StdCallsReplacementStyle { + FreeFunc, + MemberCall, + }; + + ReplaceWithStdCopyCheck(StringRef Name, ClangTidyContext *Context); + ~ReplaceWithStdCopyCheck() override = default; + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) override; + void storeOptions(ClangTidyOptions::OptionMap &Options) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus11; + } + std::optional getCheckTraversalKind() const override { + return TK_IgnoreUnlessSpelledInSource; + } + + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + void handleMemcpy(); + void handleMemset(); + + void renameFunction(DiagnosticBuilder &Diag, const CallExpr *MemcpyNode); + void reorderArgs(DiagnosticBuilder &Diag, const CallExpr *MemcpyNode); + void insertHeader(DiagnosticBuilder &Diag, const CallExpr *MemcpyNode, + SourceManager *const SM); + bool checkIsByteSequence(const ast_matchers::MatchFinder::MatchResult &Result, + std::string_view Prefix); + +private: + void tryIssueFixIt(const ast_matchers::MatchFinder::MatchResult &Result, + const DiagnosticBuilder &Diag, const CallExpr &CallNode); + llvm::ArrayRef getFlaggableCallees() const; + + utils::IncludeInserter Inserter; + // __jm__ TODO options: + // 1. list of functions that can be replaced, memmove by default, + // and memcpy as opt-in + // 2. should calls to {begin,end,size} be replaced + // with their members or free fns. + + static constexpr auto FlaggableCalleesDefault = FlaggableCallees::OnlyMemmove; + static constexpr auto StdCallsReplacementStyleDefault = + StdCallsReplacementStyle::FreeFunc; + + const FlaggableCallees FlaggableCallees_; + const StdCallsReplacementStyle StdCallsReplacementStyle_; +}; + +} // namespace clang::tidy::modernize + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_WITH_STDCOPY_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 552044356e404..94e1ea1a72386 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -319,8 +319,8 @@ Changes in existing checks member function calls too and to only expand macros starting with ``PRI`` and ``__PRI`` from ```` in the format string. -- New :doc:`modernize-replace-memcpy-with-stdcopy - ` check. +- New :doc:`modernize-replace-with-stdcopy + ` check. Replaces all occurrences of the C ``memcpy`` function by ``std::copy``. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 8af5fc8fa3cfa..0ee04dc15082b 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -287,7 +287,7 @@ Clang-Tidy Checks :doc:`modernize-raw-string-literal `, "Yes" :doc:`modernize-redundant-void-arg `, "Yes" :doc:`modernize-replace-auto-ptr `, "Yes" - :doc:`modernize-replace-memcpy-with-std-copy `, "Yes" + :doc:`modernize-replace-with-std-copy `, "Yes" :doc:`modernize-replace-disallow-copy-and-assign-macro `, "Yes" :doc:`modernize-replace-random-shuffle `, "Yes" :doc:`modernize-return-braced-init-list `, "Yes" diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/modernize-replace-memcpy-with-stdcopy.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/modernize-replace-memcpy-with-stdcopy.rst index 922a7f36e7e07..74276c37bc2c0 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/modernize/modernize-replace-memcpy-with-stdcopy.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/modernize-replace-memcpy-with-stdcopy.rst @@ -1,6 +1,6 @@ -.. title:: clang-tidy - modernize-replace-memcpy-with-stdcopy +.. title:: clang-tidy - modernize-with-stdcopy -modernize-replace-memcpy-with-stdcopy +modernize-with-stdcopy =================================== Replaces all occurrences of the C ``memcpy`` function with ``std::copy`` diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/replace-memcpy-with-stdcopy.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/replace-memcpy-with-stdcopy.rst new file mode 100644 index 0000000000000..119a3184208c2 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/replace-memcpy-with-stdcopy.rst @@ -0,0 +1,47 @@ +.. title:: clang-tidy - modernize-replace-with-stdcopy + +modernize-replace-with-stdcopy +=================================== + +Replaces all occurrences of the C ``memcpy`` function with ``std::copy`` + +Example: + +.. code-block:: c++ + + /*! + * \param destination Pointer to the destination array where the content is to be copied + * \param source Pointer to the source of data to be copied + * \param num Number of bytes to copy + */ + memcpy(destination, source, num); + +becomes + +.. code-block:: c++ + + /*! + * \param destination Pointer to the destination array where the content is to be copied + * \param source Pointer to the source of data to be copied + * \param num Number of bytes to copy + */ + std::copy(source, source + (num / sizeof *source), destination); + +Bytes to iterator conversion +---------------------------- + +Unlike ``std::copy`` that take an iterator on the last element of the source array, ``memcpy`` request the number of bytes to copy. +In order to make the check working, it will convert the size parameter to an iterator by replacing it by ``source + (num / sizeof *source)`` + +Header inclusion +---------------- + +``std::copy`` being provided by the ``algorithm`` header file, this check will include it if needed. + +Options +------- + +.. option:: IncludeStyle + + A string specifying which include-style is used, `llvm` or `google`. Default + is `llvm`. diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-memcpy-with-std-copy.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-memcpy-with-std-copy.cpp deleted file mode 100644 index ce07fc5801da6..0000000000000 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-memcpy-with-std-copy.cpp +++ /dev/null @@ -1,146 +0,0 @@ -// RUN: %check_clang_tidy %s modernize-replace-memcpy-with-std-copy %t - -// CHECK-FIXES: #include - -namespace { - -using size_t = decltype(sizeof(int)); - -namespace std { -typedef long long int64_t; -typedef short int16_t; -typedef char int8_t; - -void *memcpy(void *__restrict dest, const void *__restrict src, size_t n); - -template struct vector { - vector(size_t); - - T *data(); - size_t size() const; - void resize(size_t); - using value_type = T; -}; - -size_t size(void *); - -size_t strlen(const char *); -} // namespace std - -void *memcpy(void *__restrict dest, const void *__restrict src, size_t n); -} // namespace - -void notSupportedEx() { - char source[] = "once upon a daydream...", dest[4]; - - auto *primitiveDest = new std::int8_t; - std::memcpy(primitiveDest, source, sizeof primitiveDest); - - auto *primitiveDest2 = new std::int16_t; - std::memcpy(primitiveDest2, source, sizeof primitiveDest); - std::memcpy(primitiveDest2, source, sizeof primitiveDest2); - - double d = 0.1; - std::int64_t n; - // don't warn on calls over non-sequences - std::memcpy(&n, &d, sizeof d); - - // object creation in destination buffer - struct S { - int x{42}; - void *operator new(size_t, void *) noexcept { return nullptr; } - } s; - alignas(S) char buf[sizeof(S)]; - S *ps = new (buf) S; // placement new - // // don't warn on calls over non-sequences - std::memcpy(ps, &s, sizeof s); - - const char *pSource = "once upon a daydream..."; - char *pDest = new char[4]; - std::memcpy(dest, pSource, sizeof dest); - std::memcpy(pDest, source, 4); -} - -void noFixItEx() { - char source[] = "once upon a daydream...", dest[4]; - - // no FixIt when return value is used - auto *ptr = std::memcpy(dest, source, sizeof dest); - // CHECK-MESSAGES: [[@LINE-1]]:20: warning: prefer std::copy_n to memcpy - - std::vector vec_i16(4); - // not a supported type, should be a sequence of bytes, otherwise it is difficult to compute the n in copy_n - std::memcpy(vec_i16.data(), source, - vec_i16.size() * sizeof(decltype(vec_i16)::value_type)); - // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy -} - -void sequenceOfBytesEx() { - // the check should support memcpy conversion for the following types: - // T[] - // std::vector - // std::span - // std::deque - // std::array - // std::string - // std::string_view - // where T is byte-like - - char source[] = "once upon a daydream...", dest[4]; - std::memcpy(dest, source, sizeof dest); - // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy - // CHECK-FIXES: std::copy_n(std::begin(source), std::size(dest), std::begin(dest)); - - // __jm__ warn on global call as well - memcpy(dest, source, sizeof dest); - // CHECK-MESSAGES: [[@LINE-1]]:3: warning: prefer std::copy_n to memcpy - // CHECK-FIXES: std::copy_n(std::begin(source), std::size(dest), std::begin(dest)); - - std::vector vec_i8(4); - std::memcpy(vec_i8.data(), source, vec_i8.size()); - // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy - // CHECK-FIXES: std::copy_n(std::begin(source), std::size(vec_i8), std::begin(vec_i8)); - - // __jm__ make configurable whether stl containers should use members or free fns. - // __jm__ for now use free fns. only - - std::memcpy(dest, vec_i8.data(), vec_i8.size()); - // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy - // CHECK-FIXES: std::copy_n(std::begin(vec_i8), std::size(vec_i8), std::begin(dest)); - std::memcpy(dest, vec_i8.data(), sizeof(dest)); - // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy - // CHECK-FIXES: std::copy_n(vec_i8.data(), std::size(dest), std::begin(dest)); - std::memcpy(dest, vec_i8.data(), std::size(dest)); - // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy - // CHECK-FIXES: std::copy_n(std::begin(source), std::size(vec_i8), std::begin(vec_i8)); - - std::memcpy(dest, vec_i8.data(), 1 + vec_i8.size() / 2); - // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy - // CHECK-FIXES: std::copy_n(std::begin(vec_i8), 1 + std::size(vec_i8) / 2, std::begin(dest)); - std::memcpy(dest, vec_i8.data(), 1 + sizeof(dest) / 2); - // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy - // CHECK-FIXES: std::copy_n(vec_i8.data(), 1 + std::size(dest) / 2, std::begin(dest)); - std::memcpy(dest, vec_i8.data(), 1 + std::size(dest) / 2); - // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy - // CHECK-FIXES: std::copy_n(std::begin(source), 1 + std::size(dest) / 2, std::begin(vec_i8)); -} - -// void uninitialized_copy_ex() { -// std::vector v = {"This", "is", "an", "example"}; - -// std::string* p; -// std::size_t sz; -// std::tie(p, sz) = std::get_temporary_buffer(v.size()); -// sz = std::min(sz, v.size()); - -// std::uninitialized_copy_n(v.begin(), sz, p); - -// for (std::string* i = p; i != p + sz; ++i) -// { -// std::cout << *i << ' '; -// i->~basic_string(); -// } -// std::cout << '\n'; - -// std::return_temporary_buffer(p); -// } \ No newline at end of file diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-with-std-copy.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-with-std-copy.cpp new file mode 100644 index 0000000000000..3d0dc18051676 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-with-std-copy.cpp @@ -0,0 +1,320 @@ +// RUN: %check_clang_tidy %s modernize-replace-with-std-copy %t + +// possible call scenarios, infeasible to cover all +// replacement type: +// [ ] members when possible +// [X] all free +// source: +// type: +// [ ] T[] +// std::data: +// [ ] free +// [ ] N/A +// [ ] container +// std::data: +// [ ] free +// [ ] member +// type: +// [ ] std::vector +// [ ] std::span +// [ ] std::deque +// [ ] std::array +// [ ] std::string +// [ ] std::string_view +// dest: +// type: +// [ ] T[] +// std::data: +// [ ] free +// [ ] N/A +// [ ] container +// std::data: +// [ ] free +// [ ] member +// type: +// [ ] std::vector +// [ ] std::span +// [ ] std::deque +// [ ] std::array +// [ ] std::string +// [ ] std::string_view +// callee: +// name: +// [ ] memmove +// [ ] memcpy +// [ ] wmemmove +// [ ] wmemcpy +// qualified: +// [ ] y +// [ ] n + +// CHECK-FIXES: #include + +namespace { + +using size_t = decltype(sizeof(int)); + +namespace std { +using int64_t = long long ; +using int32_t = int ; +using int16_t = short ; +using int8_t = char ; + +using char32 = int32_t; +using char16 = int16_t; +using char8 = int8_t; + +template +class allocator {}; +template +class char_traits {}; +template +struct basic_string { + typedef basic_string _Type; + basic_string(); + basic_string(const C* p, const A& a = A()); + + const C* c_str() const; + const C* data() const; + + _Type& append(const C* s); + _Type& append(const C* s, size_t n); + _Type& assign(const C* s); + _Type& assign(const C* s, size_t n); + + int compare(const _Type&) const; + int compare(const C* s) const; + int compare(size_t pos, size_t len, const _Type&) const; + int compare(size_t pos, size_t len, const C* s) const; + + size_t find(const _Type& str, size_t pos = 0) const; + size_t find(const C* s, size_t pos = 0) const; + size_t find(const C* s, size_t pos, size_t n) const; + + _Type& insert(size_t pos, const _Type& str); + _Type& insert(size_t pos, const C* s); + _Type& insert(size_t pos, const C* s, size_t n); + + _Type& operator+=(const _Type& str); + _Type& operator+=(const C* s); + _Type& operator=(const _Type& str); + _Type& operator=(const C* s); +}; + +using string = basic_string, std::allocator>; +using wstring = basic_string,std::allocator>; +using u16string = basic_string, std::allocator>; +using u32string = basic_string, std::allocator>; + + + +void *memcpy(void *__restrict dest, const void *__restrict src, size_t n); +void *memmove(void *__restrict dest, const void *__restrict src, size_t n); + +template struct vector { + vector(size_t); + + T *data(); + const T *data() const; + size_t size() const; + void resize(size_t); + using value_type = T; +}; + +template +T* data(vector&); + +template +T* data(T[]); + +template +const T* data(const vector&); + +template +const T* data(const T[]); + +size_t size(void *); + +size_t strlen(const char *); +} // namespace std + +void *memcpy(void *__restrict dest, const void *__restrict src, size_t n); +void *memmove(void *__restrict dest, const void *__restrict src, size_t n); +} // namespace + + +namespace { +void notSupportedEx() { + char Source[] = "once upon a daydream...", Dest[4]; + + auto *PrimitiveDest = new std::int8_t; + std::memcpy(PrimitiveDest, Source, sizeof PrimitiveDest); + + auto *PrimitiveDest2 = new std::int16_t; + std::memcpy(PrimitiveDest2, Source, sizeof PrimitiveDest); + std::memcpy(PrimitiveDest2, Source, sizeof PrimitiveDest2); + + double D = 0.1; + std::int64_t N; + // don't warn on calls over non-sequences + std::memcpy(&N, &D, sizeof D); + + // object creation in destination buffer + struct StructType { + int X{42}; + void *operator new(size_t, void *) noexcept { return nullptr; } + } Struct; + alignas(StructType) char Buf[sizeof(StructType)]; + StructType *Ps = new (Buf) StructType; // placement new + // // don't warn on calls over non-sequences + std::memcpy(Ps, &Struct, sizeof Struct); + + const char *PtrSource = "once upon a daydream..."; + char *PtrDest = new char[4]; + std::memcpy(Dest, PtrSource, sizeof Dest); + std::memcpy(PtrDest, Source, 4); +} + +void noFixItEx() { + { + int Source[10]; + std::vector Dest(5); + + memmove(Source, std::data(Dest), 1); + // CHECK-MESSAGES: [[@LINE-1]]:20: warning: prefer std::copy_n to memcpy + std::memmove(Source, std::data(Dest), 1); + memmove(std::data(Source), std::data(Dest), 1); + std::memmove(std::data(Source), std::data(Dest), 1); + memmove(std::data(Source), std::data(Dest), 1); + std::memmove(std::data(Source), std::data(Dest), 1); + } + char Source[] = "once upon a daydream...", Dest[4]; + + // no FixIt when return value is used + + [](auto){}(std::memcpy(Dest, Source, sizeof Dest)); + // CHECK-MESSAGES: [[@LINE-1]]:20: warning: prefer std::copy_n to memcpy + + std::vector VecI16(4); + // not a supported type, should be a sequence of bytes, otherwise it is difficult to compute the n in copy_n + std::memcpy(VecI16.data(), Source, + VecI16.size() * sizeof(decltype(VecI16)::value_type)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy + + std::memcpy(std::data(VecI16), Source, + VecI16.size() * sizeof(decltype(VecI16)::value_type)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy +} + +void sequenceOfBytesEx() { + // the check should support memcpy conversion for the following types: + // T[] + // std::vector + // std::span + // std::deque + // std::array + // std::string + // std::string_view + // where T is byte-like + + { + char Source[] = "once upon a daydream..."; + char Dest[4]; + + std::memcpy(Dest, Source, sizeof Dest); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy + // CHECK-FIXES: std::copy_n(std::begin(Source), std::size(Dest), std::begin(Dest)); + + std::memcpy(std::data(Dest), Source, sizeof Dest); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy + // CHECK-FIXES: std::copy_n(std::begin(Source), std::size(Dest), std::begin(Dest)); + + std::memcpy(Dest, std::data(Source), sizeof Dest); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy + // CHECK-FIXES: std::copy_n(std::begin(Source), std::size(Dest), std::begin(Dest)); + + std::memcpy(std::data(Dest), std::data(Source), sizeof Dest); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy + // CHECK-FIXES: std::copy_n(std::begin(Source), std::size(Dest), std::begin(Dest)); + + // __jm__ warn on global call as well + memcpy(Dest, Source, sizeof Dest); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: prefer std::copy_n to memcpy + // CHECK-FIXES: std::copy_n(std::begin(Source), std::size(Dest), std::begin(Dest)); + + memcpy(std::data(Dest), Source, sizeof Dest); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: prefer std::copy_n to memcpy + // CHECK-FIXES: std::copy_n(std::begin(Source), std::size(Dest), std::begin(Dest)); + + memcpy(Dest, std::data(Source), sizeof Dest); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: prefer std::copy_n to memcpy + // CHECK-FIXES: std::copy_n(std::begin(Source), std::size(Dest), std::begin(Dest)); + + memcpy(std::data(Dest), std::data(Source), sizeof Dest); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: prefer std::copy_n to memcpy + // CHECK-FIXES: std::copy_n(std::begin(Source), std::size(Dest), std::begin(Dest)); + } + + { + char Source[] = "once upon a daydream..."; + std::vector Dest(4); + + std::memcpy(Dest.data(), Source, Dest.size()); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy + // CHECK-FIXES: std::copy_n(std::begin(Source), std::size(Dest), std::begin(Dest)); + + std::memcpy(std::data(Dest), Source, Dest.size()); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy + // CHECK-FIXES: std::copy_n(std::begin(Source), std::size(Dest), std::begin(Dest)); + } + + { + std::vector Source(10); + char Dest[4]; + + std::memcpy(Dest, Source.data(), Source.size()); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy + // CHECK-FIXES: std::copy_n(std::begin(Source), std::size(Source), std::begin(Dest)); + std::memcpy(Dest, Source.data(), sizeof(Dest)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy + // CHECK-FIXES: std::copy_n(Source.data(), std::size(Dest), std::begin(Dest)); + std::memcpy(Dest, Source.data(), std::size(Dest)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy + // CHECK-FIXES: std::copy_n(std::begin(Source), std::size(Source), std::begin(Source)); + + std::memcpy(Dest, Source.data(), 1 + Source.size() / 2); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy + // CHECK-FIXES: std::copy_n(std::begin(Source), 1 + std::size(Source) / 2, std::begin(Dest)); + std::memcpy(Dest, Source.data(), 1 + sizeof(Dest) / 2); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy + // CHECK-FIXES: std::copy_n(Source.data(), 1 + std::size(Dest) / 2, std::begin(Dest)); + std::memcpy(Dest, Source.data(), 1 + std::size(Dest) / 2); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy + // CHECK-FIXES: std::copy_n(std::begin(Source), 1 + std::size(Dest) / 2, std::begin(Source)); + + std::memcpy(Dest, std::data(Source), Source.size()); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy + // CHECK-FIXES: std::copy_n(std::begin(Source), std::size(Source), std::begin(Dest)); + std::memcpy(Dest, std::data(Source), sizeof(Dest)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy + // CHECK-FIXES: std::copy_n(Source.data(), std::size(Dest), std::begin(Dest)); + std::memcpy(Dest, std::data(Source), std::size(Dest)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy + // CHECK-FIXES: std::copy_n(std::begin(Source), std::size(Source), std::begin(Source)); + + std::memcpy(Dest, std::data(Source), 1 + Source.size() / 2); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy + // CHECK-FIXES: std::copy_n(std::begin(Source), 1 + std::size(Source) / 2, std::begin(Dest)); + std::memcpy(Dest, std::data(Source), 1 + sizeof(Dest) / 2); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy + // CHECK-FIXES: std::copy_n(Source.data(), 1 + std::size(Dest) / 2, std::begin(Dest)); + std::memcpy(Dest, std::data(Source), 1 + std::size(Dest) / 2); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy + // CHECK-FIXES: std::copy_n(std::begin(Source), 1 + std::size(Dest) / 2, std::begin(Source)); + } + + // __jm__ make configurable whether stl containers should use members or free fns. + // __jm__ for now use free fns. only + +} +} // namespace \ No newline at end of file From 6d8c0ffc589a80d2830ed0975eb6eb3306d0d458 Mon Sep 17 00:00:00 2001 From: kidp330 Date: Sun, 29 Dec 2024 22:43:17 +0100 Subject: [PATCH 04/15] pre debug WIP, ditch raw pointer support for now i [skip ci] --- .../modernize/ReplaceWithStdCopyCheck.cpp | 623 ++++++++++++------ .../modernize/ReplaceWithStdCopyCheck.h | 9 - 2 files changed, 404 insertions(+), 228 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.cpp b/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.cpp index beba705f5a141..b594018675989 100644 --- a/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.cpp @@ -7,9 +7,14 @@ //===----------------------------------------------------------------------===// #include "ReplaceWithStdCopyCheck.h" +#include "clang/AST/ASTContext.h" #include "clang/AST/DeclTemplate.h" +#include "clang/AST/Expr.h" #include "clang/AST/Type.h" #include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Lex/Lexer.h" +#include "clang/Tooling/Core/Replacement.h" #include "llvm/ADT/StringRef.h" #include @@ -37,72 +42,37 @@ struct OptionEnumMapping< } }; -template <> -struct OptionEnumMapping< - enum modernize::ReplaceWithStdCopyCheck::StdCallsReplacementStyle> { - static llvm::ArrayRef> - getEnumMapping() { - static constexpr std::pair< - modernize::ReplaceWithStdCopyCheck::StdCallsReplacementStyle, StringRef> - Mapping[] = { - {modernize::ReplaceWithStdCopyCheck::StdCallsReplacementStyle:: - FreeFunc, - "FreeFunc"}, - {modernize::ReplaceWithStdCopyCheck::StdCallsReplacementStyle:: - MemberCall, - "MemberCall"}, - }; - return {Mapping}; - } -}; - namespace modernize { namespace { -namespace arg { -namespace tag { -struct VariantPtrArgRef { - llvm::StringLiteral AsContainer; - llvm::StringLiteral AsCArray; - llvm::StringLiteral Fallback; -}; -struct PtrArgRefs { - VariantPtrArgRef VariantRefs; - llvm::StringLiteral CharType; -}; +constexpr llvm::StringLiteral ExpressionRef = "::std::memcpy"; +constexpr llvm::StringLiteral StdCopyHeader = ""; +constexpr llvm::StringLiteral ReturnValueDiscardedRef = + "ReturnValueDiscardedRef"; -struct DestArg { - static constexpr PtrArgRefs Refs = { - {"DestArg::AsContainer", "DestArg::AsCArray", "DestArg::Fallback"}, - "DestArg::CharType"}; -}; -struct SourceArg { - static constexpr PtrArgRefs Refs = { - {"SourceArg::AsContainer", "SourceArg::AsCArray", "SourceArg::Fallback"}, - "SourceArg::CharType" +namespace ptrarg { - }; +struct Refs { + llvm::StringLiteral AsContainer; + llvm::StringLiteral AsCArray; + size_t FallbackParameterIdx; + llvm::StringLiteral ValueType; }; -struct SizeArg {}; -} // namespace tag -template auto createPtrArgMatcher(tag::PtrArgRefs Refs) { - auto [VariantRefs, ValueTypeRef] = Refs; - auto [AsContainer, AsCArray] = VariantRefs; +template auto createPtrArgMatcher() { + constexpr Refs Refs = RefsT::Refs; auto AllowedContainerNamesM = []() { - if constexpr (IsConst) { + if constexpr (true) return hasAnyName("::std::deque", "::std::forward_list", "::std::list", "::std::vector", "::std::basic_string"); - } else { + else return hasAnyName("::std::deque", "::std::forward_list", "::std::list", "::std::vector", "::std::basic_string", "::std::basic_string_view", "::std::span"); - } }(); - auto ValueTypeM = type().bind(ValueTypeRef); + auto ValueTypeM = type().bind(Refs.ValueType); auto AllowedContainerTypeM = hasUnqualifiedDesugaredType( recordType(hasDeclaration(recordDecl(classTemplateSpecializationDecl( @@ -112,12 +82,12 @@ template auto createPtrArgMatcher(tag::PtrArgRefs Refs) { hasUnqualifiedDesugaredType(ValueTypeM))))))))); auto VariantContainerM = - expr(hasType(AllowedContainerTypeM)).bind(AsContainer); + expr(hasType(AllowedContainerTypeM)).bind(Refs.AsContainer); auto VariantCArrayM = - expr(hasType(arrayType(hasElementType(ValueTypeM)))).bind(AsCArray); + expr(hasType(arrayType(hasElementType(ValueTypeM)))).bind(Refs.AsCArray); auto StdDataReturnM = returns(pointerType(pointee( - hasUnqualifiedDesugaredType(equalsBoundNode(ValueTypeRef.str()))))); + hasUnqualifiedDesugaredType(equalsBoundNode(Refs.ValueType.str()))))); auto StdDataMemberDeclM = cxxMethodDecl(hasName("data"), parameterCountIs(0), StdDataReturnM); @@ -130,71 +100,173 @@ template auto createPtrArgMatcher(tag::PtrArgRefs Refs) { auto StdDataMemberCallM = cxxMemberCallExpr( callee(StdDataMemberDeclM), argumentCountIs(0), - on(expr(hasType(AllowedContainerTypeM)).bind(AsContainer))); + on(expr(hasType(AllowedContainerTypeM)).bind(Refs.AsContainer))); auto ArrayOrContainerM = expr(anyOf(VariantCArrayM, VariantContainerM)); auto StdDataFreeCallM = callExpr(callee(StdDataFreeDeclM), argumentCountIs(1), hasArgument(0, ArrayOrContainerM)); - return expr(anyOf(StdDataMemberCallM, StdDataFreeCallM, VariantCArrayM)); + // the last expr() in anyOf assumes previous matchers are ran eagerly from + // left to right, still need to test this is the actual behaviour + return expr( + anyOf(StdDataMemberCallM, StdDataFreeCallM, VariantCArrayM, expr())); } -template class MatcherLinker : public TAG { -public: - static auto createMatcher(); - static void handleResult(const MatchFinder::MatchResult &Result); - -private: - using AllowedTAG = std::enable_if_t || - std::is_same_v || - std::is_same_v>; +namespace tag { +struct Container {}; +struct CArray {}; +struct RawPtr {}; +} // namespace tag +struct PtrArg { + std::variant Tag; + const Expr *Node; }; -template <> auto MatcherLinker::createMatcher() { - return createPtrArgMatcher(Refs); +template +auto extractNode(const CallExpr &CallNode, + const MatchFinder::MatchResult &Result) -> PtrArg { + constexpr Refs Refs = RefT::Refs; + if (const auto *Node = Result.Nodes.getNodeAs(Refs.AsCArray)) + return {tag::CArray{}, Node}; + if (const auto *Node = Result.Nodes.getNodeAs(Refs.AsContainer)) + return {tag::Container{}, Node}; + return {tag::RawPtr{}, CallNode.getArg(Refs.FallbackParameterIdx)}; } -template <> auto MatcherLinker::createMatcher() { - return createPtrArgMatcher(Refs); +template +QualType extractValueType(const MatchFinder::MatchResult &Result) { + return *Result.Nodes.getNodeAs(RefT::Refs.ValueType); } +} // namespace ptrarg + +namespace dst { +constexpr size_t argIndex = 0; +struct RefT { + static constexpr ptrarg::Refs Refs = { + "Dst::AsContainer", + "Dst::AsCArray", + 0, + "Dst::ValueType", + }; +}; -template <> auto MatcherLinker::createMatcher() { - // cases: - // 1. call to std::size - // 2. member call to .size() - // 3. sizeof node or sizeof(node) (c-array) - // 4. N * sizeof(type) (hints at sequence-related use, however none singled out) - - // need a robust way of exchanging information between matchers that will help in sequence identification - // I see one possiblity - // size "thinks" dest is rawptr with pointee sequence - // N * sizeof - // size "thinks" src is rawptr with pointee sequence - // size agrees dest is container/c-array - // size agrees src is container/c-array - - return anything(); // __jm__ TODO +auto createMatcher() { return ptrarg::createPtrArgMatcher(); } +auto extractNode(const CallExpr &CallNode, + const MatchFinder::MatchResult &Result) { + return ptrarg::extractNode(CallNode, Result); +} +auto extractValueType(const MatchFinder::MatchResult &Result) { + return ptrarg::extractValueType(Result); } +} // namespace dst -using Dest = MatcherLinker; -using Source = MatcherLinker; -using Size = MatcherLinker; -} // namespace arg +namespace src { +constexpr size_t argIndex = 1; -constexpr llvm::StringLiteral ExpressionRef = "::std::memcpy"; -constexpr llvm::StringLiteral StdCopyHeader = ""; -constexpr llvm::StringLiteral ReturnValueDiscardedRef = - "ReturnValueDiscardedRef"; +struct SrcRefsT { + static constexpr ptrarg::Refs Refs = { + "Src::AsContainer", + "Src::AsCArray", + 1, + "Src::ValueType", + }; +}; + +auto createMatcher() { return ptrarg::createPtrArgMatcher(); } +auto extractNode(const CallExpr &CallNode, + const MatchFinder::MatchResult &Result) { + return ptrarg::extractNode(CallNode, Result); +} +auto extractValueType(const MatchFinder::MatchResult &Result) { + return ptrarg::extractValueType(Result); +} +} // namespace src + +namespace size { +constexpr size_t argIndex = 2; +// cases: +// 1. sizeof(T) where T is a type +// 2. sizeof(rec) where rec is a CArray +// 3. N * sizeof(T) +// 4. strlen(rec) [+ 1] +// 5. other expr +namespace variant { +struct SizeOfExpr { + const Expr *Arg; +}; +struct NSizeOfExpr { + const Expr *N; + const Expr *Arg; +}; +struct Strlen { + const Expr *Arg; +}; +} // namespace variant +using SizeArg = std::variant; + +struct SizeComponents { + CharUnits Unit; + std::string NExpr; +}; + +struct IArg { + virtual ~IArg() = default; + virtual SizeComponents extractSizeComponents() = 0; +}; -// Helper Matcher which applies the given QualType Matcher either directly or by -// resolving a pointer type to its pointee. Used to match v.push_back() as well -// as p->push_back(). -auto hasTypeOrPointeeType( - const ast_matchers::internal::Matcher &TypeMatcher) { - return anyOf(hasType(TypeMatcher), - hasType(pointerType(pointee(TypeMatcher)))); +static constexpr struct Refs { + llvm::StringLiteral SizeOfArg; + llvm::StringLiteral N; + llvm::StringLiteral StrlenArg; +} Refs = { + "Size::SizeOfExpr", + "Size::N", + "Size::StrlenArg", +}; + +auto createMatcher() { + auto NSizeOfT = + binaryOperator(hasOperatorName("*"), + hasOperands(expr().bind(Refs.N), + sizeOfExpr(has(expr().bind(Refs.SizeOfArg))))); + auto Strlen = callExpr(callee(functionDecl(hasAnyName( + "::strlen", "::std::strlen", "::wcslen", + "::std::wcslen", "::strnlen_s", "::std::strnlen_s", + "::wcsnlen_s", "::std::wcsnlen_s"))), + hasArgument(0, expr().bind(Refs.StrlenArg))); + auto StrlenPlusOne = binaryOperator( + hasOperatorName("+"), hasOperands(Strlen, integerLiteral(equals(1)))); + + auto SizeOfExpr = sizeOfExpr(hasArgumentOfType(type().bind(Refs.SizeOfArg))); + + return expr(anyOf(NSizeOfT, Strlen, StrlenPlusOne, SizeOfExpr)); +} + +SizeArg extractNode(const CallExpr &CallNode, + const MatchFinder::MatchResult &Result) { + if (const auto *SizeOfArgNode = Result.Nodes.getNodeAs(Refs.SizeOfArg); + SizeOfArgNode != nullptr) { + if (const auto *NNode = Result.Nodes.getNodeAs(Refs.N); + NNode != nullptr) + return variant::NSizeOfExpr{NNode, SizeOfArgNode}; + return variant::SizeOfExpr{SizeOfArgNode}; + } + if (const auto *StrlenArgNode = Result.Nodes.getNodeAs(Refs.StrlenArg); + StrlenArgNode != nullptr) { + return variant::Strlen{StrlenArgNode}; + } + return CallNode.getArg(2); } +// 1. N * sizeof(type) (commutative) - issue warning but no fixit, or fixit +// only if both src/dest are fixit friendly 1.1. This might allow fixits for +// wider-than-byte element collections +// 2. strlen(src|dest) ?(+ 1 (commutative)) -- issue warning but no fixit +// 4. sizeof(variable) only if that variable is of arrayType, if it's of +// ptrType that may indicate [de]serialization + +} // namespace size } // namespace @@ -205,9 +277,7 @@ ReplaceWithStdCopyCheck::ReplaceWithStdCopyCheck(StringRef Name, utils::IncludeSorter::IS_LLVM), areDiagsSelfContained()), FlaggableCallees_(Options.getLocalOrGlobal("FlaggableCallees", - FlaggableCalleesDefault)), - StdCallsReplacementStyle_(Options.getLocalOrGlobal( - "StdCallsReplacementStyle", StdCallsReplacementStyleDefault)) {} + FlaggableCalleesDefault)) {} void ReplaceWithStdCopyCheck::registerMatchers(MatchFinder *Finder) { const auto ReturnValueUsedM = @@ -217,18 +287,17 @@ void ReplaceWithStdCopyCheck::registerMatchers(MatchFinder *Finder) { functionDecl(parameterCountIs(3), hasAnyName(getFlaggableCallees())); // cases: - // 1. One of the arguments is definitely a sequence and the other a pointer - - // match + // 1. One of the arguments is definitely a collection and the other a pointer + // - match // 2. Both source and dest are pointers, but size is of the form ((N := // expr()) * sizeof(bytelike())) - match (false positive if N \in {0, 1}) - using namespace arg; const auto Expression = callExpr(callee(OffendingDeclM), anyOf(optionally(ReturnValueUsedM), - allOf(hasArgument(0, Dest::createMatcher()), - hasArgument(1, Source::createMatcher()), - hasArgument(2, Size::createMatcher())))); + allOf(hasArgument(0, dst::createMatcher()), + hasArgument(1, src::createMatcher()), + hasArgument(2, size::createMatcher())))); Finder->addMatcher(Expression.bind(ExpressionRef), this); } @@ -240,7 +309,6 @@ void ReplaceWithStdCopyCheck::registerPPCallbacks( void ReplaceWithStdCopyCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "IncludeStyle", Inserter.getStyle()); Options.store(Opts, "FlaggableCallees", FlaggableCallees_); - Options.store(Opts, "StdCallsReplacementStyle", StdCallsReplacementStyle_); } #include "llvm/Support/Debug.h" @@ -249,144 +317,261 @@ void ReplaceWithStdCopyCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { void ReplaceWithStdCopyCheck::check(const MatchFinder::MatchResult &Result) { const auto &CallNode = *Result.Nodes.getNodeAs(ExpressionRef); - auto Diag = diag(CallNode.getExprLoc(), "prefer std::copy_n to %0") - << cast(CallNode.getCalleeDecl()); + auto dstVT = dst::extractValueType(Result); + auto srcVT = src::extractValueType(Result); - tryIssueFixIt(Result, Diag, CallNode); + // basis for converting size argument to std::copy_n's when issuing fixit + auto valueTypeWidth = Result.Context->getTypeSizeInChars(dstVT); - { - // using namespace std::literals; - // Diag << FixItHint::CreateReplacement( - // DestArg->getSourceRange(), - // ("std::begin(" + - // tooling::fixit::getText(SrcArg->getSourceRange(), *Result.Context) + - // ")") - // .str()); - // Diag << FixItHint::CreateReplacement( - // SrcArg->getSourceRange(), - // tooling::fixit::getText(SizeArg->getSourceRange(), *Result.Context)); - // Diag << FixItHint::CreateReplacement( - // SizeArg->getSourceRange(), - // ("std::begin(" + - // tooling::fixit::getText(DestArg->getSourceRange(), *Result.Context) - // + - // ")") - // .str()); - } - // dest source size -> source size dest -} + // Don't report cases where value type widths differ, as this might indicate + // [de]serialization and hard to reason about replacements using std::copy[_n] + if (valueTypeWidth != Result.Context->getTypeSizeInChars(srcVT)) + return; -// void ReplaceWithStdCopyCheck::handleMemcpy(const CallExpr &Node) { + auto dst = dst::extractNode(CallNode, Result); + auto src = src::extractNode(CallNode, Result); + auto size = size::extractNode(CallNode, Result); + + // only have this function return a non-empty optional if the form of the size + // argument strongly indicates collection-related usage + auto exprAsString = [&](const Expr *Node) { + return Lexer::getSourceText( + CharSourceRange::getTokenRange(Node->getSourceRange()), + *Result.SourceManager, getLangOpts()) + .str(); + }; -// // FixIt -// const CharSourceRange FunctionNameSourceRange = -// CharSourceRange::getCharRange( -// Node.getBeginLoc(), Node.getArg(0)->getBeginLoc()); + auto extractComponents = [&]() -> std::optional { + if (const auto *NSizeOfExprNode = + std::get_if(&size); + NSizeOfExprNode != nullptr) { + auto &[N, Arg] = *NSizeOfExprNode; + auto sizeOfArgWidth = Result.Context->getTypeSizeInChars(Arg->getType()); + return {{sizeOfArgWidth, exprAsString(N)}}; + } + if (const auto *StrlenNode = std::get_if(&size); + StrlenNode != nullptr) { + auto strlenArgTypeWidth = Result.Context->getTypeSizeInChars( + StrlenNode->Arg->getType()->getPointeeType()); + return {{strlenArgTypeWidth, exprAsString(CallNode.getArg(2))}}; + } + return std::nullopt; + }; -// Diag << FixItHint::CreateReplacement(FunctionNameSourceRange, -// "std::copy_n("); -// } + if (auto maybeComponents = extractComponents(); maybeComponents.has_value()) { + auto [unit, nExpr] = *maybeComponents; + if (unit != valueTypeWidth) { + // __jm__ tricky to offer a helpful fixit here, as the size argument + // doesn't seem to be related to the pointee types of src and dst + return; + } + // __jm__ makes sense to assume + } + // might be a reinterpretation call, only other case where we don't flag + if (std::holds_alternative(dst.Tag) and + std::holds_alternative(src.Tag)) { + // not supported yet, need to come up with a robust heuristic first + return; + } -// bool ReplaceWithStdCopyCheck::fixItPossible( -// const MatchFinder::MatchResult &Result) {} + // both src and dst are collection expressions, enough to wrap them in + // std::begin calls + auto Diag = diag(CallNode.getExprLoc(), "prefer std::copy_n to %0") + << cast(CallNode.getCalleeDecl()); -void ReplaceWithStdCopyCheck::tryIssueFixIt( - const MatchFinder::MatchResult &Result, const DiagnosticBuilder &Diag, - const CallExpr &CallNode) { // don't issue a fixit if the result of the call is used if (bool IsReturnValueUsed = Result.Nodes.getNodeAs(ReturnValueDiscardedRef) == nullptr; IsReturnValueUsed) return; - // to make sure we can issue a FixIt, need to be pretty sure we're dealing - // with a sequence and it is a byte sequence - // 1. For now we are sure both source and dest are sequences, but not - // necessarily of bytes - // 2. We can relax conditions to where just one arg is a sequence, and the - // other can then be a sequence or raw pointer - // 3. when both dest and source are pointers (no expectations on the argument - // as everything required is enforced by the type system) we can use a - // heuristic using the form of the 3rd argument expression - // + Diag << FixItHint::CreateRemoval(CallNode.getSourceRange()); + std::string srcFixit = "std::cbegin(" + exprAsString(src.Node) + ")"; + std::string dstFixit = "std::begin(" + exprAsString(dst.Node) + ")"; - // delete the memmove/memcpy - // insert an std::copy_n - struct CArrayTag {}; - struct ContainerTag {}; - struct RawPtrTag {}; - using PtrArgVariant = std::variant; - struct PtrArg { - PtrArgVariant Tag; - const Expr *Node; - }; + auto calleeIsWideVariant = + CallNode.getDirectCallee()->getParamDecl(0)->getType()->isWideCharType(); - const auto MakePtrArg = [&](arg::tag::VariantPtrArgRef Refs) -> PtrArg { - if (const auto *Node = - Result.Nodes.getNodeAs(arg::Dest::Refs.VariantRefs.AsCArray); - Node != nullptr) { - // freestanding std::begin - return {CArrayTag{}, Node}; + auto calleeUnit = [&]() { + if (calleeIsWideVariant) { + return Result.Context->getTypeSizeInChars( + (Result.Context->getWideCharType())); } - if (const auto *Node = Result.Nodes.getNodeAs( - arg::Dest::Refs.VariantRefs.AsContainer); - Node != nullptr) { - return {ContainerTag{}, Node}; + return CharUnits::One(); + }(); + auto sizeFixit = [&]() -> std::string { + if (valueTypeWidth == calleeUnit) { + return exprAsString(CallNode.getArg(size::argIndex)); } - const auto *Node = - Result.Nodes.getNodeAs(arg::Dest::Refs.VariantRefs.Fallback); - return {RawPtrTag{}, Node}; - }; - - auto Dest = MakePtrArg(arg::Dest::Refs.VariantRefs); - auto Source = MakePtrArg(arg::Source::Refs.VariantRefs); - - if (std::holds_alternative(Dest.Tag) and - std::holds_alternative(Source.Tag) and CheckSizeArgPermitsFix()) { - - } else { - - } + // try to factor out the unit from the size expression + if (auto maybeSizeComponents = extractComponents(); + maybeSizeComponents.has_value() and + maybeSizeComponents->Unit == valueTypeWidth) { + return maybeSizeComponents->NExpr; + } + // last resort, divide by valueTypeWidth + return exprAsString(CallNode.getArg(size::argIndex)) + " / " + "sizeof(" + + dstVT.getAsString() + ")"; + }(); - Diag << Inserter.createIncludeInsertion( - Result.SourceManager->getFileID(CallNode.getBeginLoc()), StdCopyHeader); -} + Diag << FixItHint::CreateInsertion(CallNode.getEndLoc(), + "std::copy_n(" + srcFixit + ", " + + sizeFixit + ", " + dstFixit + ");"); + + // if (const auto *NSizeofExprNode = + // std::get_if(&size); + // NSizeofExprNode != nullptr) { + // auto &[N, Arg] = *NSizeofExprNode; + // auto sizeOfArgWidth = Result.Context->getTypeSizeInChars(Arg->getType()); + // issueFixitIfWidthsMatch(dst.Node, src.Node, + // {sizeOfArgWidth, exprAsString(N)}); + // return; + // } + // if (const auto *StrlenNode = std::get_if(&size); + // StrlenNode != nullptr) { + // auto strlenArgTypeWidth = Result.Context->getTypeSizeInChars( + // StrlenNode->Arg->getType()->getPointeeType()); + // issueFixitIfWidthsMatch( + // dst.Node, src.Node, + // {strlenArgTypeWidth, exprAsString(CallNode.getArg(2))}); + // return; + // } + // if (const auto *SizeOfExprNode = + // std::get_if(&size); + // SizeOfExprNode != nullptr) { + // auto &Arg = SizeOfExprNode->Arg; + // if (SizeOfExprNode->Arg->getType()->isArrayType()) { + // issueFixitIfWidthsMatch( + // dst.Node, src.Node, + // {CharUnits::One(), exprAsString(CallNode.getArg(2))}); + // return; + // } + // // __jm__ weird bc we assume dst and src are collections + // // if Arg turns out to have the same type as dst or src then just suggest + // // copy via the assignment operator + // if (auto argType = Arg->getType(); argType == dstVT or argType == srcVT) + // { + // issueFixit{valueTypeWidth, exprAsString(CallNode.getArg(2))}; + // return; + // } + // // __jm__ only flag this as suspicious with a further explanation in the + // // diagnostic TODO: a sizeof of an unrelated type when copying between + // // collections does not make a lot of sense + + // auto sizeDRE = [&]() -> const DeclRefExpr * { + // if (const auto *Variant = std::get_if(&size); + // Variant != nullptr) { + // return llvm::dyn_cast_if_present(Variant->Arg); + // } + // if (const auto *Variant = + // std::get_if(&size); + // Variant != nullptr) { + // return llvm::dyn_cast_if_present(Variant->Arg); + // } + // if (const auto *Variant = + // std::get_if(&size); + // Variant != nullptr) { + // return llvm::dyn_cast_if_present(Variant->Arg); + // } + // return nullptr; + // }(); + + // one thing the analysis of the size argument must return is an Expr* Node + // that we can lift into std::copy_n's third argument + + // 1. strlen(DeclRefExpr) also hints that the referenced variable is a + // collection + + // 2. sizeof(expr) + // 2.1. expr is a c-array DeclRefExpr to one of src/dst, copy_n size + // becomes std::size(expr) + // 2.2. both src and dst are not raw pointers and expr is a type of width + // equal to vtw, essentialy fallthrough to 3 + + // 3. N * sizeof(expr) is okay when expr is a type with width == + // valueTypeWidth and N may be verbatim lifted into copy_n's third argument -void ReplaceWithStdCopyCheck::reorderArgs(DiagnosticBuilder &Diag, - const CallExpr *MemcpyNode) { - std::array Arg; + // to make sure we can issue a FixIt, need to be pretty sure we're dealing + // with a collection and it is a byte collection + // 1. For now we are sure both source and dest are collections, but not + // necessarily of bytes + // 2. We can relax conditions to where just one arg is a collection, and the + // other can then be a collection or raw pointer. However, this is not + // robust as there are cases where this may be used for unmarshalling + // 3. when both dest and source are pointers (no expectations on the + // argument as everything required is enforced by the type system) we can + // use a heuristic using the form of the 3rd argument expression + // - LangOptions LangOpts; - LangOpts.CPlusPlus = true; - PrintingPolicy Policy(LangOpts); + // delete the memmove/memcpy + // insert an std::copy_n - // Retrieve all the arguments - for (uint8_t I = 0; I < Arg.size(); I++) { - llvm::raw_string_ostream S(Arg[I]); - MemcpyNode->getArg(I)->printPretty(S, nullptr, Policy); + // using PtrArgVariant = std::variant; + // struct PtrArg { + // PtrArgVariant Tag; + // const Expr *Node; + // }; + + // const auto MakePtrArg = [&](arg::tag::VariantPtrArgRef Refs) -> PtrArg { + // if (const auto *Node = Result.Nodes.getNodeAs( + // arg::Dest::Refs.VariantRefs.AsCArray); + // Node != nullptr) { + // // freestanding std::begin + // return {CArrayTag{}, Node}; + // } + // if (const auto *Node = Result.Nodes.getNodeAs( + // arg::Dest::Refs.VariantRefs.AsContainer); + // Node != nullptr) { + // return {ContainerTag{}, Node}; + // } + // const auto *Node = CallNode.getArg(Refs.FallbackParameterIdx); + // return {RawPtrTag{}, Node}; + // }; + + // auto Dest = MakePtrArg(arg::Dest::Refs.VariantRefs); + // auto Source = MakePtrArg(arg::Source::Refs.VariantRefs); + + // if (std::holds_alternative(Dest.Tag) and + // std::holds_alternative(Source.Tag) and + // CheckSizeArgPermitsFix()) { + + // } else { + // } + + Diag << Inserter.createIncludeInsertion( + Result.SourceManager->getFileID(CallNode.getBeginLoc()), StdCopyHeader); + { + // using namespace std::literals; + // Diag << FixItHint::CreateReplacement( + // DestArg->getSourceRange(), + // ("std::begin(" + + // tooling::fixit::getText(SrcArg->getSourceRange(), *Result.Context) + // + + // ")") + // .str()); + // Diag << FixItHint::CreateReplacement( + // SrcArg->getSourceRange(), + // tooling::fixit::getText(SizeArg->getSourceRange(), + // *Result.Context)); + // Diag << FixItHint::CreateReplacement( + // SizeArg->getSourceRange(), + // ("std::begin(" + + // tooling::fixit::getText(DestArg->getSourceRange(), + // *Result.Context) + // + + // ")") + // .str()); } - - // Create lambda that return SourceRange of an argument - auto GetSourceRange = [MemcpyNode](uint8_t ArgCount) -> SourceRange { - return SourceRange(MemcpyNode->getArg(ArgCount)->getBeginLoc(), - MemcpyNode->getArg(ArgCount)->getEndLoc()); - }; - - // Reorder the arguments - Diag << FixItHint::CreateReplacement(GetSourceRange(0), Arg[1]); - - Arg[2] = Arg[1] + " + ((" + Arg[2] + ") / sizeof(*(" + Arg[1] + ")))"; - Diag << FixItHint::CreateReplacement(GetSourceRange(1), Arg[2]); - - Diag << FixItHint::CreateReplacement(GetSourceRange(2), Arg[0]); + // dest source size -> source size dest } llvm::ArrayRef ReplaceWithStdCopyCheck::getFlaggableCallees() const { switch (FlaggableCallees_) { case FlaggableCallees::OnlyMemmove: - return {"::memmove", "::std::memmove"}; + return {"::memmove", "::std::memmove", "::wmemmove", "::std::wmemmove"}; case FlaggableCallees::MemmoveAndMemcpy: - return {"::memmove", "::std::memmove", "::memcpy", "::std::memcpy"}; + return {"::memmove", "::std::memmove", "::memcpy", "::std::memcpy", + "::wmemmove", "::std::wmemmove", "::wmemcpy", "::std::wmemcpy"}; } } } // namespace modernize diff --git a/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.h b/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.h index 16510174c1ee5..f9cf38bad1d91 100644 --- a/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.h @@ -26,11 +26,6 @@ class ReplaceWithStdCopyCheck : public ClangTidyCheck { MemmoveAndMemcpy, }; - enum class StdCallsReplacementStyle { - FreeFunc, - MemberCall, - }; - ReplaceWithStdCopyCheck(StringRef Name, ClangTidyContext *Context); ~ReplaceWithStdCopyCheck() override = default; void registerMatchers(ast_matchers::MatchFinder *Finder) override; @@ -70,11 +65,7 @@ class ReplaceWithStdCopyCheck : public ClangTidyCheck { // with their members or free fns. static constexpr auto FlaggableCalleesDefault = FlaggableCallees::OnlyMemmove; - static constexpr auto StdCallsReplacementStyleDefault = - StdCallsReplacementStyle::FreeFunc; - const FlaggableCallees FlaggableCallees_; - const StdCallsReplacementStyle StdCallsReplacementStyle_; }; } // namespace clang::tidy::modernize From 5ec498fc945045d8c2363e0e3e03a0bb90e5c350 Mon Sep 17 00:00:00 2001 From: kidp330 Date: Sun, 5 Jan 2025 18:17:10 +0100 Subject: [PATCH 05/15] Version passing the reduced test suite [skip ci] --- .../modernize/ModernizeTidyModule.cpp | 2 +- .../modernize/ReplaceWithStdCopyCheck.cpp | 417 ++++++++++-------- .../modernize/ReplaceWithStdCopyCheck.h | 26 +- .../modernize/replace-with-std-copy-small.cpp | 150 +++++++ .../modernize/replace-with-std-copy.cpp | 3 +- 5 files changed, 391 insertions(+), 207 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/replace-with-std-copy-small.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp index 0c2c351cc71eb..c1c3f8f0f5a72 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -96,7 +96,7 @@ class ModernizeModule : public ClangTidyModule { CheckFactories.registerCheck( "modernize-replace-disallow-copy-and-assign-macro"); CheckFactories.registerCheck( - "modernize-replace-with-stdcopy"); + "modernize-replace-with-std-copy"); CheckFactories.registerCheck( "modernize-replace-random-shuffle"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.cpp b/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.cpp index b594018675989..54f5a29947dda 100644 --- a/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.cpp @@ -23,30 +23,10 @@ using namespace clang::ast_matchers; namespace clang::tidy { -template <> -struct OptionEnumMapping< - enum modernize::ReplaceWithStdCopyCheck::FlaggableCallees> { - static llvm::ArrayRef> - getEnumMapping() { - static constexpr std::pair< - modernize::ReplaceWithStdCopyCheck::FlaggableCallees, StringRef> - Mapping[] = { - {modernize::ReplaceWithStdCopyCheck::FlaggableCallees:: - MemmoveAndMemcpy, - "MemmoveAndMemcpy"}, - {modernize::ReplaceWithStdCopyCheck::FlaggableCallees::OnlyMemmove, - "OnlyMemmove"}, - }; - return {Mapping}; - } -}; - namespace modernize { namespace { constexpr llvm::StringLiteral ExpressionRef = "::std::memcpy"; -constexpr llvm::StringLiteral StdCopyHeader = ""; constexpr llvm::StringLiteral ReturnValueDiscardedRef = "ReturnValueDiscardedRef"; @@ -57,8 +37,11 @@ struct Refs { llvm::StringLiteral AsCArray; size_t FallbackParameterIdx; llvm::StringLiteral ValueType; + llvm::StringLiteral PtrCastFnReturnType; + llvm::StringLiteral AsSmartPtr; }; - +// rn matchers have a problem with binding types correctly, clang-query build up +// the matchers from this file tmrw for debugging template auto createPtrArgMatcher() { constexpr Refs Refs = RefsT::Refs; @@ -72,31 +55,38 @@ template auto createPtrArgMatcher() { "::std::basic_string_view", "::std::span"); }(); - auto ValueTypeM = type().bind(Refs.ValueType); - auto AllowedContainerTypeM = hasUnqualifiedDesugaredType( recordType(hasDeclaration(recordDecl(classTemplateSpecializationDecl( AllowedContainerNamesM, - hasTemplateArgument( - 0, templateArgument(refersToType( - hasUnqualifiedDesugaredType(ValueTypeM))))))))); + hasTemplateArgument(0, templateArgument(refersToType( + qualType().bind(Refs.ValueType))))))))); + + auto CArrayTypeM = arrayType(hasElementType(qualType().bind(Refs.ValueType))); auto VariantContainerM = expr(hasType(AllowedContainerTypeM)).bind(Refs.AsContainer); - auto VariantCArrayM = - expr(hasType(arrayType(hasElementType(ValueTypeM)))).bind(Refs.AsCArray); - auto StdDataReturnM = returns(pointerType(pointee( - hasUnqualifiedDesugaredType(equalsBoundNode(Refs.ValueType.str()))))); + auto VariantCArrayM = expr(hasType(CArrayTypeM)).bind(Refs.AsCArray); + + auto VariantSmartPtrM = + expr(hasType(pointerType(pointee(recordType(hasDeclaration(cxxRecordDecl( + isSameOrDerivedFrom(classTemplateSpecializationDecl( + hasAnyName("::std::unique_ptr", "::std::shared_ptr"), + hasTemplateArgument(0, refersToType(CArrayTypeM))))))))))) + .bind(Refs.AsSmartPtr); + auto VariantRawPtrM = + expr(hasType(pointerType(pointee(qualType().bind(Refs.ValueType))))); + + auto StdDataReturnM = + returns(pointerType(pointee(qualType().bind(Refs.PtrCastFnReturnType)))); auto StdDataMemberDeclM = cxxMethodDecl(hasName("data"), parameterCountIs(0), StdDataReturnM); // ,unless(isConst()) // __jm__ ONLY difference between Dest // and Source calls, but maybe don't include it? - auto StdDataFreeDeclM = functionDecl( - hasName("::std::data"), parameterCountIs(1), - StdDataReturnM); // __jm__ possibly elaborate on argument type here? + auto StdDataFreeDeclM = functionDecl(hasAnyName("::std::data", "::data"), + parameterCountIs(1), StdDataReturnM); auto StdDataMemberCallM = cxxMemberCallExpr( callee(StdDataMemberDeclM), argumentCountIs(0), @@ -109,46 +99,62 @@ template auto createPtrArgMatcher() { // the last expr() in anyOf assumes previous matchers are ran eagerly from // left to right, still need to test this is the actual behaviour - return expr( - anyOf(StdDataMemberCallM, StdDataFreeCallM, VariantCArrayM, expr())); + return expr(anyOf(StdDataMemberCallM, StdDataFreeCallM, VariantCArrayM, + VariantSmartPtrM, VariantRawPtrM)); } namespace tag { +struct RawPtr {}; struct Container {}; struct CArray {}; -struct RawPtr {}; +struct SmartPtr {}; } // namespace tag struct PtrArg { - std::variant Tag; - const Expr *Node; + std::variant Tag; + const Expr &Node; }; template -auto extractNode(const CallExpr &CallNode, - const MatchFinder::MatchResult &Result) -> PtrArg { +PtrArg extractNode(const CallExpr &CallNode, + const MatchFinder::MatchResult &Result) { constexpr Refs Refs = RefT::Refs; - if (const auto *Node = Result.Nodes.getNodeAs(Refs.AsCArray)) - return {tag::CArray{}, Node}; - if (const auto *Node = Result.Nodes.getNodeAs(Refs.AsContainer)) - return {tag::Container{}, Node}; - return {tag::RawPtr{}, CallNode.getArg(Refs.FallbackParameterIdx)}; + if (const auto *Node = Result.Nodes.getNodeAs(Refs.AsCArray); + Node != nullptr) + return {tag::CArray{}, *Node}; + if (const auto *Node = Result.Nodes.getNodeAs(Refs.AsContainer); + Node != nullptr) + return {tag::Container{}, *Node}; + if (const auto *Node = Result.Nodes.getNodeAs(Refs.AsSmartPtr); + Node != nullptr) + return {tag::SmartPtr{}, *Node}; + return {tag::RawPtr{}, *CallNode.getArg(Refs.FallbackParameterIdx)}; } template -QualType extractValueType(const MatchFinder::MatchResult &Result) { - return *Result.Nodes.getNodeAs(RefT::Refs.ValueType); +const QualType *extractValueType(const MatchFinder::MatchResult &Result) { + constexpr Refs Refs = RefT::Refs; + const auto *MaybeRetType = + Result.Nodes.getNodeAs(Refs.PtrCastFnReturnType); + const auto *ValueType = Result.Nodes.getNodeAs(Refs.ValueType); + llvm::errs() << "MaybeRetType: " << (MaybeRetType == nullptr) << '\n'; + llvm::errs() << "ValueType: " << (ValueType == nullptr) << '\n'; + + // checking equality is done here as opposed to when matching because the + // equalsBoundNode matcher depends on the match order and the + // PtrCastFnReturnType is only present in some scenarios + if (MaybeRetType != nullptr and + MaybeRetType->getCanonicalType() != ValueType->getCanonicalType()) + return nullptr; + return ValueType; } } // namespace ptrarg namespace dst { -constexpr size_t argIndex = 0; +constexpr size_t ArgIndex = 0; struct RefT { static constexpr ptrarg::Refs Refs = { - "Dst::AsContainer", - "Dst::AsCArray", - 0, - "Dst::ValueType", - }; + "Dst::AsContainer", "Dst::AsCArray", 0, "Dst::ValueType", + "Dst::PtrCastFnReturnType", "Dst::AsSmartPtr"}; }; auto createMatcher() { return ptrarg::createPtrArgMatcher(); } @@ -162,15 +168,12 @@ auto extractValueType(const MatchFinder::MatchResult &Result) { } // namespace dst namespace src { -constexpr size_t argIndex = 1; +constexpr size_t ArgIndex = 1; struct SrcRefsT { static constexpr ptrarg::Refs Refs = { - "Src::AsContainer", - "Src::AsCArray", - 1, - "Src::ValueType", - }; + "Src::AsContainer", "Src::AsCArray", 1, "Src::ValueType", + "Src::PtrCastFnReturnType", "Src::AsSmartPtr"}; }; auto createMatcher() { return ptrarg::createPtrArgMatcher(); } @@ -184,7 +187,7 @@ auto extractValueType(const MatchFinder::MatchResult &Result) { } // namespace src namespace size { -constexpr size_t argIndex = 2; +constexpr size_t ArgIndex = 2; // cases: // 1. sizeof(T) where T is a type // 2. sizeof(rec) where rec is a CArray @@ -193,14 +196,14 @@ constexpr size_t argIndex = 2; // 5. other expr namespace variant { struct SizeOfExpr { - const Expr *Arg; + const Expr &Arg; }; struct NSizeOfExpr { - const Expr *N; - const Expr *Arg; + const Expr &N; + const Expr &Arg; }; struct Strlen { - const Expr *Arg; + const Expr &Arg; }; } // namespace variant using SizeArg = std::variant(Refs.SizeOfArg); SizeOfArgNode != nullptr) { + llvm::errs() << __LINE__ << '\n'; if (const auto *NNode = Result.Nodes.getNodeAs(Refs.N); NNode != nullptr) - return variant::NSizeOfExpr{NNode, SizeOfArgNode}; - return variant::SizeOfExpr{SizeOfArgNode}; + return variant::NSizeOfExpr{*NNode, *SizeOfArgNode}; + llvm::errs() << __LINE__ << '\n'; + return variant::SizeOfExpr{*SizeOfArgNode}; } + llvm::errs() << __LINE__ << '\n'; if (const auto *StrlenArgNode = Result.Nodes.getNodeAs(Refs.StrlenArg); - StrlenArgNode != nullptr) { - return variant::Strlen{StrlenArgNode}; - } + StrlenArgNode != nullptr) + return variant::Strlen{*StrlenArgNode}; + llvm::errs() << __LINE__ << '\n'; return CallNode.getArg(2); } // 1. N * sizeof(type) (commutative) - issue warning but no fixit, or fixit @@ -268,6 +276,15 @@ SizeArg extractNode(const CallExpr &CallNode, } // namespace size +auto createCalleeMatcher(bool FlagMemcpy) { + if (FlagMemcpy) + return hasAnyName("::memmove", "::std::memmove", "::memcpy", + "::std::memcpy", "::wmemmove", "::std::wmemmove", + "::wmemcpy", "::std::wmemcpy"); + return hasAnyName("::memmove", "::std::memmove", "::wmemmove", + "::std::wmemmove"); +} + } // namespace ReplaceWithStdCopyCheck::ReplaceWithStdCopyCheck(StringRef Name, @@ -276,15 +293,14 @@ ReplaceWithStdCopyCheck::ReplaceWithStdCopyCheck(StringRef Name, Inserter(Options.getLocalOrGlobal("IncludeStyle", utils::IncludeSorter::IS_LLVM), areDiagsSelfContained()), - FlaggableCallees_(Options.getLocalOrGlobal("FlaggableCallees", - FlaggableCalleesDefault)) {} + FlagMemcpy(Options.getLocalOrGlobal("FlagMemcpy", false)) {} void ReplaceWithStdCopyCheck::registerMatchers(MatchFinder *Finder) { const auto ReturnValueUsedM = hasParent(compoundStmt().bind(ReturnValueDiscardedRef)); const auto OffendingDeclM = - functionDecl(parameterCountIs(3), hasAnyName(getFlaggableCallees())); + functionDecl(parameterCountIs(3), createCalleeMatcher(FlagMemcpy)); // cases: // 1. One of the arguments is definitely a collection and the other a pointer @@ -292,13 +308,12 @@ void ReplaceWithStdCopyCheck::registerMatchers(MatchFinder *Finder) { // 2. Both source and dest are pointers, but size is of the form ((N := // expr()) * sizeof(bytelike())) - match (false positive if N \in {0, 1}) - const auto Expression = - callExpr(callee(OffendingDeclM), - anyOf(optionally(ReturnValueUsedM), - allOf(hasArgument(0, dst::createMatcher()), - hasArgument(1, src::createMatcher()), - hasArgument(2, size::createMatcher())))); - Finder->addMatcher(Expression.bind(ExpressionRef), this); + const auto ExpressionM = callExpr( + callee(OffendingDeclM), optionally(ReturnValueUsedM), + allOf(optionally(hasArgument(dst::ArgIndex, dst::createMatcher())), + optionally(hasArgument(src::ArgIndex, src::createMatcher())), + optionally(hasArgument(size::ArgIndex, size::createMatcher())))); + Finder->addMatcher(ExpressionM.bind(ExpressionRef), this); } void ReplaceWithStdCopyCheck::registerPPCallbacks( @@ -308,132 +323,186 @@ void ReplaceWithStdCopyCheck::registerPPCallbacks( void ReplaceWithStdCopyCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "IncludeStyle", Inserter.getStyle()); - Options.store(Opts, "FlaggableCallees", FlaggableCallees_); + Options.store(Opts, "FlagMemcpy", FlagMemcpy); } -#include "llvm/Support/Debug.h" -#define DEBUG_TYPE "clang-tidy" - void ReplaceWithStdCopyCheck::check(const MatchFinder::MatchResult &Result) { const auto &CallNode = *Result.Nodes.getNodeAs(ExpressionRef); - - auto dstVT = dst::extractValueType(Result); - auto srcVT = src::extractValueType(Result); - - // basis for converting size argument to std::copy_n's when issuing fixit - auto valueTypeWidth = Result.Context->getTypeSizeInChars(dstVT); - - // Don't report cases where value type widths differ, as this might indicate - // [de]serialization and hard to reason about replacements using std::copy[_n] - if (valueTypeWidth != Result.Context->getTypeSizeInChars(srcVT)) + llvm::errs() << __LINE__ << ' ' + << Lexer::getSourceText( + CharSourceRange::getTokenRange(CallNode.getSourceRange()), + *Result.SourceManager, getLangOpts()) + << '\n'; + + auto Dst = dst::extractNode(CallNode, Result); + auto Src = src::extractNode(CallNode, Result); + auto Size = size::extractNode(CallNode, Result); + + const auto *DstVT = dst::extractValueType(Result); + if (DstVT == nullptr) + return; + const auto *SrcVT = src::extractValueType(Result); + if (SrcVT == nullptr) return; - auto dst = dst::extractNode(CallNode, Result); - auto src = src::extractNode(CallNode, Result); - auto size = size::extractNode(CallNode, Result); - - // only have this function return a non-empty optional if the form of the size - // argument strongly indicates collection-related usage - auto exprAsString = [&](const Expr *Node) { + auto ExprAsString = [&](const Expr &Node) { return Lexer::getSourceText( - CharSourceRange::getTokenRange(Node->getSourceRange()), + CharSourceRange::getTokenRange(Node.getSourceRange()), *Result.SourceManager, getLangOpts()) .str(); }; - auto extractComponents = [&]() -> std::optional { + // only have this function return a non-empty optional if the form of the size + // argument strongly indicates collection-related usage + auto ExtractComponents = [&]() -> std::optional { if (const auto *NSizeOfExprNode = - std::get_if(&size); + std::get_if(&Size); NSizeOfExprNode != nullptr) { auto &[N, Arg] = *NSizeOfExprNode; - auto sizeOfArgWidth = Result.Context->getTypeSizeInChars(Arg->getType()); - return {{sizeOfArgWidth, exprAsString(N)}}; + auto SizeOfArgWidth = Result.Context->getTypeSizeInChars(Arg.getType()); + return {{SizeOfArgWidth, ExprAsString(N)}}; } - if (const auto *StrlenNode = std::get_if(&size); + if (const auto *StrlenNode = std::get_if(&Size); StrlenNode != nullptr) { - auto strlenArgTypeWidth = Result.Context->getTypeSizeInChars( - StrlenNode->Arg->getType()->getPointeeType()); - return {{strlenArgTypeWidth, exprAsString(CallNode.getArg(2))}}; + auto StrlenArgTypeWidth = Result.Context->getTypeSizeInChars( + StrlenNode->Arg.getType()->getPointeeType()); + return {{StrlenArgTypeWidth, ExprAsString(*CallNode.getArg(2))}}; } return std::nullopt; }; - if (auto maybeComponents = extractComponents(); maybeComponents.has_value()) { - auto [unit, nExpr] = *maybeComponents; - if (unit != valueTypeWidth) { - // __jm__ tricky to offer a helpful fixit here, as the size argument - // doesn't seem to be related to the pointee types of src and dst - return; - } - // __jm__ makes sense to assume - } - // might be a reinterpretation call, only other case where we don't flag - if (std::holds_alternative(dst.Tag) and - std::holds_alternative(src.Tag)) { - // not supported yet, need to come up with a robust heuristic first + bool DstIsRawPtr = std::holds_alternative(Dst.Tag); + bool SrcIsRawPtr = std::holds_alternative(Src.Tag); + + auto CheckIsFalsePositive = [&]() { + // This case could be supported but it requires a robust heuristic + // over the form of the size argument + // to deduce we are dealing with pointers to collections + // When only one of the arguments is a raw pointer, then there are still + // valid scenarios in which it is a singleton to be memmoved over, like + // [de]serialization. + return DstIsRawPtr or SrcIsRawPtr; + }; + if (CheckIsFalsePositive()) return; - } - // both src and dst are collection expressions, enough to wrap them in - // std::begin calls auto Diag = diag(CallNode.getExprLoc(), "prefer std::copy_n to %0") << cast(CallNode.getCalleeDecl()); - // don't issue a fixit if the result of the call is used - if (bool IsReturnValueUsed = - Result.Nodes.getNodeAs(ReturnValueDiscardedRef) == nullptr; - IsReturnValueUsed) + // basis for converting size argument to std::copy_n's when issuing fixit + auto DstTypeWidth = Result.Context->getTypeSizeInChars(*DstVT); + auto SrcTypeWidth = Result.Context->getTypeSizeInChars(*SrcVT); + + auto CheckIsFixable = [&]() { + // If the width types differ, it's hard to reason about what would be a + // helpful replacement, so just don't issue a fixit in this case + if (DstTypeWidth != SrcTypeWidth) + return false; + + // don't issue a fixit if the result of the call is used + if (bool IsReturnValueUsed = + Result.Nodes.getNodeAs(ReturnValueDiscardedRef) == nullptr; + IsReturnValueUsed) + return false; + + return true; + }; + if (not CheckIsFixable()) return; - Diag << FixItHint::CreateRemoval(CallNode.getSourceRange()); - std::string srcFixit = "std::cbegin(" + exprAsString(src.Node) + ")"; - std::string dstFixit = "std::begin(" + exprAsString(dst.Node) + ")"; + // From here we can assume dst and src have equal value type widths + const auto &ValueTypeWidth = DstTypeWidth; + + auto SrcFixit = [&]() { + auto AsString = ExprAsString(Src.Node); + if (SrcIsRawPtr) + return AsString; + return "std::cbegin(" + AsString + ")"; + }(); + + auto DstFixit = [&]() { + auto AsString = ExprAsString(Dst.Node); + if (DstIsRawPtr) + return AsString; + return "std::begin(" + AsString + ")"; + }(); - auto calleeIsWideVariant = + llvm::errs() << "SrcFixit: " << SrcFixit << '\n' + << "DstFixit: " << DstFixit << '\n'; + auto CalleeIsWideVariant = CallNode.getDirectCallee()->getParamDecl(0)->getType()->isWideCharType(); - auto calleeUnit = [&]() { - if (calleeIsWideVariant) { + auto CalleeUnit = [&]() { + if (CalleeIsWideVariant) { return Result.Context->getTypeSizeInChars( (Result.Context->getWideCharType())); } return CharUnits::One(); }(); - auto sizeFixit = [&]() -> std::string { - if (valueTypeWidth == calleeUnit) { - return exprAsString(CallNode.getArg(size::argIndex)); + llvm::errs() << __LINE__ << Size.index() << '\n'; + + llvm::errs() << __LINE__ << " CalleeUnit: " << CalleeUnit.getQuantity() + << '\n'; + auto SizeFixit = [&]() -> std::string { + if (ValueTypeWidth == CalleeUnit) { + // replace sizeof(CArray) with the more modern std::size(CArray) + if (const auto *SizeOfExprNode = + std::get_if(&Size); + SizeOfExprNode != nullptr and + SizeOfExprNode->Arg.getType()->isArrayType()) { + return "std::size(" + ExprAsString(SizeOfExprNode->Arg) + ")"; + } + + return ExprAsString(*CallNode.getArg(size::ArgIndex)); } // try to factor out the unit from the size expression - if (auto maybeSizeComponents = extractComponents(); - maybeSizeComponents.has_value() and - maybeSizeComponents->Unit == valueTypeWidth) { - return maybeSizeComponents->NExpr; + if (auto MaybeSizeComponents = ExtractComponents(); + MaybeSizeComponents.has_value() and + MaybeSizeComponents->Unit == ValueTypeWidth) { + return MaybeSizeComponents->NExpr; } - // last resort, divide by valueTypeWidth - return exprAsString(CallNode.getArg(size::argIndex)) + " / " + "sizeof(" + - dstVT.getAsString() + ")"; + // last resort, divide by ValueTypeWidth + return ExprAsString(*CallNode.getArg(size::ArgIndex)) + " / " + "sizeof(" + + DstVT->getAsString() + ")"; }(); + llvm::errs() << "SizeFixit: " << SizeFixit << '\n'; + + Diag << FixItHint::CreateRemoval(CallNode.getSourceRange()) + << FixItHint::CreateInsertion(CallNode.getBeginLoc(), + "std::copy_n(" + SrcFixit + ", " + + SizeFixit + ", " + DstFixit + ");") + << Inserter.createIncludeInsertion( + Result.SourceManager->getFileID(CallNode.getBeginLoc()), + ""); + + // Modern containers will include a std::[c]begin declaration with their + // header, but not c-arrays (or smart pointers to c-arrays), so need to ensure + // they are included. + auto IsCArrayOrWrapper = [](const decltype(ptrarg::PtrArg::Tag) &Tag) { + return std::holds_alternative(Tag) or + std::holds_alternative(Tag); + }; - Diag << FixItHint::CreateInsertion(CallNode.getEndLoc(), - "std::copy_n(" + srcFixit + ", " + - sizeFixit + ", " + dstFixit + ");"); + if (IsCArrayOrWrapper(Dst.Tag) and IsCArrayOrWrapper(Src.Tag)) + Diag << Inserter.createIncludeInsertion( + Result.SourceManager->getFileID(CallNode.getBeginLoc()), ""); // if (const auto *NSizeofExprNode = // std::get_if(&size); // NSizeofExprNode != nullptr) { // auto &[N, Arg] = *NSizeofExprNode; - // auto sizeOfArgWidth = Result.Context->getTypeSizeInChars(Arg->getType()); + // auto SizeOfArgWidth = Result.Context->getTypeSizeInChars(Arg->getType()); // issueFixitIfWidthsMatch(dst.Node, src.Node, - // {sizeOfArgWidth, exprAsString(N)}); + // {SizeOfArgWidth, ExprAsString(N)}); // return; // } // if (const auto *StrlenNode = std::get_if(&size); // StrlenNode != nullptr) { - // auto strlenArgTypeWidth = Result.Context->getTypeSizeInChars( + // auto StrlenArgTypeWidth = Result.Context->getTypeSizeInChars( // StrlenNode->Arg->getType()->getPointeeType()); // issueFixitIfWidthsMatch( // dst.Node, src.Node, - // {strlenArgTypeWidth, exprAsString(CallNode.getArg(2))}); + // {StrlenArgTypeWidth, ExprAsString(CallNode.getArg(2))}); // return; // } // if (const auto *SizeOfExprNode = @@ -443,15 +512,15 @@ void ReplaceWithStdCopyCheck::check(const MatchFinder::MatchResult &Result) { // if (SizeOfExprNode->Arg->getType()->isArrayType()) { // issueFixitIfWidthsMatch( // dst.Node, src.Node, - // {CharUnits::One(), exprAsString(CallNode.getArg(2))}); + // {CharUnits::One(), ExprAsString(CallNode.getArg(2))}); // return; // } // // __jm__ weird bc we assume dst and src are collections // // if Arg turns out to have the same type as dst or src then just suggest // // copy via the assignment operator - // if (auto argType = Arg->getType(); argType == dstVT or argType == srcVT) + // if (auto argType = Arg->getType(); argType == DstVT or argType == SrcVT) // { - // issueFixit{valueTypeWidth, exprAsString(CallNode.getArg(2))}; + // issueFixit{ValueTypeWidth, ExprAsString(CallNode.getArg(2))}; // return; // } // // __jm__ only flag this as suspicious with a further explanation in the @@ -489,7 +558,7 @@ void ReplaceWithStdCopyCheck::check(const MatchFinder::MatchResult &Result) { // equal to vtw, essentialy fallthrough to 3 // 3. N * sizeof(expr) is okay when expr is a type with width == - // valueTypeWidth and N may be verbatim lifted into copy_n's third argument + // ValueTypeWidth and N may be verbatim lifted into copy_n's third argument // to make sure we can issue a FixIt, need to be pretty sure we're dealing // with a collection and it is a byte collection @@ -538,8 +607,6 @@ void ReplaceWithStdCopyCheck::check(const MatchFinder::MatchResult &Result) { // } else { // } - Diag << Inserter.createIncludeInsertion( - Result.SourceManager->getFileID(CallNode.getBeginLoc()), StdCopyHeader); { // using namespace std::literals; // Diag << FixItHint::CreateReplacement( @@ -562,17 +629,7 @@ void ReplaceWithStdCopyCheck::check(const MatchFinder::MatchResult &Result) { // ")") // .str()); } - // dest source size -> source size dest } -llvm::ArrayRef ReplaceWithStdCopyCheck::getFlaggableCallees() const { - switch (FlaggableCallees_) { - case FlaggableCallees::OnlyMemmove: - return {"::memmove", "::std::memmove", "::wmemmove", "::std::wmemmove"}; - case FlaggableCallees::MemmoveAndMemcpy: - return {"::memmove", "::std::memmove", "::memcpy", "::std::memcpy", - "::wmemmove", "::std::wmemmove", "::wmemcpy", "::std::wmemcpy"}; - } -} } // namespace modernize } // namespace clang::tidy \ No newline at end of file diff --git a/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.h b/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.h index f9cf38bad1d91..e3e317bf7fb5e 100644 --- a/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.h @@ -13,7 +13,6 @@ #include "../utils/IncludeInserter.h" #include "clang/AST/ASTTypeTraits.h" #include "clang/Basic/LangOptions.h" -#include "llvm/ADT/SmallVector.h" namespace clang::tidy::modernize { @@ -21,11 +20,6 @@ namespace clang::tidy::modernize { // calls to std::copy or std::copy_n, depending on the context class ReplaceWithStdCopyCheck : public ClangTidyCheck { public: - enum class FlaggableCallees { - OnlyMemmove, - MemmoveAndMemcpy, - }; - ReplaceWithStdCopyCheck(StringRef Name, ClangTidyContext *Context); ~ReplaceWithStdCopyCheck() override = default; void registerMatchers(ast_matchers::MatchFinder *Finder) override; @@ -41,31 +35,13 @@ class ReplaceWithStdCopyCheck : public ClangTidyCheck { void check(const ast_matchers::MatchFinder::MatchResult &Result) override; -private: - void handleMemcpy(); - void handleMemset(); - - void renameFunction(DiagnosticBuilder &Diag, const CallExpr *MemcpyNode); - void reorderArgs(DiagnosticBuilder &Diag, const CallExpr *MemcpyNode); - void insertHeader(DiagnosticBuilder &Diag, const CallExpr *MemcpyNode, - SourceManager *const SM); - bool checkIsByteSequence(const ast_matchers::MatchFinder::MatchResult &Result, - std::string_view Prefix); - private: void tryIssueFixIt(const ast_matchers::MatchFinder::MatchResult &Result, const DiagnosticBuilder &Diag, const CallExpr &CallNode); - llvm::ArrayRef getFlaggableCallees() const; utils::IncludeInserter Inserter; - // __jm__ TODO options: - // 1. list of functions that can be replaced, memmove by default, - // and memcpy as opt-in - // 2. should calls to {begin,end,size} be replaced - // with their members or free fns. - static constexpr auto FlaggableCalleesDefault = FlaggableCallees::OnlyMemmove; - const FlaggableCallees FlaggableCallees_; + const bool FlagMemcpy; }; } // namespace clang::tidy::modernize diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-with-std-copy-small.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-with-std-copy-small.cpp new file mode 100644 index 0000000000000..cc3845dcce9cc --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-with-std-copy-small.cpp @@ -0,0 +1,150 @@ +// RUN: %check_clang_tidy %s modernize-replace-with-std-copy %t + +//CHECK-FIXES: #include + +namespace { +using size_t = decltype(sizeof(int)); + +void *memcpy(void *__restrict dest, const void *__restrict src, size_t n) { + return nullptr; +} +void *memmove(void *__restrict dest, const void *__restrict src, size_t n) { + return nullptr; +} +} // namespace + +namespace std { +using int64_t = long long ; +using int32_t = int ; +using int16_t = short ; +using int8_t = char ; + +using char32 = int32_t; +using char16 = int16_t; +using char8 = int8_t; + +template +class allocator {}; +template +class char_traits {}; +template +struct basic_string { + typedef basic_string _Type; + basic_string(); + basic_string(const C* p, const A& a = A()); + + const C* c_str() const; + const C* data() const; + + _Type& append(const C* s); + _Type& append(const C* s, size_t n); + _Type& assign(const C* s); + _Type& assign(const C* s, size_t n); + + int compare(const _Type&) const; + int compare(const C* s) const; + int compare(size_t pos, size_t len, const _Type&) const; + int compare(size_t pos, size_t len, const C* s) const; + + size_t find(const _Type& str, size_t pos = 0) const; + size_t find(const C* s, size_t pos = 0) const; + size_t find(const C* s, size_t pos, size_t n) const; + + _Type& insert(size_t pos, const _Type& str); + _Type& insert(size_t pos, const C* s); + _Type& insert(size_t pos, const C* s, size_t n); + + _Type& operator+=(const _Type& str); + _Type& operator+=(const C* s); + _Type& operator=(const _Type& str); + _Type& operator=(const C* s); +}; + +using string = basic_string, std::allocator>; +using wstring = basic_string,std::allocator>; +using u16string = basic_string, std::allocator>; +using u32string = basic_string, std::allocator>; + + + +void *memcpy(void *__restrict dest, const void *__restrict src, size_t n); +void *memmove(void *__restrict dest, const void *__restrict src, size_t n); + +template struct vector { + vector(size_t); + + T *data(); + const T *data() const; + size_t size() const; + void resize(size_t); + using value_type = T; +}; + +template +T* data(vector&); + +template +T* data(T[]); + +template +const T* data(const vector&); + +template +const T* data(const T[]); + +size_t size(void *); + +size_t strlen(const char *); +} // namespace std + + +namespace { +void notSupportedEx() { + char Source[] = "once upon a daydream..."; + + auto *PrimitiveDest = new std::int8_t; + std::memmove(PrimitiveDest, Source, sizeof PrimitiveDest); + + double D = 0.1; + std::int64_t N; + // don't warn on calls over non-sequences + std::memmove(&N, &D, sizeof D); +} + +void noFixItEx() { + // value type widths are different and so a fix is not straightforward + int Source[5]; + std::vector Dest(20); + + memmove(std::data(Dest), Source, 20); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'memmove' +} + +void sequenceOfBytesEx() { + { + char Source[] = "once upon a daydream..."; + char Dest[4]; + + std::memmove(Dest, Source, sizeof Dest); + // CHECK-MESSAGES: [[@LINE-1]]:9: warning: prefer std::copy_n to 'memmove' + // CHECK-FIXES: std::copy_n(std::cbegin(Source), std::size(Dest), std::begin(Dest)); + + memmove(std::data(Dest), std::data(Source), sizeof Dest); + // CHECK-MESSAGES: [[@LINE-1]]:9: warning: prefer std::copy_n to 'memmove' + // CHECK-FIXES: std::copy_n(std::cbegin(Source), std::size(Dest), std::begin(Dest)); + } + + { + char Source[] = "once upon a daydream..."; + std::vector Dest(4); + + std::memmove(Dest.data(), Source, Dest.size()); + // CHECK-MESSAGES: [[@LINE-1]]:9: warning: prefer std::copy_n to 'memmove' + // CHECK-FIXES: std::copy_n(std::cbegin(Source), Dest.size(), std::begin(Dest)); + + std::memmove(std::data(Dest), Source, Dest.size()); + // CHECK-MESSAGES: [[@LINE-1]]:9: warning: prefer std::copy_n to 'memmove' + // CHECK-FIXES: std::copy_n(std::cbegin(Source), Dest.size(), std::begin(Dest)); + } +} +} // namespace \ No newline at end of file diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-with-std-copy.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-with-std-copy.cpp index 3d0dc18051676..72d57f7c140e9 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-with-std-copy.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-with-std-copy.cpp @@ -1,4 +1,5 @@ -// RUN: %check_clang_tidy %s modernize-replace-with-std-copy %t +// RUN: %check_clang_tidy %s modernize-replace-with-std-copy %t -- \ +// RUN: -config='{CheckOptions: {modernize-replace-with-std-copy.FlagMemcpy: true}}' // possible call scenarios, infeasible to cover all // replacement type: From 9d03f98621dea25ea018bbb18c9ab9ab2a737694 Mon Sep 17 00:00:00 2001 From: kidp330 Date: Sun, 5 Jan 2025 18:17:34 +0100 Subject: [PATCH 06/15] Add check to clangd --- clang-tools-extra/clangd/TidyFastChecks.inc | 1 + 1 file changed, 1 insertion(+) diff --git a/clang-tools-extra/clangd/TidyFastChecks.inc b/clang-tools-extra/clangd/TidyFastChecks.inc index de1a025602fa9..a5d40a486976a 100644 --- a/clang-tools-extra/clangd/TidyFastChecks.inc +++ b/clang-tools-extra/clangd/TidyFastChecks.inc @@ -326,6 +326,7 @@ FAST(modernize-redundant-void-arg, 1.0) FAST(modernize-replace-auto-ptr, 0.0) FAST(modernize-replace-disallow-copy-and-assign-macro, -0.0) FAST(modernize-replace-random-shuffle, 1.0) +FAST(modernize-replace-with-std-copy, 1.0) FAST(modernize-return-braced-init-list, 1.0) FAST(modernize-shrink-to-fit, 1.0) FAST(modernize-type-traits, 1.0) From 6c2d8619e3574c4b71c6c6bc431dbe3d5e3c37ad Mon Sep 17 00:00:00 2001 From: kidp330 Date: Sun, 5 Jan 2025 18:17:55 +0100 Subject: [PATCH 07/15] Rename check .rst file --- ...with-stdcopy.rst => replace-with-std-copy.rst} | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) rename clang-tools-extra/docs/clang-tidy/checks/modernize/{replace-memcpy-with-stdcopy.rst => replace-with-std-copy.rst} (63%) diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/replace-memcpy-with-stdcopy.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/replace-with-std-copy.rst similarity index 63% rename from clang-tools-extra/docs/clang-tidy/checks/modernize/replace-memcpy-with-stdcopy.rst rename to clang-tools-extra/docs/clang-tidy/checks/modernize/replace-with-std-copy.rst index 119a3184208c2..7a28ab75ac2b9 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/modernize/replace-memcpy-with-stdcopy.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/replace-with-std-copy.rst @@ -3,18 +3,19 @@ modernize-replace-with-stdcopy =================================== -Replaces all occurrences of the C ``memcpy`` function with ``std::copy`` +Replaces all occurrences of the C ``memmove`` function and its wide-char variant with ``std::copy_n``. +Replacement of ``memcpy`` is optionally also supported. Example: .. code-block:: c++ /*! - * \param destination Pointer to the destination array where the content is to be copied - * \param source Pointer to the source of data to be copied - * \param num Number of bytes to copy + * \param dst Pointer to the destination array where the content is to be copied + * \param src Pointer to the source of data to be copied + * \param size Number of bytes to copy */ - memcpy(destination, source, num); + memcpy(dst, src, size); becomes @@ -25,7 +26,7 @@ becomes * \param source Pointer to the source of data to be copied * \param num Number of bytes to copy */ - std::copy(source, source + (num / sizeof *source), destination); + std::copy_n(std::cbegin(src), size, std::begin(dst)); Bytes to iterator conversion ---------------------------- @@ -36,7 +37,7 @@ In order to make the check working, it will convert the size parameter to an ite Header inclusion ---------------- -``std::copy`` being provided by the ``algorithm`` header file, this check will include it if needed. +``std::copy_n`` is provided by the ``algorithm`` header file, this check will include it if needed. Options ------- From c6afed0e96e0c781239cfc19f1d33e46b77f9eb0 Mon Sep 17 00:00:00 2001 From: kidp330 Date: Mon, 6 Jan 2025 23:46:44 +0100 Subject: [PATCH 08/15] Split test suite into multiple files --- .../modernize/replace-with-std-copy-nofix.cpp | 85 ++++ .../modernize/replace-with-std-copy-small.cpp | 8 +- .../modernize/replace-with-std-copy.cpp | 382 +++++++++--------- 3 files changed, 285 insertions(+), 190 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/replace-with-std-copy-nofix.cpp diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-with-std-copy-nofix.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-with-std-copy-nofix.cpp new file mode 100644 index 0000000000000..6cdba0d0b8f74 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-with-std-copy-nofix.cpp @@ -0,0 +1,85 @@ +// RUN: %check_clang_tidy %s modernize-replace-with-std-copy %t -- \ +// RUN: -config='{CheckOptions: {modernize-replace-with-std-copy.FlagMemcpy: true}}' \ +// RUN: | FileCheck %s -implicit-check-not="{{FIX-IT}}" + +namespace std { +using size_t = decltype(sizeof(int)); + +void *memcpy(void *__restrict dest, const void *__restrict src, size_t n); +void *memmove(void *__restrict dest, const void *__restrict src, size_t n); + +template struct vector { + vector(size_t); + + T *data(); + const T *data() const; + size_t size() const; + using value_type = T; +}; + +template +typename Container::value_type* data(Container& Arg) { + return Arg.data(); +} + +template +T* data(T Arg[]); + +template +const typename Container::value_type* data(const Container& Arg) { + return Arg.data(); +} + +template +const T* data(const T[]); + +} // namespace std + +namespace { +using size_t = std::size_t; + +void *memmove(void *__restrict dest, const void *__restrict src, size_t n) { + return nullptr; +} +} // namespace + +namespace { +void noFixItEx() { + { // different value type widths + int Source[10]; + std::vector Dest(5); + + memmove(Source, std::data(Dest), 1); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'memmove' + std::memmove(Source, std::data(Dest), 1); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'memmove' + memmove(std::data(Source), Dest.data(), 1); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'memmove' + std::memmove(std::data(Source), Dest.data(), 1); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'memmove' + memmove(std::data(Source), std::data(Dest), 1); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'memmove' + std::memmove(std::data(Source), std::data(Dest), 1); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'memmove' + } + + { // return value used + int Source[10]; + std::vector Dest(5); + + void* Vptr = nullptr; + if (memmove(Source, Dest.data(), 1) != nullptr) { + // CHECK-MESSAGES: [[@LINE-1]]:9: warning: prefer std::copy_n to 'memmove' + Vptr = memmove(Source, Dest.data(), 1); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: prefer std::copy_n to 'memmove' + } + Vptr = [&]() { + return memmove(Source, Dest.data(), 1); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: prefer std::copy_n to 'memmove' + }(); + + for (;Vptr != nullptr; Vptr = memmove(Source, Dest.data(), 1)) {} + // CHECK-MESSAGES: [[@LINE-1]]:35: warning: prefer std::copy_n to 'memmove' + } +} +} // namespace \ No newline at end of file diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-with-std-copy-small.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-with-std-copy-small.cpp index cc3845dcce9cc..469faef619619 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-with-std-copy-small.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-with-std-copy-small.cpp @@ -109,6 +109,12 @@ void notSupportedEx() { std::int64_t N; // don't warn on calls over non-sequences std::memmove(&N, &D, sizeof D); + + std::vector Dest(4); + + // don't warn on memcpy by default + memcpy(Dest.data(), Source, Dest.size()); + std::memcpy(std::data(Dest), Source, Dest.size()); } void noFixItEx() { @@ -120,7 +126,7 @@ void noFixItEx() { // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'memmove' } -void sequenceOfBytesEx() { +void supportedEx() { { char Source[] = "once upon a daydream..."; char Dest[4]; diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-with-std-copy.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-with-std-copy.cpp index 72d57f7c140e9..8927e4b1da2e4 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-with-std-copy.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-with-std-copy.cpp @@ -2,12 +2,10 @@ // RUN: -config='{CheckOptions: {modernize-replace-with-std-copy.FlagMemcpy: true}}' // possible call scenarios, infeasible to cover all -// replacement type: -// [ ] members when possible // [X] all free // source: // type: -// [ ] T[] +// [ ] c-array // std::data: // [ ] free // [ ] N/A @@ -49,22 +47,14 @@ // [ ] y // [ ] n -// CHECK-FIXES: #include - -namespace { - +namespace std { using size_t = decltype(sizeof(int)); -namespace std { using int64_t = long long ; using int32_t = int ; using int16_t = short ; using int8_t = char ; -using char32 = int32_t; -using char16 = int16_t; -using char8 = int8_t; - template class allocator {}; template @@ -75,38 +65,35 @@ struct basic_string { basic_string(); basic_string(const C* p, const A& a = A()); - const C* c_str() const; const C* data() const; + C* data(); + size_t size() const; +}; - _Type& append(const C* s); - _Type& append(const C* s, size_t n); - _Type& assign(const C* s); - _Type& assign(const C* s, size_t n); +using string = basic_string, std::allocator>; +using wstring = basic_string,std::allocator>; +using u16string = basic_string, std::allocator>; +using u32string = basic_string, std::allocator>; - int compare(const _Type&) const; - int compare(const C* s) const; - int compare(size_t pos, size_t len, const _Type&) const; - int compare(size_t pos, size_t len, const C* s) const; +template +class basic_string_view { +public: + basic_string_view(); - size_t find(const _Type& str, size_t pos = 0) const; - size_t find(const C* s, size_t pos = 0) const; - size_t find(const C* s, size_t pos, size_t n) const; + basic_string_view(const CharT *); - _Type& insert(size_t pos, const _Type& str); - _Type& insert(size_t pos, const C* s); - _Type& insert(size_t pos, const C* s, size_t n); + basic_string_view(const CharT *, size_t); - _Type& operator+=(const _Type& str); - _Type& operator+=(const C* s); - _Type& operator=(const _Type& str); - _Type& operator=(const C* s); -}; + basic_string_view(const basic_string_view &); -using string = basic_string, std::allocator>; -using wstring = basic_string,std::allocator>; -using u16string = basic_string, std::allocator>; -using u32string = basic_string, std::allocator>; + basic_string_view &operator=(const basic_string_view &); + + const CharT* data() const; + + size_t size() const; +}; +using string_view = basic_string_view; void *memcpy(void *__restrict dest, const void *__restrict src, size_t n); @@ -122,200 +109,217 @@ template struct vector { using value_type = T; }; -template -T* data(vector&); +template +struct array { + T* begin(); + T* end(); + const T* begin() const; + const T* end() const; + + size_t size() const; + + const T* data() const; + T* data(); + + T _data[N]; +}; template -T* data(T[]); +struct span { + template + span(T (&arr)[N]); + + const T* data() const; + T* data(); + + using value_type = T; +}; + +template +typename Container::value_type* data(Container& Arg) { + return Arg.data(); +} template -const T* data(const vector&); +T* data(T Arg[]); + +template +const typename Container::value_type* data(const Container& Arg) { + return Arg.data(); +} template const T* data(const T[]); size_t size(void *); +template +size_t size(const Container& c) { + return c.size(); +} + size_t strlen(const char *); + +wchar_t* wmemcpy( wchar_t* dest, const wchar_t* src, size_t count ); +wchar_t* wmemmove( wchar_t* dest, const wchar_t* src, size_t count ); + +template +struct unique_ptr { + unique_ptr(T); + T *get() const; + explicit operator bool() const; + void reset(T *ptr); + T &operator*() const; + T *operator->() const; + T& operator[](size_t i) const; +}; } // namespace std -void *memcpy(void *__restrict dest, const void *__restrict src, size_t n); -void *memmove(void *__restrict dest, const void *__restrict src, size_t n); -} // namespace +namespace { +using size_t = std::size_t; +void *memcpy(void *__restrict dest, const void *__restrict src, size_t n) { + return nullptr; +} +void *memmove(void *__restrict dest, const void *__restrict src, size_t n) { + return nullptr; +} +wchar_t* wmemcpy( wchar_t* dest, const wchar_t* src, size_t count ) { + return nullptr; +} +wchar_t* wmemmove( wchar_t* dest, const wchar_t* src, size_t count ) { + return nullptr; +} +} // namespace namespace { void notSupportedEx() { - char Source[] = "once upon a daydream...", Dest[4]; - - auto *PrimitiveDest = new std::int8_t; - std::memcpy(PrimitiveDest, Source, sizeof PrimitiveDest); - - auto *PrimitiveDest2 = new std::int16_t; - std::memcpy(PrimitiveDest2, Source, sizeof PrimitiveDest); - std::memcpy(PrimitiveDest2, Source, sizeof PrimitiveDest2); - - double D = 0.1; - std::int64_t N; - // don't warn on calls over non-sequences - std::memcpy(&N, &D, sizeof D); - - // object creation in destination buffer - struct StructType { - int X{42}; - void *operator new(size_t, void *) noexcept { return nullptr; } - } Struct; - alignas(StructType) char Buf[sizeof(StructType)]; - StructType *Ps = new (Buf) StructType; // placement new - // // don't warn on calls over non-sequences - std::memcpy(Ps, &Struct, sizeof Struct); - - const char *PtrSource = "once upon a daydream..."; - char *PtrDest = new char[4]; - std::memcpy(Dest, PtrSource, sizeof Dest); - std::memcpy(PtrDest, Source, 4); -} -void noFixItEx() { - { - int Source[10]; - std::vector Dest(5); - - memmove(Source, std::data(Dest), 1); - // CHECK-MESSAGES: [[@LINE-1]]:20: warning: prefer std::copy_n to memcpy - std::memmove(Source, std::data(Dest), 1); - memmove(std::data(Source), std::data(Dest), 1); - std::memmove(std::data(Source), std::data(Dest), 1); - memmove(std::data(Source), std::data(Dest), 1); - std::memmove(std::data(Source), std::data(Dest), 1); + { // pointee is not a collection + char Source[] = "once upon a daydream..."; + auto *PrimitiveDest = new std::int8_t; + + std::memcpy(PrimitiveDest, Source, sizeof PrimitiveDest); } - char Source[] = "once upon a daydream...", Dest[4]; - // no FixIt when return value is used + { // reinterpretation + double D = 0.1; + std::int64_t N; + // don't warn on calls over non-sequences + std::memcpy(&N, &D, sizeof D); + } + + { // [de]serialization + struct ComplexObj { + int A; + float B; + }; + auto *Obj = new ComplexObj(); + + char Src[sizeof(ComplexObj)] = {0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08}; + std::memcpy(Obj, Src, sizeof(ComplexObj)); + char Dst[sizeof(ComplexObj)]; + std::memcpy(Dst, Obj, sizeof(ComplexObj)); + } + + { // incomplete array type (should be treated the same as a raw ptr) + char Src[] = "once upon a daydream..."; + auto CopySrc = [&Src](char Dst[], size_t sz) { + std::memcpy(Dst, Src, sz); + }; - [](auto){}(std::memcpy(Dest, Source, sizeof Dest)); - // CHECK-MESSAGES: [[@LINE-1]]:20: warning: prefer std::copy_n to memcpy + char Dst[4]; + CopySrc(Dst, 4); + } - std::vector VecI16(4); - // not a supported type, should be a sequence of bytes, otherwise it is difficult to compute the n in copy_n - std::memcpy(VecI16.data(), Source, - VecI16.size() * sizeof(decltype(VecI16)::value_type)); - // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy + { // pointers + const char *Src = "once upon a daydream..."; + char *Dst = new char[64]; - std::memcpy(std::data(VecI16), Source, - VecI16.size() * sizeof(decltype(VecI16)::value_type)); - // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy + std::memcpy(Dst, Src, std::strlen(Src)); + } } -void sequenceOfBytesEx() { - // the check should support memcpy conversion for the following types: - // T[] - // std::vector - // std::span - // std::deque - // std::array - // std::string - // std::string_view - // where T is byte-like - - { - char Source[] = "once upon a daydream..."; - char Dest[4]; +void supportedEx() { + { // two wchar c-arrays + wchar_t Src[] = L"once upon a daydream..."; + wchar_t Dst[4]; - std::memcpy(Dest, Source, sizeof Dest); - // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy - // CHECK-FIXES: std::copy_n(std::begin(Source), std::size(Dest), std::begin(Dest)); + std::wmemcpy(Dst, Src, sizeof Dst); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'wmemcpy' + // CHECK-FIXES: std::copy_n(std::cbegin(Src), std::size(Dst), std::begin(Dst)); - std::memcpy(std::data(Dest), Source, sizeof Dest); - // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy - // CHECK-FIXES: std::copy_n(std::begin(Source), std::size(Dest), std::begin(Dest)); + std::wmemcpy(Dst, Src, sizeof(Dst)); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'wmemcpy' + // CHECK-FIXES: std::copy_n(std::cbegin(Src), std::size(Dst), std::begin(Dst)); - std::memcpy(Dest, std::data(Source), sizeof Dest); - // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy - // CHECK-FIXES: std::copy_n(std::begin(Source), std::size(Dest), std::begin(Dest)); + wmemcpy(Dst, Src, sizeof Src); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'wmemcpy' + // CHECK-FIXES: std::copy_n(std::cbegin(Src), std::size(Src), std::begin(Dst)); - std::memcpy(std::data(Dest), std::data(Source), sizeof Dest); - // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy - // CHECK-FIXES: std::copy_n(std::begin(Source), std::size(Dest), std::begin(Dest)); + wmemmove(Dst, Src, sizeof(Src)); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'wmemmove' + // CHECK-FIXES: std::copy_n(std::cbegin(Src), std::size(Src), std::begin(Dst)); + } - // __jm__ warn on global call as well - memcpy(Dest, Source, sizeof Dest); - // CHECK-MESSAGES: [[@LINE-1]]:3: warning: prefer std::copy_n to memcpy - // CHECK-FIXES: std::copy_n(std::begin(Source), std::size(Dest), std::begin(Dest)); + { // std::string + std::vector + std::string Src = "once upon a daydream..."; + std::vector Dst(4); - memcpy(std::data(Dest), Source, sizeof Dest); - // CHECK-MESSAGES: [[@LINE-1]]:3: warning: prefer std::copy_n to memcpy - // CHECK-FIXES: std::copy_n(std::begin(Source), std::size(Dest), std::begin(Dest)); + std::memcpy(Dst.data(), Src.data(), Dst.size()); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'memcpy' + // CHECK-FIXES: std::copy_n(std::cbegin(Src), Dst.size(), std::begin(Dst)); + } - memcpy(Dest, std::data(Source), sizeof Dest); - // CHECK-MESSAGES: [[@LINE-1]]:3: warning: prefer std::copy_n to memcpy - // CHECK-FIXES: std::copy_n(std::begin(Source), std::size(Dest), std::begin(Dest)); + { // std::string + std::vector + std::string Src = "once upon a daydream..."; + std::vector Dst(4); - memcpy(std::data(Dest), std::data(Source), sizeof Dest); - // CHECK-MESSAGES: [[@LINE-1]]:3: warning: prefer std::copy_n to memcpy - // CHECK-FIXES: std::copy_n(std::begin(Source), std::size(Dest), std::begin(Dest)); + std::memmove(Dst.data(), Src.data(), Src.size()); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'memmove' + // CHECK-FIXES: std::copy_n(std::cbegin(Src), Src.size(), std::begin(Dst)); } - { - char Source[] = "once upon a daydream..."; - std::vector Dest(4); + { // std::wstring + std::unique_ptr + std::wstring Src = L"once upon a daydream..."; + std::unique_ptr Dst(new wchar_t[16]); + + std::wmemcpy(*Dst, Src.data(), sizeof(*Dst)); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'wmemcpy' + // CHECK-FIXES: std::copy_n(std::cbegin(Src), std::size(*Dst), std::begin(*Dst)); + } - std::memcpy(Dest.data(), Source, Dest.size()); - // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy - // CHECK-FIXES: std::copy_n(std::begin(Source), std::size(Dest), std::begin(Dest)); + { // std::string_view + std::array + std::string_view Src = "once upon a daydream..."; + std::array Dst; - std::memcpy(std::data(Dest), Source, Dest.size()); - // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy - // CHECK-FIXES: std::copy_n(std::begin(Source), std::size(Dest), std::begin(Dest)); + std::memmove(Dst.data(), Src.data(), Dst.size() - 1); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'memmove' + // CHECK-FIXES: std::copy_n(std::cbegin(Src), Dst.size() - 1, std::begin(Dst)); } - { - std::vector Source(10); - char Dest[4]; - - std::memcpy(Dest, Source.data(), Source.size()); - // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy - // CHECK-FIXES: std::copy_n(std::begin(Source), std::size(Source), std::begin(Dest)); - std::memcpy(Dest, Source.data(), sizeof(Dest)); - // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy - // CHECK-FIXES: std::copy_n(Source.data(), std::size(Dest), std::begin(Dest)); - std::memcpy(Dest, Source.data(), std::size(Dest)); - // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy - // CHECK-FIXES: std::copy_n(std::begin(Source), std::size(Source), std::begin(Source)); - - std::memcpy(Dest, Source.data(), 1 + Source.size() / 2); - // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy - // CHECK-FIXES: std::copy_n(std::begin(Source), 1 + std::size(Source) / 2, std::begin(Dest)); - std::memcpy(Dest, Source.data(), 1 + sizeof(Dest) / 2); - // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy - // CHECK-FIXES: std::copy_n(Source.data(), 1 + std::size(Dest) / 2, std::begin(Dest)); - std::memcpy(Dest, Source.data(), 1 + std::size(Dest) / 2); - // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy - // CHECK-FIXES: std::copy_n(std::begin(Source), 1 + std::size(Dest) / 2, std::begin(Source)); - - std::memcpy(Dest, std::data(Source), Source.size()); - // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy - // CHECK-FIXES: std::copy_n(std::begin(Source), std::size(Source), std::begin(Dest)); - std::memcpy(Dest, std::data(Source), sizeof(Dest)); - // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy - // CHECK-FIXES: std::copy_n(Source.data(), std::size(Dest), std::begin(Dest)); - std::memcpy(Dest, std::data(Source), std::size(Dest)); - // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy - // CHECK-FIXES: std::copy_n(std::begin(Source), std::size(Source), std::begin(Source)); - - std::memcpy(Dest, std::data(Source), 1 + Source.size() / 2); - // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy - // CHECK-FIXES: std::copy_n(std::begin(Source), 1 + std::size(Source) / 2, std::begin(Dest)); - std::memcpy(Dest, std::data(Source), 1 + sizeof(Dest) / 2); - // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy - // CHECK-FIXES: std::copy_n(Source.data(), 1 + std::size(Dest) / 2, std::begin(Dest)); - std::memcpy(Dest, std::data(Source), 1 + std::size(Dest) / 2); - // CHECK-MESSAGES: [[@LINE-1]]:8: warning: prefer std::copy_n to memcpy - // CHECK-FIXES: std::copy_n(std::begin(Source), 1 + std::size(Dest) / 2, std::begin(Source)); + { // using namespace std; + using namespace std; + string_view Src = "once upon a daydream..."; + array Dst; + + memmove(Dst.data(), Src.data(), Src.size() - 2); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'memmove' + // CHECK-FIXES: std::copy_n(std::cbegin(Src), Src.size() - 2, std::begin(Dst)); } - // __jm__ make configurable whether stl containers should use members or free fns. - // __jm__ for now use free fns. only + { // NSizeOfExpr cases + std::int32_t Data[] = {1, 2, 3, 4, 1, 2, 3, 4}; + std::span Src{Data}; + std::vector Dst(8); + + memcpy(Dst.data(), Src.data(), Dst.size() * sizeof(std::int32_t)); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'memcpy' + // CHECK-FIXES: std::copy_n(std::cbegin(Src), Dst.size(), std::begin(Dst)); + memmove(std::data(Dst), std::data(Src), sizeof(int) * std::size(Dst)); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'memcpy' + // CHECK-FIXES: std::copy_n(std::cbegin(Src), std::size(Dst), std::begin(Dst)); + } } } // namespace \ No newline at end of file From 7467c7226f95a042d0da208cf0a95dbb6e9aaa2d Mon Sep 17 00:00:00 2001 From: kidp330 Date: Mon, 6 Jan 2025 23:47:20 +0100 Subject: [PATCH 09/15] WIP size argument semantics --- .../modernize/ReplaceWithStdCopyCheck.cpp | 211 ++++++++++-------- 1 file changed, 117 insertions(+), 94 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.cpp b/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.cpp index 54f5a29947dda..28d755e6767f3 100644 --- a/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "ReplaceWithStdCopyCheck.h" +#include "ReplaceAutoPtrCheck.h" #include "clang/AST/ASTContext.h" #include "clang/AST/DeclTemplate.h" #include "clang/AST/Expr.h" @@ -38,7 +39,6 @@ struct Refs { size_t FallbackParameterIdx; llvm::StringLiteral ValueType; llvm::StringLiteral PtrCastFnReturnType; - llvm::StringLiteral AsSmartPtr; }; // rn matchers have a problem with binding types correctly, clang-query build up // the matchers from this file tmrw for debugging @@ -46,13 +46,12 @@ template auto createPtrArgMatcher() { constexpr Refs Refs = RefsT::Refs; auto AllowedContainerNamesM = []() { - if constexpr (true) - return hasAnyName("::std::deque", "::std::forward_list", "::std::list", - "::std::vector", "::std::basic_string"); - else - return hasAnyName("::std::deque", "::std::forward_list", "::std::list", - "::std::vector", "::std::basic_string", - "::std::basic_string_view", "::std::span"); + // return hasAnyName("::std::deque", "::std::forward_list", "::std::list", + // "::std::vector", "::std::basic_string", + // "::std::array"); + return hasAnyName("::std::deque", "::std::forward_list", "::std::list", + "::std::vector", "::std::basic_string", "::std::array", + "::std::basic_string_view", "::std::span"); }(); auto AllowedContainerTypeM = hasUnqualifiedDesugaredType( @@ -61,19 +60,15 @@ template auto createPtrArgMatcher() { hasTemplateArgument(0, templateArgument(refersToType( qualType().bind(Refs.ValueType))))))))); - auto CArrayTypeM = arrayType(hasElementType(qualType().bind(Refs.ValueType))); + auto CArrayTypeM = + constantArrayType(hasElementType(qualType().bind(Refs.ValueType))); auto VariantContainerM = expr(hasType(AllowedContainerTypeM)).bind(Refs.AsContainer); - auto VariantCArrayM = expr(hasType(CArrayTypeM)).bind(Refs.AsCArray); + auto VariantCArrayM = expr(hasType(hasUnqualifiedDesugaredType(CArrayTypeM))) + .bind(Refs.AsCArray); - auto VariantSmartPtrM = - expr(hasType(pointerType(pointee(recordType(hasDeclaration(cxxRecordDecl( - isSameOrDerivedFrom(classTemplateSpecializationDecl( - hasAnyName("::std::unique_ptr", "::std::shared_ptr"), - hasTemplateArgument(0, refersToType(CArrayTypeM))))))))))) - .bind(Refs.AsSmartPtr); auto VariantRawPtrM = expr(hasType(pointerType(pointee(qualType().bind(Refs.ValueType))))); @@ -100,17 +95,16 @@ template auto createPtrArgMatcher() { // the last expr() in anyOf assumes previous matchers are ran eagerly from // left to right, still need to test this is the actual behaviour return expr(anyOf(StdDataMemberCallM, StdDataFreeCallM, VariantCArrayM, - VariantSmartPtrM, VariantRawPtrM)); + VariantRawPtrM)); } namespace tag { struct RawPtr {}; struct Container {}; struct CArray {}; -struct SmartPtr {}; } // namespace tag struct PtrArg { - std::variant Tag; + std::variant Tag; const Expr &Node; }; @@ -124,9 +118,6 @@ PtrArg extractNode(const CallExpr &CallNode, if (const auto *Node = Result.Nodes.getNodeAs(Refs.AsContainer); Node != nullptr) return {tag::Container{}, *Node}; - if (const auto *Node = Result.Nodes.getNodeAs(Refs.AsSmartPtr); - Node != nullptr) - return {tag::SmartPtr{}, *Node}; return {tag::RawPtr{}, *CallNode.getArg(Refs.FallbackParameterIdx)}; } @@ -136,14 +127,28 @@ const QualType *extractValueType(const MatchFinder::MatchResult &Result) { const auto *MaybeRetType = Result.Nodes.getNodeAs(Refs.PtrCastFnReturnType); const auto *ValueType = Result.Nodes.getNodeAs(Refs.ValueType); - llvm::errs() << "MaybeRetType: " << (MaybeRetType == nullptr) << '\n'; - llvm::errs() << "ValueType: " << (ValueType == nullptr) << '\n'; - // checking equality is done here as opposed to when matching because the // equalsBoundNode matcher depends on the match order and the // PtrCastFnReturnType is only present in some scenarios + // if (MaybeRetType != nullptr) + // llvm::errs() + // << "MaybeRetType: " + // << + // MaybeRetType->getCanonicalType().getUnqualifiedType().getAsString() + // << '\n'; + // if (ValueType != nullptr) + // llvm::errs() + // << "ValueType: " + // << ValueType->getCanonicalType().getUnqualifiedType().getAsString() + // << '\n'; + + // stripping qualifiers is necessary in cases like when matching a call + // to const T* data() const; of a std::vector - + // the container value_type (char) is not const qualified, + // but the return type of data() is. if (MaybeRetType != nullptr and - MaybeRetType->getCanonicalType() != ValueType->getCanonicalType()) + MaybeRetType->getCanonicalType().getUnqualifiedType() != + ValueType->getCanonicalType().getUnqualifiedType()) return nullptr; return ValueType; } @@ -152,9 +157,9 @@ const QualType *extractValueType(const MatchFinder::MatchResult &Result) { namespace dst { constexpr size_t ArgIndex = 0; struct RefT { - static constexpr ptrarg::Refs Refs = { - "Dst::AsContainer", "Dst::AsCArray", 0, "Dst::ValueType", - "Dst::PtrCastFnReturnType", "Dst::AsSmartPtr"}; + static constexpr ptrarg::Refs Refs = {"Dst::AsContainer", "Dst::AsCArray", 0, + "Dst::ValueType", + "Dst::PtrCastFnReturnType"}; }; auto createMatcher() { return ptrarg::createPtrArgMatcher(); } @@ -171,9 +176,9 @@ namespace src { constexpr size_t ArgIndex = 1; struct SrcRefsT { - static constexpr ptrarg::Refs Refs = { - "Src::AsContainer", "Src::AsCArray", 1, "Src::ValueType", - "Src::PtrCastFnReturnType", "Src::AsSmartPtr"}; + static constexpr ptrarg::Refs Refs = {"Src::AsContainer", "Src::AsCArray", 1, + "Src::ValueType", + "Src::PtrCastFnReturnType"}; }; auto createMatcher() { return ptrarg::createPtrArgMatcher(); } @@ -196,29 +201,25 @@ constexpr size_t ArgIndex = 2; // 5. other expr namespace variant { struct SizeOfExpr { + const Expr *N = nullptr; const Expr &Arg; }; -struct NSizeOfExpr { - const Expr &N; - const Expr &Arg; +struct SizeOfType { + const Expr *N = nullptr; + const QualType T; }; struct Strlen { const Expr &Arg; }; } // namespace variant -using SizeArg = std::variant; +using SizeArg = std::variant; struct SizeComponents { CharUnits Unit; std::string NExpr; }; -struct IArg { - virtual ~IArg() = default; - virtual SizeComponents extractSizeComponents() = 0; -}; - static constexpr struct Refs { llvm::StringLiteral SizeOfArg; llvm::StringLiteral N; @@ -230,35 +231,39 @@ static constexpr struct Refs { }; auto createMatcher() { - auto NSizeOfExprM = - binaryOperator(hasOperatorName("*"), - hasOperands(expr().bind(Refs.N), - sizeOfExpr(has(expr().bind(Refs.SizeOfArg))))); + auto SizeOfExprM = sizeOfExpr(expr().bind(Refs.SizeOfArg)); + + auto NSizeOfExprM = binaryOperator( + hasOperatorName("*"), hasOperands(expr().bind(Refs.N), SizeOfExprM)); + auto StrlenM = callExpr(callee(functionDecl(hasAnyName( "::strlen", "::std::strlen", "::wcslen", "::std::wcslen", "::strnlen_s", "::std::strnlen_s", "::wcsnlen_s", "::std::wcsnlen_s"))), hasArgument(0, expr().bind(Refs.StrlenArg))); + auto StrlenPlusOneM = binaryOperator( hasOperatorName("+"), hasOperands(StrlenM, integerLiteral(equals(1)))); - auto SizeOfExprM = sizeOfExpr(has(expr().bind(Refs.SizeOfArg))); - return expr(anyOf(NSizeOfExprM, StrlenM, StrlenPlusOneM, SizeOfExprM)); } SizeArg extractNode(const CallExpr &CallNode, const MatchFinder::MatchResult &Result) { + llvm::errs() << "Dumps Size:\n"; + CallNode.dump(); llvm::errs() << __LINE__ << '\n'; - if (const auto *SizeOfArgNode = Result.Nodes.getNodeAs(Refs.SizeOfArg); - SizeOfArgNode != nullptr) { + if (const auto *SizeOfExprNode = + Result.Nodes.getNodeAs(Refs.SizeOfArg); + SizeOfExprNode != nullptr) { llvm::errs() << __LINE__ << '\n'; - if (const auto *NNode = Result.Nodes.getNodeAs(Refs.N); - NNode != nullptr) - return variant::NSizeOfExpr{*NNode, *SizeOfArgNode}; + const auto *NNode = Result.Nodes.getNodeAs(Refs.N); + if (const auto *ArgAsExpr = SizeOfExprNode->getArgumentExpr(); + ArgAsExpr != nullptr) + return variant::SizeOfExpr{NNode, *ArgAsExpr}; llvm::errs() << __LINE__ << '\n'; - return variant::SizeOfExpr{*SizeOfArgNode}; + return variant::SizeOfType{NNode, SizeOfExprNode->getTypeOfArgument()}; } llvm::errs() << __LINE__ << '\n'; if (const auto *StrlenArgNode = Result.Nodes.getNodeAs(Refs.StrlenArg); @@ -327,12 +332,8 @@ void ReplaceWithStdCopyCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { } void ReplaceWithStdCopyCheck::check(const MatchFinder::MatchResult &Result) { + llvm::errs() << __LINE__ << '\n'; const auto &CallNode = *Result.Nodes.getNodeAs(ExpressionRef); - llvm::errs() << __LINE__ << ' ' - << Lexer::getSourceText( - CharSourceRange::getTokenRange(CallNode.getSourceRange()), - *Result.SourceManager, getLangOpts()) - << '\n'; auto Dst = dst::extractNode(CallNode, Result); auto Src = src::extractNode(CallNode, Result); @@ -351,23 +352,52 @@ void ReplaceWithStdCopyCheck::check(const MatchFinder::MatchResult &Result) { *Result.SourceManager, getLangOpts()) .str(); }; + llvm::errs() << "Call: " << ExprAsString(CallNode) << '\n'; // only have this function return a non-empty optional if the form of the size // argument strongly indicates collection-related usage auto ExtractComponents = [&]() -> std::optional { + llvm::errs() << __LINE__ << '\n'; + if (const auto *NSizeOfTypeNode = + std::get_if(&Size); + NSizeOfTypeNode != nullptr) { + llvm::errs() << __LINE__ << '\n'; + auto &[N, T] = *NSizeOfTypeNode; + + auto WidthT = Result.Context->getTypeSizeInChars(T); + auto StrN = N == nullptr ? "1" : ExprAsString(*N); + + return {{WidthT, StrN}}; + } if (const auto *NSizeOfExprNode = - std::get_if(&Size); + std::get_if(&Size); NSizeOfExprNode != nullptr) { + llvm::errs() << __LINE__ << '\n'; auto &[N, Arg] = *NSizeOfExprNode; + if (Arg.getType()->isConstantArrayType()) { + } auto SizeOfArgWidth = Result.Context->getTypeSizeInChars(Arg.getType()); + llvm::errs() << SizeOfArgWidth.getQuantity() << '\n'; return {{SizeOfArgWidth, ExprAsString(N)}}; } + // if (const auto *NSizeOfExprNode = + // std::get_if(&Size); + // NSizeOfExprNode != nullptr) { + // llvm::errs() << __LINE__ << '\n' << "chuj\n"; + // auto &[N, Arg] = *NSizeOfExprNode; + // auto SizeOfArgWidth = + // Result.Context->getTypeSizeInChars(Arg.getType()); llvm::errs() << + // SizeOfArgWidth.getQuantity() << '\n'; return {{SizeOfArgWidth, + // ExprAsString(N)}}; + // } if (const auto *StrlenNode = std::get_if(&Size); StrlenNode != nullptr) { + llvm::errs() << __LINE__ << '\n'; auto StrlenArgTypeWidth = Result.Context->getTypeSizeInChars( StrlenNode->Arg.getType()->getPointeeType()); return {{StrlenArgTypeWidth, ExprAsString(*CallNode.getArg(2))}}; } + llvm::errs() << __LINE__ << '\n'; return std::nullopt; }; @@ -427,10 +457,15 @@ void ReplaceWithStdCopyCheck::check(const MatchFinder::MatchResult &Result) { return "std::begin(" + AsString + ")"; }(); - llvm::errs() << "SrcFixit: " << SrcFixit << '\n' - << "DstFixit: " << DstFixit << '\n'; - auto CalleeIsWideVariant = - CallNode.getDirectCallee()->getParamDecl(0)->getType()->isWideCharType(); + auto CalleeIsWideVariant = [&]() { + const auto *Callee = CallNode.getDirectCallee(); + if (Callee == nullptr) + return false; + auto *ParamDecl = Callee->getParamDecl(0); + if (ParamDecl == nullptr) + return false; + return ParamDecl->getType()->getPointeeType()->isWideCharType(); + }(); auto CalleeUnit = [&]() { if (CalleeIsWideVariant) { @@ -439,33 +474,22 @@ void ReplaceWithStdCopyCheck::check(const MatchFinder::MatchResult &Result) { } return CharUnits::One(); }(); - llvm::errs() << __LINE__ << Size.index() << '\n'; - llvm::errs() << __LINE__ << " CalleeUnit: " << CalleeUnit.getQuantity() - << '\n'; auto SizeFixit = [&]() -> std::string { - if (ValueTypeWidth == CalleeUnit) { - // replace sizeof(CArray) with the more modern std::size(CArray) - if (const auto *SizeOfExprNode = - std::get_if(&Size); - SizeOfExprNode != nullptr and - SizeOfExprNode->Arg.getType()->isArrayType()) { - return "std::size(" + ExprAsString(SizeOfExprNode->Arg) + ")"; - } - - return ExprAsString(*CallNode.getArg(size::ArgIndex)); - } // try to factor out the unit from the size expression + llvm::errs() << __LINE__ << '\n'; if (auto MaybeSizeComponents = ExtractComponents(); - MaybeSizeComponents.has_value() and - MaybeSizeComponents->Unit == ValueTypeWidth) { + MaybeSizeComponents.has_value()) { + + // __jm__ TODO + llvm::errs() << __LINE__ << '\n'; return MaybeSizeComponents->NExpr; } // last resort, divide by ValueTypeWidth + llvm::errs() << __LINE__ << '\n'; return ExprAsString(*CallNode.getArg(size::ArgIndex)) + " / " + "sizeof(" + DstVT->getAsString() + ")"; }(); - llvm::errs() << "SizeFixit: " << SizeFixit << '\n'; Diag << FixItHint::CreateRemoval(CallNode.getSourceRange()) << FixItHint::CreateInsertion(CallNode.getBeginLoc(), @@ -475,15 +499,10 @@ void ReplaceWithStdCopyCheck::check(const MatchFinder::MatchResult &Result) { Result.SourceManager->getFileID(CallNode.getBeginLoc()), ""); - // Modern containers will include a std::[c]begin declaration with their - // header, but not c-arrays (or smart pointers to c-arrays), so need to ensure - // they are included. - auto IsCArrayOrWrapper = [](const decltype(ptrarg::PtrArg::Tag) &Tag) { - return std::holds_alternative(Tag) or - std::holds_alternative(Tag); - }; - - if (IsCArrayOrWrapper(Dst.Tag) and IsCArrayOrWrapper(Src.Tag)) + // All containers will contain an std::[c]begin declaration with their + // definition, with the exception of constant c-arrays + if (std::holds_alternative(Dst.Tag) and + std::holds_alternative(Src.Tag)) Diag << Inserter.createIncludeInsertion( Result.SourceManager->getFileID(CallNode.getBeginLoc()), ""); @@ -491,7 +510,8 @@ void ReplaceWithStdCopyCheck::check(const MatchFinder::MatchResult &Result) { // std::get_if(&size); // NSizeofExprNode != nullptr) { // auto &[N, Arg] = *NSizeofExprNode; - // auto SizeOfArgWidth = Result.Context->getTypeSizeInChars(Arg->getType()); + // auto SizeOfArgWidth = + // Result.Context->getTypeSizeInChars(Arg->getType()); // issueFixitIfWidthsMatch(dst.Node, src.Node, // {SizeOfArgWidth, ExprAsString(N)}); // return; @@ -516,14 +536,17 @@ void ReplaceWithStdCopyCheck::check(const MatchFinder::MatchResult &Result) { // return; // } // // __jm__ weird bc we assume dst and src are collections - // // if Arg turns out to have the same type as dst or src then just suggest + // // if Arg turns out to have the same type as dst or src then just + // suggest // // copy via the assignment operator - // if (auto argType = Arg->getType(); argType == DstVT or argType == SrcVT) + // if (auto argType = Arg->getType(); argType == DstVT or argType == + // SrcVT) // { // issueFixit{ValueTypeWidth, ExprAsString(CallNode.getArg(2))}; // return; // } - // // __jm__ only flag this as suspicious with a further explanation in the + // // __jm__ only flag this as suspicious with a further explanation in + // the // // diagnostic TODO: a sizeof of an unrelated type when copying between // // collections does not make a lot of sense From 80450aadafd5108b8824b9ce914d3e1b0168e6a4 Mon Sep 17 00:00:00 2001 From: kidp330 Date: Tue, 7 Jan 2025 21:45:30 +0100 Subject: [PATCH 10/15] Add more tests --- .../modernize/replace-with-std-copy.cpp | 156 ++++++++++-------- 1 file changed, 88 insertions(+), 68 deletions(-) diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-with-std-copy.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-with-std-copy.cpp index 8927e4b1da2e4..2dc4efcc65971 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-with-std-copy.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-with-std-copy.cpp @@ -1,52 +1,6 @@ // RUN: %check_clang_tidy %s modernize-replace-with-std-copy %t -- \ // RUN: -config='{CheckOptions: {modernize-replace-with-std-copy.FlagMemcpy: true}}' -// possible call scenarios, infeasible to cover all -// [X] all free -// source: -// type: -// [ ] c-array -// std::data: -// [ ] free -// [ ] N/A -// [ ] container -// std::data: -// [ ] free -// [ ] member -// type: -// [ ] std::vector -// [ ] std::span -// [ ] std::deque -// [ ] std::array -// [ ] std::string -// [ ] std::string_view -// dest: -// type: -// [ ] T[] -// std::data: -// [ ] free -// [ ] N/A -// [ ] container -// std::data: -// [ ] free -// [ ] member -// type: -// [ ] std::vector -// [ ] std::span -// [ ] std::deque -// [ ] std::array -// [ ] std::string -// [ ] std::string_view -// callee: -// name: -// [ ] memmove -// [ ] memcpy -// [ ] wmemmove -// [ ] wmemcpy -// qualified: -// [ ] y -// [ ] n - namespace std { using size_t = decltype(sizeof(int)); @@ -62,6 +16,7 @@ class char_traits {}; template struct basic_string { typedef basic_string _Type; + using value_type = C; basic_string(); basic_string(const C* p, const A& a = A()); @@ -140,38 +95,33 @@ typename Container::value_type* data(Container& Arg) { return Arg.data(); } -template -T* data(T Arg[]); - template const typename Container::value_type* data(const Container& Arg) { return Arg.data(); } -template -const T* data(const T[]); +template +constexpr T* data(T (&arr)[N]) { + return arr; +} -size_t size(void *); +template< class T, std::size_t N > +constexpr std::size_t size( const T (&array)[N] ) { + return N; +} template size_t size(const Container& c) { return c.size(); } -size_t strlen(const char *); - wchar_t* wmemcpy( wchar_t* dest, const wchar_t* src, size_t count ); wchar_t* wmemmove( wchar_t* dest, const wchar_t* src, size_t count ); template struct unique_ptr { unique_ptr(T); - T *get() const; - explicit operator bool() const; - void reset(T *ptr); T &operator*() const; - T *operator->() const; - T& operator[](size_t i) const; }; } // namespace std @@ -236,7 +186,7 @@ void notSupportedEx() { const char *Src = "once upon a daydream..."; char *Dst = new char[64]; - std::memcpy(Dst, Src, std::strlen(Src)); + std::memcpy(Dst, Src, 24); } } @@ -245,19 +195,19 @@ void supportedEx() { wchar_t Src[] = L"once upon a daydream..."; wchar_t Dst[4]; - std::wmemcpy(Dst, Src, sizeof Dst); + std::wmemcpy(Dst, Src, sizeof(Dst) / sizeof(wchar_t)); // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'wmemcpy' // CHECK-FIXES: std::copy_n(std::cbegin(Src), std::size(Dst), std::begin(Dst)); - std::wmemcpy(Dst, Src, sizeof(Dst)); + std::wmemcpy(Dst, Src, sizeof(Dst) / sizeof(wchar_t)); // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'wmemcpy' // CHECK-FIXES: std::copy_n(std::cbegin(Src), std::size(Dst), std::begin(Dst)); - wmemcpy(Dst, Src, sizeof Src); + wmemcpy(Dst, Src, std::size(Src)); // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'wmemcpy' // CHECK-FIXES: std::copy_n(std::cbegin(Src), std::size(Src), std::begin(Dst)); - wmemmove(Dst, Src, sizeof(Src)); + wmemmove(Dst, Src, std::size(Src)); // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'wmemmove' // CHECK-FIXES: std::copy_n(std::cbegin(Src), std::size(Src), std::begin(Dst)); } @@ -280,11 +230,21 @@ void supportedEx() { // CHECK-FIXES: std::copy_n(std::cbegin(Src), Src.size(), std::begin(Dst)); } + { // reference to const c-array + const char Src[] = "once upon a daydream..."; + char Dst[7]; + decltype(Dst)& RefDst = Dst; + + memcpy(RefDst, Src, sizeof(RefDst)); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'memcpy' + // CHECK-FIXES: std::copy_n(std::cbegin(Src), std::size(RefDst), std::begin(RefDst)); + } + { // std::wstring + std::unique_ptr std::wstring Src = L"once upon a daydream..."; - std::unique_ptr Dst(new wchar_t[16]); + std::unique_ptr Dst(new wchar_t[7]); - std::wmemcpy(*Dst, Src.data(), sizeof(*Dst)); + std::wmemcpy(*Dst, Src.data(), sizeof(*Dst) / sizeof(wchar_t)); // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'wmemcpy' // CHECK-FIXES: std::copy_n(std::cbegin(Src), std::size(*Dst), std::begin(*Dst)); } @@ -308,7 +268,7 @@ void supportedEx() { // CHECK-FIXES: std::copy_n(std::cbegin(Src), Src.size() - 2, std::begin(Dst)); } - { // NSizeOfExpr cases + { // various size arg cases std::int32_t Data[] = {1, 2, 3, 4, 1, 2, 3, 4}; std::span Src{Data}; std::vector Dst(8); @@ -318,8 +278,68 @@ void supportedEx() { // CHECK-FIXES: std::copy_n(std::cbegin(Src), Dst.size(), std::begin(Dst)); memmove(std::data(Dst), std::data(Src), sizeof(int) * std::size(Dst)); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'memmove' + // CHECK-FIXES: std::copy_n(std::cbegin(Src), std::size(Dst), std::begin(Dst)); + + // If both arguments of the product match sizeOfExpr then the right one is lifted + std::memcpy(std::data(Dst), std::data(Src), sizeof(int) * sizeof(std::int32_t)); // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'memcpy' - // CHECK-FIXES: std::copy_n(std::cbegin(Src), std::size(Dst), std::begin(Dst)); + // CHECK-FIXES: std::copy_n(std::cbegin(Src), sizeof(int), std::begin(Dst)); + + std::memcpy(std::data(Dst), std::data(Src), sizeof(decltype(Dst)::value_type) * 3); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'memcpy' + // CHECK-FIXES: std::copy_n(std::cbegin(Src), 3, std::begin(Dst)); + } + + { // default sizearg path + std::vector Src(64); + std::u32string Dst = U"once upon a daydream..."; + + // Don't factor out sizeof expressions that are unlikely to be related to the value type, + // this might lose semantic info + memmove(std::data(Dst), std::data(Src), sizeof(float) * 3); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'memmove' + // CHECK-FIXES: std::copy_n(std::cbegin(Src), (sizeof(float) * 3) / sizeof(char32_t), std::begin(Dst)); + + // the sizeof(value_type) is nested too deep in the product expression, + // so currently the check will leave it as is + memmove(Dst.data(), Src.data(), 1 * 2 * sizeof(char32_t) * 4); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'memmove' + // CHECK-FIXES: std::copy_n(std::cbegin(Src), (1 * 2 * sizeof(char32_t) * 4) / sizeof(char32_t), std::begin(Dst)); + + std::memcpy(Dst.data(), Src.data(), sizeof(char32_t) + sizeof(int)); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'memcpy' + // CHECK-FIXES: std::copy_n(std::cbegin(Src), (sizeof(char32_t) + sizeof(int)) / sizeof(char32_t), std::begin(Dst)); + } + + { // SizeOfDivSizeOf - width = char + const char Src[] = "once upon a daydream..."; + std::string DstStr = " "; + + // superfluous, can be simplified + std::memcpy(DstStr.data(), std::data(Src), sizeof(Src) / sizeof(char)); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'memcpy' + // CHECK-FIXES: std::copy_n(std::cbegin(Src), std::size(Src), std::begin(DstStr)); + } + + { // SizeOfDivSizeOf - width = int + const int Src[] = {1, 2, 3, 4, 5, 6, 7, 8}; + std::vector DstVec(8); + + // this is probably a bug, but the check prefers not to change behaviour + std::memcpy(DstVec.data(), std::data(Src), sizeof(Src) / sizeof(int)); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'memcpy' + // CHECK-FIXES: std::copy_n(std::cbegin(Src), (sizeof(Src) / sizeof(int)) / sizeof(int), std::begin(DstVec)); + } + + { // SizeOfDivSizeOf - width = wchar_t + const wchar_t Src[] = L"once upon a daydream..."; + std::wstring DstStr = L" "; + + // simplifies + std::wmemcpy(DstStr.data(), std::data(Src), sizeof(Src) / sizeof(wchar_t)); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: prefer std::copy_n to 'wmemcpy' + // CHECK-FIXES: std::copy_n(std::cbegin(Src), std::size(Src), std::begin(DstStr)); } } } // namespace \ No newline at end of file From 47a371b0acba71dab6cac229a16f6351d24af6a5 Mon Sep 17 00:00:00 2001 From: kidp330 Date: Tue, 7 Jan 2025 21:46:10 +0100 Subject: [PATCH 11/15] Finish size semantics, clean up comments and logs --- .../modernize/ReplaceWithStdCopyCheck.cpp | 493 ++++++------------ 1 file changed, 173 insertions(+), 320 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.cpp b/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.cpp index 28d755e6767f3..a718937614d1a 100644 --- a/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.cpp @@ -27,7 +27,7 @@ namespace clang::tidy { namespace modernize { namespace { -constexpr llvm::StringLiteral ExpressionRef = "::std::memcpy"; +constexpr llvm::StringLiteral ExpressionRef = "ReplaceWithStdCopyCheckRef"; constexpr llvm::StringLiteral ReturnValueDiscardedRef = "ReturnValueDiscardedRef"; @@ -40,8 +40,7 @@ struct Refs { llvm::StringLiteral ValueType; llvm::StringLiteral PtrCastFnReturnType; }; -// rn matchers have a problem with binding types correctly, clang-query build up -// the matchers from this file tmrw for debugging + template auto createPtrArgMatcher() { constexpr Refs Refs = RefsT::Refs; @@ -69,16 +68,11 @@ template auto createPtrArgMatcher() { auto VariantCArrayM = expr(hasType(hasUnqualifiedDesugaredType(CArrayTypeM))) .bind(Refs.AsCArray); - auto VariantRawPtrM = - expr(hasType(pointerType(pointee(qualType().bind(Refs.ValueType))))); - auto StdDataReturnM = returns(pointerType(pointee(qualType().bind(Refs.PtrCastFnReturnType)))); auto StdDataMemberDeclM = cxxMethodDecl(hasName("data"), parameterCountIs(0), StdDataReturnM); - // ,unless(isConst()) // __jm__ ONLY difference between Dest - // and Source calls, but maybe don't include it? auto StdDataFreeDeclM = functionDecl(hasAnyName("::std::data", "::data"), parameterCountIs(1), StdDataReturnM); @@ -94,8 +88,7 @@ template auto createPtrArgMatcher() { // the last expr() in anyOf assumes previous matchers are ran eagerly from // left to right, still need to test this is the actual behaviour - return expr(anyOf(StdDataMemberCallM, StdDataFreeCallM, VariantCArrayM, - VariantRawPtrM)); + return expr(anyOf(StdDataMemberCallM, StdDataFreeCallM, VariantCArrayM)); } namespace tag { @@ -124,26 +117,17 @@ PtrArg extractNode(const CallExpr &CallNode, template const QualType *extractValueType(const MatchFinder::MatchResult &Result) { constexpr Refs Refs = RefT::Refs; + + // checking equality is done here as opposed to when matching because the + // equalsBoundNode matcher depends on the match order and the + // PtrCastFnReturnType is only present in some scenarios, + // making it tricky to swap the binding order const auto *MaybeRetType = Result.Nodes.getNodeAs(Refs.PtrCastFnReturnType); const auto *ValueType = Result.Nodes.getNodeAs(Refs.ValueType); - // checking equality is done here as opposed to when matching because the - // equalsBoundNode matcher depends on the match order and the - // PtrCastFnReturnType is only present in some scenarios - // if (MaybeRetType != nullptr) - // llvm::errs() - // << "MaybeRetType: " - // << - // MaybeRetType->getCanonicalType().getUnqualifiedType().getAsString() - // << '\n'; - // if (ValueType != nullptr) - // llvm::errs() - // << "ValueType: " - // << ValueType->getCanonicalType().getUnqualifiedType().getAsString() - // << '\n'; // stripping qualifiers is necessary in cases like when matching a call - // to const T* data() const; of a std::vector - + // to const T* vector::data() const; // the container value_type (char) is not const qualified, // but the return type of data() is. if (MaybeRetType != nullptr and @@ -157,8 +141,8 @@ const QualType *extractValueType(const MatchFinder::MatchResult &Result) { namespace dst { constexpr size_t ArgIndex = 0; struct RefT { - static constexpr ptrarg::Refs Refs = {"Dst::AsContainer", "Dst::AsCArray", 0, - "Dst::ValueType", + static constexpr ptrarg::Refs Refs = {"Dst::AsContainer", "Dst::AsCArray", + ArgIndex, "Dst::ValueType", "Dst::PtrCastFnReturnType"}; }; @@ -176,8 +160,8 @@ namespace src { constexpr size_t ArgIndex = 1; struct SrcRefsT { - static constexpr ptrarg::Refs Refs = {"Src::AsContainer", "Src::AsCArray", 1, - "Src::ValueType", + static constexpr ptrarg::Refs Refs = {"Src::AsContainer", "Src::AsCArray", + ArgIndex, "Src::ValueType", "Src::PtrCastFnReturnType"}; }; @@ -193,92 +177,100 @@ auto extractValueType(const MatchFinder::MatchResult &Result) { namespace size { constexpr size_t ArgIndex = 2; -// cases: -// 1. sizeof(T) where T is a type -// 2. sizeof(rec) where rec is a CArray -// 3. N * sizeof(T) -// 4. strlen(rec) [+ 1] -// 5. other expr + namespace variant { -struct SizeOfExpr { - const Expr *N = nullptr; - const Expr &Arg; +struct SizeOfCArray { + const Expr &Array; }; -struct SizeOfType { - const Expr *N = nullptr; - const QualType T; +struct NSizeOfType { + const Expr &N; + const QualType &Arg; }; -struct Strlen { - const Expr &Arg; +struct SizeOfDivSizeOf { + const Expr &Array; + const QualType &DivSizeOfType; }; } // namespace variant -using SizeArg = std::variant; - -struct SizeComponents { - CharUnits Unit; - std::string NExpr; -}; +using SizeArg = std::variant; static constexpr struct Refs { - llvm::StringLiteral SizeOfArg; - llvm::StringLiteral N; - llvm::StringLiteral StrlenArg; -} Refs = { - "Size::SizeOfExpr", - "Size::N", - "Size::StrlenArg", -}; + llvm::StringLiteral SizeOfCArray; + llvm::StringLiteral NSizeOfTypeN; + llvm::StringLiteral NSizeOfTypeArg; + llvm::StringLiteral SizeOfDivSizeOfArray; + llvm::StringLiteral SizeOfDivSizeOfArg; +} Refs = {"Size::SizeOfCArray", "Size::NSizeOfTypeN", "Size::NSizeOfTypeArg", + "Size::SizeOfDivSizeOfArray", "Size::SizeOfDivSizeOfArg"}; auto createMatcher() { - auto SizeOfExprM = sizeOfExpr(expr().bind(Refs.SizeOfArg)); + // NOTE: this check does not detect common, invalid patterns + // like sizeof(_) * sizeof(_), etc. since other checks exist for those. + + // patterns of the size argument that may be modified : + // 1. sizeof(arr) + // - invalid if callee is a wide variant, + // should be sizeof(arr) / sizeof(wchar_like) + // - otherwise -> std::size(arr) + // 2. N * sizeof(value_like) + // - invalid if callee is a wide variant, should just be N + // - otherwise when sizeof(value_like) == sizeof(value_type) -> N + // 3. sizeof(arr) / sizeof(value_like) + // - valid if callee is a wide variant -> std::size(arr) + // - valid if sizeof(value_like) == 1 + // - invalid otherwise, will fall back to (expr) / sizeof(value_type) + + constexpr auto SizeOfCArray = [](llvm::StringLiteral Ref) { + return sizeOfExpr( + has(expr(hasType(hasUnqualifiedDesugaredType(constantArrayType()))) + .bind(Ref))); + }; - auto NSizeOfExprM = binaryOperator( - hasOperatorName("*"), hasOperands(expr().bind(Refs.N), SizeOfExprM)); + constexpr auto SizeOfType = [](llvm::StringLiteral Ref) { + return sizeOfExpr(hasArgumentOfType(qualType().bind(Ref))); + }; - auto StrlenM = - callExpr(callee(functionDecl(hasAnyName( - "::strlen", "::std::strlen", "::wcslen", "::std::wcslen", - "::strnlen_s", "::std::strnlen_s", "::wcsnlen_s", - "::std::wcsnlen_s"))), - hasArgument(0, expr().bind(Refs.StrlenArg))); + auto NSizeOfTypeM = binaryOperator( + hasOperatorName("*"), hasOperands(expr().bind(Refs.NSizeOfTypeN), + SizeOfType(Refs.NSizeOfTypeArg))); - auto StrlenPlusOneM = binaryOperator( - hasOperatorName("+"), hasOperands(StrlenM, integerLiteral(equals(1)))); + auto SizeOfDivSizeOfM = binaryOperator( + hasOperatorName("/"), hasOperands(SizeOfCArray(Refs.SizeOfDivSizeOfArray), + SizeOfType(Refs.SizeOfDivSizeOfArg))); - return expr(anyOf(NSizeOfExprM, StrlenM, StrlenPlusOneM, SizeOfExprM)); + return expr( + anyOf(NSizeOfTypeM, SizeOfCArray(Refs.SizeOfCArray), SizeOfDivSizeOfM)); } SizeArg extractNode(const CallExpr &CallNode, const MatchFinder::MatchResult &Result) { - llvm::errs() << "Dumps Size:\n"; - CallNode.dump(); - llvm::errs() << __LINE__ << '\n'; - if (const auto *SizeOfExprNode = - Result.Nodes.getNodeAs(Refs.SizeOfArg); - SizeOfExprNode != nullptr) { - llvm::errs() << __LINE__ << '\n'; - const auto *NNode = Result.Nodes.getNodeAs(Refs.N); - if (const auto *ArgAsExpr = SizeOfExprNode->getArgumentExpr(); - ArgAsExpr != nullptr) - return variant::SizeOfExpr{NNode, *ArgAsExpr}; + auto ConcreteCase = [&Nodes = Result.Nodes]() -> std::optional { + if (const auto *Array = Nodes.getNodeAs(Refs.SizeOfCArray); + Array != nullptr) + return variant::SizeOfCArray{*Array}; llvm::errs() << __LINE__ << '\n'; - return variant::SizeOfType{NNode, SizeOfExprNode->getTypeOfArgument()}; - } - llvm::errs() << __LINE__ << '\n'; - if (const auto *StrlenArgNode = Result.Nodes.getNodeAs(Refs.StrlenArg); - StrlenArgNode != nullptr) - return variant::Strlen{*StrlenArgNode}; - llvm::errs() << __LINE__ << '\n'; - return CallNode.getArg(2); -} -// 1. N * sizeof(type) (commutative) - issue warning but no fixit, or fixit -// only if both src/dest are fixit friendly 1.1. This might allow fixits for -// wider-than-byte element collections -// 2. strlen(src|dest) ?(+ 1 (commutative)) -- issue warning but no fixit -// 4. sizeof(variable) only if that variable is of arrayType, if it's of -// ptrType that may indicate [de]serialization + if (const auto *N = Nodes.getNodeAs(Refs.NSizeOfTypeN); + N != nullptr) { + if (const auto *Arg = Nodes.getNodeAs(Refs.NSizeOfTypeArg); + Arg != nullptr) + return variant::NSizeOfType{*N, *Arg}; + return std::nullopt; + } + if (const auto *Array = Nodes.getNodeAs(Refs.SizeOfDivSizeOfArray); + Array != nullptr) { + if (const auto *SizeOfArg = + Nodes.getNodeAs(Refs.SizeOfDivSizeOfArg); + SizeOfArg != nullptr) + return variant::SizeOfDivSizeOf{*Array, *SizeOfArg}; + return std::nullopt; + } + return std::nullopt; + }; + if (auto MaybeSize = ConcreteCase(); MaybeSize.has_value()) + return *MaybeSize; + return CallNode.getArg(ArgIndex); +} } // namespace size auto createCalleeMatcher(bool FlagMemcpy) { @@ -354,53 +346,6 @@ void ReplaceWithStdCopyCheck::check(const MatchFinder::MatchResult &Result) { }; llvm::errs() << "Call: " << ExprAsString(CallNode) << '\n'; - // only have this function return a non-empty optional if the form of the size - // argument strongly indicates collection-related usage - auto ExtractComponents = [&]() -> std::optional { - llvm::errs() << __LINE__ << '\n'; - if (const auto *NSizeOfTypeNode = - std::get_if(&Size); - NSizeOfTypeNode != nullptr) { - llvm::errs() << __LINE__ << '\n'; - auto &[N, T] = *NSizeOfTypeNode; - - auto WidthT = Result.Context->getTypeSizeInChars(T); - auto StrN = N == nullptr ? "1" : ExprAsString(*N); - - return {{WidthT, StrN}}; - } - if (const auto *NSizeOfExprNode = - std::get_if(&Size); - NSizeOfExprNode != nullptr) { - llvm::errs() << __LINE__ << '\n'; - auto &[N, Arg] = *NSizeOfExprNode; - if (Arg.getType()->isConstantArrayType()) { - } - auto SizeOfArgWidth = Result.Context->getTypeSizeInChars(Arg.getType()); - llvm::errs() << SizeOfArgWidth.getQuantity() << '\n'; - return {{SizeOfArgWidth, ExprAsString(N)}}; - } - // if (const auto *NSizeOfExprNode = - // std::get_if(&Size); - // NSizeOfExprNode != nullptr) { - // llvm::errs() << __LINE__ << '\n' << "chuj\n"; - // auto &[N, Arg] = *NSizeOfExprNode; - // auto SizeOfArgWidth = - // Result.Context->getTypeSizeInChars(Arg.getType()); llvm::errs() << - // SizeOfArgWidth.getQuantity() << '\n'; return {{SizeOfArgWidth, - // ExprAsString(N)}}; - // } - if (const auto *StrlenNode = std::get_if(&Size); - StrlenNode != nullptr) { - llvm::errs() << __LINE__ << '\n'; - auto StrlenArgTypeWidth = Result.Context->getTypeSizeInChars( - StrlenNode->Arg.getType()->getPointeeType()); - return {{StrlenArgTypeWidth, ExprAsString(*CallNode.getArg(2))}}; - } - llvm::errs() << __LINE__ << '\n'; - return std::nullopt; - }; - bool DstIsRawPtr = std::holds_alternative(Dst.Tag); bool SrcIsRawPtr = std::holds_alternative(Src.Tag); @@ -423,6 +368,23 @@ void ReplaceWithStdCopyCheck::check(const MatchFinder::MatchResult &Result) { auto DstTypeWidth = Result.Context->getTypeSizeInChars(*DstVT); auto SrcTypeWidth = Result.Context->getTypeSizeInChars(*SrcVT); + auto CalleeIsWideVariant = [&]() { + const auto *Callee = CallNode.getDirectCallee(); + if (Callee == nullptr) + return false; + auto *ParamDecl = Callee->getParamDecl(0); + if (ParamDecl == nullptr) + return false; + return ParamDecl->getType()->getPointeeType()->isWideCharType(); + }(); + + auto CalleeUnit = [&]() { + if (CalleeIsWideVariant) + return Result.Context->getTypeSizeInChars( + (Result.Context->getWideCharType())); + return CharUnits::One(); + }(); + auto CheckIsFixable = [&]() { // If the width types differ, it's hard to reason about what would be a // helpful replacement, so just don't issue a fixit in this case @@ -435,6 +397,11 @@ void ReplaceWithStdCopyCheck::check(const MatchFinder::MatchResult &Result) { IsReturnValueUsed) return false; + // for widechar variants assume that the value types are also + // of wchar_t width, to make analysis easier. + if (CalleeIsWideVariant and DstTypeWidth != CalleeUnit) + return false; + return true; }; if (not CheckIsFixable()) @@ -457,44 +424,77 @@ void ReplaceWithStdCopyCheck::check(const MatchFinder::MatchResult &Result) { return "std::begin(" + AsString + ")"; }(); - auto CalleeIsWideVariant = [&]() { - const auto *Callee = CallNode.getDirectCallee(); - if (Callee == nullptr) - return false; - auto *ParamDecl = Callee->getParamDecl(0); - if (ParamDecl == nullptr) - return false; - return ParamDecl->getType()->getPointeeType()->isWideCharType(); - }(); + // this function is used to specify when a type from a sizeof(_) call is + // considered equivalent to the value type of the collection. + // for now it is relaxed because the template specialization matcher + // seems to unqualify types automatically (e.g. the value type of + // std::vector will just be int) and + auto CheckIsEquivValueType = [&DstVT](QualType SizeArgType) { + return SizeArgType->getCanonicalTypeUnqualified() == + (*DstVT)->getCanonicalTypeUnqualified(); + }; - auto CalleeUnit = [&]() { - if (CalleeIsWideVariant) { - return Result.Context->getTypeSizeInChars( - (Result.Context->getWideCharType())); + auto ByteModifySizeArg = [&]() -> std::string { + if (const auto *SizeOfCArray = + std::get_if(&Size); + SizeOfCArray != nullptr) { + // simply replaces sizeof(arr) with std::size(arr) + return "std::size(" + ExprAsString(SizeOfCArray->Array) + ")"; + } + if (const auto *NSizeofExprNode = + std::get_if(&Size); + NSizeofExprNode != nullptr) { + auto &[N, Arg] = *NSizeofExprNode; + // In this case it is easy to factor out the byte multiplier + // by just dropping the sizeof expression from the size computation + if (CheckIsEquivValueType(Arg)) + return ExprAsString(N); + } + if (const auto *SizeOfDivSizeOf = + std::get_if(&Size); + SizeOfDivSizeOf != nullptr) { + auto &[Array, DivSizeOfType] = *SizeOfDivSizeOf; + if (CheckIsEquivValueType(DivSizeOfType) and ValueTypeWidth == CalleeUnit) + return "std::size(" + ExprAsString(Array) + ")"; } - return CharUnits::One(); - }(); - auto SizeFixit = [&]() -> std::string { - // try to factor out the unit from the size expression - llvm::errs() << __LINE__ << '\n'; - if (auto MaybeSizeComponents = ExtractComponents(); - MaybeSizeComponents.has_value()) { + // In the specific case where the collections' value types are byte-wide, + // no unit conversion of the size argument is necessary + if (ValueTypeWidth == CalleeUnit) + return ExprAsString(*CallNode.getArg(size::ArgIndex)); + + // For all other cases where the value type is wider than one byte, + // and the size argument is of a form that is not easy to factor the unit + // out of, perform explicit division to ensure it is in element units + return (llvm::Twine("") + "(" + + ExprAsString(*CallNode.getArg(size::ArgIndex)) + ") / sizeof(" + + DstVT->getAsString() + ")") + .str(); + }; - // __jm__ TODO - llvm::errs() << __LINE__ << '\n'; - return MaybeSizeComponents->NExpr; + auto WideModifySizeArg = [&]() { + if (const auto *SizeOfDivSizeOf = + std::get_if(&Size); + SizeOfDivSizeOf != nullptr) { + auto &[Array, DivSizeOfType] = *SizeOfDivSizeOf; + if (CheckIsEquivValueType(DivSizeOfType)) + return "std::size(" + ExprAsString(Array) + ")"; } - // last resort, divide by ValueTypeWidth - llvm::errs() << __LINE__ << '\n'; - return ExprAsString(*CallNode.getArg(size::ArgIndex)) + " / " + "sizeof(" + - DstVT->getAsString() + ")"; - }(); + // Since we assume wide variants' value type width equals wchar_t, + // the units should already be unified and no modifications to the size + // argument are necessary + return ExprAsString(*CallNode.getArg(size::ArgIndex)); + }; + + auto SizeFixit = + CalleeIsWideVariant ? WideModifySizeArg() : ByteModifySizeArg(); Diag << FixItHint::CreateRemoval(CallNode.getSourceRange()) << FixItHint::CreateInsertion(CallNode.getBeginLoc(), - "std::copy_n(" + SrcFixit + ", " + - SizeFixit + ", " + DstFixit + ");") + (llvm::Twine() + "std::copy_n(" + + SrcFixit + ", " + SizeFixit + ", " + + DstFixit + ")") + .str()) << Inserter.createIncludeInsertion( Result.SourceManager->getFileID(CallNode.getBeginLoc()), ""); @@ -505,153 +505,6 @@ void ReplaceWithStdCopyCheck::check(const MatchFinder::MatchResult &Result) { std::holds_alternative(Src.Tag)) Diag << Inserter.createIncludeInsertion( Result.SourceManager->getFileID(CallNode.getBeginLoc()), ""); - - // if (const auto *NSizeofExprNode = - // std::get_if(&size); - // NSizeofExprNode != nullptr) { - // auto &[N, Arg] = *NSizeofExprNode; - // auto SizeOfArgWidth = - // Result.Context->getTypeSizeInChars(Arg->getType()); - // issueFixitIfWidthsMatch(dst.Node, src.Node, - // {SizeOfArgWidth, ExprAsString(N)}); - // return; - // } - // if (const auto *StrlenNode = std::get_if(&size); - // StrlenNode != nullptr) { - // auto StrlenArgTypeWidth = Result.Context->getTypeSizeInChars( - // StrlenNode->Arg->getType()->getPointeeType()); - // issueFixitIfWidthsMatch( - // dst.Node, src.Node, - // {StrlenArgTypeWidth, ExprAsString(CallNode.getArg(2))}); - // return; - // } - // if (const auto *SizeOfExprNode = - // std::get_if(&size); - // SizeOfExprNode != nullptr) { - // auto &Arg = SizeOfExprNode->Arg; - // if (SizeOfExprNode->Arg->getType()->isArrayType()) { - // issueFixitIfWidthsMatch( - // dst.Node, src.Node, - // {CharUnits::One(), ExprAsString(CallNode.getArg(2))}); - // return; - // } - // // __jm__ weird bc we assume dst and src are collections - // // if Arg turns out to have the same type as dst or src then just - // suggest - // // copy via the assignment operator - // if (auto argType = Arg->getType(); argType == DstVT or argType == - // SrcVT) - // { - // issueFixit{ValueTypeWidth, ExprAsString(CallNode.getArg(2))}; - // return; - // } - // // __jm__ only flag this as suspicious with a further explanation in - // the - // // diagnostic TODO: a sizeof of an unrelated type when copying between - // // collections does not make a lot of sense - - // auto sizeDRE = [&]() -> const DeclRefExpr * { - // if (const auto *Variant = std::get_if(&size); - // Variant != nullptr) { - // return llvm::dyn_cast_if_present(Variant->Arg); - // } - // if (const auto *Variant = - // std::get_if(&size); - // Variant != nullptr) { - // return llvm::dyn_cast_if_present(Variant->Arg); - // } - // if (const auto *Variant = - // std::get_if(&size); - // Variant != nullptr) { - // return llvm::dyn_cast_if_present(Variant->Arg); - // } - // return nullptr; - // }(); - - // one thing the analysis of the size argument must return is an Expr* Node - // that we can lift into std::copy_n's third argument - - // 1. strlen(DeclRefExpr) also hints that the referenced variable is a - // collection - - // 2. sizeof(expr) - // 2.1. expr is a c-array DeclRefExpr to one of src/dst, copy_n size - // becomes std::size(expr) - // 2.2. both src and dst are not raw pointers and expr is a type of width - // equal to vtw, essentialy fallthrough to 3 - - // 3. N * sizeof(expr) is okay when expr is a type with width == - // ValueTypeWidth and N may be verbatim lifted into copy_n's third argument - - // to make sure we can issue a FixIt, need to be pretty sure we're dealing - // with a collection and it is a byte collection - // 1. For now we are sure both source and dest are collections, but not - // necessarily of bytes - // 2. We can relax conditions to where just one arg is a collection, and the - // other can then be a collection or raw pointer. However, this is not - // robust as there are cases where this may be used for unmarshalling - // 3. when both dest and source are pointers (no expectations on the - // argument as everything required is enforced by the type system) we can - // use a heuristic using the form of the 3rd argument expression - // - - // delete the memmove/memcpy - // insert an std::copy_n - - // using PtrArgVariant = std::variant; - // struct PtrArg { - // PtrArgVariant Tag; - // const Expr *Node; - // }; - - // const auto MakePtrArg = [&](arg::tag::VariantPtrArgRef Refs) -> PtrArg { - // if (const auto *Node = Result.Nodes.getNodeAs( - // arg::Dest::Refs.VariantRefs.AsCArray); - // Node != nullptr) { - // // freestanding std::begin - // return {CArrayTag{}, Node}; - // } - // if (const auto *Node = Result.Nodes.getNodeAs( - // arg::Dest::Refs.VariantRefs.AsContainer); - // Node != nullptr) { - // return {ContainerTag{}, Node}; - // } - // const auto *Node = CallNode.getArg(Refs.FallbackParameterIdx); - // return {RawPtrTag{}, Node}; - // }; - - // auto Dest = MakePtrArg(arg::Dest::Refs.VariantRefs); - // auto Source = MakePtrArg(arg::Source::Refs.VariantRefs); - - // if (std::holds_alternative(Dest.Tag) and - // std::holds_alternative(Source.Tag) and - // CheckSizeArgPermitsFix()) { - - // } else { - // } - - { - // using namespace std::literals; - // Diag << FixItHint::CreateReplacement( - // DestArg->getSourceRange(), - // ("std::begin(" + - // tooling::fixit::getText(SrcArg->getSourceRange(), *Result.Context) - // + - // ")") - // .str()); - // Diag << FixItHint::CreateReplacement( - // SrcArg->getSourceRange(), - // tooling::fixit::getText(SizeArg->getSourceRange(), - // *Result.Context)); - // Diag << FixItHint::CreateReplacement( - // SizeArg->getSourceRange(), - // ("std::begin(" + - // tooling::fixit::getText(DestArg->getSourceRange(), - // *Result.Context) - // + - // ")") - // .str()); - } } } // namespace modernize From 0abf669401c96cf4fdcfcc8890ddf0323a25de41 Mon Sep 17 00:00:00 2001 From: kidp330 Date: Mon, 13 Jan 2025 23:25:41 +0100 Subject: [PATCH 12/15] Minor fixes [skip ci] --- .../modernize/ReplaceWithStdCopyCheck.cpp | 66 ++++++++----------- 1 file changed, 29 insertions(+), 37 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.cpp b/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.cpp index a718937614d1a..214183ccd0e49 100644 --- a/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ReplaceWithStdCopyCheck.cpp @@ -7,7 +7,6 @@ //===----------------------------------------------------------------------===// #include "ReplaceWithStdCopyCheck.h" -#include "ReplaceAutoPtrCheck.h" #include "clang/AST/ASTContext.h" #include "clang/AST/DeclTemplate.h" #include "clang/AST/Expr.h" @@ -44,15 +43,13 @@ struct Refs { template auto createPtrArgMatcher() { constexpr Refs Refs = RefsT::Refs; - auto AllowedContainerNamesM = []() { - // return hasAnyName("::std::deque", "::std::forward_list", "::std::list", - // "::std::vector", "::std::basic_string", - // "::std::array"); - return hasAnyName("::std::deque", "::std::forward_list", "::std::list", - "::std::vector", "::std::basic_string", "::std::array", - "::std::basic_string_view", "::std::span"); - }(); + auto AllowedContainerNamesM = + hasAnyName("::std::vector", "::std::basic_string", "::std::array", + "::std::basic_string_view", "::std::span"); + // Note: a problem with this matcher is that it automatically desugars the + // template argument type, so e.g. std::vector will bind + // ValueType to int and not std::int32_t auto AllowedContainerTypeM = hasUnqualifiedDesugaredType( recordType(hasDeclaration(recordDecl(classTemplateSpecializationDecl( AllowedContainerNamesM, @@ -71,8 +68,8 @@ template auto createPtrArgMatcher() { auto StdDataReturnM = returns(pointerType(pointee(qualType().bind(Refs.PtrCastFnReturnType)))); - auto StdDataMemberDeclM = - cxxMethodDecl(hasName("data"), parameterCountIs(0), StdDataReturnM); + auto StdDataMemberDeclM = cxxMethodDecl(hasAnyName("data", "c_str"), + parameterCountIs(0), StdDataReturnM); auto StdDataFreeDeclM = functionDecl(hasAnyName("::std::data", "::data"), parameterCountIs(1), StdDataReturnM); @@ -86,8 +83,6 @@ template auto createPtrArgMatcher() { auto StdDataFreeCallM = callExpr(callee(StdDataFreeDeclM), argumentCountIs(1), hasArgument(0, ArrayOrContainerM)); - // the last expr() in anyOf assumes previous matchers are ran eagerly from - // left to right, still need to test this is the actual behaviour return expr(anyOf(StdDataMemberCallM, StdDataFreeCallM, VariantCArrayM)); } @@ -119,9 +114,11 @@ const QualType *extractValueType(const MatchFinder::MatchResult &Result) { constexpr Refs Refs = RefT::Refs; // checking equality is done here as opposed to when matching because the - // equalsBoundNode matcher depends on the match order and the + // equalsBoundNode matcher depends on the match order. + // Already considered swapping the role of the node + // matchers, having one bind and the other match using equalsBoundNode, but // PtrCastFnReturnType is only present in some scenarios, - // making it tricky to swap the binding order + // so it's not applicable. const auto *MaybeRetType = Result.Nodes.getNodeAs(Refs.PtrCastFnReturnType); const auto *ValueType = Result.Nodes.getNodeAs(Refs.ValueType); @@ -244,7 +241,8 @@ auto createMatcher() { SizeArg extractNode(const CallExpr &CallNode, const MatchFinder::MatchResult &Result) { - auto ConcreteCase = [&Nodes = Result.Nodes]() -> std::optional { + auto TryExtractFromBoundTags = + [&Nodes = Result.Nodes]() -> std::optional { if (const auto *Array = Nodes.getNodeAs(Refs.SizeOfCArray); Array != nullptr) return variant::SizeOfCArray{*Array}; @@ -267,7 +265,7 @@ SizeArg extractNode(const CallExpr &CallNode, return std::nullopt; }; - if (auto MaybeSize = ConcreteCase(); MaybeSize.has_value()) + if (auto MaybeSize = TryExtractFromBoundTags(); MaybeSize.has_value()) return *MaybeSize; return CallNode.getArg(ArgIndex); } @@ -299,12 +297,6 @@ void ReplaceWithStdCopyCheck::registerMatchers(MatchFinder *Finder) { const auto OffendingDeclM = functionDecl(parameterCountIs(3), createCalleeMatcher(FlagMemcpy)); - // cases: - // 1. One of the arguments is definitely a collection and the other a pointer - // - match - // 2. Both source and dest are pointers, but size is of the form ((N := - // expr()) * sizeof(bytelike())) - match (false positive if N \in {0, 1}) - const auto ExpressionM = callExpr( callee(OffendingDeclM), optionally(ReturnValueUsedM), allOf(optionally(hasArgument(dst::ArgIndex, dst::createMatcher())), @@ -324,7 +316,6 @@ void ReplaceWithStdCopyCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { } void ReplaceWithStdCopyCheck::check(const MatchFinder::MatchResult &Result) { - llvm::errs() << __LINE__ << '\n'; const auto &CallNode = *Result.Nodes.getNodeAs(ExpressionRef); auto Dst = dst::extractNode(CallNode, Result); @@ -344,7 +335,6 @@ void ReplaceWithStdCopyCheck::check(const MatchFinder::MatchResult &Result) { *Result.SourceManager, getLangOpts()) .str(); }; - llvm::errs() << "Call: " << ExprAsString(CallNode) << '\n'; bool DstIsRawPtr = std::holds_alternative(Dst.Tag); bool SrcIsRawPtr = std::holds_alternative(Src.Tag); @@ -364,7 +354,8 @@ void ReplaceWithStdCopyCheck::check(const MatchFinder::MatchResult &Result) { auto Diag = diag(CallNode.getExprLoc(), "prefer std::copy_n to %0") << cast(CallNode.getCalleeDecl()); - // basis for converting size argument to std::copy_n's when issuing fixit + // the value type widths are helpful when translating the size argument + // from byte units (memcpy etc.) to element units (std::copy_n), auto DstTypeWidth = Result.Context->getTypeSizeInChars(*DstVT); auto SrcTypeWidth = Result.Context->getTypeSizeInChars(*SrcVT); @@ -407,28 +398,27 @@ void ReplaceWithStdCopyCheck::check(const MatchFinder::MatchResult &Result) { if (not CheckIsFixable()) return; - // From here we can assume dst and src have equal value type widths + assert(DstTypeWidth == SrcTypeWidth); const auto &ValueTypeWidth = DstTypeWidth; auto SrcFixit = [&]() { auto AsString = ExprAsString(Src.Node); if (SrcIsRawPtr) return AsString; - return "std::cbegin(" + AsString + ")"; + return (llvm::Twine() + "std::cbegin(" + AsString + ")").str(); }(); auto DstFixit = [&]() { auto AsString = ExprAsString(Dst.Node); if (DstIsRawPtr) return AsString; - return "std::begin(" + AsString + ")"; + return (llvm::Twine() + "std::begin(" + AsString + ")").str(); }(); - // this function is used to specify when a type from a sizeof(_) call is + // This function is used to specify when a type from a sizeof(_) call is // considered equivalent to the value type of the collection. - // for now it is relaxed because the template specialization matcher - // seems to unqualify types automatically (e.g. the value type of - // std::vector will just be int) and + // For now it is relaxed because the matcher desugars + // the container value types automatically. auto CheckIsEquivValueType = [&DstVT](QualType SizeArgType) { return SizeArgType->getCanonicalTypeUnqualified() == (*DstVT)->getCanonicalTypeUnqualified(); @@ -439,7 +429,9 @@ void ReplaceWithStdCopyCheck::check(const MatchFinder::MatchResult &Result) { std::get_if(&Size); SizeOfCArray != nullptr) { // simply replaces sizeof(arr) with std::size(arr) - return "std::size(" + ExprAsString(SizeOfCArray->Array) + ")"; + return (llvm::Twine() + "std::size(" + ExprAsString(SizeOfCArray->Array) + + ")") + .str(); } if (const auto *NSizeofExprNode = std::get_if(&Size); @@ -455,7 +447,7 @@ void ReplaceWithStdCopyCheck::check(const MatchFinder::MatchResult &Result) { SizeOfDivSizeOf != nullptr) { auto &[Array, DivSizeOfType] = *SizeOfDivSizeOf; if (CheckIsEquivValueType(DivSizeOfType) and ValueTypeWidth == CalleeUnit) - return "std::size(" + ExprAsString(Array) + ")"; + return (llvm::Twine() + "std::size(" + ExprAsString(Array) + ")").str(); } // In the specific case where the collections' value types are byte-wide, @@ -466,7 +458,7 @@ void ReplaceWithStdCopyCheck::check(const MatchFinder::MatchResult &Result) { // For all other cases where the value type is wider than one byte, // and the size argument is of a form that is not easy to factor the unit // out of, perform explicit division to ensure it is in element units - return (llvm::Twine("") + "(" + + return (llvm::Twine() + "(" + ExprAsString(*CallNode.getArg(size::ArgIndex)) + ") / sizeof(" + DstVT->getAsString() + ")") .str(); @@ -478,7 +470,7 @@ void ReplaceWithStdCopyCheck::check(const MatchFinder::MatchResult &Result) { SizeOfDivSizeOf != nullptr) { auto &[Array, DivSizeOfType] = *SizeOfDivSizeOf; if (CheckIsEquivValueType(DivSizeOfType)) - return "std::size(" + ExprAsString(Array) + ")"; + return (llvm::Twine() + "std::size(" + ExprAsString(Array) + ")").str(); } // Since we assume wide variants' value type width equals wchar_t, // the units should already be unified and no modifications to the size From f8dd28b4cb07204e7146c4686f86f7594e45005d Mon Sep 17 00:00:00 2001 From: kidp330 Date: Mon, 13 Jan 2025 23:25:55 +0100 Subject: [PATCH 13/15] Refine docs --- .../modernize-replace-memcpy-with-stdcopy.rst | 47 ------------------ .../modernize/replace-with-std-copy.rst | 48 ++++++++----------- 2 files changed, 20 insertions(+), 75 deletions(-) delete mode 100644 clang-tools-extra/docs/clang-tidy/checks/modernize/modernize-replace-memcpy-with-stdcopy.rst diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/modernize-replace-memcpy-with-stdcopy.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/modernize-replace-memcpy-with-stdcopy.rst deleted file mode 100644 index 74276c37bc2c0..0000000000000 --- a/clang-tools-extra/docs/clang-tidy/checks/modernize/modernize-replace-memcpy-with-stdcopy.rst +++ /dev/null @@ -1,47 +0,0 @@ -.. title:: clang-tidy - modernize-with-stdcopy - -modernize-with-stdcopy -=================================== - -Replaces all occurrences of the C ``memcpy`` function with ``std::copy`` - -Example: - -.. code-block:: c++ - - /*! - * \param destination Pointer to the destination array where the content is to be copied - * \param source Pointer to the source of data to be copied - * \param num Number of bytes to copy - */ - memcpy(destination, source, num); - -becomes - -.. code-block:: c++ - - /*! - * \param destination Pointer to the destination array where the content is to be copied - * \param source Pointer to the source of data to be copied - * \param num Number of bytes to copy - */ - std::copy(source, source + (num / sizeof *source), destination); - -Bytes to iterator conversion ----------------------------- - -Unlike ``std::copy`` that take an iterator on the last element of the source array, ``memcpy`` request the number of bytes to copy. -In order to make the check working, it will convert the size parameter to an iterator by replacing it by ``source + (num / sizeof *source)`` - -Header inclusion ----------------- - -``std::copy`` being provided by the ``algorithm`` header file, this check will include it if needed. - -Options -------- - -.. option:: IncludeStyle - - A string specifying which include-style is used, `llvm` or `google`. Default - is `llvm`. diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/replace-with-std-copy.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/replace-with-std-copy.rst index 7a28ab75ac2b9..4f3060112fa5d 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/modernize/replace-with-std-copy.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/replace-with-std-copy.rst @@ -1,48 +1,40 @@ -.. title:: clang-tidy - modernize-replace-with-stdcopy +.. title:: clang-tidy - modernize-replace-with-std-copy -modernize-replace-with-stdcopy +modernize-replace-with-std-copy =================================== -Replaces all occurrences of the C ``memmove`` function and its wide-char variant with ``std::copy_n``. -Replacement of ``memcpy`` is optionally also supported. +This check will flag all calls to ``memmove`` that can be possibly replaced with ``std::copy_n``. +In some specific cases it will provide a fix automatically. +``memcpy`` may also be flagged as opt-in. It is disabled by default because it performs no safety checks for overlapping ranges +in the way ``memmove`` and ``std::copy_n`` do. +``wmemmove`` and ``wmemcpy`` are also supported. Example: .. code-block:: c++ - - /*! - * \param dst Pointer to the destination array where the content is to be copied - * \param src Pointer to the source of data to be copied - * \param size Number of bytes to copy - */ - memcpy(dst, src, size); + std::vector dst(64); + memcpy(dst.data(), std::data(src), N); becomes .. code-block:: c++ + std::vector dst(64); + std::copy_n(std::cbegin(src), (N) / sizeof(int), std::begin(dst)); - /*! - * \param destination Pointer to the destination array where the content is to be copied - * \param source Pointer to the source of data to be copied - * \param num Number of bytes to copy - */ - std::copy_n(std::cbegin(src), size, std::begin(dst)); - -Bytes to iterator conversion ----------------------------- - -Unlike ``std::copy`` that take an iterator on the last element of the source array, ``memcpy`` request the number of bytes to copy. -In order to make the check working, it will convert the size parameter to an iterator by replacing it by ``source + (num / sizeof *source)`` - -Header inclusion +Known limitations ---------------- +For now, the check works only on a limited, recognizable subset of calls, where it can infer the arguments are pointers to valid collections +in the sense that ``std::copy_n`` understands. More specifically, source/destination should be one of: +- a call to ``std::data`` or the corresponding member method. +- a fixed-size C-array. -``std::copy_n`` is provided by the ``algorithm`` header file, this check will include it if needed. +Moreover, a fix will not be issued in more complicated cases, e.g. when source and destination are collections of types that have different sizes. Options ------- .. option:: IncludeStyle + A string specifying which include-style is used, `llvm` or `google`. Default is `llvm`. - A string specifying which include-style is used, `llvm` or `google`. Default - is `llvm`. +.. option:: FlagMemcpy + A boolean specifying whether to flag calls to ``memcpy`` as well. Default is `false`. \ No newline at end of file From e40000323e68c0859fc126f5c0a1d80b10e4e166 Mon Sep 17 00:00:00 2001 From: kidp330 Date: Tue, 14 Jan 2025 17:42:38 +0100 Subject: [PATCH 14/15] Fix placement in release notes --- clang-tools-extra/docs/ReleaseNotes.rst | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 94e1ea1a72386..7e5ed2ea82f52 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -169,6 +169,11 @@ New checks Finds cases when an uninstantiated virtual member function in a template class causes cross-compiler incompatibility. +- New :doc:`modernize-replace-with-std-copy + ` check. + + Tries to replace calls to ``memmove`` and ``memcpy`` with an equivalent call to ``std::copy_n``. + New check aliases ^^^^^^^^^^^^^^^^^ @@ -319,11 +324,6 @@ Changes in existing checks member function calls too and to only expand macros starting with ``PRI`` and ``__PRI`` from ```` in the format string. -- New :doc:`modernize-replace-with-stdcopy - ` check. - - Replaces all occurrences of the C ``memcpy`` function by ``std::copy``. - - Improved :doc:`modernize-use-std-print ` check to support replacing member function calls too and to only expand macros starting with ``PRI`` From a308ba03e4582d0a04e6ec509ad26e7355832e60 Mon Sep 17 00:00:00 2001 From: kidp330 Date: Sat, 8 Feb 2025 19:23:06 +0100 Subject: [PATCH 15/15] Update TidyFastChecks --- clang-tools-extra/clangd/TidyFastChecks.inc | 765 ++++++++++---------- 1 file changed, 386 insertions(+), 379 deletions(-) diff --git a/clang-tools-extra/clangd/TidyFastChecks.inc b/clang-tools-extra/clangd/TidyFastChecks.inc index a5d40a486976a..9076779cacc01 100644 --- a/clang-tools-extra/clangd/TidyFastChecks.inc +++ b/clang-tools-extra/clangd/TidyFastChecks.inc @@ -7,437 +7,444 @@ #define SLOW(CHECK, DELTA) #endif -FAST(abseil-cleanup-ctad, -2.0) -FAST(abseil-duration-addition, 0.0) +FAST(abseil-cleanup-ctad, 3.0) +FAST(abseil-duration-addition, 2.0) FAST(abseil-duration-comparison, -1.0) -FAST(abseil-duration-conversion-cast, -1.0) -FAST(abseil-duration-division, 0.0) -FAST(abseil-duration-factory-float, 2.0) -FAST(abseil-duration-factory-scale, 1.0) -FAST(abseil-duration-subtraction, -1.0) -FAST(abseil-duration-unnecessary-conversion, -0.0) -FAST(abseil-faster-strsplit-delimiter, 3.0) -FAST(abseil-no-internal-dependencies, 1.0) -FAST(abseil-no-namespace, -0.0) +FAST(abseil-duration-conversion-cast, 1.0) +FAST(abseil-duration-division, 4.0) +FAST(abseil-duration-factory-float, -0.0) +FAST(abseil-duration-factory-scale, -2.0) +FAST(abseil-duration-subtraction, -0.0) +FAST(abseil-duration-unnecessary-conversion, 2.0) +FAST(abseil-faster-strsplit-delimiter, -3.0) +FAST(abseil-no-internal-dependencies, -2.0) +FAST(abseil-no-namespace, 2.0) FAST(abseil-redundant-strcat-calls, 1.0) -FAST(abseil-str-cat-append, -0.0) -FAST(abseil-string-find-startswith, -1.0) +FAST(abseil-str-cat-append, 1.0) +FAST(abseil-string-find-startswith, -2.0) FAST(abseil-string-find-str-contains, 4.0) -FAST(abseil-time-comparison, -1.0) -FAST(abseil-time-subtraction, 1.0) -FAST(abseil-upgrade-duration-conversions, 2.0) -SLOW(altera-id-dependent-backward-branch, 13.0) -FAST(altera-kernel-name-restriction, 4.0) -FAST(altera-single-work-item-barrier, 1.0) -FAST(altera-struct-pack-align, -0.0) -FAST(altera-unroll-loops, 2.0) -FAST(android-cloexec-accept, 0.0) -FAST(android-cloexec-accept4, 1.0) -FAST(android-cloexec-creat, 1.0) +FAST(abseil-time-comparison, 1.0) +FAST(abseil-time-subtraction, 2.0) +FAST(abseil-upgrade-duration-conversions, 1.0) +SLOW(altera-id-dependent-backward-branch, 9.0) +FAST(altera-kernel-name-restriction, -0.0) +FAST(altera-single-work-item-barrier, -3.0) +FAST(altera-struct-pack-align, 1.0) +FAST(altera-unroll-loops, 3.0) +FAST(android-cloexec-accept, 1.0) +FAST(android-cloexec-accept4, 3.0) +FAST(android-cloexec-creat, 4.0) FAST(android-cloexec-dup, 0.0) FAST(android-cloexec-epoll-create, 2.0) -FAST(android-cloexec-epoll-create1, 0.0) -FAST(android-cloexec-fopen, -1.0) -FAST(android-cloexec-inotify-init, 2.0) -FAST(android-cloexec-inotify-init1, -0.0) -FAST(android-cloexec-memfd-create, -1.0) -FAST(android-cloexec-open, 1.0) -FAST(android-cloexec-pipe, -0.0) +FAST(android-cloexec-epoll-create1, -1.0) +FAST(android-cloexec-fopen, 1.0) +FAST(android-cloexec-inotify-init, 0.0) +FAST(android-cloexec-inotify-init1, -1.0) +FAST(android-cloexec-memfd-create, -4.0) +FAST(android-cloexec-open, -0.0) +FAST(android-cloexec-pipe, 0.0) FAST(android-cloexec-pipe2, 0.0) -FAST(android-cloexec-socket, 1.0) +FAST(android-cloexec-socket, -1.0) FAST(android-comparison-in-temp-failure-retry, 1.0) -FAST(boost-use-ranges, 2.0) -FAST(boost-use-to-string, 2.0) -FAST(bugprone-argument-comment, 4.0) -FAST(bugprone-assert-side-effect, 1.0) -FAST(bugprone-assignment-in-if-condition, 2.0) -FAST(bugprone-bad-signal-to-kill-thread, 1.0) -FAST(bugprone-bool-pointer-implicit-conversion, 0.0) +FAST(boost-use-ranges, 3.0) +FAST(boost-use-to-string, 0.0) +FAST(bugprone-argument-comment, 2.0) +FAST(bugprone-assert-side-effect, 2.0) +FAST(bugprone-assignment-in-if-condition, -5.0) +FAST(bugprone-bad-signal-to-kill-thread, -0.0) +FAST(bugprone-bitwise-pointer-cast, -2.0) +FAST(bugprone-bool-pointer-implicit-conversion, 4.0) FAST(bugprone-branch-clone, 1.0) -FAST(bugprone-casting-through-void, 1.0) -FAST(bugprone-chained-comparison, 1.0) -FAST(bugprone-compare-pointer-to-member-virtual-function, -0.0) -FAST(bugprone-copy-constructor-init, 1.0) -FAST(bugprone-crtp-constructor-accessibility, 0.0) -FAST(bugprone-dangling-handle, -0.0) -FAST(bugprone-dynamic-static-initializers, 0.0) -FAST(bugprone-easily-swappable-parameters, 2.0) -FAST(bugprone-empty-catch, 1.0) -FAST(bugprone-exception-escape, 0.0) -FAST(bugprone-fold-init-type, 1.0) -FAST(bugprone-forward-declaration-namespace, 0.0) -FAST(bugprone-forwarding-reference-overload, -1.0) -FAST(bugprone-implicit-widening-of-multiplication-result, 2.0) -FAST(bugprone-inaccurate-erase, -0.0) -FAST(bugprone-inc-dec-in-conditions, 3.0) -FAST(bugprone-incorrect-enable-if, -1.0) -FAST(bugprone-incorrect-roundings, 1.0) -FAST(bugprone-infinite-loop, 1.0) -FAST(bugprone-integer-division, -0.0) -FAST(bugprone-lambda-function-name, 0.0) -FAST(bugprone-macro-parentheses, 1.0) -FAST(bugprone-macro-repeated-side-effects, 1.0) -FAST(bugprone-misplaced-operator-in-strlen-in-alloc, 0.0) -FAST(bugprone-misplaced-pointer-arithmetic-in-alloc, -0.0) -FAST(bugprone-misplaced-widening-cast, -1.0) -FAST(bugprone-move-forwarding-reference, -1.0) +FAST(bugprone-casting-through-void, -1.0) +FAST(bugprone-chained-comparison, 2.0) +FAST(bugprone-compare-pointer-to-member-virtual-function, -4.0) +FAST(bugprone-copy-constructor-init, -0.0) +FAST(bugprone-crtp-constructor-accessibility, 1.0) +FAST(bugprone-dangling-handle, 1.0) +FAST(bugprone-dynamic-static-initializers, 1.0) +FAST(bugprone-easily-swappable-parameters, 1.0) +FAST(bugprone-empty-catch, 3.0) +FAST(bugprone-exception-escape, -0.0) +FAST(bugprone-fold-init-type, -1.0) +FAST(bugprone-forward-declaration-namespace, -1.0) +FAST(bugprone-forwarding-reference-overload, -4.0) +FAST(bugprone-implicit-widening-of-multiplication-result, 3.0) +FAST(bugprone-inaccurate-erase, 1.0) +FAST(bugprone-inc-dec-in-conditions, 5.0) +FAST(bugprone-incorrect-enable-if, 2.0) +FAST(bugprone-incorrect-roundings, 2.0) +FAST(bugprone-infinite-loop, 7.0) +FAST(bugprone-integer-division, 3.0) +FAST(bugprone-lambda-function-name, 2.0) +FAST(bugprone-macro-parentheses, 2.0) +FAST(bugprone-macro-repeated-side-effects, 2.0) +FAST(bugprone-misplaced-operator-in-strlen-in-alloc, 1.0) +FAST(bugprone-misplaced-pointer-arithmetic-in-alloc, 2.0) +FAST(bugprone-misplaced-widening-cast, 1.0) +FAST(bugprone-move-forwarding-reference, -0.0) FAST(bugprone-multi-level-implicit-pointer-conversion, -1.0) -FAST(bugprone-multiple-new-in-one-expression, 0.0) -FAST(bugprone-multiple-statement-macro, 2.0) +FAST(bugprone-multiple-new-in-one-expression, -1.0) +FAST(bugprone-multiple-statement-macro, -3.0) FAST(bugprone-narrowing-conversions, 2.0) -FAST(bugprone-no-escape, 1.0) -FAST(bugprone-non-zero-enum-to-bool-conversion, 0.0) -FAST(bugprone-not-null-terminated-result, 0.0) -FAST(bugprone-optional-value-conversion, 1.0) -FAST(bugprone-parent-virtual-call, 1.0) -FAST(bugprone-pointer-arithmetic-on-polymorphic-object, 0.0) -FAST(bugprone-posix-return, -0.0) -FAST(bugprone-redundant-branch-condition, -0.0) -FAST(bugprone-reserved-identifier, -1.0) -FAST(bugprone-return-const-ref-from-parameter, -2.0) -FAST(bugprone-shared-ptr-array-mismatch, 0.0) -FAST(bugprone-signal-handler, -1.0) -FAST(bugprone-signed-char-misuse, -2.0) -FAST(bugprone-sizeof-container, -1.0) -FAST(bugprone-sizeof-expression, 1.0) -FAST(bugprone-spuriously-wake-up-functions, 1.0) -FAST(bugprone-standalone-empty, 7.0) -FAST(bugprone-string-constructor, 3.0) -FAST(bugprone-string-integer-assignment, -0.0) -FAST(bugprone-string-literal-with-embedded-nul, 1.0) -FAST(bugprone-stringview-nullptr, 4.0) +FAST(bugprone-no-escape, 2.0) +FAST(bugprone-non-zero-enum-to-bool-conversion, 1.0) +FAST(bugprone-nondeterministic-pointer-iteration-order, 2.0) +FAST(bugprone-not-null-terminated-result, 1.0) +FAST(bugprone-optional-value-conversion, 2.0) +FAST(bugprone-parent-virtual-call, -2.0) +FAST(bugprone-pointer-arithmetic-on-polymorphic-object, 1.0) +FAST(bugprone-posix-return, 2.0) +FAST(bugprone-redundant-branch-condition, 1.0) +FAST(bugprone-reserved-identifier, 2.0) +FAST(bugprone-return-const-ref-from-parameter, 2.0) +FAST(bugprone-shared-ptr-array-mismatch, 3.0) +FAST(bugprone-signal-handler, 2.0) +FAST(bugprone-signed-char-misuse, -1.0) +FAST(bugprone-sizeof-container, -2.0) +FAST(bugprone-sizeof-expression, 6.0) +FAST(bugprone-spuriously-wake-up-functions, 0.0) +FAST(bugprone-standalone-empty, 5.0) +FAST(bugprone-string-constructor, 0.0) +FAST(bugprone-string-integer-assignment, 1.0) +FAST(bugprone-string-literal-with-embedded-nul, 0.0) +FAST(bugprone-stringview-nullptr, 2.0) FAST(bugprone-suspicious-enum-usage, 2.0) FAST(bugprone-suspicious-include, 0.0) -FAST(bugprone-suspicious-memory-comparison, 0.0) -FAST(bugprone-suspicious-memset-usage, 0.0) -FAST(bugprone-suspicious-missing-comma, -2.0) -FAST(bugprone-suspicious-realloc-usage, -0.0) -FAST(bugprone-suspicious-semicolon, 6.0) -FAST(bugprone-suspicious-string-compare, 1.0) -FAST(bugprone-suspicious-stringview-data-usage, 1.0) -FAST(bugprone-swapped-arguments, 1.0) -FAST(bugprone-switch-missing-default-case, 2.0) -FAST(bugprone-terminating-continue, -1.0) -FAST(bugprone-throw-keyword-missing, 0.0) -FAST(bugprone-too-small-loop-variable, 0.0) +FAST(bugprone-suspicious-memory-comparison, -0.0) +FAST(bugprone-suspicious-memset-usage, 1.0) +FAST(bugprone-suspicious-missing-comma, -3.0) +FAST(bugprone-suspicious-realloc-usage, 2.0) +FAST(bugprone-suspicious-semicolon, 3.0) +FAST(bugprone-suspicious-string-compare, 3.0) +FAST(bugprone-suspicious-stringview-data-usage, -0.0) +FAST(bugprone-swapped-arguments, 2.0) +FAST(bugprone-switch-missing-default-case, 0.0) +FAST(bugprone-tagged-union-member-count, -2.0) +FAST(bugprone-terminating-continue, 3.0) +FAST(bugprone-throw-keyword-missing, -1.0) +FAST(bugprone-too-small-loop-variable, 3.0) FAST(bugprone-unchecked-optional-access, 2.0) FAST(bugprone-undefined-memory-manipulation, 1.0) -FAST(bugprone-undelegated-constructor, 1.0) -FAST(bugprone-unhandled-exception-at-new, -1.0) -FAST(bugprone-unhandled-self-assignment, 0.0) -FAST(bugprone-unique-ptr-array-mismatch, 0.0) -FAST(bugprone-unsafe-functions, 1.0) -FAST(bugprone-unused-local-non-trivial-variable, -1.0) -FAST(bugprone-unused-raii, 1.0) +FAST(bugprone-undelegated-constructor, 4.0) +FAST(bugprone-unhandled-exception-at-new, 2.0) +FAST(bugprone-unhandled-self-assignment, 4.0) +FAST(bugprone-unique-ptr-array-mismatch, 5.0) +FAST(bugprone-unsafe-functions, 0.0) +FAST(bugprone-unused-local-non-trivial-variable, 2.0) +FAST(bugprone-unused-raii, 3.0) FAST(bugprone-unused-return-value, 4.0) -FAST(bugprone-use-after-move, 4.0) -FAST(bugprone-virtual-near-miss, 0.0) -FAST(cert-con36-c, 1.0) -FAST(cert-con54-cpp, 2.0) -FAST(cert-ctr56-cpp, 0.0) -FAST(cert-dcl03-c, 0.0) -FAST(cert-dcl16-c, 1.0) -FAST(cert-dcl37-c, 1.0) -FAST(cert-dcl50-cpp, -1.0) -FAST(cert-dcl51-cpp, -1.0) -FAST(cert-dcl54-cpp, 0.0) -FAST(cert-dcl58-cpp, -0.0) -FAST(cert-dcl59-cpp, 1.0) -FAST(cert-env33-c, 1.0) -FAST(cert-err09-cpp, -0.0) +FAST(bugprone-use-after-move, -0.0) +FAST(bugprone-virtual-near-miss, -1.0) +FAST(cert-arr39-c, 1.0) +FAST(cert-con36-c, 2.0) +FAST(cert-con54-cpp, 3.0) +FAST(cert-ctr56-cpp, 4.0) +FAST(cert-dcl03-c, 4.0) +FAST(cert-dcl16-c, 2.0) +FAST(cert-dcl37-c, 2.0) +FAST(cert-dcl50-cpp, 1.0) +FAST(cert-dcl51-cpp, 0.0) +FAST(cert-dcl54-cpp, 3.0) +FAST(cert-dcl58-cpp, -1.0) +FAST(cert-dcl59-cpp, -4.0) +FAST(cert-env33-c, 2.0) +FAST(cert-err09-cpp, 2.0) FAST(cert-err33-c, 4.0) -FAST(cert-err34-c, -1.0) -FAST(cert-err52-cpp, -1.0) -FAST(cert-err58-cpp, -0.0) -FAST(cert-err60-cpp, -0.0) -FAST(cert-err61-cpp, 2.0) -FAST(cert-exp42-c, 1.0) -FAST(cert-fio38-c, 1.0) -FAST(cert-flp30-c, 3.0) -FAST(cert-flp37-c, 1.0) -FAST(cert-int09-c, -1.0) -FAST(cert-mem57-cpp, 0.0) +FAST(cert-err34-c, 1.0) +FAST(cert-err52-cpp, 0.0) +FAST(cert-err58-cpp, 2.0) +FAST(cert-err60-cpp, 1.0) +FAST(cert-err61-cpp, 4.0) +FAST(cert-exp42-c, 2.0) +FAST(cert-fio38-c, -0.0) +FAST(cert-flp30-c, 2.0) +FAST(cert-flp37-c, 2.0) +FAST(cert-int09-c, 2.0) +FAST(cert-mem57-cpp, 2.0) FAST(cert-msc24-c, 0.0) -FAST(cert-msc30-c, 0.0) -FAST(cert-msc32-c, -0.0) +FAST(cert-msc30-c, 1.0) +FAST(cert-msc32-c, -1.0) FAST(cert-msc33-c, 2.0) -FAST(cert-msc50-cpp, -0.0) -FAST(cert-msc51-cpp, 2.0) -FAST(cert-msc54-cpp, -0.0) -FAST(cert-oop11-cpp, -0.0) -FAST(cert-oop54-cpp, 2.0) -FAST(cert-oop57-cpp, -0.0) -FAST(cert-oop58-cpp, 0.0) -FAST(cert-pos44-c, 2.0) -FAST(cert-pos47-c, 0.0) -FAST(cert-sig30-c, 1.0) -FAST(cert-str34-c, 2.0) -FAST(concurrency-mt-unsafe, 3.0) -FAST(concurrency-thread-canceltype-asynchronous, 1.0) -FAST(cppcoreguidelines-avoid-c-arrays, 0.0) +FAST(cert-msc50-cpp, 3.0) +FAST(cert-msc51-cpp, -0.0) +FAST(cert-msc54-cpp, 0.0) +FAST(cert-oop11-cpp, -3.0) +FAST(cert-oop54-cpp, 1.0) +FAST(cert-oop57-cpp, -1.0) +FAST(cert-oop58-cpp, 8.0) +FAST(cert-pos44-c, 1.0) +FAST(cert-pos47-c, 4.0) +FAST(cert-sig30-c, 3.0) +FAST(cert-str34-c, 1.0) +FAST(concurrency-mt-unsafe, -2.0) +FAST(concurrency-thread-canceltype-asynchronous, -1.0) +FAST(cppcoreguidelines-avoid-c-arrays, 2.0) FAST(cppcoreguidelines-avoid-capturing-lambda-coroutines, -1.0) -FAST(cppcoreguidelines-avoid-const-or-ref-data-members, -2.0) -FAST(cppcoreguidelines-avoid-do-while, -1.0) -FAST(cppcoreguidelines-avoid-goto, -1.0) -FAST(cppcoreguidelines-avoid-magic-numbers, -2.0) -FAST(cppcoreguidelines-avoid-non-const-global-variables, -0.0) -FAST(cppcoreguidelines-avoid-reference-coroutine-parameters, -0.0) -FAST(cppcoreguidelines-c-copy-assignment-signature, 1.0) -FAST(cppcoreguidelines-explicit-virtual-functions, 0.0) -FAST(cppcoreguidelines-init-variables, 1.0) +FAST(cppcoreguidelines-avoid-const-or-ref-data-members, 1.0) +FAST(cppcoreguidelines-avoid-do-while, 6.0) +FAST(cppcoreguidelines-avoid-goto, -0.0) +FAST(cppcoreguidelines-avoid-magic-numbers, 4.0) +FAST(cppcoreguidelines-avoid-non-const-global-variables, -1.0) +FAST(cppcoreguidelines-avoid-reference-coroutine-parameters, 1.0) +FAST(cppcoreguidelines-c-copy-assignment-signature, -1.0) +FAST(cppcoreguidelines-explicit-virtual-functions, -1.0) +FAST(cppcoreguidelines-init-variables, 4.0) FAST(cppcoreguidelines-interfaces-global-init, 1.0) -FAST(cppcoreguidelines-macro-to-enum, 0.0) -FAST(cppcoreguidelines-macro-usage, -0.0) -FAST(cppcoreguidelines-misleading-capture-default-by-value, -1.0) -FAST(cppcoreguidelines-missing-std-forward, 0.0) -FAST(cppcoreguidelines-narrowing-conversions, 2.0) -FAST(cppcoreguidelines-no-malloc, -1.0) -FAST(cppcoreguidelines-no-suspend-with-lock, 1.0) -FAST(cppcoreguidelines-noexcept-destructor, -0.0) -FAST(cppcoreguidelines-noexcept-move-operations, 2.0) -FAST(cppcoreguidelines-noexcept-swap, -2.0) -FAST(cppcoreguidelines-non-private-member-variables-in-classes, 1.0) -FAST(cppcoreguidelines-owning-memory, 3.0) +FAST(cppcoreguidelines-macro-to-enum, 3.0) +FAST(cppcoreguidelines-macro-usage, -2.0) +FAST(cppcoreguidelines-misleading-capture-default-by-value, 1.0) +FAST(cppcoreguidelines-missing-std-forward, 3.0) +FAST(cppcoreguidelines-narrowing-conversions, 3.0) +FAST(cppcoreguidelines-no-malloc, -2.0) +FAST(cppcoreguidelines-no-suspend-with-lock, 2.0) +FAST(cppcoreguidelines-noexcept-destructor, 3.0) +FAST(cppcoreguidelines-noexcept-move-operations, 4.0) +FAST(cppcoreguidelines-noexcept-swap, 0.0) +FAST(cppcoreguidelines-non-private-member-variables-in-classes, 2.0) +FAST(cppcoreguidelines-owning-memory, 2.0) FAST(cppcoreguidelines-prefer-member-initializer, 2.0) FAST(cppcoreguidelines-pro-bounds-array-to-pointer-decay, 2.0) FAST(cppcoreguidelines-pro-bounds-constant-array-index, 1.0) -FAST(cppcoreguidelines-pro-bounds-pointer-arithmetic, 0.0) -FAST(cppcoreguidelines-pro-type-const-cast, -1.0) -FAST(cppcoreguidelines-pro-type-cstyle-cast, 2.0) -FAST(cppcoreguidelines-pro-type-member-init, 1.0) -FAST(cppcoreguidelines-pro-type-reinterpret-cast, -1.0) -FAST(cppcoreguidelines-pro-type-static-cast-downcast, 0.0) -FAST(cppcoreguidelines-pro-type-union-access, 0.0) -FAST(cppcoreguidelines-pro-type-vararg, -1.0) -FAST(cppcoreguidelines-rvalue-reference-param-not-moved, 1.0) -FAST(cppcoreguidelines-slicing, 1.0) -FAST(cppcoreguidelines-special-member-functions, -1.0) -FAST(cppcoreguidelines-use-default-member-init, 0.0) -FAST(cppcoreguidelines-virtual-class-destructor, -0.0) -FAST(darwin-avoid-spinlock, 2.0) -FAST(darwin-dispatch-once-nonstatic, 0.0) -FAST(fuchsia-default-arguments-calls, 1.0) -FAST(fuchsia-default-arguments-declarations, -0.0) -FAST(fuchsia-header-anon-namespaces, -0.0) -FAST(fuchsia-multiple-inheritance, 0.0) -FAST(fuchsia-overloaded-operator, 4.0) -FAST(fuchsia-statically-constructed-objects, -0.0) -FAST(fuchsia-trailing-return, 1.0) -FAST(fuchsia-virtual-inheritance, 1.0) -FAST(google-build-explicit-make-pair, 3.0) +FAST(cppcoreguidelines-pro-bounds-pointer-arithmetic, 1.0) +FAST(cppcoreguidelines-pro-type-const-cast, 1.0) +FAST(cppcoreguidelines-pro-type-cstyle-cast, -3.0) +FAST(cppcoreguidelines-pro-type-member-init, 5.0) +FAST(cppcoreguidelines-pro-type-reinterpret-cast, 1.0) +FAST(cppcoreguidelines-pro-type-static-cast-downcast, 2.0) +FAST(cppcoreguidelines-pro-type-union-access, 2.0) +FAST(cppcoreguidelines-pro-type-vararg, -0.0) +FAST(cppcoreguidelines-rvalue-reference-param-not-moved, 0.0) +FAST(cppcoreguidelines-slicing, -2.0) +FAST(cppcoreguidelines-special-member-functions, 1.0) +FAST(cppcoreguidelines-use-default-member-init, 1.0) +FAST(cppcoreguidelines-virtual-class-destructor, 2.0) +FAST(darwin-avoid-spinlock, 5.0) +FAST(darwin-dispatch-once-nonstatic, 3.0) +FAST(fuchsia-default-arguments-calls, 0.0) +FAST(fuchsia-default-arguments-declarations, 3.0) +FAST(fuchsia-header-anon-namespaces, 6.0) +FAST(fuchsia-multiple-inheritance, 2.0) +FAST(fuchsia-overloaded-operator, 3.0) +FAST(fuchsia-statically-constructed-objects, 1.0) +FAST(fuchsia-trailing-return, -0.0) +FAST(fuchsia-virtual-inheritance, 0.0) +FAST(google-build-explicit-make-pair, 2.0) FAST(google-build-namespaces, -1.0) -FAST(google-build-using-namespace, -0.0) -FAST(google-default-arguments, 0.0) -FAST(google-explicit-constructor, 2.0) -FAST(google-global-names-in-headers, 0.0) -FAST(google-objc-avoid-nsobject-new, 1.0) -FAST(google-objc-avoid-throwing-exception, -0.0) -FAST(google-objc-function-naming, -1.0) -FAST(google-objc-global-variable-declaration, 0.0) +FAST(google-build-using-namespace, 1.0) +FAST(google-default-arguments, -0.0) +FAST(google-explicit-constructor, 0.0) +FAST(google-global-names-in-headers, -0.0) +FAST(google-objc-avoid-nsobject-new, -0.0) +FAST(google-objc-avoid-throwing-exception, -3.0) +FAST(google-objc-function-naming, -2.0) +FAST(google-objc-global-variable-declaration, -1.0) FAST(google-readability-avoid-underscore-in-googletest-name, 1.0) -FAST(google-readability-braces-around-statements, 0.0) -FAST(google-readability-casting, -0.0) -FAST(google-readability-function-size, 3.0) -FAST(google-readability-namespace-comments, -0.0) -FAST(google-readability-todo, 1.0) -FAST(google-runtime-int, 0.0) -FAST(google-runtime-operator, 0.0) -FAST(google-upgrade-googletest-case, 1.0) -FAST(hicpp-avoid-c-arrays, 1.0) -FAST(hicpp-avoid-goto, -0.0) -FAST(hicpp-braces-around-statements, 0.0) -FAST(hicpp-deprecated-headers, 1.0) -FAST(hicpp-exception-baseclass, -1.0) -FAST(hicpp-explicit-conversions, 1.0) -FAST(hicpp-function-size, 1.0) -FAST(hicpp-ignored-remove-result, 3.0) +FAST(google-readability-braces-around-statements, -3.0) +FAST(google-readability-casting, 2.0) +FAST(google-readability-function-size, 4.0) +FAST(google-readability-namespace-comments, 2.0) +FAST(google-readability-todo, 2.0) +FAST(google-runtime-int, 2.0) +FAST(google-runtime-operator, -6.0) +FAST(google-upgrade-googletest-case, -0.0) +FAST(hicpp-avoid-c-arrays, -1.0) +FAST(hicpp-avoid-goto, -1.0) +FAST(hicpp-braces-around-statements, -0.0) +FAST(hicpp-deprecated-headers, -1.0) +FAST(hicpp-exception-baseclass, 1.0) +FAST(hicpp-explicit-conversions, 2.0) +FAST(hicpp-function-size, 2.0) +FAST(hicpp-ignored-remove-result, 2.0) FAST(hicpp-invalid-access-moved, 4.0) FAST(hicpp-member-init, 2.0) -FAST(hicpp-move-const-arg, 3.0) -FAST(hicpp-multiway-paths-covered, 0.0) -FAST(hicpp-named-parameter, 2.0) -FAST(hicpp-new-delete-operators, -0.0) -FAST(hicpp-no-array-decay, 4.0) +FAST(hicpp-move-const-arg, 1.0) +FAST(hicpp-multiway-paths-covered, -0.0) +FAST(hicpp-named-parameter, 1.0) +FAST(hicpp-new-delete-operators, 2.0) +FAST(hicpp-no-array-decay, 2.0) FAST(hicpp-no-assembler, 1.0) FAST(hicpp-no-malloc, 2.0) -FAST(hicpp-noexcept-move, -0.0) -FAST(hicpp-signed-bitwise, -1.0) -FAST(hicpp-special-member-functions, -2.0) -FAST(hicpp-static-assert, 4.0) -FAST(hicpp-undelegated-constructor, 6.0) -FAST(hicpp-uppercase-literal-suffix, 5.0) +FAST(hicpp-noexcept-move, 1.0) +FAST(hicpp-signed-bitwise, 5.0) +FAST(hicpp-special-member-functions, 0.0) +FAST(hicpp-static-assert, 5.0) +FAST(hicpp-undelegated-constructor, -1.0) +FAST(hicpp-uppercase-literal-suffix, 2.0) FAST(hicpp-use-auto, 0.0) -FAST(hicpp-use-emplace, 3.0) +FAST(hicpp-use-emplace, -0.0) FAST(hicpp-use-equals-default, 2.0) -FAST(hicpp-use-equals-delete, 1.0) -FAST(hicpp-use-noexcept, -0.0) -FAST(hicpp-use-nullptr, 1.0) -FAST(hicpp-use-override, -1.0) -FAST(hicpp-vararg, 0.0) -FAST(linuxkernel-must-check-errs, -0.0) -FAST(llvm-else-after-return, -1.0) -FAST(llvm-header-guard, 3.0) -FAST(llvm-include-order, 0.0) -FAST(llvm-namespace-comment, 0.0) -FAST(llvm-prefer-isa-or-dyn-cast-in-conditionals, 3.0) -FAST(llvm-prefer-register-over-unsigned, -0.0) -FAST(llvm-qualified-auto, 4.0) -FAST(llvm-twine-local, -0.0) -FAST(llvmlibc-callee-namespace, -0.0) -FAST(llvmlibc-implementation-in-namespace, 1.0) -FAST(llvmlibc-inline-function-decl, 3.0) +FAST(hicpp-use-equals-delete, 0.0) +FAST(hicpp-use-noexcept, -1.0) +FAST(hicpp-use-nullptr, 2.0) +FAST(hicpp-use-override, 1.0) +FAST(hicpp-vararg, -3.0) +FAST(linuxkernel-must-check-errs, 0.0) +FAST(llvm-else-after-return, 1.0) +FAST(llvm-header-guard, 2.0) +FAST(llvm-include-order, 1.0) +FAST(llvm-namespace-comment, 1.0) +FAST(llvm-prefer-isa-or-dyn-cast-in-conditionals, 1.0) +FAST(llvm-prefer-register-over-unsigned, -4.0) +FAST(llvm-qualified-auto, -1.0) +FAST(llvm-twine-local, -2.0) +FAST(llvmlibc-callee-namespace, 0.0) +FAST(llvmlibc-implementation-in-namespace, -0.0) +FAST(llvmlibc-inline-function-decl, 1.0) FAST(llvmlibc-restrict-system-libc-headers, 0.0) -FAST(misc-confusable-identifiers, -1.0) -SLOW(misc-const-correctness, 67.0) -FAST(misc-coroutine-hostile-raii, 1.0) +FAST(misc-confusable-identifiers, 1.0) +FAST(misc-const-correctness, 5.0) +FAST(misc-coroutine-hostile-raii, -2.0) FAST(misc-definitions-in-headers, -1.0) -SLOW(misc-header-include-cycle, 10.0) -FAST(misc-include-cleaner, 5.0) -FAST(misc-misleading-bidirectional, 1.0) -FAST(misc-misleading-identifier, 3.0) -FAST(misc-misplaced-const, -2.0) -FAST(misc-new-delete-overloads, 1.0) -FAST(misc-no-recursion, 0.0) -FAST(misc-non-copyable-objects, 0.0) -FAST(misc-non-private-member-variables-in-classes, -1.0) -FAST(misc-redundant-expression, 1.0) -FAST(misc-static-assert, 3.0) -FAST(misc-throw-by-value-catch-by-reference, -0.0) +FAST(misc-header-include-cycle, 2.0) +FAST(misc-include-cleaner, 15.0) +FAST(misc-misleading-bidirectional, 0.0) +FAST(misc-misleading-identifier, 1.0) +FAST(misc-misplaced-const, -1.0) +FAST(misc-new-delete-overloads, 2.0) +FAST(misc-no-recursion, 1.0) +FAST(misc-non-copyable-objects, 2.0) +FAST(misc-non-private-member-variables-in-classes, -0.0) +FAST(misc-redundant-expression, 2.0) +FAST(misc-static-assert, 2.0) +FAST(misc-throw-by-value-catch-by-reference, 0.0) FAST(misc-unconventional-assign-operator, 1.0) -FAST(misc-uniqueptr-reset-release, 2.0) -FAST(misc-unused-alias-decls, 2.0) -FAST(misc-unused-parameters, 3.0) -FAST(misc-unused-using-decls, 1.0) -FAST(misc-use-anonymous-namespace, 1.0) -FAST(misc-use-internal-linkage, 1.0) -FAST(modernize-avoid-bind, 1.0) -FAST(modernize-avoid-c-arrays, 2.0) -FAST(modernize-concat-nested-namespaces, -0.0) +FAST(misc-uniqueptr-reset-release, 1.0) +FAST(misc-unused-alias-decls, 3.0) +FAST(misc-unused-parameters, 2.0) +FAST(misc-unused-using-decls, 0.0) +FAST(misc-use-anonymous-namespace, 4.0) +FAST(misc-use-internal-linkage, 3.0) +FAST(modernize-avoid-bind, -2.0) +FAST(modernize-avoid-c-arrays, 3.0) +FAST(modernize-concat-nested-namespaces, 1.0) FAST(modernize-deprecated-headers, 0.0) FAST(modernize-deprecated-ios-base-aliases, 0.0) -FAST(modernize-loop-convert, 2.0) +FAST(modernize-loop-convert, 4.0) FAST(modernize-macro-to-enum, 0.0) -FAST(modernize-make-shared, 2.0) +FAST(modernize-make-shared, -2.0) FAST(modernize-make-unique, 1.0) -FAST(modernize-min-max-use-initializer-list, 1.0) -FAST(modernize-pass-by-value, 0.0) -FAST(modernize-raw-string-literal, 2.0) -FAST(modernize-redundant-void-arg, 1.0) -FAST(modernize-replace-auto-ptr, 0.0) -FAST(modernize-replace-disallow-copy-and-assign-macro, -0.0) -FAST(modernize-replace-random-shuffle, 1.0) +FAST(modernize-min-max-use-initializer-list, -1.0) +FAST(modernize-pass-by-value, -1.0) +FAST(modernize-raw-string-literal, 1.0) +FAST(modernize-redundant-void-arg, -2.0) +FAST(modernize-replace-auto-ptr, 1.0) +FAST(modernize-replace-disallow-copy-and-assign-macro, -1.0) +FAST(modernize-replace-random-shuffle, 0.0) FAST(modernize-replace-with-std-copy, 1.0) -FAST(modernize-return-braced-init-list, 1.0) -FAST(modernize-shrink-to-fit, 1.0) -FAST(modernize-type-traits, 1.0) +FAST(modernize-return-braced-init-list, -1.0) +FAST(modernize-shrink-to-fit, -2.0) +FAST(modernize-type-traits, 3.0) FAST(modernize-unary-static-assert, 1.0) -FAST(modernize-use-auto, 0.0) -FAST(modernize-use-bool-literals, 1.0) -FAST(modernize-use-constraints, 1.0) -FAST(modernize-use-default-member-init, -0.0) +FAST(modernize-use-auto, -2.0) +FAST(modernize-use-bool-literals, -2.0) +FAST(modernize-use-constraints, -0.0) +FAST(modernize-use-default-member-init, 2.0) FAST(modernize-use-designated-initializers, 1.0) -FAST(modernize-use-emplace, 2.0) +FAST(modernize-use-emplace, 1.0) FAST(modernize-use-equals-default, 1.0) -FAST(modernize-use-equals-delete, 2.0) -FAST(modernize-use-nodiscard, -2.0) -FAST(modernize-use-noexcept, -2.0) -FAST(modernize-use-nullptr, 1.0) -FAST(modernize-use-override, 0.0) -FAST(modernize-use-ranges, 0.0) -FAST(modernize-use-starts-ends-with, 0.0) -FAST(modernize-use-std-format, -1.0) -FAST(modernize-use-std-numbers, 0.0) -FAST(modernize-use-std-print, -0.0) -FAST(modernize-use-trailing-return-type, 3.0) -FAST(modernize-use-transparent-functors, 0.0) -FAST(modernize-use-uncaught-exceptions, 0.0) -FAST(modernize-use-using, 1.0) -FAST(objc-assert-equals, 1.0) -FAST(objc-avoid-nserror-init, -0.0) -FAST(objc-dealloc-in-category, 0.0) -FAST(objc-forbidden-subclassing, 2.0) -FAST(objc-missing-hash, -0.0) -FAST(objc-nsdate-formatter, 0.0) -FAST(objc-nsinvocation-argument-lifetime, 0.0) -FAST(objc-property-declaration, -0.0) -FAST(objc-super-self, -2.0) -FAST(openmp-exception-escape, -1.0) -FAST(openmp-use-default-none, 2.0) -FAST(performance-avoid-endl, 2.0) -FAST(performance-enum-size, -1.0) -FAST(performance-faster-string-find, 1.0) -FAST(performance-for-range-copy, 1.0) -FAST(performance-implicit-conversion-in-loop, 0.0) -FAST(performance-inefficient-algorithm, 1.0) -FAST(performance-inefficient-string-concatenation, 1.0) -FAST(performance-inefficient-vector-operation, -0.0) -FAST(performance-move-const-arg, 2.0) -FAST(performance-move-constructor-init, 2.0) -FAST(performance-no-automatic-move, 2.0) -FAST(performance-no-int-to-ptr, 0.0) -FAST(performance-noexcept-destructor, -2.0) +FAST(modernize-use-equals-delete, -1.0) +FAST(modernize-use-integer-sign-comparison, 1.0) +FAST(modernize-use-nodiscard, -0.0) +FAST(modernize-use-noexcept, 0.0) +FAST(modernize-use-nullptr, 2.0) +FAST(modernize-use-override, -1.0) +FAST(modernize-use-ranges, -2.0) +FAST(modernize-use-starts-ends-with, 2.0) +FAST(modernize-use-std-format, -3.0) +FAST(modernize-use-std-numbers, 1.0) +FAST(modernize-use-std-print, -2.0) +FAST(modernize-use-trailing-return-type, -0.0) +FAST(modernize-use-transparent-functors, 1.0) +FAST(modernize-use-uncaught-exceptions, -3.0) +FAST(modernize-use-using, -1.0) +FAST(objc-assert-equals, 2.0) +FAST(objc-avoid-nserror-init, 1.0) +FAST(objc-dealloc-in-category, -1.0) +FAST(objc-forbidden-subclassing, 1.0) +FAST(objc-missing-hash, 3.0) +FAST(objc-nsdate-formatter, -1.0) +FAST(objc-nsinvocation-argument-lifetime, -0.0) +FAST(objc-property-declaration, 0.0) +FAST(objc-super-self, -1.0) +FAST(openmp-exception-escape, 1.0) +FAST(openmp-use-default-none, -1.0) +FAST(performance-avoid-endl, 3.0) +FAST(performance-enum-size, 1.0) +FAST(performance-faster-string-find, -1.0) +FAST(performance-for-range-copy, 3.0) +FAST(performance-implicit-conversion-in-loop, 1.0) +FAST(performance-inefficient-algorithm, 0.0) +FAST(performance-inefficient-string-concatenation, -2.0) +FAST(performance-inefficient-vector-operation, -1.0) +FAST(performance-move-const-arg, -2.0) +FAST(performance-move-constructor-init, 1.0) +FAST(performance-no-automatic-move, 0.0) +FAST(performance-no-int-to-ptr, -2.0) +FAST(performance-noexcept-destructor, 1.0) FAST(performance-noexcept-move-constructor, 1.0) -FAST(performance-noexcept-swap, -2.0) -FAST(performance-trivially-destructible, 3.0) -FAST(performance-type-promotion-in-math-fn, 2.0) -FAST(performance-unnecessary-copy-initialization, 2.0) -FAST(performance-unnecessary-value-param, 2.0) -FAST(portability-restrict-system-includes, 1.0) +FAST(performance-noexcept-swap, -3.0) +FAST(performance-trivially-destructible, -1.0) +FAST(performance-type-promotion-in-math-fn, 3.0) +FAST(performance-unnecessary-copy-initialization, 5.0) +FAST(performance-unnecessary-value-param, 0.0) +FAST(portability-restrict-system-includes, 0.0) FAST(portability-simd-intrinsics, 1.0) -FAST(portability-std-allocator-const, 3.0) -FAST(readability-avoid-const-params-in-decls, -0.0) -FAST(readability-avoid-nested-conditional-operator, -1.0) -FAST(readability-avoid-return-with-void-value, 0.0) -FAST(readability-avoid-unconditional-preprocessor-if, -1.0) -FAST(readability-braces-around-statements, 1.0) -FAST(readability-const-return-type, -1.0) -FAST(readability-container-contains, 3.0) -FAST(readability-container-data-pointer, -1.0) -SLOW(readability-container-size-empty, 13.0) -FAST(readability-convert-member-functions-to-static, 4.0) -FAST(readability-delete-null-pointer, -1.0) -FAST(readability-duplicate-include, 2.0) -FAST(readability-else-after-return, 0.0) -FAST(readability-enum-initial-value, 0.0) -FAST(readability-function-cognitive-complexity, 0.0) -FAST(readability-function-size, 0.0) +FAST(portability-std-allocator-const, 1.0) +FAST(portability-template-virtual-member-function, 3.0) +FAST(readability-avoid-const-params-in-decls, 2.0) +FAST(readability-avoid-nested-conditional-operator, 1.0) +FAST(readability-avoid-return-with-void-value, -1.0) +FAST(readability-avoid-unconditional-preprocessor-if, 4.0) +FAST(readability-braces-around-statements, -1.0) +FAST(readability-const-return-type, 0.0) +FAST(readability-container-contains, 9.0) +FAST(readability-container-data-pointer, 1.0) +FAST(readability-container-size-empty, 1.0) +FAST(readability-convert-member-functions-to-static, 1.0) +FAST(readability-delete-null-pointer, 0.0) +FAST(readability-duplicate-include, -0.0) +FAST(readability-else-after-return, 2.0) +FAST(readability-enum-initial-value, 4.0) +FAST(readability-function-cognitive-complexity, 3.0) +FAST(readability-function-size, -3.0) FAST(readability-identifier-length, 2.0) -FAST(readability-identifier-naming, 1.0) -FAST(readability-implicit-bool-conversion, 3.0) -FAST(readability-inconsistent-declaration-parameter-name, -0.0) -FAST(readability-isolate-declaration, 0.0) -FAST(readability-magic-numbers, 4.0) +FAST(readability-identifier-naming, 2.0) +FAST(readability-implicit-bool-conversion, 1.0) +FAST(readability-inconsistent-declaration-parameter-name, 2.0) +FAST(readability-isolate-declaration, -1.0) +FAST(readability-magic-numbers, -1.0) FAST(readability-make-member-function-const, 1.0) -FAST(readability-math-missing-parentheses, 1.0) +FAST(readability-math-missing-parentheses, 2.0) FAST(readability-misleading-indentation, 1.0) -FAST(readability-misplaced-array-index, 0.0) -FAST(readability-named-parameter, -0.0) +FAST(readability-misplaced-array-index, -1.0) +FAST(readability-named-parameter, -1.0) FAST(readability-non-const-parameter, 2.0) -FAST(readability-operators-representation, 0.0) -FAST(readability-qualified-auto, 0.0) -FAST(readability-redundant-access-specifiers, 1.0) -FAST(readability-redundant-casting, -0.0) +FAST(readability-operators-representation, -2.0) +FAST(readability-qualified-auto, 1.0) +FAST(readability-redundant-access-specifiers, -1.0) +FAST(readability-redundant-casting, -1.0) FAST(readability-redundant-control-flow, 1.0) -FAST(readability-redundant-declaration, 1.0) +FAST(readability-redundant-declaration, -2.0) FAST(readability-redundant-function-ptr-dereference, 0.0) -FAST(readability-redundant-inline-specifier, 0.0) +FAST(readability-redundant-inline-specifier, -0.0) FAST(readability-redundant-member-init, 1.0) FAST(readability-redundant-preprocessor, -0.0) -FAST(readability-redundant-smartptr-get, 4.0) -FAST(readability-redundant-string-cstr, 1.0) -FAST(readability-redundant-string-init, -0.0) -FAST(readability-reference-to-constructed-temporary, 3.0) -FAST(readability-simplify-boolean-expr, -0.0) -FAST(readability-simplify-subscript-expr, 1.0) -FAST(readability-static-accessed-through-instance, 0.0) -FAST(readability-static-definition-in-anonymous-namespace, 0.0) -FAST(readability-string-compare, -0.0) -FAST(readability-suspicious-call-argument, -1.0) -FAST(readability-uniqueptr-delete-release, 3.0) -FAST(readability-uppercase-literal-suffix, 0.0) -FAST(readability-use-anyofallof, 2.0) -FAST(readability-use-std-min-max, -1.0) -FAST(zircon-temporary-objects, 1.0) +FAST(readability-redundant-smartptr-get, -1.0) +FAST(readability-redundant-string-cstr, -1.0) +FAST(readability-redundant-string-init, 1.0) +FAST(readability-reference-to-constructed-temporary, -0.0) +FAST(readability-simplify-boolean-expr, 2.0) +FAST(readability-simplify-subscript-expr, 0.0) +FAST(readability-static-accessed-through-instance, 1.0) +FAST(readability-static-definition-in-anonymous-namespace, 1.0) +FAST(readability-string-compare, 1.0) +FAST(readability-suspicious-call-argument, 1.0) +FAST(readability-uniqueptr-delete-release, -0.0) +FAST(readability-uppercase-literal-suffix, -3.0) +FAST(readability-use-anyofallof, -1.0) +FAST(readability-use-std-min-max, 2.0) +FAST(zircon-temporary-objects, 0.0) #undef FAST #undef SLOW +