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