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: documentation sends mixed signals regarding == operator on time.Time values #19510

Closed
dmitshur opened this issue Mar 11, 2017 · 1 comment

Comments

Projects
None yet
3 participants
@dmitshur
Copy link
Member

commented Mar 11, 2017

On tip, in time package documentation, there are currently at least 3 places that talk about the == operator on time.Time values. They've been added at different times and are somewhat out of sync, sending mixed signals about status of == operator on time.Time values.

  1. As part of time.Time.Equal method documentation, modified 4 months ago from:

    // Note that using == with Time values produces unpredictable results.
    

    To:

    // Do not use == with Time values.
    
  2. As part of time.Time type documentation, added 3 years ago:

    // Note that the Go == operator compares not just the time instant but also the
    // Location. Therefore, Time values should not be used as map or database keys
    // without first guaranteeing that the identical Location has been set for all
    // values, which can be achieved through use of the UTC or Local method.
    
  3. As part of package comment, added a month ago:

    // Note that the Go == operator includes the monotonic clock reading in
    // its comparison. If time values returned from time.Now and time values
    // constructed by other means (for example, by time.Parse or time.Unix)
    // are meant to compare equal when used as map keys, the times returned
    // by time.Now must have the monotonic clock reading stripped, by setting
    // t = t.AddDate(0, 0, 0). In general, prefer t.Equal(u) to t == u, since
    // t.Equal uses the most accurate comparison available and correctly
    // handles the case when only one of its arguments has a monotonic clock
    // reading.
    

    (I like this version the best, it seems most accurate, clear and well-phrased. It's also the latest touched and mentions the recent monotonic support.)

This issue is primarily about the following phrase:

Do not use == with Time values.

Which was added on Oct 30, 2016 as part of CL 32411, and the commit message was:

time: simplify: tell people to not use == with Time values

When I first saw that, I understood that to mean that valid Go code should never use == operator on time.Time values. I filed a suggestion in staticcheck to detect and report instances of == operator being used on time.Time values, but in the discussion there it turned out that using == operator on time.Time values is okay given certain precautions are taken. As I understand it, the actual reality is that people should "prefer Time.Equal" but it's not illegal to use == operator, one just needs to be aware of certain rules.

"Do not use == with Time values." means something very different and doesn't seem to accurately represent the current state. This phrase is much better:

In general, prefer t.Equal(u) to t == u, since ...

@ianlancetaylor ianlancetaylor added this to the Go1.9 milestone Mar 11, 2017

zegl added a commit to zegl/go that referenced this issue Mar 24, 2017

encoding/base32: unpadded and custom padded encodings
Fixes: golang#19510

Change-Id: I87c76609d652bb8f478d16963223bb6f6a0e2d3a
@gopherbot

This comment has been minimized.

Copy link

commented Jun 14, 2017

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

@gopherbot gopherbot closed this in dd94bac Jun 14, 2017

gopherbot pushed a commit that referenced this issue Jun 14, 2017

time: remove some redundant equality comparison documentation
Updates to CL 45698

Updates #19510

Change-Id: Iec7a455b6c4d5f96d0b674459bf1455c99102d62
Reviewed-on: https://go-review.googlesource.com/45779
Reviewed-by: Rob Pike <r@golang.org>

@golang golang locked and limited conversation to collaborators Jun 14, 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.