-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
all: revise reflect.Type/reflect.Value NumMethod usage after CL 259237 #41882
Comments
Change https://golang.org/cl/260857 mentions this issue: |
So as feared, it looks like some users are relying on the undocument behavior. In particular, they're using Google's internal Code Search for There's a reasonable and backwards-compatible alternative using package reflect's public API like:
I see two questions:
/cc @rsc @ianlancetaylor |
@mdempsky For 1), I think it should be ok, since when Go 1 compat guidelines also states that user's code which rely on buggy behavior may break. |
That many cases makes this sound like a pretty problematic change. While it's true that the compatibility guarantee permits us to fix bugs, we still have to be careful about breaking code. The fact that some of the code we are breaking is in the standard library is a particular bad sign. Also, we only changed If we're going to go ahead with this change I think we do need to add some sort of Want to open a proposal issue? Thanks and sorry. |
Thanks, I filled #41896 |
While we wait for the proposal committee to review #41896 for the public facing API, maybe we should at least add an |
Change https://golang.org/cl/263078 mentions this issue: |
I added one in CL 263078 |
For the record: if we roll back https://golang.org/cl/259237 as suggested in #42123, then we don't have to do anything here. |
Closed as the change was rolled back. |
With CL 259237 merged, the bug in #22075 is fixed. But I found some of the usage in standard library, which relies on old (wrong) behavior of
NumMethod()
to detect empty interface.Example, this program:
go1.14 and go1.15 will print:
While on tip:
We should fix wrong checking
NumMethod() == 0
to detect empty interface.cc @mdempsky @cherrymui
The text was updated successfully, but these errors were encountered: