aaron.ballman added inline comments.
================
Comment at: clang-tidy/abseil/DurationComparisonCheck.cpp:47-48
// if nothing needs to be done.
- if (!IsValidMacro(Result, Binop->getLHS()) ||
- !IsValidMacro(Result, Binop->getRHS()))
+ if (!isNotInMacro(Result, Binop->getLHS()) ||
+ !isNotInMacro(Result, Binop->getRHS()))
return;
----------------
I think this change (and the related removal) should be landed separately as a
NFC commit once this one lands, as it's logically separate from the new check.
================
Comment at: clang-tidy/abseil/DurationConversionCastCheck.cpp:46
+ const auto *Arg = Result.Nodes.getNodeAs<Expr>("arg");
+ llvm::StringRef ConversionFuncName = FuncDecl->getName();
+
----------------
You can drop the `llvm::` qualifiers here and elsewhere.
================
Comment at: clang-tidy/abseil/DurationConversionCastCheck.cpp:52
+
+ // Casting a double to an integer
+ if (MatchedCast->getTypeAsWritten()->isIntegerType() &&
----------------
Add a full stop to the end of the comment (same below).
================
Comment at: clang-tidy/abseil/DurationConversionCastCheck.cpp:57
+
+ diag(MatchedCast->getBeginLoc(), "convert duration directly to integer")
+ << FixItHint::CreateReplacement(
----------------
This diagnostic message does not tell the user what they did wrong with their
code. How about `duration should be converted directly to
integer|floating-point rather than through a type cast` or something along
those lines?
================
Comment at: test/clang-tidy/abseil-duration-conversion-cast.cpp:10
+
+ i = static_cast<int>(absl::ToDoubleHours(d1));
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: convert duration directly to
integer [abseil-duration-conversion-cast]
----------------
Can you add an example like:
```
typedef int FancyInt;
FancyInt j = static_cast<FancyInt>(absl::ToDoubleHours(d1));
```
to make sure the cast is looking through types as expected? Please add a
similar test for floating-point casts.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56532/new/
https://reviews.llvm.org/D56532
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
