ioeric added inline comments.
================
Comment at: clang-tidy/abseil/TimeComparisonCheck.cpp:23
+ auto Matcher =
+ binaryOperator(anyOf(hasOperatorName(">"), hasOperatorName(">="),
+ hasOperatorName("=="), hasOperatorName("<="),
----------------
`DurationComparisonCheck.cpp` has a very similar matcher pattern.
Maybe pull out a common matcher for this? E.g.
`comparisonOperatorWithCallee(...)`?
================
Comment at: clang-tidy/abseil/TimeComparisonCheck.cpp:42
+
+ // In most cases, we'll only need to rewrite one of the sides, but we also
+ // want to handle the case of rewriting both sides. This is much simpler if
----------------
Move this comment closer the replacement logic below?
================
Comment at: clang-tidy/abseil/TimeComparisonCheck.cpp:46
+ // if nothing needs to be done.
+ if (!isNotInMacro(Result, Binop->getLHS()) ||
+ !isNotInMacro(Result, Binop->getRHS()))
----------------
nit: double negation is a bit hard to read. maybe `!(isNotInMacro(LSH) &&
isNotInMacro(RHS))`? Ideally, the helper should be `isInMacro` instead of
`isNotInMacro`.
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58977/new/
https://reviews.llvm.org/D58977
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
