golang / protobuf

Go support for Google's protocol buffers
BSD 3-Clause "New" or "Revised" License
9.74k stars 1.58k forks source link

Add a generic proto.Clone #1594

Open akshayjshah opened 7 months ago

akshayjshah commented 7 months ago

Is your feature request related to a problem? Please describe.

Use of proto.Clone is awkward because it returns a proto.Message, even though the caller often has a concrete type in hand. In those cases, working with the cloned message requires a type assertion and (more importantly) error handling. For example:

now := timestamppb.Now()
clone, ok := proto.Clone(now).(*timestamppb.Timestamp)
if !ok {
  return errors.New("boom")
}
fmt.Println(clone.AsTime())

The error-handling path is unnecessary - Clone always returns a *timestamppb.Timestamp, but that isn't visible to the type system. Of course, the caller can instead panic if the type assertion fails...but modern Go offers a more palatable option.

Describe the solution you'd like

Now that generics are available in all supported Go versions, it'd be nice to have a type-safe, generic variant of proto.Clone:

func Clone2[T proto.Message](msg T) T {
  // implementation
}

Then our example code becomes:

now := timestamppb.Now()
clone := proto.Clone2(now)
fmt.Println(clone.AsTime())

Virtually all implementations of proto.Message are pointers to structs and have the same gcshape, so use of this generic function shouldn't significantly affect binary size (at least, with the current generics implementation).

The new function could be called DeepCopy, CloneMessage, CloneT, or whatever else seems appropriate. DeepCopy feels the most natural to me, since it's how the current GoDoc describe's Clone's behavior.

Describe alternatives you've considered

Additional context I'm happy to open a CL for this if we can agree on naming.

znkr commented 7 months ago

We discussed this internally and more or less aligned on

// CloneOf returns a deep copy of m. If the top-level message is invalid, 
// it returns an invalid message as well.
func CloneOf[M Message](m M) M {
        return proto.Clone(m).(M)
}

We decided to not move forward with it because at the time some of the tooling we use internally wasn't ready for generics. The remaining difficulty is that we are still supporting go 1.17, but we can work around that with build tags.

We're happy to accept a contribution. :)

Regarding Merge and Equal a separate proposal would be good.