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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits