Skip to content

Commit

Permalink
starlark: bring floating-point into spec compliance
Browse files Browse the repository at this point in the history
This change makes go.starlark.net's floating-point implementation
match the proposed spec (see bazelbuild/starlark#119),
and thus much more closely match the behavior of the Java implementation.

The major changes are:
- Float values are totally ordered; NaN compares greater than +Inf.
- The string form of a finite float value always contains an exponent
  or a decimal point, so they are self-evidently not int values.
- Operations that would cause a large integer to become rounded to
  an infinite float are now an error.

The resolve.AllowFloat boolean, and the corresponding -float command-line
flag, now have no effect. Floating point support is always enabled.

Change-Id: Ia692f765f575cf0d13438906e70a3c29dac2ae59
  • Loading branch information
adonovan committed Nov 10, 2020
1 parent dff0ae5 commit 6f16b5c
Show file tree
Hide file tree
Showing 20 changed files with 1,467 additions and 398 deletions.
2 changes: 1 addition & 1 deletion cmd/starlark/starlark.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func init() {
flag.BoolVar(&compile.Disassemble, "disassemble", compile.Disassemble, "show disassembly during compilation of each function")

// non-standard dialect flags
flag.BoolVar(&resolve.AllowFloat, "float", resolve.AllowFloat, "allow floating-point numbers")
flag.BoolVar(&resolve.AllowFloat, "float", resolve.AllowFloat, "obsolete; no effect")
flag.BoolVar(&resolve.AllowSet, "set", resolve.AllowSet, "allow set data type")
flag.BoolVar(&resolve.AllowLambda, "lambda", resolve.AllowLambda, "allow lambda expressions")
flag.BoolVar(&resolve.AllowNestedDef, "nesteddef", resolve.AllowNestedDef, "allow nested def statements")
Expand Down
31 changes: 30 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,37 @@ module go.starlark.net
go 1.13

require (
github.com/NebulousLabs/go-upnp v0.0.0-20181203152547-b32978b8ccbf // indirect
github.com/aead/chacha20 v0.0.0-20180709150244-8b13a72661da // indirect
github.com/btcsuite/btcwallet v0.11.0 // indirect
github.com/btcsuite/fastsha256 v0.0.0-20160815193821-637e65642941 // indirect
github.com/btcsuite/golangcrypto v0.0.0-20150304025918-53f62d9b43e8 // indirect
github.com/chzyer/logex v1.1.10 // indirect
github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e
github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1 // indirect
golang.org/x/sys v0.0.0-20200625212154-ddb9806d33ae // indirect
github.com/go-errors/errors v1.1.1 // indirect
github.com/google/pprof v0.0.0-20201023163331-3e6fc7fc9c4c // indirect
github.com/grpc-ecosystem/grpc-gateway v1.16.0 // indirect
github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639 // indirect
github.com/jackpal/gateway v1.0.6 // indirect
github.com/kkdai/bstream v1.0.0 // indirect
github.com/lightninglabs/gozmq v0.0.0-20191113021534-d20a764486bf // indirect
github.com/lightninglabs/neutrino v0.11.0 // indirect
github.com/lightningnetwork/lnd v0.0.2 // indirect
github.com/ltcsuite/ltcd v0.20.1-beta // indirect
github.com/miekg/dns v1.1.35 // indirect
github.com/nbutton23/zxcvbn-go v0.0.0-20180912185939-ae427f1e4c1d // indirect
github.com/roasbeef/btcd v0.0.0-20180418012700-a03db407e40d // indirect
github.com/roasbeef/btcrpcclient v0.0.0-20170622074026-d0f4db8b4dad // indirect
github.com/roasbeef/btcutil v0.0.0-20180406014609-dfb640c57141 // indirect
github.com/tv42/zbase32 v0.0.0-20190604154422-aacc64a8f915 // indirect
gitlab.com/NebulousLabs/go-upnp v0.0.0-20181011194642-3a71999ed0d3 // indirect
golang.org/x/net/http2/h2demo v0.0.0-20201031054903-ff519b6c9102 // indirect
golang.org/x/tools/gopls v0.5.2 // indirect
golang.org/x/vgo v0.0.0-20180912184537-9d567625acf4 // indirect
google.golang.org/grpc v1.33.1 // indirect
google.golang.org/grpc/examples v0.0.0-20201030225255-4e179b8d3ec4 // indirect
gopkg.in/errgo.v1 v1.0.1 // indirect
gopkg.in/macaroon-bakery.v2 v2.2.0 // indirect
gopl.io v0.0.0-20200323155855-65c318dde95e // indirect
)
685 changes: 685 additions & 0 deletions go.sum

Large diffs are not rendered by default.

11 changes: 1 addition & 10 deletions resolve/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ const doesnt = "this Starlark dialect does not "
var (
AllowNestedDef = false // allow def statements within function bodies
AllowLambda = false // allow lambda expressions
AllowFloat = false // allow floating point literals, the 'float' built-in, and x / y
AllowFloat = false // obsolete; no effect
AllowSet = false // allow the 'set' built-in
AllowGlobalReassign = false // allow reassignment to top-level names; also, allow if/for/while at top-level
AllowRecursion = false // allow while statements and recursive functions
Expand Down Expand Up @@ -418,9 +418,6 @@ func (r *resolver) useToplevel(use use) (bind *Binding) {
r.predeclared[id.Name] = bind // save it
} else if r.isUniversal(id.Name) {
// use of universal name
if !AllowFloat && id.Name == "float" {
r.errorf(id.NamePos, doesnt+"support floating point")
}
if !AllowSet && id.Name == "set" {
r.errorf(id.NamePos, doesnt+"support sets")
}
Expand Down Expand Up @@ -636,9 +633,6 @@ func (r *resolver) expr(e syntax.Expr) {
r.use(e)

case *syntax.Literal:
if !AllowFloat && e.Token == syntax.FLOAT {
r.errorf(e.TokenPos, doesnt+"support floating point")
}

case *syntax.ListExpr:
for _, x := range e.List {
Expand Down Expand Up @@ -713,9 +707,6 @@ func (r *resolver) expr(e syntax.Expr) {
r.expr(e.X)

case *syntax.BinaryExpr:
if !AllowFloat && e.Op == syntax.SLASH {
r.errorf(e.OpPos, doesnt+"support floating point (use //)")
}
r.expr(e.X)
r.expr(e.Y)

Expand Down
3 changes: 1 addition & 2 deletions resolve/resolve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
)

func setOptions(src string) {
resolve.AllowFloat = option(src, "float")
resolve.AllowGlobalReassign = option(src, "globalreassign")
resolve.AllowLambda = option(src, "lambda")
resolve.AllowNestedDef = option(src, "nesteddef")
Expand All @@ -38,7 +37,7 @@ func TestResolve(t *testing.T) {
continue
}

// A chunk may set options by containing e.g. "option:float".
// A chunk may set options by containing e.g. "option:nesteddef".
setOptions(chunk.Source)

if err := resolve.File(f, isPredeclared, isUniversal); err != nil {
Expand Down
7 changes: 1 addition & 6 deletions resolve/testdata/resolve.star
Original file line number Diff line number Diff line change
Expand Up @@ -307,12 +307,7 @@ def h(kwargs, a, **kwargs): pass ### "duplicate parameter: kwargs"
def i(*x, **x): pass ### "duplicate parameter: x"

---
# No floating point
a = float("3.141") ### `dialect does not support floating point`
b = 1 / 2 ### `dialect does not support floating point \(use //\)`
c = 3.141 ### `dialect does not support floating point`
---
# Floating point support (option:float)
# Floating-point support is now standard.
a = float("3.141")
b = 1 / 2
c = 3.141
Expand Down
106 changes: 71 additions & 35 deletions starlark/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -713,14 +713,22 @@ func Binary(op syntax.Token, x, y Value) (Value, error) {
case Int:
return x.Add(y), nil
case Float:
return x.Float() + y, nil
xf, err := x.finiteFloat()
if err != nil {
return nil, err
}
return xf + y, nil
}
case Float:
switch y := y.(type) {
case Float:
return x + y, nil
case Int:
return x + y.Float(), nil
yf, err := y.finiteFloat()
if err != nil {
return nil, err
}
return x + yf, nil
}
case *List:
if y, ok := y.(*List); ok {
Expand All @@ -745,14 +753,22 @@ func Binary(op syntax.Token, x, y Value) (Value, error) {
case Int:
return x.Sub(y), nil
case Float:
return x.Float() - y, nil
xf, err := x.finiteFloat()
if err != nil {
return nil, err
}
return xf - y, nil
}
case Float:
switch y := y.(type) {
case Float:
return x - y, nil
case Int:
return x - y.Float(), nil
yf, err := y.finiteFloat()
if err != nil {
return nil, err
}
return x - yf, nil
}
}

Expand All @@ -763,7 +779,11 @@ func Binary(op syntax.Token, x, y Value) (Value, error) {
case Int:
return x.Mul(y), nil
case Float:
return x.Float() * y, nil
xf, err := x.finiteFloat()
if err != nil {
return nil, err
}
return xf * y, nil
case String:
return stringRepeat(y, x)
case *List:
Expand All @@ -780,7 +800,11 @@ func Binary(op syntax.Token, x, y Value) (Value, error) {
case Float:
return x * y, nil
case Int:
return x * y.Float(), nil
yf, err := y.finiteFloat()
if err != nil {
return nil, err
}
return x * yf, nil
}
case String:
if y, ok := y.(Int); ok {
Expand All @@ -804,30 +828,40 @@ func Binary(op syntax.Token, x, y Value) (Value, error) {
case syntax.SLASH:
switch x := x.(type) {
case Int:
xf, err := x.finiteFloat()
if err != nil {
return nil, err
}
switch y := y.(type) {
case Int:
yf := y.Float()
yf, err := y.finiteFloat()
if err != nil {
return nil, err
}
if yf == 0.0 {
return nil, fmt.Errorf("real division by zero")
return nil, fmt.Errorf("floating-point division by zero")
}
return x.Float() / yf, nil
return xf / yf, nil
case Float:
if y == 0.0 {
return nil, fmt.Errorf("real division by zero")
return nil, fmt.Errorf("floating-point division by zero")
}
return x.Float() / y, nil
return xf / y, nil
}
case Float:
switch y := y.(type) {
case Float:
if y == 0.0 {
return nil, fmt.Errorf("real division by zero")
return nil, fmt.Errorf("floating-point division by zero")
}
return x / y, nil
case Int:
yf := y.Float()
yf, err := y.finiteFloat()
if err != nil {
return nil, err
}
if yf == 0.0 {
return nil, fmt.Errorf("real division by zero")
return nil, fmt.Errorf("floating-point division by zero")
}
return x / yf, nil
}
Expand All @@ -843,10 +877,14 @@ func Binary(op syntax.Token, x, y Value) (Value, error) {
}
return x.Div(y), nil
case Float:
xf, err := x.finiteFloat()
if err != nil {
return nil, err
}
if y == 0.0 {
return nil, fmt.Errorf("floored division by zero")
}
return floor((x.Float() / y)), nil
return floor(xf / y), nil
}
case Float:
switch y := y.(type) {
Expand All @@ -856,7 +894,10 @@ func Binary(op syntax.Token, x, y Value) (Value, error) {
}
return floor(x / y), nil
case Int:
yf := y.Float()
yf, err := y.finiteFloat()
if err != nil {
return nil, err
}
if yf == 0.0 {
return nil, fmt.Errorf("floored division by zero")
}
Expand All @@ -874,23 +915,31 @@ func Binary(op syntax.Token, x, y Value) (Value, error) {
}
return x.Mod(y), nil
case Float:
xf, err := x.finiteFloat()
if err != nil {
return nil, err
}
if y == 0 {
return nil, fmt.Errorf("float modulo by zero")
return nil, fmt.Errorf("floating-point modulo by zero")
}
return x.Float().Mod(y), nil
return xf.Mod(y), nil
}
case Float:
switch y := y.(type) {
case Float:
if y == 0.0 {
return nil, fmt.Errorf("float modulo by zero")
return nil, fmt.Errorf("floating-point modulo by zero")
}
return x.Mod(y), nil
case Int:
if y.Sign() == 0 {
return nil, fmt.Errorf("float modulo by zero")
return nil, fmt.Errorf("floating-point modulo by zero")
}
return x.Mod(y.Float()), nil
yf, err := y.finiteFloat()
if err != nil {
return nil, err
}
return x.Mod(yf), nil
}
case String:
return interpolate(string(x), y)
Expand Down Expand Up @@ -1497,20 +1546,7 @@ func interpolate(format string, x Value) (Value, error) {
if !ok {
return nil, fmt.Errorf("%%%c format requires float, not %s", c, arg.Type())
}
switch c {
case 'e':
fmt.Fprintf(buf, "%e", f)
case 'f':
fmt.Fprintf(buf, "%f", f)
case 'g':
fmt.Fprintf(buf, "%g", f)
case 'E':
fmt.Fprintf(buf, "%E", f)
case 'F':
fmt.Fprintf(buf, "%F", f)
case 'G':
fmt.Fprintf(buf, "%G", f)
}
Float(f).format(buf, c)
case 'c':
switch arg := arg.(type) {
case Int:
Expand Down
1 change: 0 additions & 1 deletion starlark/eval_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (

// A test may enable non-standard options by containing (e.g.) "option:recursion".
func setOptions(src string) {
resolve.AllowFloat = option(src, "float")
resolve.AllowGlobalReassign = option(src, "globalreassign")
resolve.LoadBindsGlobally = option(src, "loadbindsglobally")
resolve.AllowLambda = option(src, "lambda")
Expand Down
10 changes: 10 additions & 0 deletions starlark/int.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,16 @@ func (i Int) Float() Float {
return Float(iSmall)
}

// finiteFloat returns the finite float value nearest i,
// or an error if the magnitude is too large.
func (i Int) finiteFloat() (Float, error) {
f := i.Float()
if math.IsInf(float64(f), 0) {
return 0, fmt.Errorf("int too large to convert to float")
}
return f, nil
}

func (x Int) Sign() int {
xSmall, xBig := x.get()
if xBig != nil {
Expand Down
Loading

0 comments on commit 6f16b5c

Please sign in to comment.