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

proposal: plugin: add Syms method #52513

Open
Guilherme-De-Marchi opened this issue Apr 24, 2022 · 4 comments
Open

proposal: plugin: add Syms method #52513

Guilherme-De-Marchi opened this issue Apr 24, 2022 · 4 comments
Labels
Milestone

Comments

@Guilherme-De-Marchi
Copy link

@Guilherme-De-Marchi Guilherme-De-Marchi commented Apr 24, 2022

Hello.
I would like to suggest a tiny change on the plugin package.
The Plugin struct have a field called syms, that holds all exported variable or function names. It's safe for concurrent too. But, we can't access this field when using the Plugin struct, and would be useful to use that map, e.g in the case of a programs that wants to list to the user all the functions that a plugin exports.
So, my suggestion is to turn the field syms to Syms, so we can use when import the plugin package.

@gopherbot gopherbot added this to the Proposal milestone Apr 24, 2022
@DeedleFake
Copy link

@DeedleFake DeedleFake commented Apr 24, 2022

What might be more likely is a func (p *Plugin) Syms() []string method. Unfortunately, as far as I know the plugin package is essentially unmaintained at the moment, though this change is pretty small.

@Guilherme-De-Marchi
Copy link
Author

@Guilherme-De-Marchi Guilherme-De-Marchi commented Apr 24, 2022

What might be more likely is a func (p *Plugin) Syms() []string method. Unfortunately, as far as I know the plugin package is essentially unmaintained at the moment, though this change is pretty small.

Oh, i really would like to have access to this kind of data XD. Is it possible for someone to make this change in this package? I'm quite a noob when it comes to the github issue system, this is the first one I open, by the way.
The method that you suggest is valid too, but someone may want to get a list of the interfaces that the syms map hold, etc. So, i think that just export this field is a nice and useful solution. Thanks for the answer, too :-)

@DeedleFake
Copy link

@DeedleFake DeedleFake commented Apr 24, 2022

Exporting the field isn't likely, for several reasons, but one of the biggest is that it would then become mutable. Even if you had a method that just returned it, such as func (p *Plugin) Syms() map[string]any { return p.syms }, it would still be mutable because maps are actually a pointer internally, and copying them only copies that pointer. A full copy could be performed, but that would be fairly expensive and have little benefit over just returning a slice of the keys, which could then be manually fed into Lookup to get the actual symbols. Plus, if you look at the definition of Lookup, it actually calls another function that could be platform specific, so allowing access to the map directly would allow the user to potentially bypass platform-specific code, which could be quite messy.

All of that being said, I'm not a maintainer. While I, obviously, think that I'm right about all of this, it's ultimately up to the actual project maintainers.

@Guilherme-De-Marchi
Copy link
Author

@Guilherme-De-Marchi Guilherme-De-Marchi commented Apr 24, 2022

Exporting the field isn't likely, for several reasons, but one of the biggest is that it would then become mutable. Even if you had a method that just returned it, such as func (p *Plugin) Syms() map[string]any { return p.syms }, it would still be mutable because maps are actually a pointer internally, and copying them only copies that pointer. A full copy could be performed, but that would be fairly expensive and have little benefit over just returning a slice of the keys, which could then be manually fed into Lookup to get the actual symbols. Plus, if you look at the definition of Lookup, it actually calls another function that could be platform specific, so allowing access to the map directly would allow the user to potentially bypass platform-specific code, which could be quite messy.

All of that being said, I'm not a maintainer. While I, obviously, think that I'm right about all of this, it's ultimately up to the actual project maintainers.

Yeah, you are right, thanks for the correction. I think so that your suggestion of a function that return the map keys is what should be some nice method to be added on the Plugin struct. Thanks, again S2

@ianlancetaylor ianlancetaylor changed the title proposal: affected/package: small change on plugin package proposal: plugin: add Syms method Apr 25, 2022
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Proposals
Incoming
Development

No branches or pull requests

3 participants