Skip to content

Commit

Permalink
starlark: add AsInt helper function for unpacking values to Go ints (g…
Browse files Browse the repository at this point in the history
…oogle#329)

Also, use it in Unpack* functions.
This is a minor breaking change: any call that assumed values
unpacked to 'int' were always in the int32 range will need to be modified
to handle possible int64 values.
  • Loading branch information
adonovan authored Dec 10, 2020
1 parent 2627fba commit e81fc95
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 10 deletions.
29 changes: 29 additions & 0 deletions starlark/eval_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"math"
"os/exec"
"path/filepath"
"reflect"
"sort"
"strings"
"testing"
Expand Down Expand Up @@ -712,6 +713,34 @@ func TestUnpackCustomUnpacker(t *testing.T) {
}
}

func TestAsInt(t *testing.T) {
for _, test := range []struct {
val starlark.Value
ptr interface{}
want string
}{
{starlark.MakeInt(42), new(int32), "42"},
{starlark.MakeInt(-1), new(int32), "-1"},
{starlark.MakeInt(1 << 40), new(int32), "1099511627776 out of range (want value in signed 32-bit range)"},
{starlark.MakeInt(-1 << 40), new(int32), "-1099511627776 out of range (want value in signed 32-bit range)"},

{starlark.MakeInt(42), new(uint16), "42"},
{starlark.MakeInt(0xffff), new(uint16), "65535"},
{starlark.MakeInt(0x10000), new(uint16), "65536 out of range (want value in unsigned 16-bit range)"},
{starlark.MakeInt(-1), new(uint16), "-1 out of range (want value in unsigned 16-bit range)"},
} {
var got string
if err := starlark.AsInt(test.val, test.ptr); err != nil {
got = err.Error()
} else {
got = fmt.Sprint(reflect.ValueOf(test.ptr).Elem().Interface())
}
if got != test.want {
t.Errorf("AsInt(%s, %T): got %q, want %q", test.val, test.ptr, got, test.want)
}
}
}

func TestDocstring(t *testing.T) {
globals, _ := starlark.ExecFile(&starlark.Thread{}, "doc.star", `
def somefunc():
Expand Down
56 changes: 56 additions & 0 deletions starlark/int.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"math"
"math/big"
"reflect"
"strconv"

"go.starlark.net/syntax"
Expand Down Expand Up @@ -334,6 +335,61 @@ func AsInt32(x Value) (int, error) {
return int(iSmall), nil
}

// AsInt sets *ptr to the value of Starlark int x, if it is exactly representable,
// otherwise it returns an error.
// The type of ptr must be one of the pointer types *int, *int8, *int16, *int32, or *int64,
// or one of their unsigned counterparts including *uintptr.
func AsInt(x Value, ptr interface{}) error {
xint, ok := x.(Int)
if !ok {
return fmt.Errorf("got %s, want int", x.Type())
}

bits := reflect.TypeOf(ptr).Elem().Size() * 8
switch ptr.(type) {
case *int, *int8, *int16, *int32, *int64:
i, ok := xint.Int64()
if !ok || bits < 64 && !(-1<<(bits-1) <= i && i < 1<<(bits-1)) {
return fmt.Errorf("%s out of range (want value in signed %d-bit range)", xint, bits)
}
switch ptr := ptr.(type) {
case *int:
*ptr = int(i)
case *int8:
*ptr = int8(i)
case *int16:
*ptr = int16(i)
case *int32:
*ptr = int32(i)
case *int64:
*ptr = int64(i)
}

case *uint, *uint8, *uint16, *uint32, *uint64, *uintptr:
i, ok := xint.Uint64()
if !ok || bits < 64 && i >= 1<<bits {
return fmt.Errorf("%s out of range (want value in unsigned %d-bit range)", xint, bits)
}
switch ptr := ptr.(type) {
case *uint:
*ptr = uint(i)
case *uint8:
*ptr = uint8(i)
case *uint16:
*ptr = uint16(i)
case *uint32:
*ptr = uint32(i)
case *uint64:
*ptr = uint64(i)
case *uintptr:
*ptr = uintptr(i)
}
default:
panic(fmt.Sprintf("invalid argument type: %T", ptr))
}
return nil
}

// NumberToInt converts a number x to an integer value.
// An int is returned unchanged, a float is truncated towards zero.
// NumberToInt reports an error for all other values.
Expand Down
1 change: 0 additions & 1 deletion starlark/library.go
Original file line number Diff line number Diff line change
Expand Up @@ -740,7 +740,6 @@ func range_(thread *Thread, b *Builtin, args Tuple, kwargs []Tuple) (Value, erro
return nil, err
}

// TODO(adonovan): analyze overflow/underflows cases for 32-bit implementations.
if len(args) == 1 {
// range(stop)
start, stop = 0, start
Expand Down
3 changes: 2 additions & 1 deletion starlark/testdata/builtins.star
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ assert.eq(list(range(10)[1:11:2]), [1, 3, 5, 7, 9])
assert.eq(list(range(10)[::-2]), [9, 7, 5, 3, 1])
assert.eq(list(range(0, 10, 2)[::2]), [0, 4, 8])
assert.eq(list(range(0, 10, 2)[::-2]), [8, 4, 0])
assert.fails(lambda: range(3000000000), "3000000000 out of range") # signed 32-bit values only
# range() is limited by the width of the Go int type (int32 or int64).
assert.fails(lambda: range(1<<64), "... out of range .want value in signed ..-bit range")
assert.eq(len(range(0x7fffffff)), 0x7fffffff) # O(1)
# Two ranges compare equal if they denote the same sequence:
assert.eq(range(0), range(2, 1, 3)) # []
Expand Down
13 changes: 5 additions & 8 deletions starlark/unpack.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ type Unpacker interface {
// supplied parameter variables. pairs is an alternating list of names
// and pointers to variables.
//
// If the variable is a bool, int, string, *List, *Dict, Callable,
// If the variable is a bool, integer, string, *List, *Dict, Callable,
// Iterable, or user-defined implementation of Value,
// UnpackArgs performs the appropriate type check.
// An int uses the AsInt32 check.
// Predeclared Go integer types uses the AsInt check.
// If the parameter name ends with "?",
// it and all following parameters are optional.
//
Expand Down Expand Up @@ -199,12 +199,9 @@ func unpackOneArg(v Value, ptr interface{}) error {
return fmt.Errorf("got %s, want bool", v.Type())
}
*ptr = bool(b)
case *int:
i, err := AsInt32(v)
if err != nil {
return err
}
*ptr = i
case *int, *int8, *int16, *int32, *int64,
*uint, *uint8, *uint16, *uint32, *uint64, *uintptr:
return AsInt(v, ptr)
case **List:
list, ok := v.(*List)
if !ok {
Expand Down

0 comments on commit e81fc95

Please sign in to comment.