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

[Clement Courbet via cfe-commits]

courbet added a comment.

Thanks, PTAL.


================
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) {
----------------
aaron.ballman wrote:
> 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?
That would be cool, but I'm afraid it would give the false sense of covering all cases. Unfortunately the user can specialize iterator_traits for their iterator type, which would not be easy to cover.

================
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) {
----------------
courbet wrote:
> aaron.ballman wrote:
> > 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?
> That would be cool, but I'm afraid it would give the false sense of covering all cases. Unfortunately the user can specialize iterator_traits for their iterator type, which would not be easy to cover.
Done, and that's indeed much nicer to read.

================
Comment at: clang-tidy/misc/FoldInitTypeCheck.cpp:79
@@ +78,3 @@
+      callExpr(callee(functionDecl(
+                   hasName("std::accumulate"),
+                   hasParameter(0, parmVarDecl().bind("iterator")),
----------------
aaron.ballman wrote:
> 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.
> }
> }
> ```
Added a test, thanks for the catch !


http://reviews.llvm.org/D18442