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.

Sign up for GitHub

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

Jump to bottom

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
Go1.16

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
This was referenced Oct 21, 2020
Closed
Closed
Closed
@gopherbot
Copy link
Contributor

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

@ianlancetaylor ianlancetaylor mentioned this issue Oct 21, 2020
Closed
@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
Contributor

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

@gazerro gazerro mentioned this issue Oct 30, 2020
Closed
gopherbot pushed a commit that referenced this issue Nov 3, 2020
@cuonglm @rsc
reflect: update NumMethod doc for interface type
ebc1b8e 
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
@dsnet
Revert "Adjust for reflect.Type.NumMethod change in Go1.16 (#240)"
07015e1 
This reverts commit ab46b8b.
The upstream change in Go1.16 has been rolled back.
See golang/go#42123
@dsnet dsnet mentioned this issue Nov 11, 2020
Merged
dsnet added a commit to google/go-cmp that referenced this issue Nov 12, 2020
@dsnet
Revert "Adjust for reflect.Type.NumMethod change in Go1.16 (#240)" (#242
d3c8501 
)

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
@zlasd @ianlancetaylor
reflect: fix Value.NumMethod docs
ac31352 
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.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.16
Development

No branches or pull requests
5 participants
@rsc @ianlancetaylor @cuonglm @gopherbot @cherrymui