Index: clang-tidy/modernize/CMakeLists.txt =================================================================== --- clang-tidy/modernize/CMakeLists.txt +++ clang-tidy/modernize/CMakeLists.txt @@ -14,6 +14,7 @@ RedundantVoidArgCheck.cpp ReplaceAutoPtrCheck.cpp ShrinkToFitCheck.cpp + UseAlgorithmCheck.cpp UseAutoCheck.cpp UseBoolLiteralsCheck.cpp UseDefaultCheck.cpp Index: clang-tidy/modernize/ModernizeTidyModule.cpp =================================================================== --- clang-tidy/modernize/ModernizeTidyModule.cpp +++ clang-tidy/modernize/ModernizeTidyModule.cpp @@ -20,6 +20,7 @@ #include "RedundantVoidArgCheck.h" #include "ReplaceAutoPtrCheck.h" #include "ShrinkToFitCheck.h" +#include "UseAlgorithmCheck.h" #include "UseAutoCheck.h" #include "UseBoolLiteralsCheck.h" #include "UseDefaultCheck.h" @@ -52,6 +53,7 @@ CheckFactories.registerCheck( "modernize-replace-auto-ptr"); CheckFactories.registerCheck("modernize-shrink-to-fit"); + CheckFactories.registerCheck("modernize-use-algorithm"); CheckFactories.registerCheck("modernize-use-auto"); CheckFactories.registerCheck( "modernize-use-bool-literals"); Index: clang-tidy/modernize/UseAlgorithmCheck.h =================================================================== --- /dev/null +++ clang-tidy/modernize/UseAlgorithmCheck.h @@ -0,0 +1,42 @@ +//===--- UseAlgorithmCheck.h - clang-tidy-------------------------- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USE_ALGORITHM_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USE_ALGORITHM_H + +#include "../ClangTidy.h" +#include "../utils/IncludeInserter.h" + +namespace clang { +namespace tidy { +namespace modernize { + +/// Replaces `memcpy` and `memset` with `std::copy` and `std::fill`, +/// respectively. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/modernize-use-algorithm.html +class UseAlgorithmCheck : public ClangTidyCheck { +public: + UseAlgorithmCheck(StringRef Name, ClangTidyContext *Context); + + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void registerPPCallbacks(CompilerInstance &Compiler) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + std::unique_ptr Inserter; + const utils::IncludeSorter::IncludeStyle IncludeStyle; +}; + +} // namespace modernize +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USE_ALGORITHM_H Index: clang-tidy/modernize/UseAlgorithmCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/modernize/UseAlgorithmCheck.cpp @@ -0,0 +1,166 @@ +//===--- UseAlgorithmCheck.cpp - clang-tidy--------------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "UseAlgorithmCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Frontend/CompilerInstance.h" +#include "clang/Lex/Lexer.h" +#include "clang/Lex/Preprocessor.h" +#include "clang/Tooling/FixIt.h" + +#include + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace modernize { + +static QualType getStrippedType(QualType T) { + while (const auto *PtrType = T->getAs()) + T = PtrType->getPointeeType(); + + if (const auto *ArrType = dyn_cast(T)) + T = ArrType->getElementType(); + + return T.getCanonicalType().getUnqualifiedType(); +} + +static bool doesNotSupportPointerArithmetic(QualType T) { + return T->isVoidPointerType() || T->isFunctionPointerType(); +} + +static bool needsParensAroundExpressions(const Expr *E) { + E = E->IgnoreImpCasts(); + + if (isa(E) || isa(E)) + return true; + + if (const auto *Op = dyn_cast(E)) + return Op->getNumArgs() == 2 && Op->getOperator() != OO_Call && + Op->getOperator() != OO_Subscript; + + return false; +} + +UseAlgorithmCheck::UseAlgorithmCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + IncludeStyle(utils::IncludeSorter::parseIncludeStyle( + Options.get("IncludeStyle", "llvm"))) {} + +void UseAlgorithmCheck::registerMatchers(MatchFinder *Finder) { + // Only register the matchers for C++. + if (!getLangOpts().CPlusPlus) + return; + + // Match calls to memcpy or memset with exactly 3 arguments. + Finder->addMatcher( + callExpr(callee(functionDecl(hasAnyName("::std::memcpy", "::std::memset", + "::memcpy", "::memset")) + .bind("callee")), + argumentCountIs(3)) + .bind("expr"), + this); +} + +void UseAlgorithmCheck::registerPPCallbacks(CompilerInstance &Compiler) { + // Only register the preprocessor callbacks for C++; the functionality + // currently does not provide any benefit to other languages, despite being + // benign. + if (!getLangOpts().CPlusPlus) + return; + + Inserter = llvm::make_unique( + Compiler.getSourceManager(), Compiler.getLangOpts(), IncludeStyle); + Compiler.getPreprocessor().addPPCallbacks(Inserter->CreatePPCallbacks()); +} + +void UseAlgorithmCheck::check(const MatchFinder::MatchResult &Result) { + const auto *MatchedExpr = Result.Nodes.getNodeAs("expr"); + const auto *Callee = Result.Nodes.getNodeAs("callee"); + + const StringRef MatchedName = Callee->getName(); + + // Check if matched name is in map of replacements. + StringRef ReplacedName; + if (MatchedName == "memcpy") { + ReplacedName = "std::copy"; + } else if (MatchedName == "memset") { + ReplacedName = "std::fill"; + } else { + return; + } + + const SourceLocation Loc = MatchedExpr->getExprLoc(); + auto Diag = diag(Loc, "%0 reduces type-safety, consider using '%1' instead") + << Callee << ReplacedName; + + // Don't make replacements in macro. + if (Loc.isMacroID()) + return; + + const QualType Arg0Type = MatchedExpr->getArg(0)->IgnoreImpCasts()->getType(); + const QualType Arg1Type = MatchedExpr->getArg(1)->IgnoreImpCasts()->getType(); + + // Don't make replacements when destination is const. + if (Arg0Type.isConstQualified()) + return; + + // Don't make replacements when types are not compatible. + if (getStrippedType(Arg0Type) != getStrippedType(Arg1Type)) + return; + + // Don't make replacements when types do not support pointer arithmetic. + if (doesNotSupportPointerArithmetic(Arg0Type) || + doesNotSupportPointerArithmetic(Arg1Type)) + return; + + StringRef Arg0Text = + tooling::fixit::getText(*MatchedExpr->getArg(0), *Result.Context); + if (needsParensAroundExpressions(MatchedExpr->getArg(0))) + Arg0Text = ("(" + Arg0Text + ")").str(); + + StringRef Arg1Text = + tooling::fixit::getText(*MatchedExpr->getArg(1), *Result.Context); + if (needsParensAroundExpressions(MatchedExpr->getArg(1))) + Arg1Text = ("(" + Arg1Text + ")").str(); + + StringRef Arg2Text = + tooling::fixit::getText(*MatchedExpr->getArg(2), *Result.Context); + if (needsParensAroundExpressions(MatchedExpr->getArg(2))) + Arg2Text = ("(" + Arg2Text + ")").str(); + + std::string Insertion; + if (MatchedName == "memset") { + // Rearrangement of arguments for memset: + // (dest, ch, count) becomes (dest, dest + count, ch). + Insertion = (ReplacedName + "(" + Arg0Text + ", " + Arg0Text + " + " + + Arg2Text + ", " + Arg1Text + ")") + .str(); + } else { + // Rearrangement of arguments for memcpy: + // (dest, src, count) becomes (src, src + count, dest). + Insertion = (ReplacedName + "(" + Arg1Text + ", " + Arg1Text + " + " + + Arg2Text + ", " + Arg0Text + ")") + .str(); + } + + Diag << FixItHint::CreateReplacement(MatchedExpr->getSourceRange(), + Insertion); + + if (auto IncludeFixit = Inserter->CreateIncludeInsertion( + Result.SourceManager->getFileID(Loc), "algorithm", + /*IsAngled=*/true)) + Diag << *IncludeFixit; +} + +} // namespace modernize +} // namespace tidy +} // namespace clang Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -69,6 +69,12 @@ Flags classes where some, but not all, special member functions are user-defined. +- New `modernize-use-algorithm + `_ check + + Replaces calls to ``memcpy`` and ``memset`` with their respective algorithm + counterparts ``std::copy`` and ``std::fill``. + Improvements to include-fixer ----------------------------- Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -103,6 +103,7 @@ modernize-redundant-void-arg modernize-replace-auto-ptr modernize-shrink-to-fit + modernize-use-algorithm modernize-use-auto modernize-use-bool-literals modernize-use-default Index: docs/clang-tidy/checks/modernize-use-algorithm.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/modernize-use-algorithm.rst @@ -0,0 +1,62 @@ +.. title:: clang-tidy - modernize-use-algorithm + +modernize-use-algorithm +======================= + +Replaces calls to ``std::memcpy`` and ``std::memset`` with ``std::copy`` and +``std::fill``, respectively. The advantages of using these algorithm functions +is that they are at least as efficient, more general, and type-aware. + +By using the algorithms the types remain intact as opposed to being discarded +by the C-style functions. This allows the implementation to make use use of +type information to further optimize. One example of such optimization is +taking advantage of 64-bit alignment when copying an array of +``std::uint64_t``. + +memcpy +------ + +.. code:: c++ + + std::memcpy(dest, source, sizeof dest); + + // transforms to: + + std::copy(source, source + sizeof dest, dest); + +memset +------ + +.. code:: c++ + + std::memset(dest, ch, count); + + // transforms to: + + std::fill(dest, dest + count, ch) + +Limitations +----------- + +Because of the precedence of the additieve operator, it is possible that the +resulting transformation changes the semantics of the original code. An example +is given below. + +.. code:: c++ + + std::memcpy(dest, foo ? bar : baz, size); + + // transforms to: + + std::copy(foo ? bar : baz, foo ? baz : baz + size, dest); + +The meaning of the expression changes because the ternary conditional operator +has a lower precendence than the additive operator. + +.. code:: c++ + + std::copy(foo ? bar : baz, foo ? baz : (baz + size), dest); + + // but should have been: + + std::copy(foo ? bar : baz, (foo ? baz : baz) + size, dest); Index: test/clang-tidy/modernize-use-algorithm.cpp =================================================================== --- /dev/null +++ test/clang-tidy/modernize-use-algorithm.cpp @@ -0,0 +1,156 @@ +// RUN: %check_clang_tidy %s modernize-use-algorithm %t +// CHECK-FIXES: #include + +namespace std { +typedef unsigned int size_t; +void *memcpy(void *dest, const void *src, std::size_t count); +void *memset(void *dest, int ch, std::size_t count); + +template +OutputIt copy(InputIt first, InputIt last, OutputIt d_first); + +template +OutputIt move(InputIt first, InputIt last, OutputIt d_first); + +template +void fill(ForwardIt first, ForwardIt last, const T &value); +} + +typedef unsigned int size_t; +void *memcpy(void *dest, const void *source, size_t count); +void *memset(void *dest, int ch, size_t count); + +namespace alternative_std { +using ::memcpy; +using ::memset; +} + +namespace awful { +void memcpy(int, int, int); +} + +typedef unsigned int size_t; +void *memcpy(void *dest, const void *source, size_t size); + +void a() { + char foo[] = "foo", bar[4]; + std::memcpy(bar, foo, sizeof bar); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm] + // CHECK-FIXES: std::copy(foo, foo + sizeof bar, bar); + + alternative_std::memcpy(bar, foo, sizeof bar); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm] + // CHECK-FIXES: std::copy(foo, foo + sizeof bar, bar); + + void *baz = bar; + std::memcpy(baz, foo, sizeof bar); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm] + + memcpy(bar, foo, sizeof bar); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm] + // CHECK-FIXES: std::copy(foo, foo + sizeof bar, bar); + + std::copy(foo, foo + sizeof bar, bar); +} + +void b() { + char foo[] = "foobar"; + std::memset(foo, 'a', 3); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memset' reduces type-safety, consider using 'std::fill' instead [modernize-use-algorithm] + // CHECK-FIXES: std::fill(foo, foo + 3, 'a'); + + alternative_std::memset(foo, 'a', 3); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memset' reduces type-safety, consider using 'std::fill' instead [modernize-use-algorithm] + // CHECK-FIXES: std::fill(foo, foo + 3, 'a'); + + std::memset(foo, 1, 3); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memset' reduces type-safety, consider using 'std::fill' instead [modernize-use-algorithm] + + std::fill(foo, foo + 2, 'a'); +} + +#define MEMCPY(dest, src) std::memcpy((dest), (src), sizeof(dest)) +void c() { + char foo[] = "foo", bar[3]; + MEMCPY(bar, foo); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm] +} + +void d() { + typedef char foo_t; + typedef char bar_t; + foo_t foo[] = "foo"; + bar_t bar[4]; + std::memcpy(bar, foo, sizeof bar); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm] + // CHECK-FIXES: std::copy(foo, foo + sizeof bar, bar); +} + +void e() { + typedef const char *foo_t; + typedef const char *bar_t; + foo_t foo[] = {"foo", "bar"}; + bar_t bar[2]; + std::memcpy(bar, foo, sizeof bar); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm] + // CHECK-FIXES: std::copy(foo, foo + sizeof bar, bar); +} + +void f() { + typedef int some_type; + typedef some_type *const *volatile *foo_ptr; + + typedef int *const some_other_type; + typedef some_other_type *volatile *bar_ptr; + + foo_ptr foo[4]; + bar_ptr bar[3]; + std::memcpy(bar, foo, sizeof bar); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm] + // CHECK-FIXES: std::copy(foo, foo + sizeof bar, bar); +} + +void g() { + int foo[3][4] = {{0, 1, 2, 3}, {4, 5, 6, 7}, {8, 9, 10, 11}}; + int bar[2][4]; + + std::memcpy(bar, foo, sizeof bar); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm] + // CHECK-FIXES: std::copy(foo, foo + sizeof bar, bar); + + int baz[3][3] = {{0, 1, 2}, {3, 4, 5}, {6, 7, 8}}; + int qux[2][4]; + std::memcpy(qux, baz, sizeof qux); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm] +} + +void h() { + int a = 1; + int b = 2; + int c = 3; + + char foo[] = "foobar"; + char bar[3]; + + std::memcpy((bar), (foo + b), (a + c - 1)); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm] + // CHECK-FIXES: std::copy((foo + b), (foo + b) + (a + c - 1), (bar)); +} + +void i() { + using namespace awful; + int foo = 1; + int bar = 2; + memcpy(bar, foo, sizeof bar); +} + +void j() { + void f(); + typedef void(__cdecl * fp)(); + + fp f1 = f; + fp f2; + std::memcpy(&f2, &f1, sizeof(fp)); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm] + // CHECK-FIXES: std::copy(&f1, &f1 + sizeof(fp), &f2); +}