alexfh added a comment.
LG In general, but see a few comments inline.
================
Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:25
+llvm::Optional<llvm::APSInt>
+TruncateIfIntegral(const FloatingLiteral &FloatLiteral) {
+ double value = FloatLiteral.getValueAsApproximateDouble();
----------------
Please use the LLVM style (`functionName`). See
http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
================
Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:27
+ double value = FloatLiteral.getValueAsApproximateDouble();
+ if (std::fmod(value, 1) == 0) {
+ bool is_negative = false;
----------------
Probably doesn't matter much, but would `std::modf` be more appropriate in this
context?
================
Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:65-66
+ // Don't try and replace things inside of macro definitions.
+ if (MatchedCall->getBeginLoc().isMacroID() ||
+ MatchedCall->getEndLoc().isMacroID())
+ return;
----------------
Lexer::makeFileCharRange may be a better (and less strict) way to test whether
we can safely replace a range.
================
Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:87
+ // Check for floats without fractional components
+ if (const FloatingLiteral *LitFloat =
+ Result.Nodes.getNodeAs<FloatingLiteral>("float_literal")) {
----------------
`const auto *` will be as readable and less verbose.
================
Comment at: docs/ReleaseNotes.rst:73
+
+ FIXME: add release notes.
+
----------------
Please remove the FIXME
https://reviews.llvm.org/D53339
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
