Skip to content

time.Time{}.Add() behaviour has changed in 1.17 #48608

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

Closed
itmecho opened this issue Sep 24, 2021 · 4 comments
Closed

time.Time{}.Add() behaviour has changed in 1.17 #48608

itmecho opened this issue Sep 24, 2021 · 4 comments

Comments

@itmecho
Copy link

itmecho commented Sep 24, 2021

What version of Go are you using (go version)?

$ go version
go version go1.17.1 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/iain/.cache/go-build"
GOENV="/home/iain/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/iain/src/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/iain/src/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.17.1"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build284408099=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Adding 1000 (1e3) to a time.Time with Nanoseconds set to 999999999 doesn't increment the second anymore. This worked in go 1.16.8 but is now not incrementing the second in go 1.17.1

https://play.golang.org/p/DxYFLO40v6R

package main

import (
	"fmt"
	"time"
)

func main() {
	t := time.Unix(1<<63-62135596801, 999999999)
	fmt.Println(t)
	fmt.Println(t.Add(1e3))
}

go 1.16 output:

292277024627-12-06 15:30:07.999999999 +0000 UTC
292277024627-12-06 15:30:08.000000999 +0000 UTC

go 1.17 output:

292277024627-12-06 15:30:07.999999999 +0000 UTC
292277024627-12-06 15:30:07.000000999 +0000 UTC

What did you expect to see?

Same behaviour where the second is incremented correctly

What did you see instead?

The nanoseconds loop back around in the same way but the second is no longer incremented

@itmecho
Copy link
Author

itmecho commented Sep 24, 2021

I'm more confused by the fact that it still works for 1 second either side of that number: https://play.golang.org/p/kj7t6ZYNl79

@itmecho
Copy link
Author

itmecho commented Sep 24, 2021

OK I think I understand the issue a bit better now. Time.ext is an int64 so the logic added here only returns false when the addition operation wraps around to the negative value. If you create a time where Time.ext is already negative (i.e time.Unix(1<<63-62135596800, 999999999)), then if (sum > t.ext) == (d > 0) will be true as it's effectively (-9223372036854775807 > -9223372036854775808) == (1000 > 0)

I think this is pretty edge case as it will only happen on that specific second (time.Unix(1<<63-62135596802, 999999999)) and it's a odd case to be using dates that large.

Here's some info from walking through with a debugger with a breakpoint here:

        t := time.Unix(1<<63-62135596801, 999999999)
	fmt.Println(t)
	fmt.Println(t.Add(1e3).String())

image

        t2 := time.Unix(1<<63-62135596800, 999999999)
	fmt.Println(t2)
	fmt.Println(t2.Add(1e3).String())

image

@ianlancetaylor
Copy link
Contributor

This changed due to https://golang.org/cl/300890. I think some odd handling in the overflow case is inevitable. I don't think there is anything to do here, but please comment if you have a suggestion for something we could change.

@itmecho
Copy link
Author

itmecho commented Sep 24, 2021

Unfortunately I have no idea how to handle it nicely! This turned into more of a learning experience, hopefully others might find it useful/interesting!

Cheers

@golang golang locked and limited conversation to collaborators Sep 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants