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

Eugene Zelenko via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 16 17:05:53 PDT 2018


Eugene.Zelenko added inline comments.


================
Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:24
+// Returns an integer if the fractional part of a `FloatingLiteral` is 0.
+llvm::Optional<llvm::APSInt>
+TruncateIfIntegral(const FloatingLiteral &FloatLiteral) {
----------------
Please use static keyword instead of anonymous namespace.


================
Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:70
+  const Expr *Arg = MatchedCall->getArg(0)->IgnoreImpCasts();
+  // We don't mess with macros.
+  if (Arg->getBeginLoc().isMacroID())
----------------
May be //Macros are ignored// will sound better?


================
Comment at: docs/ReleaseNotes.rst:70
 
+- New :doc:`abseil-duration-factory-float
+  <clang-tidy/checks/abseil-duration-factory-float>` check.
----------------
Please sort new check lists alphabetically.


================
Comment at: docs/clang-tidy/checks/abseil-duration-factory-float.rst:8
+``absl::Seconds`` or ``absl::Hours``) are providing a floating point value
+when an integer could be used instead.  The integer factory function overload
+is more efficient and readable.
----------------
Please fix double space.


https://reviews.llvm.org/D53339





More information about the cfe-commits mailing list