[PATCH] D54246: [clang-tidy] Add the abseil-duration-factory-scale check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!



================
Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:73
+  case DurationScale::Hours:
+    if (Multiplier <= 1.0 / 60.0)
+      return std::make_tuple(DurationScale::Minutes, Multiplier * 60.0);
----------------
hwright wrote:
> aaron.ballman wrote:
> > This doesn't seem quite right. `1.0/6000.0` is less than `1.0/60.0` but isn't a `Minutes` scaling.
> After spending two days chasing this down, it finally occurred to me: we don't actually handle this case.  Our (somewhat conservative) matchers only look for literal values, not division expressions like `1.0/60.0`.
> 
> And for this iteration of the tool, I think I'm fine with that, since I expect those kinds of things to be exceedingly rare, and probably tricky to handle, anyway.
Oh jeez -- good point on not covering constant expressions from the matcher. If you think this is going to be exceedingly rare, I'm okay with punting until we need it. Thank you for looking into it!


https://reviews.llvm.org/D54246