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

reflect: roll back NumMethod change for interfaces #42123

Closed
rsc opened this issue Oct 21, 2020 · 5 comments
Closed

reflect: roll back NumMethod change for interfaces #42123

rsc opened this issue Oct 21, 2020 · 5 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@rsc
Copy link
Contributor

rsc commented Oct 21, 2020

The NumMethod change for interfaces (#22075) broke code in the standard library that assumed for interfaces that NumMethod() == 0 if and only if it is an empty interface. There is also a lot of code testing NumMethod() against 0 in the wild. See attached nummethod.txt. There are dups in that file but still, there's a bunch.

All that code will break with the change. We should probably roll it back and live with the difference between NumMethod on interfaces and NumMethod on non-interfaces.

nummethod.txt

@rsc rsc added this to the Go1.16 milestone Oct 21, 2020
@rsc rsc added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 21, 2020
@gopherbot
Copy link

Change https://golang.org/cl/264026 mentions this issue: Revert "cmd/compile: split exported/non-exported methods for interface type"

@cuonglm
Copy link
Member

cuonglm commented Oct 21, 2020

See also #41882

@cherrymui
Copy link
Member

If we decide not to change NumMethod, should we change the doc to make it match the code? (It currently says "NumMethod returns the number of exported methods in the type's method set.")

@ianlancetaylor
Copy link
Contributor

Yes, we should correct the docs.

@gopherbot
Copy link

Change https://golang.org/cl/264357 mentions this issue: reflect: update NumMethod doc for interface type

gopherbot pushed a commit that referenced this issue Nov 3, 2020
Updates #42123

Change-Id: Ieb43b65c88d15b2475b6f3dd9672c44e7831cc34
Reviewed-on: https://go-review.googlesource.com/c/go/+/264357
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
dsnet added a commit to google/go-cmp that referenced this issue Nov 11, 2020
This reverts commit ab46b8b.
The upstream change in Go1.16 has been rolled back.
See golang/go#42123
dsnet added a commit to google/go-cmp that referenced this issue Nov 12, 2020
)

This reverts commit ab46b8b.
The upstream change in Go1.16 has been rolled back.
See golang/go#42123
@golang golang locked and limited conversation to collaborators Oct 28, 2021
gopherbot pushed a commit that referenced this issue Apr 4, 2022
NumMethod counts unexported methods for interface types. This
behavior is documented in Type.NumMethod

Fixes #42123

Change-Id: Ia5aba353a8cc64190c701d1521972d57e8903564
Reviewed-on: https://go-review.googlesource.com/c/go/+/396075
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Cherry Mui <cherryyz@google.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants