From 3f0a3703c02aaff109c1d5df37be01fee784d4fc Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Mon, 8 Apr 2024 11:28:05 -0400 Subject: [PATCH] starlark: eta-abstract the new iterators (#537) According to https://github.com/golang/go/issues/66626#issuecomment-2035188011, the convention for iterator methods is that they should return an iterator, not be an iterator. The caller thus uses this form: for _ elem := range v.Elements() { ... } not this one: for _ elem := range v.Elements { ... } This is unfortunately a breaking API change, but since go1.23 is not yet released, no-one should be using the new range syntax yet. --- starlark/iterator_test.go | 8 +++---- starlark/value.go | 50 ++++++++++++++++++++++----------------- 2 files changed, 32 insertions(+), 26 deletions(-) diff --git a/starlark/iterator_test.go b/starlark/iterator_test.go index 0ecaad47..c9104962 100644 --- a/starlark/iterator_test.go +++ b/starlark/iterator_test.go @@ -25,7 +25,7 @@ func TestTupleElements(t *testing.T) { tuple := Tuple{MakeInt(1), MakeInt(2), MakeInt(3)} var got []string - for elem := range tuple.Elements { + for elem := range tuple.Elements() { got = append(got, fmt.Sprint(elem)) if len(got) == 2 { break // skip 3 @@ -47,7 +47,7 @@ func TestListElements(t *testing.T) { list := NewList([]Value{MakeInt(1), MakeInt(2), MakeInt(3)}) var got []string - for elem := range list.Elements { + for elem := range list.Elements() { got = append(got, fmt.Sprint(elem)) if len(got) == 2 { break // skip 3 @@ -72,7 +72,7 @@ func TestSetElements(t *testing.T) { set.Insert(MakeInt(3)) var got []string - for elem := range set.Elements { + for elem := range set.Elements() { got = append(got, fmt.Sprint(elem)) if len(got) == 2 { break // skip 3 @@ -97,7 +97,7 @@ func TestDictEntries(t *testing.T) { dict.SetKey(String("three"), MakeInt(3)) var got []string - for k, v := range dict.Entries { + for k, v := range dict.Entries() { got = append(got, fmt.Sprintf("%v %v", k, v)) if len(got) == 2 { break // skip 3 diff --git a/starlark/value.go b/starlark/value.go index af16239e..f24a3c81 100644 --- a/starlark/value.go +++ b/starlark/value.go @@ -854,7 +854,7 @@ func (d *Dict) Type() string { return "dict" func (d *Dict) Freeze() { d.ht.freeze() } func (d *Dict) Truth() Bool { return d.Len() > 0 } func (d *Dict) Hash() (uint32, error) { return 0, fmt.Errorf("unhashable type: dict") } -func (d *Dict) Entries(yield func(k, v Value) bool) { d.ht.entries(yield) } +func (d *Dict) Entries() func(yield func(k, v Value) bool) { return d.ht.entries } func (x *Dict) Union(y *Dict) *Dict { z := new(Dict) @@ -962,19 +962,21 @@ func (l *List) Iterate() Iterator { return &listIterator{l: l} } -// Elements is a go1.23 iterator over the elements of the list. +// Elements returns a go1.23 iterator over the elements of the list. // // Example: // -// for elem := range list.Elements { ... } -func (l *List) Elements(yield func(Value) bool) { - if !l.frozen { - l.itercount++ - defer func() { l.itercount-- }() - } - for _, x := range l.elems { - if !yield(x) { - break +// for elem := range list.Elements() { ... } +func (l *List) Elements() func(yield func(Value) bool) { + return func(yield func(Value) bool) { + if !l.frozen { + l.itercount++ + defer func() { l.itercount-- }() + } + for _, x := range l.elems { + if !yield(x) { + break + } } } } @@ -1079,15 +1081,17 @@ func (t Tuple) Slice(start, end, step int) Value { func (t Tuple) Iterate() Iterator { return &tupleIterator{elems: t} } -// Elements is a go1.23 iterator over the elements of the tuple. +// Elements returns a go1.23 iterator over the elements of the tuple. // // (A Tuple is a slice, so it is of course directly iterable. This // method exists to provide a fast path for the [Elements] standalone // function.) -func (t Tuple) Elements(yield func(Value) bool) { - for _, x := range t { - if !yield(x) { - break +func (t Tuple) Elements() func(yield func(Value) bool) { + return func(yield func(Value) bool) { + for _, x := range t { + if !yield(x) { + break + } } } } @@ -1163,8 +1167,10 @@ func (s *Set) Truth() Bool { return s.Len() > 0 } func (s *Set) Attr(name string) (Value, error) { return builtinAttr(s, name, setMethods) } func (s *Set) AttrNames() []string { return builtinAttrNames(setMethods) } -func (s *Set) Elements(yield func(k Value) bool) { - s.ht.entries(func(k, _ Value) bool { return yield(k) }) +func (s *Set) Elements() func(yield func(k Value) bool) { + return func(yield func(k Value) bool) { + s.ht.entries(func(k, _ Value) bool { return yield(k) }) + } } func (x *Set) CompareSameType(op syntax.Token, y_ Value, depth int) (bool, error) { @@ -1617,10 +1623,10 @@ func Iterate(x Value) Iterator { func Elements(iterable Iterable) func(yield func(Value) bool) { // Use specialized push iterator if available (*List, Tuple, *Set). type hasElements interface { - Elements(yield func(k Value) bool) + Elements() func(yield func(k Value) bool) } if iterable, ok := iterable.(hasElements); ok { - return iterable.Elements + return iterable.Elements() } iter := iterable.Iterate() @@ -1648,10 +1654,10 @@ func Entries(mapping IterableMapping) func(yield func(k, v Value) bool) { // If available (e.g. *Dict), use specialized push iterator, // as it gets k and v in one shot. type hasEntries interface { - Entries(yield func(k, v Value) bool) + Entries() func(yield func(k, v Value) bool) } if mapping, ok := mapping.(hasEntries); ok { - return mapping.Entries + return mapping.Entries() } iter := mapping.Iterate()