alexfh added a comment. Nice! See a few comments inline.
================ Comment at: clang-tidy/misc/IncorrectRoundings.cpp:38 @@ +37,3 @@ + // Match a floating point expression. + auto FloatType = expr(anyOf(hasType(qualType(asString("long double"))), + hasType(qualType(asString("double"))), ---------------- We should use a more effective way of checking whether the type is a (long)? double or float. There'a `builtinType()` matcher that we could extend with a narrowing matcher calling `BuiltinType::isFloatingPoint()` (that can be added to ASTMatchers.h, I think). ================ Comment at: clang-tidy/misc/IncorrectRoundings.cpp:50 @@ +49,3 @@ + auto OneSideHalf = anyOf( + allOf(hasLHS(FloatOrCastHalf), hasRHS(FloatType.bind("ExprOnRhs"))), + allOf(hasRHS(FloatOrCastHalf), hasLHS(FloatType.bind("ExprOnLhs")))); ---------------- Since these two `.bind()` calls are in alternative branches, they can use the same identifier. This would also simplify the code in the callback. ================ Comment at: clang-tidy/misc/IncorrectRoundings.cpp:71 @@ +70,3 @@ + "casting (double + 0.5) to integer leads to incorrect rounding; " + "consider using lrint (#include <cmath>) instead"); +} ---------------- I don't think we should recommend `lrint`, since its behavior can be changed by [[ http://www.cplusplus.com/reference/cfenv/fesetround | fesetround() ]]. [[ http://www.cplusplus.com/reference/cmath/lround/ | lround() ]] and [[ http://www.cplusplus.com/reference/cmath/llround/ | llround() ]] are better alternatives, IMO. We could also suggest an automated fix for this. http://reviews.llvm.org/D16764 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits