protobuf icon indicating copy to clipboard operation
protobuf copied to clipboard

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

Open chrisguiney opened this issue 5 years ago • 5 comments

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.

chrisguiney avatar Aug 30 '20 18:08 chrisguiney

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

dsnet avatar Aug 30 '20 20:08 dsnet

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

puellanivis avatar Sep 08 '20 08:09 puellanivis

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".

neild avatar Sep 08 '20 18:09 neild

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?

chrisguiney avatar Sep 09 '20 20:09 chrisguiney

@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.

neild avatar Sep 09 '20 20:09 neild