JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land.
I am happy now :) Thank you for the patch, LGTM ================ Comment at: test/clang-tidy/abseil-time-subtraction.cpp:12 + + d = absl::Hours(absl::ToUnixHours(t) - x); + // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the time domain [abseil-time-subtraction] ---------------- hwright wrote: > JonasToth wrote: > > hwright wrote: > > > JonasToth wrote: > > > > please add tests where `x` itself is a calculation with different > > > > precedence of its operators (multiplication, addition) to ensure these > > > > cases are transformed properly as well. > > > This doesn't actually matter in this case: `x` will be wrapped in a > > > function call. > > > > > > It does matter in the case where we //unwrap// the first argument (below) > > > and I've already got a test which uses multiplication in this case. I've > > > also added one for division. > > Yes, it should not matter if `x` is an expr itself or just a variable. > > Thats why it should be tested its actually true. > Added, though this seems more a test of the matcher infrastructure than the > tool itself. True, but there are always some inconsistencies, so better to test what we want :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58137/new/ https://reviews.llvm.org/D58137 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
