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