apple / swift-protobuf

Plugin and runtime library for using protobuf with Swift
Apache License 2.0
4.58k stars 452 forks source link

`MessageExtension` should capture the default #965

Open thomasvl opened 4 years ago

thomasvl commented 4 years ago

The generated code for the Swift extensions we create for proto extension fields includes the default value. That means we can't safely mark those generated apis as inlinable because the calling code would end up with the default and not get an updated default if a default was changed (without a rebuild).

If we instead capture the default in MessageExtension and make ExtensibleMessage.getExtensionValue return the default (instead of an optional), the generated properties could be tagged as inlinable.

aside - the has/clear methods could get tagged as inline safely right now, just not sure if it is worth it vs. doing everything.

thomasvl commented 2 years ago

Giving this some more thought lately, for the POD extensions, this doesn't seem too bad, MessageExtension can hold the default, and getExtensionValue could return it when the lookup fails. There could be some optimizations to help reduce the costs/sizes of things:

But the more problem case(s) are when the extension field is a Message/Group. We don't really want to have to create an instance all the time, and the current flow always it to be created on demand when needed.

Maybe the right way to do this is instead have there be some closure on MessageExtension to get the default value? That would let the Message/Group cases create on demand, and might be enough to have generic support for the repeated and zero pod cases?