reflect/prototype: hoist semantic options into builders
Add fields to the Message and Field builder structs which hold the value
of MessageOptions.map_entry, FieldOptions.packed, and FieldOptions.weak
options. Remove all access to the contents of options messages from the
prototype package.
Change IsPacked to always return false for unpackable field types,
which is consistent with the equivalent C++ API.
This change helps avoid dependency cycles between prototype and the
options messages. (Previously this was resolved by accessing options
with reflection, but just breaking the dependency from prototype to the
options message is cleaner and simpler.)
Change-Id: I756aefe2e04cfa8fea31eaaaa0b5a99d4ac9e851
Reviewed-on: https://go-review.googlesource.com/c/153517
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
diff --git a/reflect/protodesc/protodesc.go b/reflect/protodesc/protodesc.go
index 38d03a7..e2b402b 100644
--- a/reflect/protodesc/protodesc.go
+++ b/reflect/protodesc/protodesc.go
@@ -122,13 +122,23 @@
var m prototype.Message
m.Name = protoreflect.Name(md.GetName())
m.Options = md.GetOptions()
+ m.IsMapEntry = md.GetOptions().GetMapEntry()
for _, fd := range md.GetField() {
var f prototype.Field
f.Name = protoreflect.Name(fd.GetName())
f.Number = protoreflect.FieldNumber(fd.GetNumber())
f.Cardinality = protoreflect.Cardinality(fd.GetLabel())
f.Kind = protoreflect.Kind(fd.GetType())
- f.Options = fd.GetOptions()
+ opts := fd.GetOptions()
+ f.Options = opts
+ if opts != nil && opts.Packed != nil {
+ if *opts.Packed {
+ f.IsPacked = prototype.True
+ } else {
+ f.IsPacked = prototype.False
+ }
+ }
+ f.IsWeak = opts.GetWeak()
f.JSONName = fd.GetJsonName()
if fd.DefaultValue != nil {
f.Default, err = defval.Unmarshal(fd.GetDefaultValue(), f.Kind, defval.Descriptor)
@@ -143,7 +153,6 @@
}
f.OneofName = protoreflect.Name(md.GetOneofDecl()[i].GetName())
}
- opts, _ := f.Options.(*descriptorpb.FieldOptions)
switch f.Kind {
case protoreflect.EnumKind:
f.EnumType, err = findEnumDescriptor(fd.GetTypeName(), r)
diff --git a/reflect/prototype/protofile.go b/reflect/prototype/protofile.go
index d7dca23..8ca96b0 100644
--- a/reflect/prototype/protofile.go
+++ b/reflect/prototype/protofile.go
@@ -11,6 +11,13 @@
// information must be provided such as the full type name and the proto syntax.
// When creating an entire file, the syntax and full name is derived from
// the parent type.
+//
+// Most types contain options, defined as messages in descriptor.proto.
+// To avoid cyclic dependencies, the prototype package treats these options
+// as opaque protoreflect.ProtoMessage values. In some cases where the option
+// contains semantically important information (e.g.,
+// google.protobuf.MessageOptions.map_entry), this information must be provided
+// as a field of the corresponding type (e.g., prototype.Message.MapEntry).
package prototype
import "github.com/golang/protobuf/v2/reflect/protoreflect"
@@ -103,6 +110,7 @@
ExtensionRanges [][2]protoreflect.FieldNumber
ExtensionRangeOptions []protoreflect.ProtoMessage
Options protoreflect.ProtoMessage
+ IsMapEntry bool
Enums []Enum
Messages []Message
@@ -131,6 +139,8 @@
MessageType protoreflect.MessageDescriptor
EnumType protoreflect.EnumDescriptor
Options protoreflect.ProtoMessage
+ IsPacked OptionalBool
+ IsWeak bool
*fieldMeta
}
@@ -154,6 +164,7 @@
EnumType protoreflect.EnumDescriptor
ExtendedType protoreflect.MessageDescriptor
Options protoreflect.ProtoMessage
+ IsPacked OptionalBool
*extensionMeta
}
@@ -206,3 +217,13 @@
*methodMeta
}
+
+// OptionalBool is a tristate boolean.
+type OptionalBool uint8
+
+// Tristate boolean values.
+const (
+ DefaultBool OptionalBool = iota
+ True
+ False
+)
diff --git a/reflect/prototype/protofile_type.go b/reflect/prototype/protofile_type.go
index 4e2e0c4..442ad92 100644
--- a/reflect/prototype/protofile_type.go
+++ b/reflect/prototype/protofile_type.go
@@ -165,7 +165,6 @@
ms messagesMeta
es enumsMeta
xs extensionsMeta
- mo messageOptions
}
type messageDesc struct{ m *Message }
@@ -176,7 +175,7 @@
func (t messageDesc) FullName() pref.FullName { return t.m.fullName }
func (t messageDesc) IsPlaceholder() bool { return false }
func (t messageDesc) Options() pref.ProtoMessage { return altOptions(t.m.Options, optionTypes.Message) }
-func (t messageDesc) IsMapEntry() bool { return t.m.mo.lazyInit(t).isMapEntry }
+func (t messageDesc) IsMapEntry() bool { return t.m.IsMapEntry }
func (t messageDesc) Fields() pref.FieldDescriptors { return t.m.fs.lazyInit(t, t.m.Fields) }
func (t messageDesc) Oneofs() pref.OneofDescriptors { return t.m.os.lazyInit(t, t.m.Oneofs) }
func (t messageDesc) ReservedNames() pref.Names { return (*names)(&t.m.ReservedNames) }
@@ -207,22 +206,6 @@
return m
}
-type messageOptions struct {
- once sync.Once
- isMapEntry bool
-}
-
-func (p *messageOptions) lazyInit(m pref.MessageDescriptor) *messageOptions {
- p.once.Do(func() {
- if m.Options() != optionTypes.Message {
- const mapEntryFieldNumber = 7 // google.protobuf.MessageOptions.map_entry
- fs := m.Options().ProtoReflect().KnownFields()
- p.isMapEntry = fs.Get(mapEntryFieldNumber).Bool()
- }
- })
- return p
-}
-
type fieldMeta struct {
inheritedMeta
@@ -231,25 +214,29 @@
ot oneofReference
mt messageReference
et enumReference
- fo fieldOptions
}
type fieldDesc struct{ f *Field }
-func (t fieldDesc) Parent() (pref.Descriptor, bool) { return t.f.parent, true }
-func (t fieldDesc) Index() int { return t.f.index }
-func (t fieldDesc) Syntax() pref.Syntax { return t.f.syntax }
-func (t fieldDesc) Name() pref.Name { return t.f.Name }
-func (t fieldDesc) FullName() pref.FullName { return t.f.fullName }
-func (t fieldDesc) IsPlaceholder() bool { return false }
-func (t fieldDesc) Options() pref.ProtoMessage { return altOptions(t.f.Options, optionTypes.Field) }
-func (t fieldDesc) Number() pref.FieldNumber { return t.f.Number }
-func (t fieldDesc) Cardinality() pref.Cardinality { return t.f.Cardinality }
-func (t fieldDesc) Kind() pref.Kind { return t.f.Kind }
-func (t fieldDesc) HasJSONName() bool { return t.f.JSONName != "" }
-func (t fieldDesc) JSONName() string { return t.f.js.lazyInit(t.f) }
-func (t fieldDesc) IsPacked() bool { return t.f.fo.lazyInit(t).isPacked }
-func (t fieldDesc) IsWeak() bool { return t.f.fo.lazyInit(t).isWeak }
-func (t fieldDesc) IsMap() bool { return t.f.fo.lazyInit(t).isMap }
+func (t fieldDesc) Parent() (pref.Descriptor, bool) { return t.f.parent, true }
+func (t fieldDesc) Index() int { return t.f.index }
+func (t fieldDesc) Syntax() pref.Syntax { return t.f.syntax }
+func (t fieldDesc) Name() pref.Name { return t.f.Name }
+func (t fieldDesc) FullName() pref.FullName { return t.f.fullName }
+func (t fieldDesc) IsPlaceholder() bool { return false }
+func (t fieldDesc) Options() pref.ProtoMessage { return altOptions(t.f.Options, optionTypes.Field) }
+func (t fieldDesc) Number() pref.FieldNumber { return t.f.Number }
+func (t fieldDesc) Cardinality() pref.Cardinality { return t.f.Cardinality }
+func (t fieldDesc) Kind() pref.Kind { return t.f.Kind }
+func (t fieldDesc) HasJSONName() bool { return t.f.JSONName != "" }
+func (t fieldDesc) JSONName() string { return t.f.js.lazyInit(t.f) }
+func (t fieldDesc) IsPacked() bool {
+ return isPacked(t.f.IsPacked, t.f.syntax, t.f.Cardinality, t.f.Kind)
+}
+func (t fieldDesc) IsWeak() bool { return t.f.IsWeak }
+func (t fieldDesc) IsMap() bool {
+ mt := t.MessageType()
+ return mt != nil && mt.IsMapEntry()
+}
func (t fieldDesc) HasDefault() bool { return t.f.Default.IsValid() }
func (t fieldDesc) Default() pref.Value { return t.f.dv.value(t, t.f.Default) }
func (t fieldDesc) DefaultEnumValue() pref.EnumValueDescriptor { return t.f.dv.enum(t, t.f.Default) }
@@ -261,6 +248,20 @@
func (t fieldDesc) ProtoType(pref.FieldDescriptor) {}
func (t fieldDesc) ProtoInternal(pragma.DoNotImplement) {}
+func isPacked(packed OptionalBool, s pref.Syntax, c pref.Cardinality, k pref.Kind) bool {
+ if packed == False || (packed == DefaultBool && s == pref.Proto2) {
+ return false
+ }
+ if c != pref.Repeated {
+ return false
+ }
+ switch k {
+ case pref.StringKind, pref.BytesKind, pref.MessageKind, pref.GroupKind:
+ return false
+ }
+ return true
+}
+
type jsonName struct {
once sync.Once
name string
@@ -310,77 +311,6 @@
return p.otyp
}
-type fieldOptions struct {
- once sync.Once
- isPacked bool
- isWeak bool
- isMap bool
-}
-
-func (p *fieldOptions) lazyInit(f pref.FieldDescriptor) *fieldOptions {
- p.once.Do(func() {
- if f.Cardinality() == pref.Repeated {
- // In proto3, repeated fields of scalar numeric types use
- // packed encoding by default.
- // See https://developers.google.com/protocol-buffers/docs/proto3
- if f.Syntax() == pref.Proto3 {
- p.isPacked = isScalarNumeric[f.Kind()]
- }
- if f.Kind() == pref.MessageKind {
- p.isMap = f.MessageType().IsMapEntry()
- }
- }
-
- if f.Options() != optionTypes.Field {
- const packedFieldNumber = 2 // google.protobuf.FieldOptions.packed
- const weakFieldNumber = 10 // google.protobuf.FieldOptions.weak
- fs := f.Options().ProtoReflect().KnownFields()
- if fs.Has(packedFieldNumber) {
- p.isPacked = fs.Get(packedFieldNumber).Bool()
- }
- p.isWeak = fs.Get(weakFieldNumber).Bool()
- }
- })
- return p
-}
-
-// isPacked reports whether the packed options is set.
-func isPacked(m pref.ProtoMessage) (isPacked bool) {
- if m != optionTypes.Field {
- const packedFieldNumber = 2 // google.protobuf.FieldOptions.packed
- fs := m.ProtoReflect().KnownFields()
- isPacked = fs.Get(packedFieldNumber).Bool()
- }
- return isPacked
-}
-
-// isWeak reports whether the weak options is set.
-func isWeak(m pref.ProtoMessage) (isWeak bool) {
- if m != optionTypes.Field {
- const weakFieldNumber = 10 // google.protobuf.FieldOptions.weak
- fs := m.ProtoReflect().KnownFields()
- isWeak = fs.Get(weakFieldNumber).Bool()
- }
- return isWeak
-}
-
-var isScalarNumeric = map[pref.Kind]bool{
- pref.BoolKind: true,
- pref.EnumKind: true,
- pref.Int32Kind: true,
- pref.Sint32Kind: true,
- pref.Uint32Kind: true,
- pref.Int64Kind: true,
- pref.Sint64Kind: true,
- pref.Uint64Kind: true,
- pref.Sfixed32Kind: true,
- pref.Fixed32Kind: true,
- pref.FloatKind: true,
- pref.Sfixed64Kind: true,
- pref.Fixed64Kind: true,
- pref.DoubleKind: true,
-}
-
type oneofMeta struct {
inheritedMeta
@@ -410,19 +340,22 @@
}
type extensionDesc struct{ x *Extension }
-func (t extensionDesc) Parent() (pref.Descriptor, bool) { return t.x.parent, true }
-func (t extensionDesc) Syntax() pref.Syntax { return t.x.syntax }
-func (t extensionDesc) Index() int { return t.x.index }
-func (t extensionDesc) Name() pref.Name { return t.x.Name }
-func (t extensionDesc) FullName() pref.FullName { return t.x.fullName }
-func (t extensionDesc) IsPlaceholder() bool { return false }
-func (t extensionDesc) Options() pref.ProtoMessage { return altOptions(t.x.Options, optionTypes.Field) }
-func (t extensionDesc) Number() pref.FieldNumber { return t.x.Number }
-func (t extensionDesc) Cardinality() pref.Cardinality { return t.x.Cardinality }
-func (t extensionDesc) Kind() pref.Kind { return t.x.Kind }
-func (t extensionDesc) HasJSONName() bool { return false }
-func (t extensionDesc) JSONName() string { return "" }
-func (t extensionDesc) IsPacked() bool { return isPacked(t.Options()) }
+func (t extensionDesc) Parent() (pref.Descriptor, bool) { return t.x.parent, true }
+func (t extensionDesc) Syntax() pref.Syntax { return t.x.syntax }
+func (t extensionDesc) Index() int { return t.x.index }
+func (t extensionDesc) Name() pref.Name { return t.x.Name }
+func (t extensionDesc) FullName() pref.FullName { return t.x.fullName }
+func (t extensionDesc) IsPlaceholder() bool { return false }
+func (t extensionDesc) Options() pref.ProtoMessage { return altOptions(t.x.Options, optionTypes.Field) }
+func (t extensionDesc) Number() pref.FieldNumber { return t.x.Number }
+func (t extensionDesc) Cardinality() pref.Cardinality { return t.x.Cardinality }
+func (t extensionDesc) Kind() pref.Kind { return t.x.Kind }
+func (t extensionDesc) HasJSONName() bool { return false }
+func (t extensionDesc) JSONName() string { return "" }
+func (t extensionDesc) IsPacked() bool {
+ // Extensions always use proto2 defaults for packing.
+ return isPacked(t.x.IsPacked, pref.Proto2, t.x.Cardinality, t.x.Kind)
+}
func (t extensionDesc) IsWeak() bool { return false }
func (t extensionDesc) IsMap() bool { return false }
func (t extensionDesc) HasDefault() bool { return t.x.Default.IsValid() }
diff --git a/reflect/prototype/standalone.go b/reflect/prototype/standalone.go
index c934c5c..4184e82 100644
--- a/reflect/prototype/standalone.go
+++ b/reflect/prototype/standalone.go
@@ -24,11 +24,11 @@
ExtensionRanges [][2]protoreflect.FieldNumber
ExtensionRangeOptions []protoreflect.ProtoMessage
Options protoreflect.ProtoMessage
+ IsMapEntry bool
- fields fieldsMeta
- oneofs oneofsMeta
- nums numbersMeta
- options messageOptions
+ fields fieldsMeta
+ oneofs oneofsMeta
+ nums numbersMeta
}
// NewMessage creates a new protoreflect.MessageDescriptor.
@@ -67,7 +67,7 @@
for i, f := range t.Fields {
// Resolve placeholder messages with a concrete standalone message.
// If this fails, validateMessage will complain about it later.
- if f.MessageType != nil && f.MessageType.IsPlaceholder() && !isWeak(f.Options) {
+ if f.MessageType != nil && f.MessageType.IsPlaceholder() && !f.IsWeak {
if m, ok := ms[f.MessageType.FullName()]; ok {
t.Fields[i].MessageType = m
}
@@ -118,6 +118,7 @@
EnumType protoreflect.EnumDescriptor
ExtendedType protoreflect.MessageDescriptor
Options protoreflect.ProtoMessage
+ IsPacked OptionalBool
dv defaultValue
}
diff --git a/reflect/prototype/standalone_type.go b/reflect/prototype/standalone_type.go
index 4728c4e..a087a9d 100644
--- a/reflect/prototype/standalone_type.go
+++ b/reflect/prototype/standalone_type.go
@@ -23,7 +23,7 @@
func (t standaloneMessage) Options() pref.ProtoMessage {
return altOptions(t.m.Options, optionTypes.Message)
}
-func (t standaloneMessage) IsMapEntry() bool { return t.m.options.lazyInit(t).isMapEntry }
+func (t standaloneMessage) IsMapEntry() bool { return t.m.IsMapEntry }
func (t standaloneMessage) Fields() pref.FieldDescriptors { return t.m.fields.lazyInit(t, t.m.Fields) }
func (t standaloneMessage) Oneofs() pref.OneofDescriptors { return t.m.oneofs.lazyInit(t, t.m.Oneofs) }
func (t standaloneMessage) ReservedNames() pref.Names { return (*names)(&t.m.ReservedNames) }
@@ -76,11 +76,13 @@
func (t standaloneExtension) Kind() pref.Kind { return t.x.Kind }
func (t standaloneExtension) HasJSONName() bool { return false }
func (t standaloneExtension) JSONName() string { return "" }
-func (t standaloneExtension) IsPacked() bool { return isPacked(t.Options()) }
-func (t standaloneExtension) IsWeak() bool { return false }
-func (t standaloneExtension) IsMap() bool { return false }
-func (t standaloneExtension) HasDefault() bool { return t.x.Default.IsValid() }
-func (t standaloneExtension) Default() pref.Value { return t.x.dv.value(t, t.x.Default) }
+func (t standaloneExtension) IsPacked() bool {
+ return isPacked(t.x.IsPacked, pref.Proto2, t.x.Cardinality, t.x.Kind)
+}
+func (t standaloneExtension) IsWeak() bool { return false }
+func (t standaloneExtension) IsMap() bool { return false }
+func (t standaloneExtension) HasDefault() bool { return t.x.Default.IsValid() }
+func (t standaloneExtension) Default() pref.Value { return t.x.dv.value(t, t.x.Default) }
func (t standaloneExtension) DefaultEnumValue() pref.EnumValueDescriptor {
return t.x.dv.enum(t, t.x.Default)
}
diff --git a/reflect/prototype/type_test.go b/reflect/prototype/type_test.go
index 4bac6c6..56edebb 100644
--- a/reflect/prototype/type_test.go
+++ b/reflect/prototype/type_test.go
@@ -41,6 +41,7 @@
MapEntry: scalar.Bool(true),
Deprecated: scalar.Bool(true),
},
+ IsMapEntry: true,
Fields: []ptype.Field{{
Name: "key", // "test.A.key"
Number: 1,
@@ -92,6 +93,7 @@
Cardinality: pref.Repeated,
Kind: pref.Int32Kind,
Options: &descriptorpb.FieldOptions{Packed: scalar.Bool(true)},
+ IsPacked: ptype.True,
}, {
Name: "field_six", // "test.B.field_six"
Number: 6,
@@ -132,6 +134,7 @@
Cardinality: pref.Repeated,
Kind: pref.MessageKind,
Options: &descriptorpb.FieldOptions{Packed: scalar.Bool(false)},
+ IsPacked: ptype.False,
MessageType: ptype.PlaceholderMessage("test.C"),
ExtendedType: ptype.PlaceholderMessage("test.B"),
}},
@@ -156,6 +159,7 @@
Cardinality: pref.Repeated,
Kind: pref.MessageKind,
Options: &descriptorpb.FieldOptions{Packed: scalar.Bool(true)},
+ IsPacked: ptype.True,
MessageType: ptype.PlaceholderMessage("test.C"),
ExtendedType: ptype.PlaceholderMessage("test.B"),
}},
@@ -601,7 +605,7 @@
"Number": pref.FieldNumber(1000),
"Cardinality": pref.Repeated,
"Kind": pref.MessageKind,
- "IsPacked": true,
+ "IsPacked": false,
"MessageType": M{"FullName": pref.FullName("test.C"), "IsPlaceholder": false},
"ExtendedType": M{"FullName": pref.FullName("test.B"), "IsPlaceholder": false},
"Options": &descriptorpb.FieldOptions{Packed: scalar.Bool(true)},
@@ -862,7 +866,6 @@
Number: 1000
Cardinality: repeated
Kind: message
- IsPacked: true
ExtendedType: test.B
MessageType: test.C
}]