[PATCH] D22725: [clang-tidy] Add check 'modernize-use-algorithm'

Piotr Padlewski via cfe-commits cfe-commits at lists.llvm.org
Sun Jul 31 15:45:17 PDT 2016


Prazek added a comment.

I see you solved the void and conmpatible types problems. Excellent!

Can you post a patch with changes after running LLVM? I would wait for Alex or Aaron to accept it.


================
Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:58-60
@@ +57,5 @@
+
+  for (const auto &KeyValue :
+       std::vector<std::pair<llvm::StringRef, std::string>>(
+           {{"memcpy", "std::copy"}, {"memset", "std::fill"}})) {
+    Replacements.insert(KeyValue);
----------------
Try just initializer list instead of creating vector here.
Just 
for (const auto &keyValue : {std::make_pair("memcpy", "std::copy"), {"memset", "std::fill"}}

You just have to specify the type of the first argument and it will try to convert each next to it.

================
Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:98-99
@@ +97,4 @@
+  const auto it = Replacements.find(MatchedName);
+  if (it == Replacements.end())
+    return;
+  const auto ReplacedName = it->second;
----------------
this should not happen. You can change it to assert

================
Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:139
@@ +138,3 @@
+    // Cannot do pointer arithmetic on void type.
+    if (getStrippedType(Arg1Type)->isVoidType())
+      return;
----------------
Would it be the same to check here for the first argument instead of second?
So then you could take this if before the if(MatchedName == "memset")

================
Comment at: docs/clang-tidy/checks/modernize-use-algorithm.rst:37
@@ +36,2 @@
+    std::fill(dest, dest + count, ch)
+
----------------
You should add 2 notes here
1. The check runs just with C++.
2. There are some rare cases that require adding parens (and put example), so the check could right now generate wrong output.
 (which is something that I would not require to have, but it is good to leave not about it in documentation)

Also leave Fixit comment in the check.


Repository:
  rL LLVM

https://reviews.llvm.org/D22725





More information about the cfe-commits mailing list