[PATCH] D18442: A clang-tidy check for std:accumulate.

[Alexander Kornienko via cfe-commits]

alexfh requested changes to this revision.
This revision now requires changes to proceed.

================
Comment at: clang-tidy/misc/FoldInitTypeCheck.cpp:23
@@ +22,3 @@
+  // Note: Right now we check only builtin types.
+  const auto BuiltinTypeWithId = [](const char *id) {
+    return hasCanonicalType(builtinType().bind(id));
----------------
s/id/ID/g
http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly

================
Comment at: clang-tidy/misc/FoldInitTypeCheck.cpp:39
@@ +38,3 @@
+          callee(functionDecl(
+              anyOf(hasName("::std::accumulate"), hasName("::std::reduce")),
+              hasParameter(0, parmVarDecl(hasType(hasCanonicalType(
----------------
Use the more effective `hasAnyName` matcher.

================
Comment at: clang-tidy/misc/FoldInitTypeCheck.cpp:52
@@ +51,3 @@
+              hasName("::std::inner_product"),
+              hasParameter(0, parmVarDecl(hasType(hasCanonicalType(
+                                  IteratorWithValueType("IterValueType"))))),
----------------
You can pull a few common parts to local variables to make the code slightly shorter and easier to read:

  auto IteratorParam = parmVarDecl(hasType(hasCanonicalType(IteratorWithValueType("IterValueType"))));
  auto Iterator2Param = parmVarDecl(hasType(hasCanonicalType(IteratorWithValueType("Iter2ValueType"))));
  auto InitParam = parmVarDecl(hasType(BuiltinTypeWithId("InitType")));


================
Comment at: clang-tidy/misc/FoldInitTypeCheck.cpp:89
@@ +88,3 @@
+
+/// Returns true if ValueType is allowed to fold into InitType, i.e. if:
+///   static_cast<InitType>(ValueType{some_value})
----------------
Is "fold" a commonly used term in this context? At least, it's not a language standard jargon and not something I heard of frequently. Maybe we should try to find a more clear word?


http://reviews.llvm.org/D18442