pkg/eval: Handle non-positive step and overflow in the range builtin.

pkg/eval/builtin_fn_container.go

@@ -6,6 +6,7 @@ import (
 	"math"
 	"math/big"
 	"sort"
+	"strconv"
 
 	"github.com/xiaq/persistent/hashmap"
 	"src.elv.sh/pkg/eval/errs"
@@ -140,7 +141,8 @@ func makeMap(input Inputs) (vals.Map, error) {
 // ```
 //
 // Output `$low`, `$low` + `$step`, ..., proceeding as long as smaller than
-// `$high`. If not given, `$low` defaults to 0.
+// `$high` or until overflow. If not given, `$low` defaults to 0. The `$step`
+// must be positive.
 //
 // This command is [exactness-preserving](#exactness-preserving).
 //
@@ -158,8 +160,8 @@ func makeMap(input Inputs) (vals.Map, error) {
 // ▶ 5
 // ```
 //
-// When using floating-point numbers, beware that computation errors can result
+// When using floating-point numbers, beware that numerical errors can result in
-// in an unexpected number of outputs:
+// an incorrect number of outputs:
 //
 // ```elvish-transcript
 // ~> range 0.9 &step=0.3
@@ -169,7 +171,7 @@ func makeMap(input Inputs) (vals.Map, error) {
 // ▶ (num 0.8999999999999999)
 // ```
 //
-// Using exact rationals can avoid this problem:
+// Avoid this problem by using exact rationals:
 //
 // ```elvish-transcript
 // ~> range 9/10 &step=3/10
@@ -195,6 +197,28 @@ func rangeFn(fm *Frame, opts rangeOpts, args ...vals.Num) error {
 	default:
 		return ErrArgs
 	}
+	switch step := opts.Step.(type) {
+	case int:
+		if step <= 0 {
+			return errs.BadValue{
+				What: "step", Valid: "positive", Actual: strconv.Itoa(step)}
+		}
+	case *big.Int:
+		if step.Sign() <= 0 {
+			return errs.BadValue{
+				What: "step", Valid: "positive", Actual: step.String()}
+		}
+	case *big.Rat:
+		if step.Sign() <= 0 {
+			return errs.BadValue{
+				What: "step", Valid: "positive", Actual: step.String()}
+		}
+	case float64:
+		if step <= 0 {
+			return errs.BadValue{
+				What: "step", Valid: "positive", Actual: vals.ToString(step)}
+		}
+	}
 	nums := vals.UnifyNums(rawNums, vals.FixInt)
 
 	out := fm.OutputChan()
@@ -203,6 +227,10 @@ func rangeFn(fm *Frame, opts rangeOpts, args ...vals.Num) error {
 		lower, upper, step := nums[0], nums[1], nums[2]
 		for cur := lower; cur < upper; cur += step {
 			out <- vals.FromGo(cur)
+			if cur+step <= cur {
+				// Overflow
+				break
+			}
 		}
 	case []*big.Int:
 		lower, upper, step := nums[0], nums[1], nums[2]
@@ -224,8 +252,12 @@ func rangeFn(fm *Frame, opts rangeOpts, args ...vals.Num) error {
 		}
 	case []float64:
 		lower, upper, step := nums[0], nums[1], nums[2]
-		for f := lower; f < upper; f += step {
+		for cur := lower; cur < upper; cur += step {
-			out <- vals.FromGo(f)
+			out <- vals.FromGo(cur)
+			if cur+step <= cur {
+				// Overflow
+				break
+			}
 		}
 	default:
 		panic("unreachable")

pkg/eval/builtin_fn_container_test.go

@@ -4,6 +4,7 @@ import (
 	"math"
 	"math/big"
 	"testing"
+	"unsafe"
 
 	. "src.elv.sh/pkg/eval"
 	"src.elv.sh/pkg/eval/errs"
@@ -44,27 +45,51 @@ func TestMakeMap(t *testing.T) {
 	)
 }
 
+var maxInt = 1<<((unsafe.Sizeof(0)*8)-1) - 1
+var maxDenseIntInFloat = float64(1 << 53)
+
 func TestRange(t *testing.T) {
 	Test(t,
-		That(`range 3`).Puts(0, 1, 2),
+		That("range 3").Puts(0, 1, 2),
-		That(`range 1 3`).Puts(1, 2),
+		That("range 1 3").Puts(1, 2),
-		That(`range 0 10 &step=3`).Puts(0, 3, 6, 9),
+		That("range 0 10 &step=3").Puts(0, 3, 6, 9),
+		// int overflow
+		That("range &step=2 "+args(vals.ToString(maxInt-3), vals.ToString(maxInt))).
+			Puts(maxInt-3, maxInt-1),
+		// non-positive int step
+		That("range &step=0 10").
+			Throws(errs.BadValue{What: "step", Valid: "positive", Actual: "0"}),
 
-		That(`range 10_000_000_000_000_000_000 10_000_000_000_000_000_003`).
+		That("range 10_000_000_000_000_000_000 10_000_000_000_000_000_003").
 			Puts(
 				vals.ParseNum("10_000_000_000_000_000_000"),
 				vals.ParseNum("10_000_000_000_000_000_001"),
 				vals.ParseNum("10_000_000_000_000_000_002")),
-		That(`range 10_000_000_000_000_000_000 10_000_000_000_000_000_003 &step=2`).
+		That("range 10_000_000_000_000_000_000 10_000_000_000_000_000_003 &step=2").
 			Puts(
 				vals.ParseNum("10_000_000_000_000_000_000"),
 				vals.ParseNum("10_000_000_000_000_000_002")),
+		// non-positive bigint step
+		That("range &step=-"+z+" 10").
+			Throws(errs.BadValue{What: "step", Valid: "positive", Actual: "-" + z}),
 
-		That(`range 23/10`).Puts(0, 1, 2),
+		That("range 23/10").Puts(0, 1, 2),
-		That(`range 1/10 23/10`).Puts(
+		That("range 1/10 23/10").Puts(
 			big.NewRat(1, 10), big.NewRat(11, 10), big.NewRat(21, 10)),
-		That(`range 1/10 9/10 &step=3/10`).Puts(
+		That("range 1/10 9/10 &step=3/10").Puts(
 			big.NewRat(1, 10), big.NewRat(4, 10), big.NewRat(7, 10)),
+		// non-positive bigrat step
+		That("range &step=-1/2 10").
+			Throws(errs.BadValue{What: "step", Valid: "positive", Actual: "-1/2"}),
+
+		That("range 1.2").Puts(0.0, 1.0),
+		That("range &step=0.5 1 3").Puts(1.0, 1.5, 2.0, 2.5),
+		// float64 overflow
+		That("range "+args(vals.ToString(maxDenseIntInFloat-2), "+inf")).
+			Puts(maxDenseIntInFloat-2, maxDenseIntInFloat-1, maxDenseIntInFloat),
+		// non-positive float64 step
+		That("range &step=-0.5 10").
+			Throws(errs.BadValue{What: "step", Valid: "positive", Actual: "-0.5"}),
 	)
 }