JonasToth added inline comments.
================
Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:34
+ llvm::APSInt ApInt(/*BitWidth=*/64, /*isUnsigned=*/false);
+ ApInt = static_cast<unsigned long long>(value);
+ if (is_negative)
----------------
Wouldn't it make more sense to use `std::uint64_t` instead to correspond to the
line above? And where does the signedness difference come from?
(`/*isUnsigned=*/false`)
================
Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:61
+ // Don't try and replace things inside of macro definitions.
+ if (!clang::Lexer::makeFileCharRange(
+ clang::CharSourceRange::getCharRange(MatchedCall->getSourceRange()),
----------------
That is worth a helper function taking a `SourceRange` as argument.
================
Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:69
+ // Macros are ignored.
+ if (Arg->getBeginLoc().isMacroID())
+ return;
----------------
maybe `assert` instead, as your comment above suggests that macros are already
filtered out?
================
Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:72
+
+ // Check for static casts to float
+ if (const auto *MaybeCastArg = Result.Nodes.getNodeAs<Expr>("cast_arg")) {
----------------
missing full stop.
================
Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:84
+
+ // Check for floats without fractional components
+ if (const auto *LitFloat =
----------------
missing full stop
================
Comment at: docs/clang-tidy/checks/list.rst:12
abseil-redundant-strcat-calls
- abseil-string-find-startswith
abseil-str-cat-append
----------------
spurious change
================
Comment at: test/clang-tidy/abseil-duration-factory-float.cpp:32
+void ConvertFloatTest() {
+ absl::Duration d;
+
----------------
Could you also provide test cases with hexadecimal floating literals, which are
C++17? The thousand-separators could be checked as well (dunno the correct term
right now, but the `1'000'000` feature).
Please add test cases for negative literals, too.
https://reviews.llvm.org/D53339
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
