[PATCH] D53339: [clang-tidy] Add the abseil-duration-factory-float check
Hyrum Wright via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 23 11:26:42 PDT 2018
hwright marked 2 inline comments as done.
hwright added a comment.
I do not have commit privileges, so somebody else would need to submit it.
We've had a version of this check running over our internal codebase for several months with no unexpected problems.
================
Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:51
+ hasArgument(0,
+ anyOf(cxxStaticCastExpr(
+ hasDestinationType(realFloatingPointType()),
----------------
JonasToth wrote:
> Nit: the duplication in the cast matcher can be removed with a variable `auto CastToFloat = hasDestinationType(realFloatingPointType()), hasSourceExpression(expr().bind("cast_arg"));` and then used in the matcher.
This turns out to be harder than it looks: `auto` can't be used here because it would be ambiguous, and the actual type is part of the `internal` namespace so I'm hesitant to use it here.
I've left the duplication as-is; if there's a better way to de-dupe, I'm happy to try it.
================
Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:99
+ }
+ }
+}
----------------
JonasToth wrote:
> is it logically valid to fall reach the end of the method?
> If not please add an `llvm_unreachable()`
It is. Any case we don't catch would fall through to the end of this function.
https://reviews.llvm.org/D53339
More information about the cfe-commits
mailing list