reflect/protoreflect: add Has and Clear to KnownFields and Map
Change the API to use explicit Has/Clear methods instead of relying on the
"invalid" form of Value to represent nullability.
There are several reasons for this change:
* Much of the ecosystem around protobufs do not care about nullability alone.
For example, for the encoder to determine whether to emit a field:
it needs to first check if a field is nulled, and if not, it still needs to go
through a series of type-assertion to check whether the value is the zero value.
It is much easier to be able to just call Has.
* It enables representing the default value as part of the value API, rather
than only as part of the descriptor API.
* The C++ API also uses explicit has and clear methods. However, we differ from
them by defining Has for proto3 scalars (while C++ panics instead).
For internal consistency, we also use a Has/Clear API for Maps.
Change-Id: I30eda482c959d3e1454d72d9fc761c761ace27a6
Reviewed-on: https://go-review.googlesource.com/134998
Reviewed-by: Damien Neil <dneil@google.com>
diff --git a/reflect/protoreflect/type.go b/reflect/protoreflect/type.go
index 223532d..a729c3b 100644
--- a/reflect/protoreflect/type.go
+++ b/reflect/protoreflect/type.go
@@ -141,11 +141,11 @@
// Get returns the ith field. It panics if out of bounds.
// The FieldDescriptor is guaranteed to be non-nil, while the Value
- // may be Null if the proto file did not specify this option.
+ // may be invalid if the proto file did not specify this option.
Get(i int) (FieldDescriptor, Value)
// ByName looks a field up by full name and
- // returns (nil, Null) if not found.
+ // returns (nil, Value{}) if not found.
//
// As a special-case, non-extension fields in the options type can be
// directly accessed by the field name alone (e.g., "map_entry" may be used
@@ -153,7 +153,7 @@
ByName(s FullName) (FieldDescriptor, Value)
// ByNumber looks a field up by the field number and
- // returns (nil, Null) if not found.
+ // returns (nil, Value{}) if not found.
ByNumber(n FieldNumber) (FieldDescriptor, Value)
doNotImplement
diff --git a/reflect/protoreflect/value.go b/reflect/protoreflect/value.go
index 7744876..e6db47d 100644
--- a/reflect/protoreflect/value.go
+++ b/reflect/protoreflect/value.go
@@ -20,7 +20,7 @@
Number() EnumNumber
}
-// Message is a reflection interface to a concrete message value,
+// Message is a reflective interface for a concrete message value,
// which provides type information and getters/setters for individual fields.
//
// Concrete types may implement interfaces defined in proto/protoiface,
@@ -51,22 +51,12 @@
// underlying repeated element type is determined by the FieldDescriptor.Kind.
// See Value for a list of Go types associated with each Kind.
//
-// Some fields have the property of nullability where it is possible to
-// distinguish between the zero value of a field and whether the field was
-// explicitly populated with the zero value. Only scalars in proto2,
-// members of a oneof field, and singular messages are nullable.
-// In the presence of unset fields, KnownFields.Get does not return defaults;
-// use the corresponding FieldDescriptor.DefaultValue for that information.
-//
// Field extensions are handled as known fields once the extension type has been
// registered with KnownFields.ExtensionTypes.
//
-// List, Len, Get, Range, and ExtensionTypes are safe for concurrent access.
+// List, Len, Has, Get, Range, and ExtensionTypes are safe for concurrent use.
type KnownFields interface {
- // List returns a new, unordered list of all fields that are populated.
- // A nullable field is populated only if explicitly set.
- // A scalar field in proto3 is populated if it contains a non-zero value.
- // A repeated field is populated only if it is non-empty.
+ // List returns a new, unordered list of all populated fields.
List() []FieldNumber
// Len reports the number of fields that are populated.
@@ -74,22 +64,26 @@
// Invariant: f.Len() == len(f.List())
Len() int
- // TODO: Should Get return FieldDescriptor.Default if unpopulated instead of
- // returning the Null variable? If so, we loose the ability to represent
- // nullability in Get and Set calls and also need to add Has and Clear.
+ // Has reports whether a field is populated.
+ //
+ // Some fields have the property of nullability where it is possible to
+ // distinguish between the default value of a field and whether the field
+ // was explicitly populated with the default value. Only scalars in proto2,
+ // member fields of a oneof, and singular messages are nullable.
+ //
+ // A nullable field is populated only if explicitly set.
+ // A scalar field in proto3 is populated if it contains a non-zero value.
+ // A repeated field is populated only if it is non-empty.
+ Has(FieldNumber) bool
- // Get retrieves the value for field with the given field number.
- // It returns Null for non-existent or nulled fields.
+ // Get retrieves the value for a field with the given field number.
+ // It returns the default value for unpopulated fields.
+ // It returns an invalid value for unknown fields.
Get(FieldNumber) Value
- // TODO: Document memory aliasing behavior when a field is cleared?
- // For example, if Mutable is called later, can it reuse memory?
-
// Set stores the value for a field with the given field number.
// Setting a field belonging to a oneof implicitly clears any other field
// that may be currently set by the same oneof.
- // Null may be used to explicitly clear a field containing a proto2 scalar,
- // a member of oneof, or a singular message.
//
// When setting a composite type, it is unspecified whether the set
// value aliases the source's memory in any way.
@@ -98,6 +92,15 @@
// in MessageDescriptor.Fields or an extension field in ExtensionTypes.
Set(FieldNumber, Value)
+ // TODO: Document memory aliasing behavior when a field is cleared?
+ // For example, if Mutable is called later, can it reuse memory?
+
+ // Clear clears the field such that a subsequent call to Has reports false.
+ //
+ // It panics if the field number does not correspond with a known field
+ // in MessageDescriptor.Fields or an extension field in ExtensionTypes.
+ Clear(FieldNumber)
+
// Mutable returns a reference value for a field with a given field number.
// If the field is unset, Mutable implicitly initializes the field with
// a zero value instance of the Go type for that field.
@@ -105,7 +108,7 @@
// The returned Mutable reference is never nil, and is only valid until the
// next Set or Mutable call.
//
- // It panics if FieldNumber does not correspond with a known field
+ // It panics if the field number does not correspond with a known field
// in MessageDescriptor.Fields or an extension field in ExtensionTypes.
Mutable(FieldNumber) Mutable
@@ -124,7 +127,7 @@
// However, the relative ordering of fields with different field numbers
// is undefined.
//
-// List, Len, Get, and Range are safe for concurrent access.
+// List, Len, Get, and Range are safe for concurrent use.
type UnknownFields interface {
// List returns a new, unordered list of all fields that are set.
List() []FieldNumber
@@ -184,7 +187,7 @@
// ExtensionFieldTypes are the extension field types that this message instance
// has been extended with.
//
-// List, Len, Get, and Range are safe for concurrent access.
+// List, Len, Get, and Range are safe for concurrent use.
type ExtensionFieldTypes interface {
// List returns a new, unordered list of known extension field types.
List() []ExtensionType
@@ -219,12 +222,12 @@
Range(f func(ExtensionType) bool)
}
-// Vector is an ordered list. Every element is always considered populated
-// (i.e., Get never provides and Set never accepts Null).
+// Vector is an ordered list. Every element is considered populated
+// (i.e., Get never provides and Set never accepts invalid Values).
// The element Value type is determined by the associated FieldDescriptor.Kind
// and cannot be a Map or Vector.
//
-// Len and Get are safe for concurrent access.
+// Len and Get are safe for concurrent use.
type Vector interface {
// Len reports the number of entries in the Vector.
// Get, Set, Mutable, and Truncate panic with out of bound indexes.
@@ -237,16 +240,12 @@
//
// When setting a composite type, it is unspecified whether the set
// value aliases the source's memory in any way.
- //
- // It panics if the value is Null.
Set(int, Value)
// Append appends the provided value to the end of the vector.
//
// When appending a composite type, it is unspecified whether the appended
// value aliases the source's memory in any way.
- //
- // It panics if the value is Null.
Append(Value)
// Mutable returns a Mutable reference for the element with a given index.
@@ -274,7 +273,7 @@
// is considered populated. The entry Value type is determined by the associated
// FieldDescripto.Kind and cannot be a Map or Vector.
//
-// List, Len, Get, and Range are safe for concurrent access.
+// List, Len, Has, Get, and Range are safe for concurrent use.
type Map interface {
// List returns an unordered list of keys for all entries in the map.
List() []MapKey
@@ -284,7 +283,11 @@
// Invariant: f.Len() == len(f.List())
Len() int
+ // Has reports whether an entry with the given key is in the map.
+ Has(MapKey) bool
+
// Get retrieves the value for an entry with the given key.
+ // It returns an invalid value for non-existent entries.
Get(MapKey) Value
// Set stores the value for an entry with the given key.
@@ -292,9 +295,12 @@
// When setting a composite type, it is unspecified whether the set
// value aliases the source's memory in any way.
//
- // It panics if either the key or value are Null.
+ // It panics if either the key or value are invalid.
Set(MapKey, Value)
+ // Clear clears the entry associated with they given key.
+ Clear(MapKey)
+
// Mutable returns a Mutable reference for the element with a given key,
// allocating a new entry if necessary.
//
@@ -303,6 +309,7 @@
Mutable(MapKey) Mutable
// Range calls f sequentially for each key and value present in the map.
+ // The Value provided is guaranteed to be valid.
// If f returns false, range stops the iteration.
Range(f func(MapKey, Value) bool)
diff --git a/reflect/protoreflect/value_test.go b/reflect/protoreflect/value_test.go
index 69d9966..2b56f4a 100644
--- a/reflect/protoreflect/value_test.go
+++ b/reflect/protoreflect/value_test.go
@@ -20,7 +20,6 @@
in Value
want interface{}
}{
- {in: Null},
{in: Value{}},
{in: ValueOf(nil)},
{in: ValueOf(true), want: true},
@@ -43,8 +42,8 @@
t.Errorf("Value(%v).Interface() = %v, want %v", tt.in, got, tt.want)
}
- if got := tt.in.IsNull(); got != (tt.want == nil) {
- t.Errorf("Value(%v).IsNull() = %v, want %v", tt.in, got, tt.want == nil)
+ if got := tt.in.IsValid(); got != (tt.want != nil) {
+ t.Errorf("Value(%v).IsValid() = %v, want %v", tt.in, got, tt.want != nil)
}
switch want := tt.want.(type) {
case int32:
diff --git a/reflect/protoreflect/value_union.go b/reflect/protoreflect/value_union.go
index 351b606..aa47348 100644
--- a/reflect/protoreflect/value_union.go
+++ b/reflect/protoreflect/value_union.go
@@ -35,7 +35,7 @@
//
// Multiple protobuf Kinds may be represented by a single Go type if the type
// can losslessly represent the information for the proto kind. For example,
-// Int64Kind, Sint64Kind, and Sfixed64Kind all represent int64,
+// Int64Kind, Sint64Kind, and Sfixed64Kind are all represented by int64,
// but use different integer encoding methods.
//
// The Vector or Map types are used if the FieldDescriptor.Cardinality of the
@@ -47,14 +47,6 @@
// retrieve an int64 from a string.
type Value value
-// Null is an unpopulated Value.
-//
-// Since Value is incomparable, call Value.IsNull instead to test whether
-// a Value is empty.
-//
-// It is equivalent to Value{} or ValueOf(nil).
-var Null Value
-
// The protoreflect API uses a custom Value union type instead of interface{}
// to keep the future open for performance optimizations. Using an interface{}
// always incurs an allocation for primitives (e.g., int64) since it needs to
@@ -110,9 +102,9 @@
}
}
-// IsNull reports whether v is empty (has no value).
-func (v Value) IsNull() bool {
- return v.typ == nilType
+// IsValid reports whether v is populated with a value.
+func (v Value) IsValid() bool {
+ return v.typ != nilType
}
// Interface returns v as an interface{}.
@@ -282,9 +274,9 @@
// converting a Value to a MapKey with an invalid type panics.
type MapKey value
-// IsNull reports whether v is empty (has no value).
-func (k MapKey) IsNull() bool {
- return Value(k).IsNull()
+// IsValid reports whether k is populated with a value.
+func (k MapKey) IsValid() bool {
+ return Value(k).IsValid()
}
// Interface returns k as an interface{}.
diff --git a/reflect/prototype/protofile_desc.go b/reflect/prototype/protofile_desc.go
index 466e53e..8ab3279 100644
--- a/reflect/prototype/protofile_desc.go
+++ b/reflect/prototype/protofile_desc.go
@@ -386,5 +386,5 @@
return protoreflect.ValueOf([]byte(s)), nil
}
}
- return protoreflect.Null, errors.New("invalid default value for %v: %q", k, s)
+ return protoreflect.Value{}, errors.New("invalid default value for %v: %q", k, s)
}
diff --git a/reflect/prototype/protofile_type.go b/reflect/prototype/protofile_type.go
index f058403..ef22823 100644
--- a/reflect/prototype/protofile_type.go
+++ b/reflect/prototype/protofile_type.go
@@ -439,7 +439,7 @@
func (p *defaultValue) lazyInit(t pref.FieldDescriptor, v pref.Value) pref.Value {
p.once.Do(func() {
p.val = v
- if !v.IsNull() {
+ if v.IsValid() {
switch t.Kind() {
case pref.EnumKind:
// Treat a string value as an identifier referencing some enum
