all: make handling of zero-value composites more consistent
We occasionally need to work with immutable, empty lists, maps, and
messages. Notably, Message.Get on an empty repeated field will return a
"frozen" empty value.
Move handling of these immutable, zero-length composites into Converter,
to unify the behavior of regular and extension fields.
Add a Zero method to Converter, MessageType, and ExtensionType, to
provide a consistent way to get an empty, frozen value of a composite
type. Adding this method to the public {Message,Extension}Type
interfaces does increase our API surface, but lets us (for example)
cleanly represent an empty map as a nil map rather than a non-nil
one wrapped in a frozenMap type.
Drop the frozen{List,Map,Message} types as no longer necessary.
(These types did have support for creating a read-only view of a
non-empty value, but we are not currently using that feature.)
Change-Id: Ia76f149d591da07b40ce75b7404a7ab8a60cb9d8
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/189339
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
diff --git a/internal/impl/message_field.go b/internal/impl/message_field.go
index 3dfac23..7eb6199 100644
--- a/internal/impl/message_field.go
+++ b/internal/impl/message_field.go
@@ -42,10 +42,6 @@
}
conv := NewConverter(ot.Field(0).Type, fd)
isMessage := fd.Message() != nil
- var frozenEmpty pref.Value
- if isMessage {
- frozenEmpty = pref.ValueOf(frozenMessage{conv.New().Message()})
- }
// TODO: Implement unsafe fast path?
fieldOffset := offsetOf(fs, x)
@@ -74,17 +70,11 @@
},
get: func(p pointer) pref.Value {
if p.IsNil() {
- if frozenEmpty.IsValid() {
- return frozenEmpty
- }
- return defaultValueOf(fd)
+ return conv.Zero()
}
rv := p.Apply(fieldOffset).AsValueOf(fs.Type).Elem()
if rv.IsNil() || rv.Elem().Type().Elem() != ot {
- if frozenEmpty.IsValid() {
- return frozenEmpty
- }
- return defaultValueOf(fd)
+ return conv.Zero()
}
rv = rv.Elem().Elem().Field(0)
return conv.PBValueOf(rv)
@@ -126,7 +116,6 @@
panic(fmt.Sprintf("invalid type: got %v, want map kind", ft))
}
conv := NewConverter(ft, fd)
- frozenEmpty := pref.ValueOf(frozenMap{conv.New().Map()})
// TODO: Implement unsafe fast path?
fieldOffset := offsetOf(fs, x)
@@ -145,12 +134,9 @@
},
get: func(p pointer) pref.Value {
if p.IsNil() {
- return frozenEmpty
+ return conv.Zero()
}
rv := p.Apply(fieldOffset).AsValueOf(fs.Type).Elem()
- if rv.Len() == 0 {
- return frozenEmpty
- }
return conv.PBValueOf(rv)
},
set: func(p pointer, v pref.Value) {
@@ -176,7 +162,6 @@
panic(fmt.Sprintf("invalid type: got %v, want slice kind", ft))
}
conv := NewConverter(reflect.PtrTo(ft), fd)
- frozenEmpty := pref.ValueOf(frozenList{conv.New().List()})
// TODO: Implement unsafe fast path?
fieldOffset := offsetOf(fs, x)
@@ -195,12 +180,9 @@
},
get: func(p pointer) pref.Value {
if p.IsNil() {
- return frozenEmpty
+ return conv.Zero()
}
rv := p.Apply(fieldOffset).AsValueOf(fs.Type)
- if rv.Elem().Len() == 0 {
- return frozenEmpty
- }
return conv.PBValueOf(rv)
},
set: func(p pointer, v pref.Value) {
@@ -269,12 +251,12 @@
},
get: func(p pointer) pref.Value {
if p.IsNil() {
- return defaultValueOf(fd)
+ return conv.Zero()
}
rv := p.Apply(fieldOffset).AsValueOf(fs.Type).Elem()
if nullable {
if rv.IsNil() {
- return defaultValueOf(fd)
+ return conv.Zero()
}
if rv.Kind() == reflect.Ptr {
rv = rv.Elem()
@@ -312,7 +294,6 @@
var once sync.Once
var messageType pref.MessageType
- var frozenEmpty pref.Value
lazyInit := func() {
once.Do(func() {
messageName := fd.Message().FullName()
@@ -320,7 +301,6 @@
if messageType == nil {
panic(fmt.Sprintf("weak message %v is not linked in", messageName))
}
- frozenEmpty = pref.ValueOf(frozenMessage{messageType.New()})
})
}
@@ -342,12 +322,12 @@
get: func(p pointer) pref.Value {
lazyInit()
if p.IsNil() {
- return frozenEmpty
+ return pref.ValueOf(messageType.Zero())
}
fs := p.Apply(weakOffset).WeakFields()
m, ok := (*fs)[num]
if !ok {
- return frozenEmpty
+ return pref.ValueOf(messageType.Zero())
}
return pref.ValueOf(m.(pref.ProtoMessage).ProtoReflect())
},
@@ -390,7 +370,6 @@
func fieldInfoForMessage(fd pref.FieldDescriptor, fs reflect.StructField, x exporter) fieldInfo {
ft := fs.Type
conv := NewConverter(ft, fd)
- frozenEmpty := pref.ValueOf(frozenMessage{conv.New().Message()})
// TODO: Implement unsafe fast path?
fieldOffset := offsetOf(fs, x)
@@ -409,12 +388,9 @@
},
get: func(p pointer) pref.Value {
if p.IsNil() {
- return frozenEmpty
+ return conv.Zero()
}
rv := p.Apply(fieldOffset).AsValueOf(fs.Type).Elem()
- if rv.IsNil() {
- return frozenEmpty
- }
return conv.PBValueOf(rv)
},
set: func(p pointer, v pref.Value) {
@@ -461,76 +437,3 @@
},
}
}
-
-// defaultValueOf returns the default value for the field.
-func defaultValueOf(fd pref.FieldDescriptor) pref.Value {
- if fd == nil {
- return pref.Value{}
- }
- pv := fd.Default() // invalid Value for messages and repeated fields
- if fd.Kind() == pref.BytesKind && pv.IsValid() && len(pv.Bytes()) > 0 {
- return pref.ValueOf(append([]byte(nil), pv.Bytes()...)) // copy default bytes for safety
- }
- return pv
-}
-
-// frozenValueOf returns a frozen version of any composite value.
-func frozenValueOf(v pref.Value) pref.Value {
- switch v := v.Interface().(type) {
- case pref.Message:
- if _, ok := v.(frozenMessage); !ok {
- return pref.ValueOf(frozenMessage{v})
- }
- case pref.List:
- if _, ok := v.(frozenList); !ok {
- return pref.ValueOf(frozenList{v})
- }
- case pref.Map:
- if _, ok := v.(frozenMap); !ok {
- return pref.ValueOf(frozenMap{v})
- }
- }
- return v
-}
-
-type frozenMessage struct{ pref.Message }
-
-func (m frozenMessage) ProtoReflect() pref.Message { return m }
-func (m frozenMessage) Interface() pref.ProtoMessage { return m }
-func (m frozenMessage) Range(f func(pref.FieldDescriptor, pref.Value) bool) {
- m.Message.Range(func(fd pref.FieldDescriptor, v pref.Value) bool {
- return f(fd, frozenValueOf(v))
- })
-}
-func (m frozenMessage) Get(fd pref.FieldDescriptor) pref.Value {
- v := m.Message.Get(fd)
- return frozenValueOf(v)
-}
-func (frozenMessage) Clear(pref.FieldDescriptor) { panic("invalid on read-only Message") }
-func (frozenMessage) Set(pref.FieldDescriptor, pref.Value) { panic("invalid on read-only Message") }
-func (frozenMessage) Mutable(pref.FieldDescriptor) pref.Value { panic("invalid on read-only Message") }
-func (frozenMessage) SetUnknown(pref.RawFields) { panic("invalid on read-only Message") }
-
-type frozenList struct{ pref.List }
-
-func (ls frozenList) Get(i int) pref.Value {
- v := ls.List.Get(i)
- return frozenValueOf(v)
-}
-func (frozenList) Set(i int, v pref.Value) { panic("invalid on read-only List") }
-func (frozenList) Append(v pref.Value) { panic("invalid on read-only List") }
-func (frozenList) Truncate(i int) { panic("invalid on read-only List") }
-
-type frozenMap struct{ pref.Map }
-
-func (ms frozenMap) Get(k pref.MapKey) pref.Value {
- v := ms.Map.Get(k)
- return frozenValueOf(v)
-}
-func (ms frozenMap) Range(f func(pref.MapKey, pref.Value) bool) {
- ms.Map.Range(func(k pref.MapKey, v pref.Value) bool {
- return f(k, frozenValueOf(v))
- })
-}
-func (frozenMap) Set(k pref.MapKey, v pref.Value) { panic("invalid n read-only Map") }
-func (frozenMap) Clear(k pref.MapKey) { panic("invalid on read-only Map") }