JonasToth added inline comments.
================ Comment at: clang-tidy/abseil/DurationAdditionCheck.cpp:22 +void DurationAdditionCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + binaryOperator(hasOperatorName("+"), ---------------- This whole file is structurally similar to the `DurationSubtractionCheck.cpp` equivalent (which is expected :)), maybe some parts could be refactored out? Will there be a check for duration scaling or the like? ================ Comment at: clang-tidy/abseil/DurationAdditionCheck.cpp:50 + diag(Binop->getBeginLoc(), "perform addition in the duration domain") + << FixItHint::CreateReplacement( + Binop->getSourceRange(), ---------------- The only difference between the two `diag` seems to be the `FixItHint`. I would rather refactor the diag out of the `if`. ================ Comment at: clang-tidy/abseil/DurationAdditionCheck.cpp:60 + } else { + assert(Call == Binop->getRHS()->IgnoreImpCast() && "Call should be found on the RHS"); + diag(Binop->getBeginLoc(), "perform addition in the duration domain") ---------------- What happens with paren-casts, are they ignored as well? (See one comment in the test-cases) ================ Comment at: test/clang-tidy/abseil-duration-addition.cpp:28 + + i = 3 + absl::ToUnixHours(t); + // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform addition in the duration domain [abseil-duration-addition] ---------------- Call is on the right side, what happens for `3 + (abs::ToUnixHours(t));` ================ Comment at: test/clang-tidy/abseil-duration-addition.cpp:48 + // Undoing inverse conversions + i = absl::ToUnixMicros(t) + absl::ToInt64Microseconds(absl::Seconds(1)); + // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform addition in the duration domain [abseil-duration-addition] ---------------- Is something like this ``` i += absl::ToInt64MicroSeconds(absl::Seconds(1)); ``` possible (syntactically and semantically)? The compound operators are currently not covered by this (and the subtraction-check), but in this case it looks like a possible use-case. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57185/new/ https://reviews.llvm.org/D57185 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits