Kaynağa Gözat

reflect/protoreflect: remove List method

Remove List from KnownFields, UnknownFields, ExtensionFieldTypes, and Map.

Rationale:
* Each of those interfaces already have a Range method, which provides
a superset of the functionality of List. Furthermore, Range is more expressive
as it allows you to terminate iteration and provides both keys and values.
* List must always allocate a slice and populates it.
* Range is allocation free in some cases. For example, if you simply wanted to
iterate over all the populated fields to clear them, there is no need for a
closure, so a static version of the function can be directly referenced
(i.e., there is no need to create a stub function header that references the
closed-over variables).
* In the cases where a closure is needed, the allocation cost is O(1) since
there are a finite number of variables being closed over.
* In highly performance sensitive cases, the closured function could close over
a struct, such that the function and struct are stored in a sync.Pool when not
in use. For example:

	type MapLister struct {
		Entries []struct{K MapKey; V Value}
		f       func(MapKey, Value) true
	}
	func (m *MapLister) Ranger() func(MapKey, Value) bool {
		if m.f != nil {
			m.f = func(k MapKey, v Value) bool {
				m.Entries = append(m.Entries, ...)
				return true
			}
		}
		m.Entries = m.Entries[:0]
		return m.f
	}

The main benefit of List is the ease of use:

	for _, num := range knownFields.List() {
		...
	}

as opposed to:

	knownFields.Range(func(n FieldNumber, v Value) bool {
		...
		return true
	})

However, this is a marginal benefit.
Thus, remove List as it mostly inferior to Range.

Change-Id: I25586c6ea07a4706072ba06b1cf25cb6efb5e8a7
Reviewed-on: https://go-review.googlesource.com/c/142888
Reviewed-by: Damien Neil <dneil@google.com>
Joe Tsai 7 yıl önce
ebeveyn
işleme
3903b218df

+ 0 - 11
internal/impl/message.go

@@ -228,15 +228,6 @@ func (m *message) ProtoMutable() {}
 
 type knownFields messageDataType
 
