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

[Hyrum Wright via Phabricator via cfe-commits]

hwright added inline comments.


================
Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:74
+  case DurationScale::Minutes:
+    if (Multiplier == 60.0)
+      return DurationScale::Hours;
----------------
hokein wrote:
> we are comparing with floats, I think we should use something `AlmostEquals(Multiplier, 60.0)`.
I can't find AlmostEquals elsewhere; could you point me to it so I can investigate it's use?

But I am also curious why you think we should use this.  We are checking for exact values.  60.0 is representable exactly as a `double`, and even values which aren't (e.g., 1e-3) will have the same representation in this function as they do in the code being transformed, so equality is still appropriate.


================
Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:202
+  // Don't try and replace things inside of macro definitions.
+  if (InsideMacroDefinition(Result, Call->getSourceRange()))
+    return;
----------------
hokein wrote:
> isn't `Call->getExprLoc().isMacroID()` enough?
Thanks!  I've also added tests to verify this.


https://reviews.llvm.org/D54246