JDevlieghere updated this revision to Diff 71835.
JDevlieghere added a comment.
Herald added subscribers: mgorny, beanz.
Still working on comment #2 from Alex but wanted to update my diff since it's
been a while and I haven't gotten around to looking into it further. So no need
to review yet.
Repository:
rL LLVM
https://reviews.llvm.org/D22725
Files:
clang-tidy/modernize/CMakeLists.txt
clang-tidy/modernize/ModernizeTidyModule.cpp
clang-tidy/modernize/UseAlgorithmCheck.cpp
clang-tidy/modernize/UseAlgorithmCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/modernize-use-algorithm.rst
test/clang-tidy/modernize-use-algorithm.cpp
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 <algorithm>
+
+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 <class InputIt, class OutputIt>
+OutputIt copy(InputIt first, InputIt last, OutputIt d_first);
+
+template <class InputIt, class OutputIt>
+OutputIt move(InputIt first, InputIt last, OutputIt d_first);
+
+template <class ForwardIt, class T>
+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);
+}
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: 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/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
+ <http://clang.llvm.org/extra/clang-tidy/checks/modernize-use-algorithm.html>`_ check
+
+ Replaces calls to ``memcpy`` and ``memset`` with their respective algorithm
+ counterparts ``std::copy`` and ``std::fill``.
+
Improvements to include-fixer
-----------------------------
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<utils::IncludeInserter> 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 <string>
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+static QualType getStrippedType(QualType T) {
+ while (const auto *PtrType = T->getAs<PointerType>())
+ T = PtrType->getPointeeType();
+
+ if (const auto *ArrType = dyn_cast<ArrayType>(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<BinaryOperator>(E) || isa<ConditionalOperator>(E))
+ return true;
+
+ if (const auto *Op = dyn_cast<CXXOperatorCallExpr>(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<utils::IncludeInserter>(
+ Compiler.getSourceManager(), Compiler.getLangOpts(), IncludeStyle);
+ Compiler.getPreprocessor().addPPCallbacks(Inserter->CreatePPCallbacks());
+}
+
+void UseAlgorithmCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *MatchedExpr = Result.Nodes.getNodeAs<CallExpr>("expr");
+ const auto *Callee = Result.Nodes.getNodeAs<FunctionDecl>("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: 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<ReplaceAutoPtrCheck>(
"modernize-replace-auto-ptr");
CheckFactories.registerCheck<ShrinkToFitCheck>("modernize-shrink-to-fit");
+ CheckFactories.registerCheck<UseAlgorithmCheck>("modernize-use-algorithm");
CheckFactories.registerCheck<UseAutoCheck>("modernize-use-auto");
CheckFactories.registerCheck<UseBoolLiteralsCheck>(
"modernize-use-bool-literals");
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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
