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

[Jonas Toth via Phabricator via cfe-commits]

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