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

cmd/compile: partition exported and unexported methods in type reflect data #22074

Closed
mdempsky opened this issue Sep 28, 2017 · 2 comments

Comments

Projects
None yet
3 participants
@mdempsky
Copy link
Member

commented Sep 28, 2017

Package reflect has to do some extra work to dynamically build slices of exported methods for types, but it looks like this could be handled at compile-time by simply statically partitioning exported and unexported methods, and then using the unused uint16 field in runtime.uncommontype for the boundary.

This could potentially cause problems for any code that iterates over method sets and expects a particular ordering. I looked over the interface-implements logic in runtime and reflect at least, and they appear to be insensitive to the particular method ordering as long as its consistent.

Notably, sorting exported methods before unexported methods is identical to the status quo when names start with an ASCII character, which is the common case in the standard repo. This could cause ordering assumptions to go unnoticed, so maybe it's preferable to do the opposite (i.e., unexported methods first).

Related #22073.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Mar 29, 2018

Is there anything to do here that was not done in CL 100846?

@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Mar 29, 2018

@mdempsky

This comment has been minimized.

Copy link
Member Author

commented Mar 29, 2018

Nope.

@mdempsky mdempsky closed this Mar 29, 2018

@golang golang locked and limited conversation to collaborators Mar 29, 2019

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.