JDevlieghere retitled this revision from "[clang-tidy] Add check 
'misc-replace-memcpy'" to "[clang-tidy] Add check 'modernize-use-algorithm'".
JDevlieghere updated the summary for this revision.
JDevlieghere set the repository for this revision to rL LLVM.
JDevlieghere updated this revision to Diff 66247.
JDevlieghere added a comment.

- Addressed concerns raised by Etienne Bergeron
- Show warning when using void pointers but don't replace
- Don't make replacement when destination is const
- Don't make replacement when types are not compatible


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,50 @@
+// 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);
+}
+
+void a() {
+  char foo[] = "foo", bar[3];
+  std::memcpy(bar, foo, sizeof bar);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Use std::copy instead of memcpy [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: Use std::copy instead of memcpy [modernize-use-algorithm]
+
+  std::copy(foo, foo + sizeof bar, bar);
+}
+
+void b() {
+  char foo[] = "foobar";
+  std::memset(foo, 'a', 3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Use std::fill instead of memset [modernize-use-algorithm]
+  // CHECK-FIXES: std::fill(foo, foo + 3, 'a');
+
+  std::memset(foo, 1, 3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Use std::fill instead of memset [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: Use std::copy instead of memcpy [modernize-use-algorithm]
+}
Index: docs/clang-tidy/checks/modernize-use-algorithm.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/modernize-use-algorithm.rst
@@ -0,0 +1,37 @@
+.. title:: clang-tidy - modernize-use-algorithm
+
+modernize-use-algorithm
+=======================
+
+Replaces calls to ``memcpy`` and ``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.
+
+Furthermore, 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)
+
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -105,6 +105,7 @@
    modernize-shrink-to-fit
    modernize-use-auto
    modernize-use-bool-literals
+   modernize-use-algorithm
    modernize-use-default
    modernize-use-emplace
    modernize-use-nullptr
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 with std::copy
+///
+/// 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;
+  llvm::StringMap<std::string> Replacements;
+};
+
+} // 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,160 @@
+//===--- 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 <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 areTypesCompatible(QualType Left, QualType Right) {
+  return getStrippedType(Left) == getStrippedType(Right);
+}
+
+static StringRef getText(const Expr *Expression, const SourceManager &SM,
+                         const LangOptions &LangOpts) {
+  return Lexer::getSourceText(
+      CharSourceRange::getTokenRange(Expression->getSourceRange()), SM,
+      LangOpts);
+}
+
+static std::string getReplacement(const StringRef Function,
+                                  const StringRef Arg0, const StringRef Arg1,
+                                  const StringRef Arg2) {
+  return Function.str() + "(" + Arg0.str() + ", " + Arg1.str() + ", " +
+         Arg2.str() + ")";
+}
+
+UseAlgorithmCheck::UseAlgorithmCheck(StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
+          Options.get("IncludeStyle", "llvm"))) {
+
+  for (const auto &KeyValue :
+       std::vector<std::pair<llvm::StringRef, std::string>>(
+           {{"memcpy", "std::copy"}, {"memset", "std::fill"}})) {
+    Replacements.insert(KeyValue);
+  }
+}
+
+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("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) {
+    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 auto MatchedName = Callee->getNameAsString();
+
+  // Check if matched name is in map of replacements.
+  const auto it = Replacements.find(MatchedName);
+  if (it == Replacements.end())
+    return;
+  const auto ReplacedName = it->second;
+
+  const auto Loc = MatchedExpr->getExprLoc();
+  auto Diag = diag(Loc, "Use " + ReplacedName + " instead of " + MatchedName);
+
+  // Don't make replacements in macro.
+  if (Loc.isMacroID())
+    return;
+
+  const auto Arg0Type = MatchedExpr->getArg(0)->IgnoreImpCasts()->getType();
+  const auto 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 (!areTypesCompatible(Arg0Type, Arg1Type))
+    return;
+
+  const auto &SM = *Result.SourceManager;
+  const auto &LangOptions = Result.Context->getLangOpts();
+
+  StringRef Arg0Text = getText(MatchedExpr->getArg(0), SM, LangOptions);
+  StringRef Arg1Text = getText(MatchedExpr->getArg(1), SM, LangOptions);
+  StringRef Arg2Text = getText(MatchedExpr->getArg(2), SM, LangOptions);
+
+  std::string Insertion;
+  if (MatchedName == "memset") {
+    // Cannot do pointer arithmetic on void type.
+    if (getStrippedType(Arg0Type)->isVoidType())
+      return;
+
+    // Rearrangement of arguments for memset:
+    // (dest, ch, count) becomes (dest, dest + count, ch).
+    Insertion = getReplacement(ReplacedName, Arg0Text,
+                               (Arg0Text + " + " + Arg2Text).str(), Arg1Text);
+  } else {
+    // Cannot do pointer arithmetic on void type.
+    if (getStrippedType(Arg1Type)->isVoidType())
+      return;
+
+    // Rearrangement of arguments for memcpy
+    // (dest, src, count) becomes (src, src + count, dest).
+    Insertion = getReplacement(ReplacedName, Arg1Text,
+                               (Arg1Text + " + " + Arg2Text).str(), Arg0Text);
+  }
+
+  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
@@ -22,6 +22,7 @@
 #include "ShrinkToFitCheck.h"
 #include "UseAutoCheck.h"
 #include "UseBoolLiteralsCheck.h"
+#include "UseAlgorithmCheck.h"
 #include "UseDefaultCheck.h"
 #include "UseEmplaceCheck.h"
 #include "UseNullptrCheck.h"
@@ -55,6 +56,8 @@
     CheckFactories.registerCheck<UseAutoCheck>("modernize-use-auto");
     CheckFactories.registerCheck<UseBoolLiteralsCheck>(
         "modernize-use-bool-literals");
+    CheckFactories.registerCheck<UseAlgorithmCheck>(
+        "modernize-use-algorithm");
     CheckFactories.registerCheck<UseDefaultCheck>("modernize-use-default");
     CheckFactories.registerCheck<UseEmplaceCheck>("modernize-use-emplace");
     CheckFactories.registerCheck<UseNullptrCheck>("modernize-use-nullptr");
Index: clang-tidy/modernize/CMakeLists.txt
===================================================================
--- clang-tidy/modernize/CMakeLists.txt
+++ clang-tidy/modernize/CMakeLists.txt
@@ -16,6 +16,7 @@
   ShrinkToFitCheck.cpp
   UseAutoCheck.cpp
   UseBoolLiteralsCheck.cpp
+  UseAlgorithmCheck.cpp
   UseDefaultCheck.cpp
   UseEmplaceCheck.cpp
   UseNullptrCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to