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>
diff --git a/internal/impl/message.go b/internal/impl/message.go
index 2a908c2..13babe0 100644
--- a/internal/impl/message.go
+++ b/internal/impl/message.go
@@ -228,15 +228,6 @@
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 @@
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 @@
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 }
diff --git a/internal/impl/message_field.go b/internal/impl/message_field.go
index 71fb1de..1525ffd 100644
--- a/internal/impl/message_field.go
+++ b/internal/impl/message_field.go
@@ -74,13 +74,6 @@
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()
}
diff --git a/internal/impl/message_test.go b/internal/impl/message_test.go
index a789247..79bcbd5 100644
--- a/internal/impl/message_test.go
+++ b/internal/impl/message_test.go
@@ -90,7 +90,7 @@
setMap map[interface{}]pref.Value
clearMap map[interface{}]bool
rangeMap map[interface{}]pref.Value
- // TODO: List, Mutable
+ // TODO: Mutable
)
func TestScalarProto2(t *testing.T) {