Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

time: Round broken for d > 2^62, r >= 2^62 #19807

Closed
rsc opened this issue Mar 31, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@rsc
Copy link
Contributor

commented Mar 31, 2017

time.Round uses r+r < d to decide whether the remainder is above or below the halfway (round up or round down) point. This is wrong when r+r wraps negative, because it should be > d but looks < d.

No one will ever care about rounding to a d > 2^62 (d > 146 years), but might as well get it right.

CL on its way.

@gopherbot

This comment has been minimized.

Copy link

commented Mar 31, 2017

CL https://golang.org/cl/39152 mentions this issue.

@gopherbot

This comment has been minimized.

Copy link

commented Mar 31, 2017

CL https://golang.org/cl/39151 mentions this issue.

@gopherbot gopherbot closed this in 105cc2b Mar 31, 2017

gopherbot pushed a commit that referenced this issue Apr 3, 2017

testing/quick: generate all possible int64, uint64 values
When generating a random int8, uint8, int16, uint16, int32, uint32,
quick.Value chooses among all possible values.

But when generating a random int64 or uint64, it only chooses
values in the range [-2⁶², 2⁶²) (even for uint64).
It should, like for all the other integers, use the full range.

If it had, this would have caught #19807 earlier.
Instead it let us discover the presence of #19809.

While we are here, also make the default source of
randomness not completely deterministic.

Fixes #19808.

Change-Id: I070f852531c92b3670bd76523326c9132bfc9416
Reviewed-on: https://go-review.googlesource.com/39152
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>

lparth added a commit to lparth/go that referenced this issue Apr 13, 2017

time: test and fix Time.Round, Duration.Round for d > 2⁶²
Round uses r+r < d to decide whether the remainder is
above or below half of d (to decide whether to round up or down).
This is wrong when r+r wraps negative, because it looks < d
but is really > d.

No one will ever care about rounding to a multiple of
d > 2⁶² (about 146 years), but might as well get it right.

Fixes golang#19807.

Change-Id: I1b55a742dc36e02a7465bc778bf5dd74fe71f7c0
Reviewed-on: https://go-review.googlesource.com/39151
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

lparth added a commit to lparth/go that referenced this issue Apr 13, 2017

testing/quick: generate all possible int64, uint64 values
When generating a random int8, uint8, int16, uint16, int32, uint32,
quick.Value chooses among all possible values.

But when generating a random int64 or uint64, it only chooses
values in the range [-2⁶², 2⁶²) (even for uint64).
It should, like for all the other integers, use the full range.

If it had, this would have caught golang#19807 earlier.
Instead it let us discover the presence of golang#19809.

While we are here, also make the default source of
randomness not completely deterministic.

Fixes golang#19808.

Change-Id: I070f852531c92b3670bd76523326c9132bfc9416
Reviewed-on: https://go-review.googlesource.com/39152
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>

@golang golang locked and limited conversation to collaborators Mar 31, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.