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: AddDate() should use UTC if loc is nil #15852

Closed
KilledKenny opened this issue May 26, 2016 · 5 comments

Comments

Projects
None yet
4 participants
@KilledKenny
Copy link
Contributor

commented May 26, 2016

I added all my env info at the end since everything is reproducible on play.golang.org

What did you do?

I called time's AddDate whit a time object that was missing a location

https://play.golang.org/p/S-ZZKabdyL

package main

import (
    "time"
)

func main() {
    time.Time{}.AddDate(0,0,0)
}

What did you expect to see?

I expected that AddDate would follow the behavior specified by loc documentation that it should use the default UTC as location when calling Date

Only the zero Time has a nil Location.
In that case it is interpreted to mean UTC.

Ref: https://golang.org/src/time/time.go?s=2261:2352

The expected behavior can be seen here: https://play.golang.org/p/-9Bp0rjphc

What did you see instead?

panic: time: missing Location in call to Date

goroutine 1 [running]:
panic(0xd6660, 0x1030a078)
    /usr/local/go/src/runtime/panic.go:481 +0x700
time.Date(0x1, 0x1, 0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
    /usr/local/go/src/time/time.go:1030 +0xa0
time.Time.AddDate(0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
    /usr/local/go/src/time/time.go:658 +0x100
main.main()
    /tmp/sandbox596209771/main.go:8 +0x60

My suggested solution

$ git diff
diff --git a/src/time/time.go b/src/time/time.go
index d9dbd34..6ead1ce 100644
--- a/src/time/time.go
+++ b/src/time/time.go
@@ -652,7 +652,8 @@ func Since(t Time) Duration {
 func (t Time) AddDate(years int, months int, days int) Time {
        year, month, day := t.Date()
        hour, min, sec := t.Clock()
-       return Date(year+years, month+Month(months), day+days, hour, min, sec, int(t.nsec), t.loc)
+       location := t.Location()
+       return Date(year+years, month+Month(months), day+days, hour, min, sec, int(t.nsec), location)
 }

 const (

Dose the fix look good? Should I submit it for review?

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

$ go version
go version go1.6.1 linux/amd64

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

$ go env
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/simonr/go"
GORACE=""
GOROOT="/usr/lib/go"
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GO15VENDOREXPERIMENT="1"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0"
CXX="g++"
CGO_ENABLED="1"

@quentinmit quentinmit added the NeedsFix label May 26, 2016

@quentinmit

This comment has been minimized.

Copy link
Contributor

commented May 26, 2016

It looks like this panic has been in place since at least 2011; it's a simple change but I think it's probably too late for Go 1.7.

@KilledKenny Can you please submit your patch via Gerrit? (see https://golang.org/doc/contribute.html)

@rsc @bradfitz You seem to own the time package, do you think we should change this and do you agree we should wait for Go 1.8?

@quentinmit quentinmit added this to the Go1.8 milestone May 26, 2016

@KilledKenny

This comment has been minimized.

Copy link
Contributor Author

commented May 27, 2016

@quentinmit Yep, I'm aware of the contribute guidelines, I will attempt to submit the fix during the weekend.

@rsc

This comment has been minimized.

Copy link
Contributor

commented May 27, 2016

It should probably be fixed, but let's wait for Go 1.8. It's clearly not critical since it has gone this long. Thanks.

@KilledKenny

This comment has been minimized.

Copy link
Contributor Author

commented May 28, 2016

@gopherbot

This comment has been minimized.

Copy link

commented May 28, 2016

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

@gopherbot gopherbot closed this in b4f3c93 Oct 6, 2016

@golang golang locked and limited conversation to collaborators Oct 6, 2017

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.