Repository: google/starlark-go
Pull request #329 (2020-12-10 13:14:30)

starlark: add ToInt helper function for unpacking values to Go ints
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.



Patch:
diff --git a/starlark/int.go b/starlark/int.go
index 397be86..c13c8dd 100644
--- a/starlark/int.go
+++ b/starlark/int.go
@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"math"
 	"math/big"
+	"reflect"
 	"strconv"
 
 	"go.starlark.net/syntax"
@@ -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.
diff --git a/starlark/library.go b/starlark/library.go
index e9e2f94..1763824 100644
--- a/starlark/library.go
+++ b/starlark/library.go
@@ -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
diff --git a/starlark/unpack.go b/starlark/unpack.go
index 65ed08c..86f9c4a 100644
--- a/starlark/unpack.go
+++ b/starlark/unpack.go
@@ -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.
 //
@@ -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 {


Test patch:
diff --git a/starlark/eval_test.go b/starlark/eval_test.go
index 3725d17..81f8c58 100644
--- a/starlark/eval_test.go
+++ b/starlark/eval_test.go
@@ -10,6 +10,7 @@ import (
 	"math"
 	"os/exec"
 	"path/filepath"
+	"reflect"
 	"sort"
 	"strings"
 	"testing"
@@ -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():
diff --git a/starlark/testdata/builtins.star b/starlark/testdata/builtins.star
index 8dcee46..c6591b8 100644
--- a/starlark/testdata/builtins.star
+++ b/starlark/testdata/builtins.star
@@ -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))       # []