aaron.ballman added inline comments.
================ Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:29-31 + llvm::APSInt ApInt(/*BitWidth=*/64, /*isUnsigned=*/false); + ApInt = static_cast<std::int64_t>(Value); + return ApInt; ---------------- I believe you can do `return APSInt::get(static_cast<int64_t>(Value));` instead -- it should do the same thing. ================ Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:36 + +static bool InsideMacroDefinition(const MatchFinder::MatchResult &Result, + SourceRange Range) { ---------------- This function name doesn't seem to relate to the behavior of the function? Rather than try to pin it down like that, perhaps rename to "CanRangeBeSafelyReplaced()` and add a comment as to why this is the correct approach for answering the question. ================ Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:54 + hasSourceExpression(expr().bind("cast_arg"))), + cStyleCastExpr( + hasDestinationType(realFloatingPointType()), ---------------- What about function-style casts? e.g. `float(1)` ================ Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:74 + + // Check for static casts to `float`. + if (const auto *MaybeCastArg = Result.Nodes.getNodeAs<Expr>("cast_arg")) { ---------------- Comment is now out of date, this checks for a few kinds of casts. https://reviews.llvm.org/D53339 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits