[PATCH] D57185: [clang-tidy] Add the abseil-duration-addition check

[Jonas Toth via Phabricator via cfe-commits]

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