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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
