JonasToth added inline comments.
================
Comment at: clang-tidy/abseil/DurationAdditionCheck.cpp:22
+void DurationAdditionCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ binaryOperator(hasOperatorName("+"),
----------------
hwright wrote:
> JonasToth wrote:
> > 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?
> I think that most of it already has been factored out (e.g.,
> `rewriteExprFromNumberToDuration` and `getScaleForTimeInverse`, etc) . The
> actual meat here is pretty light: the matcher, and then the diagnostic
> generation.
>
> A scaling change may also follow. Our experience has been that scaling isn't
> quite straight forward, as the scaling factor may have semantic meaning which
> changes the result, but which isn't captured by the type system. Probably
> not a design discussion to have here, but suffice it to say that I don't know
> if this is yet settled.
Your right, there is not too much to gain. Only if there are more checks coming
with the same structure it makes sense to think again.
================
Comment at: test/clang-tidy/abseil-duration-addition.cpp:84
+#undef PLUS_FIVE
+}
----------------
a view template test-cases would be good to have.
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
