JonasToth added inline comments.

================
Comment at: clang-tidy/abseil/DurationDivisionCheck.cpp:29
+          hasSourceExpression(ignoringParenCasts(cxxOperatorCallExpr(
+              hasOverloadedOperatorName("/"), argumentCountIs(2),
+              hasArgument(0, expr(IsDuration)),
----------------
The `argumentCountIs` should be redundant, not? Operator/ has always two 
operands and must be overloaded as an binary operator.


================
Comment at: clang-tidy/abseil/DurationDivisionCheck.cpp:31
+              hasArgument(0, expr(IsDuration)),
+              hasArgument(1, expr(IsDuration)), expr().bind("OpCall")))),
+          hasImplicitDestinationType(qualType(unless(isInteger()))),
----------------
I dont understand the `expr().bind`, you can directly bind to the 
cxxOperatorCallExpr. That should be equivalent.


================
Comment at: clang-tidy/abseil/DurationDivisionCheck.h:19
+
+// Find potential incorrect uses of integer division of absl::Duration objects.
+class DurationDivisionCheck : public ClangTidyCheck {
----------------
The common `For the user-facing documentation see: <LinkToDoc>` is missing here.
All clang-tidy checks do have this (as it is autogenerated by the add-check 
tool) so i think it would be better to stay consistent with it.


https://reviews.llvm.org/D50389



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to