starlark: bring floating-point into spec compliance (google#313)

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.

cmd/starlark/starlark.go

@@ -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")

go.mod

@@ -6,5 +6,5 @@ require (
 	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
+	golang.org/x/sys v0.0.0-20200803210538-64077c9b5642 // indirect
 )

go.sum

@@ -4,5 +4,5 @@ github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e h1:fY5BOSpyZCqRo5O
 github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e/go.mod h1:nSuG5e5PlCu98SY8svDHJxuZscDgtXS6KTTbou5AhLI=
 github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1 h1:q763qf9huN11kDQavWsoZXJNW3xEE4JJyHa5Q25/sd8=
 github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMnBNeIyt5eFwwo7qiLfzFZmjNmxjkiQlU=
-golang.org/x/sys v0.0.0-20200625212154-ddb9806d33ae h1:Ih9Yo4hSPImZOpfGuA4bR/ORKTAbhZo2AbWNRCnevdo=
-golang.org/x/sys v0.0.0-20200625212154-ddb9806d33ae/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20200803210538-64077c9b5642 h1:B6caxRw+hozq68X2MY7jEpZh/cr4/aHLv9xU8Kkadrw=
+golang.org/x/sys v0.0.0-20200803210538-64077c9b5642/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=

resolve/resolve.go

@@ -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
@@ -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")
 		}
@@ -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 {
@@ -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)
 

resolve/resolve_test.go

@@ -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")
@@ -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 {

resolve/testdata/resolve.star

@@ -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

starlark/eval.go

@@ -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 {
@@ -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
 			}
 		}
 
@@ -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:
@@ -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 {
@@ -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
 			}
@@ -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) {
@@ -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")
 				}
@@ -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)
@@ -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:

starlark/eval_test.go

@@ -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")

starlark/int.go

@@ -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 {