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

protoc-gen-go: generate allocating accessors for message fields #1192

Open
chrisguiney opened this issue Aug 30, 2020 · 5 comments
Open

protoc-gen-go: generate allocating accessors for message fields #1192

chrisguiney opened this issue Aug 30, 2020 · 5 comments
Milestone

Comments

@chrisguiney
Copy link

Is your feature request related to a problem? Please describe.
My problem is that when I have a message that contains another message in a field, I have to manually check if it's been set or not and potentially allocate it myself. This results in me writing very boilerplate code to provide such an accessor method.

Describe the solution you'd like
I would like protoc-gen-go to automatically generate these accessors. Something along the lines of

func (x *Message1) GetAllocatedMessage2() *Message2 {
    if x.Message2 ==  nil {
        x.Message2 = new(Message2)
    }
    return x.Message2
}

Describe alternatives you've considered

  • hand writing these accessor methods (what i'm currently doing)
  • writing a script to generate them (maybe the next step)
  • sticking with gogo/protobuf forever
  • writing a custom generator (seems like it'd require a ton of work/essentially forking protoc-gen-go)

Additional context

  • I'm trying to migrate from gogo/protobuf, which enabled me to have messages directly embedded in the parent, where nilness wasn't ever a possibility.
  • the c++ generated code doesn't seem to have a problem allocating where necessary
  • a more terse name than GetAllocatedMessage2() would be appreciated.
@dsnet
Copy link
Member

dsnet commented Aug 30, 2020

\cc @neild who has been interested in adding a "MutableFoo" accessor method which does exactly this.

@puellanivis
Copy link
Collaborator

Definitely seems like the MutableFoo would be useful here, when one is programaticly assigning values, instead of just creating complex literal messages.

@neild
Copy link
Contributor

neild commented Sep 8, 2020

I very much want to add this. The sticking point has been binary and linker input size concerns.

The name MutableFoo is a bit weird, since it could seem to imply that GetFoo returns a non-mutable reference, but is consistent with method naming in other languages' APIs and (to me). I think it's justifiable as, "get me a value of foo that is definitely mutable, as opposed to one which may or may not be".

@chrisguiney
Copy link
Author

Just to give some feedback on the naming bikeshed: MutableFoo(), while it does align with the c++ api, makes no sense in the go ecosystem. Please don't try to force people working in go to write c++ in go. It'll be like nails on chalkboard.

The sticking point has been binary and linker input size concerns.

@nield Can you expand on this?

@neild
Copy link
Contributor

neild commented Sep 9, 2020

@nield Can you expand on this?

More code means larger linker inputs, and potentially larger binaries. Linker input and binary sizes are a particular area of concern for us, especially in programs which contain large amounts of generated protobuf code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants