[PATCH] D53339: [clang-tidy] Add the abseil-duration-factory-float check

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.


================
Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:51
+          hasArgument(0,
+                      anyOf(cxxStaticCastExpr(
+                                hasDestinationType(realFloatingPointType()),
----------------
What about c-style casts?


================
Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:67
+  const Expr *Arg = MatchedCall->getArg(0)->IgnoreImpCasts();
+  // Macros are ignored.
+  if (Arg->getBeginLoc().isMacroID())
----------------
Please clarfiy this comment a bit more, like `Marcros as arguments are ignored.` or the like.


================
Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:86
+          Result.Nodes.getNodeAs<FloatingLiteral>("float_literal")) {
+    // Attempt to simplify a Duration factory call with a literal argument.
+    if (llvm::Optional<llvm::APSInt> IntValue = truncateIfIntegral(*LitFloat)) {
----------------
please highlight the code construct ('Duration' in this case) here and in other comments to clarify its about the class in user-code.


================
Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.h:19
+
+/// Prefer integer Duration factories when possible.
+///
----------------
Please add more to the doc here, like `This check finds ... and transforms these calls into ...` or similar.


================
Comment at: docs/clang-tidy/checks/abseil-duration-factory-float.rst:6
+
+Finds cases where callers of ``absl::Duration`` factory functions (such as
+``absl::Seconds`` or ``absl::Hours``) are providing a floating point value
----------------
Please synchronize the first paragraph with the release notes (I would prefer the release notes version)


https://reviews.llvm.org/D53339