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

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

Sorry for the delay (mostly due to the holidays here).

The check looks very useful, at least, the pattern is hard to spot by manual inspection. A few comments though, mostly style-related.


================
Comment at: clang-tidy/misc/FoldInitTypeCheck.cpp:21
@@ +20,3 @@
+/// Returns the value_type for an InputIterator type.
+static QualType getInputIteratorValueType(const Type &IteratorType,
+                                          const ASTContext &context) {
----------------
I suspect, a significant part of this could be done in the matcher. I might be wrong, but it seems worth trying. The resulting code is usually much shorter and cleaner.

================
Comment at: clang-tidy/misc/FoldInitTypeCheck.cpp:57
@@ +56,3 @@
+  //  - a bigger int, regardless of signedness.
+  //  - FIXME: should it be a warning to fold into floating point ?
+  if (ValueType.isInteger()) {
----------------
nit: no space needed before the question mark.

================
Comment at: clang-tidy/misc/FoldInitTypeCheck.cpp:101
@@ +100,3 @@
+      getInputIteratorValueType(*Iterator->getType(), *Result.Context);
+  if (ValueType.isNull()) {
+    return;
----------------
nit: No braces needed for one line `if` bodies. Here and elsewhere.

================
Comment at: test/clang-tidy/misc-fold-init-type.cpp:17
@@ +16,3 @@
+  return std::accumulate(a, a + 1, 0);
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'float' into type
+  // 'int' might result in loss of precision [misc-fold-init-type]
----------------
The CHECK lines shouldn't be broken. Looks like your clang-format doesn't use -style=file by default.

================
Comment at: test/clang-tidy/misc-fold-init-type.cpp:24
@@ +23,3 @@
+  return std::accumulate(it, it, 0);
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'float' into type
+  // 'int' might result in loss of precision [misc-fold-init-type]
----------------
We usually specify each unique message completely once and truncate static parts of all other patterns to a make the test more readable. E.g. here I would remove everything after 'int'.


http://reviews.llvm.org/D18442