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

[Aaron Ballman via cfe-commits]

aaron.ballman added a subscriber: aaron.ballman.

================
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) {
----------------
alexfh wrote:
> 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.
I kind of wonder if it would be generally useful as a matcher helper utility -- expose everything from `std::iterator_traits` with a nice AST matcher interface. Something that can be used like: `iteratorTrait(hasValueType(<blah>), hasPointerType(<yada>))` Or is that overkill?

================
Comment at: clang-tidy/misc/FoldInitTypeCheck.cpp:23
@@ +22,3 @@
+                                          const ASTContext &context) {
+  if (IteratorType.isPointerType()) {
+    return IteratorType.getPointeeType().getCanonicalType();
----------------
Should elide the braces (here and elsewhere).

================
Comment at: clang-tidy/misc/FoldInitTypeCheck.cpp:43
@@ +42,3 @@
+
+/// Returns true if ValueType is allwed to fold into InitType.
+static bool isValidBuiltinFold(const BuiltinType &ValueType,
----------------
s/allwed/allowed.

Also, may want to clarify what "fold" means in this context (can be confusing because there are fold expressions as well as constant folding).

================
Comment at: clang-tidy/misc/FoldInitTypeCheck.cpp:79
@@ +78,3 @@
+      callExpr(callee(functionDecl(
+                   hasName("std::accumulate"),
+                   hasParameter(0, parmVarDecl().bind("iterator")),
----------------
Are there plans to apply this to other STL algorithms, like `inner_product`, `reduce`, etc? If so, it may be good to add a chunk of those up front.

Also, the name we should check should be `::std::accumulate`. May want to include a test like:
```
namespace blah {
namespace std {
template <typename Iter, typename T>
T accumulate(Iter, Iter, T); // We should not care about this one.
}
}
```

================
Comment at: clang-tidy/misc/FoldInitTypeCheck.cpp:118
@@ +117,3 @@
+           "loss of precision")
+          << ValueBuiltingType->getName(Policy)
+          << InitBuiltinType->getName(Policy);
----------------
You should be able to just pass the type directly into the diagnostic engine instead of mucking about with the printing policy directly.


http://reviews.llvm.org/D18442