-func (fs *knownFields) List() (nums []pref.FieldNumber) {
-	for n, fi := range fs.mi.fields {
-		if fi.has(fs.p) {
-			nums = append(nums, n)
-		}
-	}
-	// TODO: Handle extension fields.
-	return nums
-}
 func (fs *knownFields) Len() (cnt int) {
 	for _, fi := range fs.mi.fields {
 		if fi.has(fs.p) {
@@ -299,7 +290,6 @@ func (fs *knownFields) ExtensionTypes() pref.ExtensionFieldTypes {
 
 type extensionFieldTypes messageDataType // TODO
 
-func (fs *extensionFieldTypes) List() []pref.ExtensionType                   { return nil }
 func (fs *extensionFieldTypes) Len() int                                     { return 0 }
 func (fs *extensionFieldTypes) Register(pref.ExtensionType)                  { return }
 func (fs *extensionFieldTypes) Remove(pref.ExtensionType)                    { return }
@@ -309,7 +299,6 @@ func (fs *extensionFieldTypes) Range(f func(pref.ExtensionType) bool)        { r
 
 type unknownFields messageDataType // TODO
 
-func (fs *unknownFields) List() []pref.FieldNumber                            { return nil }
 func (fs *unknownFields) Len() int                                            { return 0 }
 func (fs *unknownFields) Get(n pref.FieldNumber) pref.RawFields               { return nil }
 func (fs *unknownFields) Set(n pref.FieldNumber, b pref.RawFields)            { return }

+ 0 - 7
internal/impl/message_field.go

@@ -74,13 +74,6 @@ type mapReflect struct {
 	valConv converter
 }
 
-func (ms mapReflect) List() []pref.MapKey {
-	var ks []pref.MapKey
-	for _, k := range ms.v.MapKeys() {
-		ks = append(ks, ms.keyConv.toPB(k).MapKey())
-	}
-	return ks
-}
 func (ms mapReflect) Len() int {
 	return ms.v.Len()
 }

+ 1 - 1
internal/impl/message_test.go

@@ -90,7 +90,7 @@ type (
 	setMap   map[interface{}]pref.Value
 	clearMap map[interface{}]bool
 	rangeMap map[interface{}]pref.Value
-	// TODO: List, Mutable
+	// TODO: Mutable
 )
 
 func TestScalarProto2(t *testing.T) {

+ 14 - 27
reflect/protoreflect/value.go

@@ -54,14 +54,9 @@ type Message interface {
 // Field extensions are handled as known fields once the extension type has been
 // registered with KnownFields.ExtensionTypes.
 //
-// List, Len, Has, Get, Range, and ExtensionTypes are safe for concurrent use.
+// Len, Has, Get, Range, and ExtensionTypes are safe for concurrent use.
 type KnownFields interface {
-	// List returns a new, unordered list of all populated fields.
-	List() []FieldNumber
-
 	// Len reports the number of fields that are populated.
-	//
-	// Invariant: f.Len() == len(f.List())
 	Len() int
 
 	// Has reports whether a field is populated.
@@ -114,6 +109,8 @@ type KnownFields interface {
 
 	// Range calls f sequentially for each known field that is populated.
 	// If f returns false, range stops the iteration.
+	// Assuming f always returns true and no mutations occur,
+	// the function is called exactly Len times.
 	Range(f func(FieldNumber, Value) bool)
 
 	// ExtensionTypes are extension field types that are known by this
@@ -127,18 +124,13 @@ type KnownFields interface {
 // However, the relative ordering of fields with different field numbers
 // is undefined.
 //
-// List, Len, Get, and Range are safe for concurrent use.
+// 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
-
-	// Len reports the number of fields that are set.
-	//
-	// Invariant: f.Len() == len(f.List())
+	// Len reports the number of fields that are populated.
 	Len() int
 
 	// Get retrieves the raw bytes of fields with the given field number.
-	// It returns an empty RawFields if there are no set fields.
+	// It returns an empty RawFields if there are no populated fields.
 	//
 	// The caller must not mutate the content of the retrieved RawFields.
 	Get(FieldNumber) RawFields
@@ -153,6 +145,8 @@ type UnknownFields interface {
 
 	// Range calls f sequentially for each unknown field that is populated.
 	// If f returns false, range stops the iteration.
+	// Assuming f always returns true and no mutations occur,
+	// the function is called exactly Len times.
 	Range(f func(FieldNumber, RawFields) bool)
 
 	// TODO: Should IsSupported be renamed as ReadOnly?
@@ -187,14 +181,9 @@ func (b RawFields) IsValid() bool {
 // ExtensionFieldTypes are the extension field types that this message instance
 // has been extended with.
 //
-// List, Len, Get, and Range are safe for concurrent use.
+// 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
-
 	// Len reports the number of field extensions.
-	//
-	// Invariant: f.Len() == len(f.List())
 	Len() int
 
 	// Register stores an ExtensionType.
@@ -219,6 +208,8 @@ type ExtensionFieldTypes interface {
 
 	// Range calls f sequentially for each registered extension field type.
 	// If f returns false, range stops the iteration.
+	// Assuming f always returns true and no mutations occur,
+	// the function is called exactly Len times.
 	Range(f func(ExtensionType) bool)
 }
 
@@ -273,14 +264,9 @@ type Vector interface {
 // is considered populated. The entry Value type is determined by the associated
 // FieldDescripto.Kind and cannot be a Map or Vector.
 //
-// List, Len, Has, Get, and Range are safe for concurrent use.
+// 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
-
 	// Len reports the number of elements in the map.
-	//
-	// Invariant: f.Len() == len(f.List())
 	Len() int
 
 	// Has reports whether an entry with the given key is in the map.
@@ -309,8 +295,9 @@ type Map interface {
 	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.
+	// Assuming f always returns true and no mutations occur,
+	// the function is called exactly Len times.
 	Range(f func(MapKey, Value) bool)
 
 	// ProtoMutable is a marker method to implement the Mutable interface.