JonasToth added inline comments.
================ Comment at: clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp:167 + // likely needed. + diag(ArgExpr->getBeginLoc(), Message); + return; ---------------- Minor, but you can move `diag(...)` out the if condition like this: ``` auto Diag = diag(...); ``` and remove the duplication in the diag creation. The `<<` works the same on the variable. ================ Comment at: test/clang-tidy/abseil-upgrade-duration-conversions.cpp:142 + +template <typename T> void templateForOpsSpecialization(T) {} +template <> ---------------- astrelni wrote: > JonasToth wrote: > > what about non-type template parameters? Do they need consideration for the > > check as well? > > If i am not mistaken floats are allowed in newwer standards as well. > IIUC non-type template parameters should be no different for this check. This > particular case is to make sure explicit specializations are treated > differently from instantiations and get fix-it hints. Please test in that case that the behaviour is as expected. ================ Comment at: test/clang-tidy/abseil-upgrade-duration-conversions.cpp:53 +void arithmeticOperatorBasicPositive() { + absl::Duration d; + d *= ConvertibleTo<int64_t>(); ---------------- Please add tests where the RHS is a complex arithmetic expression. ``` d = number1 + number2 + number3; ``` And the like, that trigger the warning and please ensure, the provided transformation is correct. https://reviews.llvm.org/D53830 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits