Skip to content

Commit

Permalink
starlark: eta-abstract the new iterators (#537)
Browse files Browse the repository at this point in the history
According to
golang/go#66626 (comment),
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.
  • Loading branch information
adonovan committed Apr 8, 2024
1 parent e6e8e7c commit 3f0a370
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 26 deletions.
8 changes: 4 additions & 4 deletions starlark/iterator_test.go
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
50 changes: 28 additions & 22 deletions starlark/value.go
Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
}
}
}
Expand Down Expand Up @@ -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
}
}
}
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit 3f0a370

Please sign in to comment.