[PATCH] D50389: [clang-tidy] new check for Abseil

Deanna Garcia via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 9 14:45:36 PDT 2018


deannagarcia marked 9 inline comments as done.
deannagarcia added inline comments.


================
Comment at: clang-tidy/abseil/DurationDivisionCheck.cpp:32
+          hasImplicitDestinationType(qualType(unless(isInteger()))),
+          unless(hasParent(cxxStaticCastExpr()))),
+      this);
----------------
JonasToth wrote:
> What about different kinds of casts, like C-Style casts?
> Doesn't the `hasImplicitDestinationType` already remove the possibility for an cast as destination?
I know the test fails without this line and flags the casts, so I'm pretty sure it's necessary but I'm not exactly sure why hasImplicitDestinationType doesn't catch it.


================
Comment at: docs/clang-tidy/checks/abseil-duration-division.rst:8
+division of two `absl::Duration` objects returns an `int64` with any fractional
+component truncated toward 0.
+
----------------
JonasToth wrote:
> Please add one more sentence, why this is something you don't want, so it gets clear that floating point contextes are the interesting here.
Does this link work or do you still want more?


================
Comment at: docs/clang-tidy/checks/abseil-duration-division.rst:3
+  
+abseil-duration-division
+========================
----------------
hokein wrote:
> This is a nice document. Does abseil have similar thing in its official guideline/practice? If yes, we can add the link in the document as well.
I linked our general documentation on absl::Duration, specifically the stuff on duration arithmetic. Is that what you wanted?


https://reviews.llvm.org/D50389





More information about the cfe-commits mailing list