Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

starlark: use Unhashable errors to indicate hash failures #165

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

starlark: use Unhashable errors to indicate hash failures #165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions starlark/eval.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -806,9 +806,12 @@ func Binary(op syntax.Token, x, y Value) (Value, error) {
}
return False, nil
case Mapping: // e.g. dict
// Ignore error from Get as we cannot distinguish true
_, found, err := y.Get(x)
if _, ok := err.(Unhashable); ok {
return nil, err
}
// Ignore other errors from Get as we cannot distinguish true
// errors (value cycle, type error) from "key not found".
_, found, _ := y.Get(x)
return Bool(found), nil
case *Set:
ok, err := y.Has(x)
Expand Down
2 changes: 1 addition & 1 deletion starlark/eval_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ func (t fib) Freeze() {}
func (t fib) String() string { return "fib" }
func (t fib) Type() string { return "fib" }
func (t fib) Truth() starlark.Bool { return true }
func (t fib) Hash() (uint32, error) { return 0, fmt.Errorf("fib is unhashable") }
func (t fib) Hash() (uint32, error) { return 0, starlark.Unhashable("fib is unhashable") }
func (t fib) Iterate() starlark.Iterator { return &fibIterator{0, 1} }

type fibIterator struct{ x, y int }
Expand Down
2 changes: 1 addition & 1 deletion starlark/library.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -936,7 +936,7 @@ func (r rangeValue) String() string {
}
func (r rangeValue) Type() string { return "range" }
func (r rangeValue) Truth() Bool { return r.len > 0 }
func (r rangeValue) Hash() (uint32, error) { return 0, fmt.Errorf("unhashable: range") }
func (r rangeValue) Hash() (uint32, error) { return 0, Unhashable("unhashable: range") }

func (x rangeValue) CompareSameType(op syntax.Token, y_ Value, depth int) (bool, error) {
y := y_.(rangeValue)
Expand Down
2 changes: 1 addition & 1 deletion starlark/testdata/builtins.star
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ assert.true(4 not in (1, 2, 3))
assert.fails(lambda: 3 in "foo", "in.*requires string as left operand")
assert.true(123 in {123: ""})
assert.true(456 not in {123:""})
assert.true([] not in {123: ""})
assert.fails(lambda: [] not in {123: ""}, "unhashable type: list")

# sorted
assert.eq(sorted([42, 123, 3]), [3, 42, 123])
Expand Down
14 changes: 10 additions & 4 deletions starlark/value.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,15 @@ type Value interface {
// Hash returns a function of x such that Equals(x, y) => Hash(x) == Hash(y).
// Hash may fail if the value's type is not hashable, or if the value
// contains a non-hashable value.
// Values are encouraged to return an error of type Unhashable in the latter case.
Hash() (uint32, error)
}

// An Unhashable error indicates that a call to Hash has failed because the Value is not hashable.
type Unhashable string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UnhashableError


func (e Unhashable) Error() string { return string(e) }

// A Comparable is a value that defines its own equivalence relation and
// perhaps ordered comparisons.
type Comparable interface {
Expand Down Expand Up @@ -524,7 +530,7 @@ func (si stringIterable) Type() string {
}
func (si stringIterable) Freeze() {} // immutable
func (si stringIterable) Truth() Bool { return True }
func (si stringIterable) Hash() (uint32, error) { return 0, fmt.Errorf("unhashable: %s", si.Type()) }
func (si stringIterable) Hash() (uint32, error) { return 0, Unhashable("unhashable: " + si.Type()) }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Helper function?

func unhashable(x Value) (uint, error) {
return 0, Unhashable(fmt.Sprintf("unhashable: " + x.Type())
}

func (si stringIterable) Iterate() Iterator { return &stringIterator{si, 0} }

type stringIterator struct {
Expand Down Expand Up @@ -675,7 +681,7 @@ func (d *Dict) String() string { return toStrin
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) Hash() (uint32, error) { return 0, Unhashable("unhashable type: dict") }

func (d *Dict) Attr(name string) (Value, error) { return builtinAttr(d, name, dictMethods) }
func (d *Dict) AttrNames() []string { return builtinAttrNames(dictMethods) }
Expand Down Expand Up @@ -746,7 +752,7 @@ func (l *List) checkMutable(verb string) error {

func (l *List) String() string { return toString(l) }
func (l *List) Type() string { return "list" }
func (l *List) Hash() (uint32, error) { return 0, fmt.Errorf("unhashable type: list") }
func (l *List) Hash() (uint32, error) { return 0, Unhashable("unhashable type: list") }
func (l *List) Truth() Bool { return l.Len() > 0 }
func (l *List) Len() int { return len(l.elems) }
func (l *List) Index(i int) Value { return l.elems[i] }
Expand Down Expand Up @@ -930,7 +936,7 @@ func (s *Set) String() string { return toString(s) }
func (s *Set) Type() string { return "set" }
func (s *Set) elems() []Value { return s.ht.keys() }
func (s *Set) Freeze() { s.ht.freeze() }
func (s *Set) Hash() (uint32, error) { return 0, fmt.Errorf("unhashable type: set") }
func (s *Set) Hash() (uint32, error) { return 0, Unhashable("unhashable type: set") }
func (s *Set) Truth() Bool { return s.Len() > 0 }

func (s *Set) Attr(name string) (Value, error) { return builtinAttr(s, name, setMethods) }
Expand Down
2 changes: 1 addition & 1 deletion starlarkstruct/module.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ var _ starlark.HasAttrs = (*Module)(nil)
func (m *Module) Attr(name string) (starlark.Value, error) { return m.Members[name], nil }
func (m *Module) AttrNames() []string { return m.Members.Keys() }
func (m *Module) Freeze() { m.Members.Freeze() }
func (m *Module) Hash() (uint32, error) { return 0, fmt.Errorf("unhashable: %s", m.Type()) }
func (m *Module) Hash() (uint32, error) { return 0, starlark.Unhashable("unhashable: " + m.Type()) }
func (m *Module) String() string { return fmt.Sprintf("<module %q>", m.Name) }
func (m *Module) Truth() starlark.Bool { return true }
func (m *Module) Type() string { return "module" }
Expand Down
6 changes: 3 additions & 3 deletions starlarkstruct/struct_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import (
"path/filepath"
"testing"

"go.starlark.net/starlark"
"go.starlark.net/resolve"
"go.starlark.net/starlark"
"go.starlark.net/starlarkstruct"
"go.starlark.net/starlarktest"
)
Expand Down Expand Up @@ -67,8 +67,8 @@ func (sym *symbol) Name() string { return sym.name }
func (sym *symbol) String() string { return sym.name }
func (sym *symbol) Type() string { return "symbol" }
func (sym *symbol) Freeze() {} // immutable
func (sym *symbol) Truth() starlark.Bool { return starlark.True }
func (sym *symbol) Hash() (uint32, error) { return 0, fmt.Errorf("unhashable: %s", sym.Type()) }
func (sym *symbol) Truth() starlark.Bool { return starlark.True }
func (sym *symbol) Hash() (uint32, error) { return 0, starlark.Unhashable("unhashable: " + sym.Type()) }

func (sym *symbol) CallInternal(thread *starlark.Thread, args starlark.Tuple, kwargs []starlark.Tuple) (starlark.Value, error) {
if len(args) > 0 {
Expand Down