[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2022-01-09 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added inline comments.
Herald added a subscriber: carlosgalvezp.



Comment at: clang-tools-extra/clang-tidy/modernize/CMakeLists.txt:19
   ReplaceAutoPtrCheck.cpp
+  ReplaceMemcpyByStdCopy.cpp
   ReplaceRandomShuffleCheck.cpp

In English, it's more idiomatic to say "Replace `memcpy` with `std::copy`",
but perhaps an even better name for this check is `modernize-use-std-copy`.
This opens the door to future improvements that recognize hand-written
for loops that are essentially just invocations of `std::copy` or `std::copy_n`.
(Recently I was considering writing a check like that while reviewing some
code at work that was full of hand-written for loops that just copied elements
from one container to another.)



Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp:35
+   isExpansionInSystemHeader())),
+   isExpansionInMainFile())
+  .bind("memcpy_function");

I erroneously had this in some checks I had written; it prevents fixits
from being applied to user supplied header files and is undesirable.

I would suggest removing both the `isExpansionInSystemHeader` and
`isExpansionInMainFile` narrowing matchers.



Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp:70
+const CallExpr *MemcpyNode) {
+  const CharSourceRange FunctionNameSourceRange = 
CharSourceRange::getCharRange(
+  MemcpyNode->getBeginLoc(), MemcpyNode->getArg(0)->getBeginLoc());

In the clang code base, they prefer not to use `const` on local variables
unless the local is a pointer to a `const` object and even then, it's only
the pointed-to object that is declared `const` not the pointer itself.



Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp:78
+ const CallExpr *MemcpyNode) {
+  std::array arg;
+

LLVM naming convention is `CapCamelCase` for variables.
See here and variables declared on lines 85, 86, 91, etc.



Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp:91
+  // Create lambda that return SourceRange of an argument
+  auto getSourceRange = [MemcpyNode](uint8_t ArgCount) -> SourceRange {
+return SourceRange(MemcpyNode->getArg(ArgCount)->getBeginLoc(),

Prefer `int` over `uint8_t` here as there is no requirement for
a type of a specific bit width.  The explicit return type is also
unnecessary, but please consult the LLVM style guide to make
sure you're following any prescriptions for lambdas.

Additionally, `ArgCount` doesn't feel like the right name for this
parameter.  It's not the count of anything, it's an index into the
arguments, so just `Arg` feels better.



Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp:99
+
+  arg[2] = arg[1] + " + ((" + arg[2] + ") / sizeof(*(" + arg[1] + ")))";
+  Diag << FixItHint::CreateReplacement(getSourceRange(1), arg[2]);

I think it would be worthwhile to probe the AST of the argument here and see if 
it is
already of the form `N*sizeof(T)`.

This can probably be handled by enhancing the matchers on the argument so that 
you
aren't probing the AST in procedural code.



Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp:11
+#include "../utils/OptionsUtils.h"
+
+#include 

Eugene.Zelenko wrote:
> Please remove unnecessary empty line.
> Please remove unnecessary empty line.

Where is this in the LLVM coding style guide?



Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h:26
+  ReplaceMemcpyByStdCopy(StringRef Name, ClangTidyContext *Context);
+  virtual ~ReplaceMemcpyByStdCopy() {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;

Eugene.Zelenko wrote:
> Should be override and = default. See modernize-use-override and 
> modernize-use-equals-default.
> Should be override and = default

Why do you need to override it if you're just using the compiler's default?



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/modernize-replace-memcpy-by-stdcopy.rst:33
+
+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)``

Please use a line length in the `.rst` files that is consistent with the LLVM 
line length
for code files.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63324/new/

https://reviews.llvm.org/D63324

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https:

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-12-30 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment.

if(num != 0) memmove is missed optimization opportunity. Such branch can be 
removed (if profitable).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63324/new/

https://reviews.llvm.org/D63324



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-12-30 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

Thinking about it more, i have some negative reservations about this :/

As it can be seen form https://godbolt.org/z/3BZmCM, it seems any and every(?) 
alternative C++ algorithm
replacement is a performance pessimization in the general case, because 
`memcpy` requires/'guarantees'
that there must be no overlap between source and destination ranges,
while there is no such restriction for C++ algorithms (=> they will get 
optimized into `memmove`).
That is unless optimizer also manages to prove that it is safe to optimize 
`memmove` into `memcpy` there.
(Extra bloat `if(num != 0) memmove` is also not always good)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63324/new/

https://reviews.llvm.org/D63324



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-12-30 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.
Herald added subscribers: wuzish, whisperity.

You need tests for this check. Please harden them against macro-interference as 
well. I think the transformation should happen as function call that return 
`Optional` and if any failure happens the diagnostic can still be emitted.




Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp:27
+void ReplaceMemcpyByStdCopy::registerMatchers(MatchFinder *Finder) {
+  assert(Finder != nullptr);
+

this assertion is not necessary. `Finder` is always not-null.



Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp:33
+  auto MemcpyMatcher =
+  callExpr(hasDeclaration(functionDecl(hasName("memcpy"),
+   isExpansionInSystemHeader())),

please match on `::memcpy` and `::std::memcpy`.



Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp:53
+  const auto *MemcpyNode = Result.Nodes.getNodeAs("memcpy_function");
+  assert(MemcpyNode != nullptr);
+

this assertion is not necessary, as you only match on one thing and that means 
it exists. (otherwise everything is broken already).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63324/new/

https://reviews.llvm.org/D63324



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-06-18 Thread Thomas Manceau via Phabricator via cfe-commits
Blackhart updated this revision to Diff 205354.
Blackhart marked 4 inline comments as done.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63324/new/

https://reviews.llvm.org/D63324

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/modernize-replace-memcpy-by-stdcopy.rst

Index: clang-tools-extra/docs/clang-tidy/checks/modernize-replace-memcpy-by-stdcopy.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/modernize-replace-memcpy-by-stdcopy.rst
@@ -0,0 +1,47 @@
+.. title:: clang-tidy - modernize-replace-memcpy-by-stdcopy
+
+modernize-replace-memcpy-by-stdcopy
+===
+
+Replaces all occurrences of the C ``memcpy`` function by ``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`.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -211,6 +211,7 @@
modernize-raw-string-literal
modernize-redundant-void-arg
modernize-replace-auto-ptr
+   modernize-replace-memcpy-by-stdcopy
modernize-replace-random-shuffle
modernize-return-braced-init-list
modernize-shrink-to-fit
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -186,6 +186,11 @@
   finds and replaces cases that match the pattern ``var &&
   isa(var)``, where ``var`` is evaluated twice.
 
+- New :doc:`modernize-replace-memcpy-by-stdcopy
+  ` check.
+  
+  Replaces all occurrences of the C ``memcpy`` function by ``std::copy``.
+
 - New :doc:`modernize-use-trailing-return-type
   ` check.
 
Index: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
@@ -0,0 +1,48 @@
+//===--- ReplaceMemcpyByStdCopy.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_BY_STDCOPY_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
+
+#include "../ClangTidyCheck.h"
+#include "../utils/IncludeInserter.h"
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// Replace the C memcpy function by std::copy
+class ReplaceMemcpyByStdCopy : public ClangTidyCheck {
+public:
+  ReplaceMemcpyByStdCopy(StringRef Name, ClangTidyContext *Context);
+  ~ReplaceMemcpyByStdCopy() 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

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-06-18 Thread Thomas Manceau via Phabricator via cfe-commits
Blackhart updated this revision to Diff 205341.
Blackhart marked 3 inline comments as done.
Blackhart added a comment.

Remove "//this check//" from documentation and release note


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63324/new/

https://reviews.llvm.org/D63324

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/modernize-replace-memcpy-by-stdcopy.rst

Index: clang-tools-extra/docs/clang-tidy/checks/modernize-replace-memcpy-by-stdcopy.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/modernize-replace-memcpy-by-stdcopy.rst
@@ -0,0 +1,47 @@
+.. title:: clang-tidy - modernize-replace-memcpy-by-stdcopy
+
+modernize-replace-memcpy-by-stdcopy
+===
+
+Replaces all occurrences of the C ``memcpy`` function by ``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)``
+
+Include
+---
+
+``std::copy`` being provided by the ``algorithm`` include 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`.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -211,6 +211,7 @@
modernize-raw-string-literal
modernize-redundant-void-arg
modernize-replace-auto-ptr
+   modernize-replace-memcpy-by-stdcopy
modernize-replace-random-shuffle
modernize-return-braced-init-list
modernize-shrink-to-fit
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -186,6 +186,11 @@
   finds and replaces cases that match the pattern ``var &&
   isa(var)``, where ``var`` is evaluated twice.
 
+- New :doc:`modernize-replace-memcpy-by-stdcopy
+  ` check.
+  
+  Replaces all occurrences of the C ``memcpy`` function by ``std::copy``.
+
 - New :doc:`modernize-use-trailing-return-type
   ` check.
 
Index: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
@@ -0,0 +1,48 @@
+//===--- ReplaceMemcpyByStdCopy.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_BY_STDCOPY_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
+
+#include "../ClangTidyCheck.h"
+#include "../utils/IncludeInserter.h"
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// Replace the C memcpy function by std::copy
+class ReplaceMemcpyByStdCopy : public ClangTidyCheck {
+public:
+  ReplaceMemcpyByStdCopy(StringRef Name, ClangTidyContext *Context);
+  ~ReplaceMemcpyByStdCopy() 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:
+ 

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-06-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

Still missing tests




Comment at: 
clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp:34-35
+  callExpr(hasDeclaration(functionDecl(hasName("memcpy"),
+   isExpansionInSystemHeader())),
+   isExpansionInMainFile())
+  .bind("memcpy_function");

Why these restrictions?



Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h:26
+  ReplaceMemcpyByStdCopy(StringRef Name, ClangTidyContext *Context);
+  ~ReplaceMemcpyByStdCopy() override = default;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;

I don't think you need this one


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63324/new/

https://reviews.llvm.org/D63324



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-06-18 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:192
+  
+  This check replaces all occurrences of the C ``memcpy`` function by 
``std::copy``.
+

Please remove //This check//. Same in first statement in documentation.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63324/new/

https://reviews.llvm.org/D63324



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-06-18 Thread Thomas Manceau via Phabricator via cfe-commits
Blackhart added a comment.

Hello,

I've updated the documentation with more informations about the transformations 
the check do.

I'm not very confident in writing documentation in english.
Please review it and submit me your change requests.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63324/new/

https://reviews.llvm.org/D63324



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-06-18 Thread Thomas Manceau via Phabricator via cfe-commits
Blackhart updated this revision to Diff 205331.
Blackhart added a comment.

- Reorder release note changes
- Update documentation




CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63324/new/

https://reviews.llvm.org/D63324

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/modernize-replace-memcpy-by-stdcopy.rst

Index: clang-tools-extra/docs/clang-tidy/checks/modernize-replace-memcpy-by-stdcopy.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/modernize-replace-memcpy-by-stdcopy.rst
@@ -0,0 +1,47 @@
+.. title:: clang-tidy - modernize-replace-memcpy-by-stdcopy
+
+modernize-replace-memcpy-by-stdcopy
+===
+
+This check replaces all occurrences of the C ``memcpy`` function by ``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)``
+
+Include
+---
+
+``std::copy`` being provided by the ``algorithm`` include 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`.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -211,6 +211,7 @@
modernize-raw-string-literal
modernize-redundant-void-arg
modernize-replace-auto-ptr
+   modernize-replace-memcpy-by-stdcopy
modernize-replace-random-shuffle
modernize-return-braced-init-list
modernize-shrink-to-fit
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -186,6 +186,11 @@
   finds and replaces cases that match the pattern ``var &&
   isa(var)``, where ``var`` is evaluated twice.
 
+- New :doc:`modernize-replace-memcpy-by-stdcopy
+  ` check.
+  
+  This check replaces all occurrences of the C ``memcpy`` function by ``std::copy``.
+
 - New :doc:`modernize-use-trailing-return-type
   ` check.
 
Index: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
@@ -0,0 +1,48 @@
+//===--- ReplaceMemcpyByStdCopy.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_BY_STDCOPY_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
+
+#include "../ClangTidyCheck.h"
+#include "../utils/IncludeInserter.h"
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// Replace the C memcpy function by std::copy
+class ReplaceMemcpyByStdCopy : public ClangTidyCheck {
+public:
+  ReplaceMemcpyByStdCopy(StringRef Name, ClangTidyContext *Context);
+  ~ReplaceMemcpyByStdCopy() 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(Diagn

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-06-15 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h:41
+  std::unique_ptr Inserter;
+  const utils::IncludeSorter::IncludeStyle IncludeStyle;
+};

I think this could be determined from .clang-format.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:204
+  
+- New :doc:`modernize-replace-memcpy-by-stdcopy
+  ` check.

Please move into new checks list (in alphabetical order).



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:207
+  
+  Replace all occurences of the C "memcpy" function by "std::copy".
 

Replace**s**. Please use double back-ticks to highlight language constructs.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/modernize-replace-memcpy-by-stdcopy.rst:6
+
+This check replaces all occurences of the C "memcpy" function by "std::copy"
+

Please make first sentance same as in Release Notes.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/modernize-replace-memcpy-by-stdcopy.rst:35
+
+   A string specifying which include-style is used, `llvm` or `google`. Default
+   is `llvm`.

I think this could be determined from project's .clang-format.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63324/new/

https://reviews.llvm.org/D63324



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-06-15 Thread Thomas Manceau via Phabricator via cfe-commits
Blackhart updated this revision to Diff 204932.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63324/new/

https://reviews.llvm.org/D63324

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/modernize-replace-memcpy-by-stdcopy.rst

Index: clang-tools-extra/docs/clang-tidy/checks/modernize-replace-memcpy-by-stdcopy.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/modernize-replace-memcpy-by-stdcopy.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - modernize-replace-memcpy-by-stdcopy
+
+modernize-replace-memcpy-by-stdcopy
+===
+
+This check replaces all occurences of the C "memcpy" function by "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);
+
+Options
+---
+
+.. option:: IncludeStyle
+
+   A string specifying which include-style is used, `llvm` or `google`. Default
+   is `llvm`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -200,6 +200,11 @@
 
   If set to true, the check will provide fix-its with literal initializers
   (``int i = 0;``) instead of curly braces (``int i{};``).
+  
+- New :doc:`modernize-replace-memcpy-by-stdcopy
+  ` check.
+  
+  Replace all occurences of the C "memcpy" function by "std::copy".
 
 Improvements to include-fixer
 -
Index: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
@@ -0,0 +1,48 @@
+//===--- ReplaceMemcpyByStdCopy.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_BY_STDCOPY_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
+
+#include "../ClangTidyCheck.h"
+#include "../utils/IncludeInserter.h"
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// Replace the C memcpy function by std::copy
+class ReplaceMemcpyByStdCopy : public ClangTidyCheck {
+public:
+  ReplaceMemcpyByStdCopy(StringRef Name, ClangTidyContext *Context);
+  ~ReplaceMemcpyByStdCopy() 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;
+  const utils::IncludeSorter::IncludeStyle IncludeStyle;
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
Index: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
@@ -0,0 +1,117 @@
+//===--- ReplaceMemcpyByStdCopy.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
+//
+//===-

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-06-15 Thread Thomas Manceau via Phabricator via cfe-commits
Blackhart updated this revision to Diff 204930.
Blackhart marked 2 inline comments as done.
Blackhart added a comment.

- Update releasenotes
- Add documentation


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63324/new/

https://reviews.llvm.org/D63324

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/modernize-replace-memcpy-by-stdcopy.rst

Index: clang-tools-extra/docs/clang-tidy/checks/modernize-replace-memcpy-by-stdcopy.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/modernize-replace-memcpy-by-stdcopy.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - modernize-replace-memcpy-by-stdcopy
+
+modernize-replace-memcpy-by-stdcopy
+===
+
+This check all occurences of the C "memcpy" function by "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);
+
+Options
+---
+
+.. option:: IncludeStyle
+
+   A string specifying which include-style is used, `llvm` or `google`. Default
+   is `llvm`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -200,6 +200,11 @@
 
   If set to true, the check will provide fix-its with literal initializers
   (``int i = 0;``) instead of curly braces (``int i{};``).
+  
+- New :doc:`modernize-replace-memcpy-by-stdcopy
+  ` check.
+  
+  Replace all occurences of the C "memcpy" function by "std::copy".
 
 Improvements to include-fixer
 -
Index: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
@@ -0,0 +1,48 @@
+//===--- ReplaceMemcpyByStdCopy.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_BY_STDCOPY_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
+
+#include "../ClangTidyCheck.h"
+#include "../utils/IncludeInserter.h"
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// Replace the C memcpy function by std::copy
+class ReplaceMemcpyByStdCopy : public ClangTidyCheck {
+public:
+  ReplaceMemcpyByStdCopy(StringRef Name, ClangTidyContext *Context);
+  ~ReplaceMemcpyByStdCopy() 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;
+  const utils::IncludeSorter::IncludeStyle IncludeStyle;
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
Index: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
@@ -0,0 +1,117 @@
+//===--- ReplaceMemcpyByStdCopy.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.
+// S

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-06-15 Thread Thomas Manceau via Phabricator via cfe-commits
Blackhart updated this revision to Diff 204926.
Blackhart marked an inline comment as done.
Blackhart added a comment.

- Remove unnessecary empty line
- Use "array.size()" instead of numerical constant


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63324/new/

https://reviews.llvm.org/D63324

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h

Index: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
@@ -0,0 +1,48 @@
+//===--- ReplaceMemcpyByStdCopy.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_BY_STDCOPY_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
+
+#include "../ClangTidyCheck.h"
+#include "../utils/IncludeInserter.h"
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// Replace the C memcpy function by std::copy
+class ReplaceMemcpyByStdCopy : public ClangTidyCheck {
+public:
+  ReplaceMemcpyByStdCopy(StringRef Name, ClangTidyContext *Context);
+  ~ReplaceMemcpyByStdCopy() 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;
+  const utils::IncludeSorter::IncludeStyle IncludeStyle;
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
Index: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
@@ -0,0 +1,117 @@
+//===--- ReplaceMemcpyByStdCopy.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 "ReplaceMemcpyByStdCopy.h"
+#include "../utils/OptionsUtils.h"
+#include 
+
+using namespace clang;
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+ReplaceMemcpyByStdCopy::ReplaceMemcpyByStdCopy(StringRef Name,
+   ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
+  Options.getLocalOrGlobal("IncludeStyle", "llvm"))) {}
+
+void ReplaceMemcpyByStdCopy::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 ReplaceMemcpyByStdCopy::registerPPCallbacks(
+const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
+  if (!getLangOpts().CPlusPlus)
+return;
+
+  Inserter = llvm::make_unique(SM, getLangOpts(),
+   IncludeStyle);
+  PP->addPPCallbacks(Inserter->CreatePPCallbacks());
+}
+
+void ReplaceMemcpyByStdCopy::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.SourceMan

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-06-15 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp:11
+#include "../utils/OptionsUtils.h"
+
+#include 

Please remove unnecessary empty line.



Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp:86
+  // Retrieve all the arguments
+  for (uint8_t i = 0; i < 3; i++) {
+llvm::raw_string_ostream s(arg[i]);

Please use arg,size() instead of 3.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63324/new/

https://reviews.llvm.org/D63324



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-06-15 Thread Thomas Manceau via Phabricator via cfe-commits
Blackhart updated this revision to Diff 204925.
Blackhart marked an inline comment as done.
Blackhart added a comment.

Use "std::array" instead of the raw array


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63324/new/

https://reviews.llvm.org/D63324

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h

Index: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
@@ -0,0 +1,48 @@
+//===--- ReplaceMemcpyByStdCopy.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_BY_STDCOPY_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
+
+#include "../ClangTidyCheck.h"
+#include "../utils/IncludeInserter.h"
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// Replace the C memcpy function by std::copy
+class ReplaceMemcpyByStdCopy : public ClangTidyCheck {
+public:
+  ReplaceMemcpyByStdCopy(StringRef Name, ClangTidyContext *Context);
+  ~ReplaceMemcpyByStdCopy() 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;
+  const utils::IncludeSorter::IncludeStyle IncludeStyle;
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
Index: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
@@ -0,0 +1,118 @@
+//===--- ReplaceMemcpyByStdCopy.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 "ReplaceMemcpyByStdCopy.h"
+#include "../utils/OptionsUtils.h"
+
+#include 
+
+using namespace clang;
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+ReplaceMemcpyByStdCopy::ReplaceMemcpyByStdCopy(StringRef Name,
+   ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
+  Options.getLocalOrGlobal("IncludeStyle", "llvm"))) {}
+
+void ReplaceMemcpyByStdCopy::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 ReplaceMemcpyByStdCopy::registerPPCallbacks(
+const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
+  if (!getLangOpts().CPlusPlus)
+return;
+
+  Inserter = llvm::make_unique(SM, getLangOpts(),
+   IncludeStyle);
+  PP->addPPCallbacks(Inserter->CreatePPCallbacks());
+}
+
+void ReplaceMemcpyByStdCopy::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 ReplaceMemcpyByStdCop

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-06-15 Thread Thomas Manceau via Phabricator via cfe-commits
Blackhart updated this revision to Diff 204924.
Blackhart marked an inline comment as done.
Blackhart added a comment.

Remove "auto" keyword when type is created from same statement


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63324/new/

https://reviews.llvm.org/D63324

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h

Index: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
@@ -0,0 +1,48 @@
+//===--- ReplaceMemcpyByStdCopy.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_BY_STDCOPY_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
+
+#include "../ClangTidyCheck.h"
+#include "../utils/IncludeInserter.h"
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// Replace the C memcpy function by std::copy
+class ReplaceMemcpyByStdCopy : public ClangTidyCheck {
+public:
+  ReplaceMemcpyByStdCopy(StringRef Name, ClangTidyContext *Context);
+  ~ReplaceMemcpyByStdCopy() 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;
+  const utils::IncludeSorter::IncludeStyle IncludeStyle;
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
Index: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
@@ -0,0 +1,116 @@
+//===--- ReplaceMemcpyByStdCopy.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 "ReplaceMemcpyByStdCopy.h"
+#include "../utils/OptionsUtils.h"
+
+using namespace clang;
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+ReplaceMemcpyByStdCopy::ReplaceMemcpyByStdCopy(StringRef Name,
+   ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
+  Options.getLocalOrGlobal("IncludeStyle", "llvm"))) {}
+
+void ReplaceMemcpyByStdCopy::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 ReplaceMemcpyByStdCopy::registerPPCallbacks(
+const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
+  if (!getLangOpts().CPlusPlus)
+return;
+
+  Inserter = llvm::make_unique(SM, getLangOpts(),
+   IncludeStyle);
+  PP->addPPCallbacks(Inserter->CreatePPCallbacks());
+}
+
+void ReplaceMemcpyByStdCopy::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 ReplaceMemcpy

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-06-15 Thread Thomas Manceau via Phabricator via cfe-commits
Blackhart updated this revision to Diff 204922.
Blackhart added a comment.

Add override keyword to the destructor and set it as "= default"


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63324/new/

https://reviews.llvm.org/D63324

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h

Index: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
@@ -0,0 +1,48 @@
+//===--- ReplaceMemcpyByStdCopy.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_BY_STDCOPY_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
+
+#include "../ClangTidyCheck.h"
+#include "../utils/IncludeInserter.h"
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// Replace the C memcpy function by std::copy
+class ReplaceMemcpyByStdCopy : public ClangTidyCheck {
+public:
+  ReplaceMemcpyByStdCopy(StringRef Name, ClangTidyContext *Context);
+  ~ReplaceMemcpyByStdCopy() 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;
+  const utils::IncludeSorter::IncludeStyle IncludeStyle;
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
Index: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
@@ -0,0 +1,115 @@
+//===--- ReplaceMemcpyByStdCopy.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 "ReplaceMemcpyByStdCopy.h"
+#include "../utils/OptionsUtils.h"
+
+using namespace clang;
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+ReplaceMemcpyByStdCopy::ReplaceMemcpyByStdCopy(StringRef Name,
+   ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
+  Options.getLocalOrGlobal("IncludeStyle", "llvm"))) {}
+
+void ReplaceMemcpyByStdCopy::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 ReplaceMemcpyByStdCopy::registerPPCallbacks(
+const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
+  if (!getLangOpts().CPlusPlus)
+return;
+
+  Inserter = llvm::make_unique(SM, getLangOpts(),
+   IncludeStyle);
+  PP->addPPCallbacks(Inserter->CreatePPCallbacks());
+}
+
+void ReplaceMemcpyByStdCopy::check(const MatchFinder::MatchResult &Result) {
+  const auto *MemcpyNode = Result.Nodes.getNodeAs("memcpy_function");
+  assert(MemcpyNode != nullptr);
+
+  auto Diag = diag(MemcpyNode->getExprLoc(), "use std::copy instead of memcpy");
+
+  renameFunction(Diag, MemcpyNode);
+  reorderArgs(Diag, MemcpyNode);
+  insertHeader(Diag, MemcpyNode, Result.SourceManager);
+}
+
+void ReplaceMemcpyByStdCopy::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-06-15 Thread Thomas Manceau via Phabricator via cfe-commits
Blackhart updated this revision to Diff 204920.
Blackhart added a comment.

Revert "Modernize memcpy only if C++20 is enabled"


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63324/new/

https://reviews.llvm.org/D63324

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h

Index: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
@@ -0,0 +1,48 @@
+//===--- ReplaceMemcpyByStdCopy.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_BY_STDCOPY_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
+
+#include "../ClangTidyCheck.h"
+#include "../utils/IncludeInserter.h"
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// Replace the C memcpy function by std::copy
+class ReplaceMemcpyByStdCopy : public ClangTidyCheck {
+public:
+  ReplaceMemcpyByStdCopy(StringRef Name, ClangTidyContext *Context);
+  virtual ~ReplaceMemcpyByStdCopy() {}
+  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;
+  const utils::IncludeSorter::IncludeStyle IncludeStyle;
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
Index: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
@@ -0,0 +1,115 @@
+//===--- ReplaceMemcpyByStdCopy.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 "ReplaceMemcpyByStdCopy.h"
+#include "../utils/OptionsUtils.h"
+
+using namespace clang;
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+ReplaceMemcpyByStdCopy::ReplaceMemcpyByStdCopy(StringRef Name,
+   ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
+  Options.getLocalOrGlobal("IncludeStyle", "llvm"))) {}
+
+void ReplaceMemcpyByStdCopy::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 ReplaceMemcpyByStdCopy::registerPPCallbacks(
+const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
+  if (!getLangOpts().CPlusPlus)
+return;
+
+  Inserter = llvm::make_unique(SM, getLangOpts(),
+   IncludeStyle);
+  PP->addPPCallbacks(Inserter->CreatePPCallbacks());
+}
+
+void ReplaceMemcpyByStdCopy::check(const MatchFinder::MatchResult &Result) {
+  const auto *MemcpyNode = Result.Nodes.getNodeAs("memcpy_function");
+  assert(MemcpyNode != nullptr);
+
+  auto Diag = diag(MemcpyNode->getExprLoc(), "use std::copy instead of memcpy");
+
+  renameFunction(Diag, MemcpyNode);
+  reorderArgs(Diag, MemcpyNode);
+  insertHeader(Diag, MemcpyNode, Result.SourceManager);
+}
+
+void ReplaceMemcpyByStdCopy::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-06-15 Thread Thomas Manceau via Phabricator via cfe-commits
Blackhart added a comment.

In D63324#1543782 , @xbolva00 wrote:

> This might not be currently ideal recommendation since std::copy produces 
> memmove with -O3.


What do you recommend ?
There is a way to disable this modernization if the user compile with the -O3 
flag ?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63324/new/

https://reviews.llvm.org/D63324



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-06-15 Thread Thomas Manceau via Phabricator via cfe-commits
Blackhart added a comment.

In D63324#1543706 , @lebedev.ri wrote:

> In D63324#1543626 , @Blackhart wrote:
>
> > In D63324#1543609 , @lebedev.ri 
> > wrote:
> >
> > > In D63324#1543607 , @Blackhart 
> > > wrote:
> > >
> > > > Modernize memcpy only if C++20 is enabled
> > >
> > >
> > > ... why?
> > >  This is also missing documentation,releasenotes changes.
> >
> >
> > According with the C++ reference, std::copy is only available since C++20.
>
>
> I don't see it there, can you quote?
>  https://godbolt.org/z/q2ryJi
>
> > There is another std::copy signature available since C++17, but it needs an 
> > extra parameter. I can implement it also.
> >  https://en.cppreference.com/w/cpp/algorithm/copy
> > 
> > I'm working on the unit tests and I'll make documentation/releasenotes 
> > after that.
> > 
> > Thanks for reviewing @lebedev.ri ;)


Sorry, I misread the reference. It's "until" C++20.
So from C++ birth to today ...

I'll revert my changes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63324/new/

https://reviews.llvm.org/D63324



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-06-14 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment.

New check must be mentioned in Release Notes and its documentation provided.




Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp:68
+const CallExpr *MemcpyNode) {
+  const auto FunctionNameSourceRange = CharSourceRange::getCharRange(
+  MemcpyNode->getBeginLoc(), MemcpyNode->getArg(0)->getBeginLoc());

Return type could not be deduced from same statement, so please don't use auto.



Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp:76
+ const CallExpr *MemcpyNode) {
+  std::string arg[3];
+

Will be good idea to use std::array and its size instead of numerical constant.



Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h:26
+  ReplaceMemcpyByStdCopy(StringRef Name, ClangTidyContext *Context);
+  virtual ~ReplaceMemcpyByStdCopy() {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;

Should be override and = default. See modernize-use-override and 
modernize-use-equals-default.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63324/new/

https://reviews.llvm.org/D63324



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-06-14 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment.

This might not be currently ideal recommendation since std::copy produces 
memmove with -O3.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63324/new/

https://reviews.llvm.org/D63324



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-06-14 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

In D63324#1543626 , @Blackhart wrote:

> In D63324#1543609 , @lebedev.ri 
> wrote:
>
> > In D63324#1543607 , @Blackhart 
> > wrote:
> >
> > > Modernize memcpy only if C++20 is enabled
> >
> >
> > ... why?
> >  This is also missing documentation,releasenotes changes.
>
>
> According with the C++ reference, std::copy is only available since C++20.


I don't see it there, can you quote?
https://godbolt.org/z/q2ryJi

> There is another std::copy signature available since C++17, but it needs an 
> extra parameter. I can implement it also.
>  https://en.cppreference.com/w/cpp/algorithm/copy
> 
> I'm working on the unit tests and I'll make documentation/releasenotes after 
> that.
> 
> Thanks for reviewing @lebedev.ri ;)




CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63324/new/

https://reviews.llvm.org/D63324



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-06-14 Thread Thomas Manceau via Phabricator via cfe-commits
Blackhart added a comment.

In D63324#1543609 , @lebedev.ri wrote:

> In D63324#1543607 , @Blackhart wrote:
>
> > Modernize memcpy only if C++20 is enabled
>
>
> ... why?
>  This is also missing documentation,releasenotes changes.


According with the C++ reference, std::copy is only available since C++20.
There is another std::copy signature available since C++17, but it needs an 
extra parameter. I can implement it also.
https://en.cppreference.com/w/cpp/algorithm/copy

I'm working on the unit tests and I'll make documentation/releasenotes after 
that.

Thanks for reviewing @lebedev.ri ;)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63324/new/

https://reviews.llvm.org/D63324



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-06-14 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

In D63324#1543607 , @Blackhart wrote:

> Modernize memcpy only if C++20 is enabled


... why?
This is also missing documentation,releasenotes changes.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63324/new/

https://reviews.llvm.org/D63324



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-06-14 Thread Thomas Manceau via Phabricator via cfe-commits
Blackhart updated this revision to Diff 204770.
Blackhart added a comment.

Modernize memcpy only if C++20 is enabled


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63324/new/

https://reviews.llvm.org/D63324

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h

Index: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
@@ -0,0 +1,48 @@
+//===--- ReplaceMemcpyByStdCopy.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_BY_STDCOPY_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
+
+#include "../ClangTidyCheck.h"
+#include "../utils/IncludeInserter.h"
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// Replace the C memcpy function by std::copy
+class ReplaceMemcpyByStdCopy : public ClangTidyCheck {
+public:
+  ReplaceMemcpyByStdCopy(StringRef Name, ClangTidyContext *Context);
+  virtual ~ReplaceMemcpyByStdCopy() {}
+  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;
+  const utils::IncludeSorter::IncludeStyle IncludeStyle;
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
Index: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
@@ -0,0 +1,115 @@
+//===--- ReplaceMemcpyByStdCopy.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 "ReplaceMemcpyByStdCopy.h"
+#include "../utils/OptionsUtils.h"
+
+using namespace clang;
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+ReplaceMemcpyByStdCopy::ReplaceMemcpyByStdCopy(StringRef Name,
+   ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
+  Options.getLocalOrGlobal("IncludeStyle", "llvm"))) {}
+
+void ReplaceMemcpyByStdCopy::registerMatchers(MatchFinder *Finder) {
+  assert(Finder != nullptr);
+
+  if (!getLangOpts().CPlusPlus2a)
+return;
+
+  auto MemcpyMatcher =
+  callExpr(hasDeclaration(functionDecl(hasName("memcpy"),
+   isExpansionInSystemHeader())),
+   isExpansionInMainFile())
+  .bind("memcpy_function");
+
+  Finder->addMatcher(MemcpyMatcher, this);
+}
+
+void ReplaceMemcpyByStdCopy::registerPPCallbacks(
+const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
+  if (!getLangOpts().CPlusPlus2a)
+return;
+
+  Inserter = llvm::make_unique(SM, getLangOpts(),
+   IncludeStyle);
+  PP->addPPCallbacks(Inserter->CreatePPCallbacks());
+}
+
+void ReplaceMemcpyByStdCopy::check(const MatchFinder::MatchResult &Result) {
+  const auto *MemcpyNode = Result.Nodes.getNodeAs("memcpy_function");
+  assert(MemcpyNode != nullptr);
+
+  auto Diag = diag(MemcpyNode->getExprLoc(), "use std::copy instead of memcpy");
+
+  renameFunction(Diag, MemcpyNode);
+  reorderArgs(Diag, MemcpyNode);
+  insertHeader(Diag, MemcpyNode, Result.SourceManager);
+}
+
+void ReplaceMemcpyByStdCopy::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "Inclu

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-06-14 Thread Thomas Manceau via Phabricator via cfe-commits
Blackhart updated this revision to Diff 204768.
Blackhart marked 2 inline comments as done.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63324/new/

https://reviews.llvm.org/D63324

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h

Index: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
@@ -0,0 +1,48 @@
+//===--- ReplaceMemcpyByStdCopy.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_BY_STDCOPY_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
+
+#include "../ClangTidyCheck.h"
+#include "../utils/IncludeInserter.h"
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// Replace the C memcpy function by std::copy
+class ReplaceMemcpyByStdCopy : public ClangTidyCheck {
+public:
+  ReplaceMemcpyByStdCopy(StringRef Name, ClangTidyContext *Context);
+  virtual ~ReplaceMemcpyByStdCopy() {}
+  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;
+  const utils::IncludeSorter::IncludeStyle IncludeStyle;
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
Index: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
@@ -0,0 +1,115 @@
+//===--- ReplaceMemcpyByStdCopy.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 "ReplaceMemcpyByStdCopy.h"
+#include "../utils/OptionsUtils.h"
+
+using namespace clang;
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+ReplaceMemcpyByStdCopy::ReplaceMemcpyByStdCopy(StringRef Name,
+   ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
+  Options.getLocalOrGlobal("IncludeStyle", "llvm"))) {}
+
+void ReplaceMemcpyByStdCopy::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 ReplaceMemcpyByStdCopy::registerPPCallbacks(
+const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
+  if (!getLangOpts().CPlusPlus)
+return;
+
+  Inserter = llvm::make_unique(SM, getLangOpts(),
+   IncludeStyle);
+  PP->addPPCallbacks(Inserter->CreatePPCallbacks());
+}
+
+void ReplaceMemcpyByStdCopy::check(const MatchFinder::MatchResult &Result) {
+  const auto *MemcpyNode = Result.Nodes.getNodeAs("memcpy_function");
+  assert(MemcpyNode != nullptr);
+
+  auto Diag = diag(MemcpyNode->getExprLoc(), "use std::copy instead of memcpy");
+
+  renameFunction(Diag, MemcpyNode);
+  reorderArgs(Diag, MemcpyNode);
+  insertHeader(Diag, MemcpyNode, Result.SourceManager);
+}
+
+void ReplaceMemcpyByStdCopy::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IncludeStyle",
+util

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-06-14 Thread Thomas Manceau via Phabricator via cfe-commits
Blackhart updated this revision to Diff 204759.
Blackhart added a comment.

Fix file comments typo


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63324/new/

https://reviews.llvm.org/D63324

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h

Index: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
@@ -0,0 +1,48 @@
+//===--- ReplaceMemcpyByStdCopy.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_BY_STDCOPY_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
+
+#include "../ClangTidyCheck.h"
+#include "../utils/IncludeInserter.h"
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// Replace the C memcpy function by std::copy
+class ReplaceMemcpyByStdCopy : public ClangTidyCheck {
+public:
+  ReplaceMemcpyByStdCopy(StringRef Name, ClangTidyContext *Context);
+  virtual ~ReplaceMemcpyByStdCopy() {}
+  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;
+  const utils::IncludeSorter::IncludeStyle IncludeStyle;
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
Index: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
@@ -0,0 +1,118 @@
+//===--- ReplaceMemcpyByStdCopy.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 "ReplaceMemcpyByStdCopy.h"
+#include "../utils/OptionsUtils.h"
+
+using namespace clang;
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+ReplaceMemcpyByStdCopy::ReplaceMemcpyByStdCopy(StringRef Name,
+   ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
+  Options.getLocalOrGlobal("IncludeStyle", "llvm"))) {}
+
+void ReplaceMemcpyByStdCopy::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 ReplaceMemcpyByStdCopy::registerPPCallbacks(
+const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
+  // 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(SM, getLangOpts(),
+   IncludeStyle);
+  PP->addPPCallbacks(Inserter->CreatePPCallbacks());
+}
+
+void ReplaceMemcpyByStdCopy::check(const MatchFinder::MatchResult &Result) {
+  const auto *MemcpyNode = Result.Nodes.getNodeAs("memcpy_function");
+  assert(MemcpyNode != nullptr);
+
+  auto Diag = diag(MemcpyNode->getExprLoc(), "use std::copy instead of memcpy");
+
+  renameFunction(Diag, MemcpyNode);
+  reorderArgs(Diag, MemcpyNode);
+  insertHead

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-06-14 Thread Thomas Manceau via Phabricator via cfe-commits
Blackhart updated this revision to Diff 204758.
Blackhart added a comment.
Herald added a subscriber: jsji.

Add missing "override" keywords


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63324/new/

https://reviews.llvm.org/D63324

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
  mypatch.patch

Index: mypatch.patch
===
--- /dev/null
+++ mypatch.patch
@@ -0,0 +1,378 @@
+commit 62ee7f02e1e236f75226337e1057a4e42f72dfb7
+Author: Thomas Manceau 
+Date:   Wed Jun 12 17:51:14 2019 +0200
+
+[clang-tidy] Replace memcpy by std::copy
+
+diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
+index 36193f0a6d1..295d1c4a010 100644
+--- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
 b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
+@@ -1,47 +1,48 @@
+ set(LLVM_LINK_COMPONENTS support)
+ 
+ add_clang_library(clangTidyModernizeModule
+   AvoidBindCheck.cpp
+   AvoidCArraysCheck.cpp
+   ConcatNestedNamespacesCheck.cpp
+   DeprecatedHeadersCheck.cpp
+   DeprecatedIosBaseAliasesCheck.cpp
+   LoopConvertCheck.cpp
+   LoopConvertUtils.cpp
+   MakeSharedCheck.cpp
+   MakeSmartPtrCheck.cpp
+   MakeUniqueCheck.cpp
+   ModernizeTidyModule.cpp
+   PassByValueCheck.cpp
+   RawStringLiteralCheck.cpp
+   RedundantVoidArgCheck.cpp
+   ReplaceAutoPtrCheck.cpp
++  ReplaceMemcpyByStdCopy.cpp
+   ReplaceRandomShuffleCheck.cpp
+   ReturnBracedInitListCheck.cpp
+   ShrinkToFitCheck.cpp
+   UnaryStaticAssertCheck.cpp
+   UseAutoCheck.cpp
+   UseBoolLiteralsCheck.cpp
+   UseDefaultMemberInitCheck.cpp
+   UseEmplaceCheck.cpp
+   UseEqualsDefaultCheck.cpp
+   UseEqualsDeleteCheck.cpp
+   UseNodiscardCheck.cpp
+   UseNoexceptCheck.cpp
+   UseNullptrCheck.cpp
+   UseOverrideCheck.cpp
+   UseTrailingReturnTypeCheck.cpp
+   UseTransparentFunctorsCheck.cpp
+   UseUncaughtExceptionsCheck.cpp
+   UseUsingCheck.cpp
+ 
+   LINK_LIBS
+   clangAST
+   clangASTMatchers
+   clangBasic
+   clangLex
+   clangTidy
+   clangTidyReadabilityModule
+   clangTidyUtils
+   clangTooling
+   )
+diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
+index 6280f9c991e..d665c1c5169 100644
+--- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
 b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
+@@ -1,129 +1,132 @@
+ //===--- ModernizeTidyModule.cpp - clang-tidy -===//
+ //
+ // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+ // See https://llvm.org/LICENSE.txt for license information.
+ // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ //
+ //===--===//
+ 
+ #include "../ClangTidy.h"
+ #include "../ClangTidyModule.h"
+ #include "../ClangTidyModuleRegistry.h"
+ #include "AvoidBindCheck.h"
+ #include "AvoidCArraysCheck.h"
+ #include "ConcatNestedNamespacesCheck.h"
+ #include "DeprecatedHeadersCheck.h"
+ #include "DeprecatedIosBaseAliasesCheck.h"
+ #include "LoopConvertCheck.h"
+ #include "MakeSharedCheck.h"
+ #include "MakeUniqueCheck.h"
+ #include "PassByValueCheck.h"
+ #include "RawStringLiteralCheck.h"
+ #include "RedundantVoidArgCheck.h"
+ #include "ReplaceAutoPtrCheck.h"
++#include "ReplaceMemcpyByStdCopy.h"
+ #include "ReplaceRandomShuffleCheck.h"
+ #include "ReturnBracedInitListCheck.h"
+ #include "ShrinkToFitCheck.h"
+ #include "UnaryStaticAssertCheck.h"
+ #include "UseAutoCheck.h"
+ #include "UseBoolLiteralsCheck.h"
+ #include "UseDefaultMemberInitCheck.h"
+ #include "UseEmplaceCheck.h"
+ #include "UseEqualsDefaultCheck.h"
+ #include "UseEqualsDeleteCheck.h"
+ #include "UseNodiscardCheck.h"
+ #include "UseNoexceptCheck.h"
+ #include "UseNullptrCheck.h"
+ #include "UseOverrideCheck.h"
+ #include "UseTrailingReturnTypeCheck.h"
+ #include "UseTransparentFunctorsCheck.h"
+ #include "UseUncaughtExceptionsCheck.h"
+ #include "UseUsingCheck.h"
+ 
+ using namespace clang::ast_matchers;
+ 
+ namespace clang {
+ namespace tidy {
+ namespace modernize {
+ 
+ class ModernizeModule : public ClangTidyModule {
+ public:
+   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+ CheckFactories.registerCheck("modernize-avoid-bind");
+ CheckFactories.registerCheck("modernize-avoid-c-arrays");
+ CheckFactories.registerCheck(
+ "modernize-concat-nested-namespaces");
+ CheckFactories.registerCheck(
+ "modernize-deprecated-headers");
+ CheckFactories.registerCheck(
+ "modernize-deprecated-ios-base-aliases");
+ CheckFactories.registerCheck("modernize-loop-convert");
+ CheckFactories.reg

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-06-14 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

Missing tests.




Comment at: 
clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp:120
+} // namespace clang
\ No newline at end of file


please add all the missing newlines



Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h:1-2
+//===--- ReplaceMemcpyByStdCopy.h - clang-tidy--*- C++
+//-*-===//
+//

should be something like
```
//===--- ReplaceMemcpyByStdCopy.h - clang-tidy -*- C++-*-===//
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63324/new/

https://reviews.llvm.org/D63324



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-06-14 Thread Thomas Manceau via Phabricator via cfe-commits
Blackhart updated this revision to Diff 204757.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63324/new/

https://reviews.llvm.org/D63324

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h

Index: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
@@ -0,0 +1,49 @@
+//===--- ReplaceMemcpyByStdCopy.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_BY_STDCOPY_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
+
+#include "../ClangTidyCheck.h"
+#include "../utils/IncludeInserter.h"
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// Replace the C memcpy function by std::copy
+class ReplaceMemcpyByStdCopy : public ClangTidyCheck {
+public:
+  ReplaceMemcpyByStdCopy(StringRef Name, ClangTidyContext *Context);
+  virtual ~ReplaceMemcpyByStdCopy() {}
+  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);
+
+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;
+  const utils::IncludeSorter::IncludeStyle IncludeStyle;
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
\ No newline at end of file
Index: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
@@ -0,0 +1,119 @@
+//===--- ReplaceMemcpyByStdCopy.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 "ReplaceMemcpyByStdCopy.h"
+#include "../utils/OptionsUtils.h"
+
+using namespace clang;
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+ReplaceMemcpyByStdCopy::ReplaceMemcpyByStdCopy(StringRef Name,
+   ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
+  Options.getLocalOrGlobal("IncludeStyle", "llvm"))) {}
+
+void ReplaceMemcpyByStdCopy::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 ReplaceMemcpyByStdCopy::registerPPCallbacks(
+const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
+  // 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(SM, getLangOpts(),
+   IncludeStyle);
+  PP->addPPCallbacks(Inserter->CreatePPCallbacks());
+}
+
+void ReplaceMemcpyByStdCopy::check(const MatchFinder::MatchResult &Result) {
+  const auto *MemcpyNode = Result.Nodes.getNodeAs("memcpy_function");
+  assert(MemcpyNode != nullptr);
+
+  auto Diag = diag(MemcpyNode->getExprLoc(), "use std::copy instead of memcpy");
+
+  renameFunction(Diag, MemcpyNode);
+  reorderArgs(Diag, MemcpyNode);
+  insertHeader(Diag, MemcpyNode, Resu

Re: [PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-06-14 Thread Roman Lebedev via cfe-commits
On Fri, Jun 14, 2019 at 12:48 PM Thomas Manceau via Phabricator via
cfe-commits  wrote:
>
> Blackhart created this revision.

> Blackhart created this object with edit policy "Only User: Blackhart (Thomas 
> Manceau)".
You might want to unset that :)

> Blackhart added a project: clang-tools-extra.
> Herald added subscribers: cfe-commits, xazax.hun, mgorny.
> Herald added a project: clang.
>
> This patch will:
>
> - replace all occurrence of the C "memcpy" function by "std::copy".
> - reorder its arguments.
> - insert the "algorithm" headers if needed.
>
>
> Repository:
>   rCTE Clang Tools Extra
>
> https://reviews.llvm.org/D63324
>
> Files:
>   clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
>   clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
>   clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
>   clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2019-06-14 Thread Thomas Manceau via Phabricator via cfe-commits
Blackhart created this revision.
Blackhart created this object with edit policy "Only User: Blackhart (Thomas 
Manceau)".
Blackhart added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, xazax.hun, mgorny.
Herald added a project: clang.

This patch will:

- replace all occurrence of the C "memcpy" function by "std::copy".
- reorder its arguments.
- insert the "algorithm" headers if needed.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D63324

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h

Index: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
@@ -0,0 +1,49 @@
+//===--- ReplaceMemcpyByStdCopy.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_BY_STDCOPY_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
+
+#include "../ClangTidyCheck.h"
+#include "../utils/IncludeInserter.h"
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// Replace the C memcpy function by std::copy
+class ReplaceMemcpyByStdCopy : public ClangTidyCheck {
+public:
+  ReplaceMemcpyByStdCopy(StringRef Name, ClangTidyContext *Context);
+  virtual ~ReplaceMemcpyByStdCopy() {}
+  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);
+
+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;
+  const utils::IncludeSorter::IncludeStyle IncludeStyle;
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
\ No newline at end of file
Index: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
@@ -0,0 +1,119 @@
+//===--- ReplaceMemcpyByStdCopy.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 "ReplaceMemcpyByStdCopy.h"
+#include "../utils/OptionsUtils.h"
+
+using namespace clang;
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+ReplaceMemcpyByStdCopy::ReplaceMemcpyByStdCopy(StringRef Name,
+   ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
+  Options.getLocalOrGlobal("IncludeStyle", "llvm"))) {}
+
+void ReplaceMemcpyByStdCopy::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 ReplaceMemcpyByStdCopy::registerPPCallbacks(
+const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
+  // 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(SM, getLangOpts(),
+   IncludeStyle);
+  PP->addPPCallbacks(Inserter->CreatePPCallbacks());
+}
+
+void ReplaceMemcpyByStdCopy::check(cons