Repository navigation
simple: suggest replacing "t1.UnixNano() >= t2.UnixNano()" with "!t1.Before(t2)" and vice-versa #995
Description
(Perhaps more of a quickfix, I'm not sure.)
Apart from a couple of nanoseconds of difference in benchmarks (on my machine), they are equivalent, but one is shorter and simpler than the other.
Reacted by Eugene Burkov
Activity
dominikh commentedon May 14, 2021
I don't believe that they are the same in the presence of monotonic time.
ainar-g commentedon May 14, 2021
Well, in the code where I've seen this pattern, either one or both of
t1andt2were decoded from somewhere, which means that the monotonic clock isn't really an issue, as monotonic clock comparison is only used when both values have monotonic clock readings.In fact, I'm pretty sure that that makes the
UnixNanoform worse, because if you compare two values with monotonic clock readings by stripping those readings away withUnixNano, you're up for trouble eventually. That's why the monotonic clock stuff is even in there in the first place.Although we would need a more advanced tracing of value origins to make it 100 % accurate (pta?), I'd say that this is a
simplecheck in case of non-monotonic comparison and astaticcheckone when both are monotonic.dominikh commentedon May 14, 2021
Unless your intention is to explicitly do that (for example to check if the system clock went backwards.) We can't tell intentional stripping from accidental stripping.
To avoid false positives, we could only flag values that we are certain have no monotonic time. Doing so the naive way will rarely ever produce true positives, and doing it the complex way doesn't seem worth it to me.
I don't think this is the right place to enforce this. You might want to write your own rule using semgrep or ruleguard if you want to ban this pattern.
dominikh commentedon Oct 17, 2021
Reopening under the
aggressivelabel, for a future set of checks specific to code review.