Range#max: beginless Integer ranges, as 4.0 answers them

internal/vm/builtins.go

@@ -5711,14 +5711,28 @@ func (vm *VM) bootstrap() {
 	})
 	vm.cRange.defineArgc("max", -1, func(vm *VM, self object.Value, args []object.Value, blk *Proc) object.Value {
 		r := self.(*object.Range)
-		if len(args) > 0 { // max(n): the n largest, descending
-			if object.IsNil(r.Hi) {
-				raise("RangeError", "cannot get the maximum of endless range")
+		if object.IsNil(r.Hi) {
+			raise("RangeError", "cannot get the maximum of endless range")
+		}
+		// ⛔ range_max asks this BEFORE looking at the n argument, and the order
+		// is observable: `(..1).max(2) { |a,b| a<=>b }` raises in MRI, where
+		// answering the n first would hand back [1, 0]. A block, or an exclusive
+		// range whose end is not Numeric, has to walk elements, and a beginless
+		// range has none to offer.
+		if blk != nil || (r.Exclusive && !isNumericValue(r.Hi)) {
+			if object.IsNil(r.Lo) {
+				raise("RangeError", "cannot get the maximum of beginless range with custom comparison method")
 			}
-			// A beginless range cannot be walked from its start, so its n largest
-			// are unobtainable (MRI raises RangeError rather than iterating).
+		}
+		if len(args) > 0 { // max(n): the n largest, descending
+			// A beginless NON-Integer range is not walkable at all: MRI's
+			// reverse_each falls through to Enumerable, whose #each raises
+			// "can't iterate from NilClass". A TypeError, not the RangeError
+			// that pre-4.0 refused every beginless range with.
 			if object.IsNil(r.Lo) {
-				raise("RangeError", "cannot get the maximum of beginless range")
+				if _, ok := toBigInt(r.Hi); !ok {
+					raise("TypeError", "can't iterate from NilClass")
+				}
 			}
 			// ⛔ An Integer range answers ARITHMETICALLY. Building the range to
 			// take n off the end costs O(size of the range) and nothing about
@@ -5740,30 +5754,38 @@ func (vm *VM) bootstrap() {
 		if blk != nil {
 			return vm.rangeExtremeByBlock(r, blk, false)
 		}
-		// The maximum comes from the endpoints without iterating: endless raises, an
-		// empty range is nil, an inclusive range is the end. An exclusive range with
-		// an Integer end is end-1; with any other end it is the last element reached
+		// The maximum comes from the endpoints without iterating: an empty range
+		// is nil, an inclusive range is the end. An exclusive range with an
+		// Integer end is end-1; with any other end it is the last element reached
 		// by #succ (a String range yields it, a Float range raises TypeError).
-		if object.IsNil(r.Hi) {
-			raise("RangeError", "cannot get the maximum of endless range")
-		}
 		if rangeEmpty(vm, r) {
 			return object.NilV
 		}
 		if r.Exclusive {
-			// An Integer (or Bignum) end excludes by yielding its predecessor — but
-			// only for a bounded range: excluding an Integer end still requires an
-			// Integer begin (MRI 3.x), so a beginless exclusive Integer range raises.
+			// An Integer (or Bignum) end excludes by yielding its predecessor.
+			//
+			// ⛔ Until 4.0 this also demanded an Integer BEGIN, so `(...1).max`
+			// raised "cannot exclude end value with non Integer begin value" — a
+			// nil begin counting as "non Integer". range_max now guards that
+			// check with `!NIL_P(b) &&`, so a beginless range skips it and the
+			// answer is the predecessor of the end: MRI 4.0.7 gives `(...1).max`
+			// as 0, and `(...-2**63).max` as -9223372036854775809.
 			switch r.Hi.(type) {
 			case object.Integer, *object.Bignum:
-				if object.IsNil(r.Lo) {
-					raise("TypeError", "cannot exclude end value with non Integer begin value")
+				if !object.IsNil(r.Lo) {
+					switch r.Lo.(type) {
+					case object.Integer, *object.Bignum:
+					default:
+						raise("TypeError", "cannot exclude end value with non Integer begin value")
+					}
 				}
 				return vm.send(r.Hi, "-", []object.Value{object.IntValue(1)}, nil)
 			}
-			// A numeric (Float/Rational) end, or a beginless range, has no
-			// predecessor reachable by #succ.
-			if object.IsNil(r.Lo) || isNumericValue(r.Hi) {
+			// A numeric (Float/Rational) end has no predecessor reachable by
+			// #succ. A beginless range never arrives here: a non-Numeric end took
+			// the walk-the-elements guard at the top of the method, and a Numeric
+			// one is caught by this raise.
+			if isNumericValue(r.Hi) {
 				raise("TypeError", "cannot exclude non Integer end value")
 			}
 			// A non-empty iterable range (e.g. String): the last element before
@@ -11991,49 +12013,97 @@ func (vm *VM) rangeFirstN(r *object.Range, n int) []object.Value {
 
 // rangeInts extracts integer endpoints. ok is false when either endpoint is not
 // an Integer (string/float ranges are not iterable in this phase).
-// integerRangeMaxN answers Range#max(n) for a bounded Integer range without
-// building the range, and reports false for anything else (a String range, a
-// Float end, a Bignum endpoint) so the caller keeps its existing path.
+// integerRangeMaxN answers Range#max(n) for an Integer range without building
+// the range, and reports false for anything else (a String range, a Float end)
+// so the caller keeps its existing path.
 //
 // ⛔ THE COUNT IS CLAMPED WITHOUT COMPUTING THE SIZE. `top - lo + 1` overflows
 // int64 for a range like (-2**62..2**62), and an overflowed size clamps n to a
 // negative number — a wrong answer on exactly the inputs this function exists
-// to make cheap. The span is taken in uint64, where it is exact for every
-// int64 pair with top >= lo, and compared against n instead.
+// to make cheap. The arithmetic is done in big.Int, where no pair of endpoints
+// overflows, and only the clamped count comes back to int64.
+//
+// Beginless ranges answer here too, since Ruby 4.0: range.c's range_max reaches
+// its n-argument branch with `c = NIL_P(b) ? -1 : …`, so a nil begin is simply
+// "below the end" rather than a refusal, and reverse_each walks
+// `(NIL_P(beg) || RB_INTEGER_TYPE_P(beg)) && RB_INTEGER_TYPE_P(end)` downward
+// from the end with nothing to stop it but n. Measured against MRI 4.0.7:
+// `(..1).max(3)` is `[1, 0, -1]`.
 func integerRangeMaxN(r *object.Range, n int64) (object.Value, bool) {
-	lo, hi, ok := rangeInts(r)
+	lo, hi, ok := rangeBigEnds(r)
 	if !ok {
 		return nil, false
 	}
 	if n < 0 {
-		raise("ArgumentError", "negative array size")
+		raise("ArgumentError", "negative array size (or size too big)")
 	}
-	top := hi
+	top := new(big.Int).Set(hi)
 	if r.Exclusive {
-		// An exclusive Integer end excludes by yielding its predecessor. If hi
-		// is the smallest int64 there is no predecessor, and the range is empty
-		// anyway for any lo.
-		if hi == math.MinInt64 {
+		// An exclusive Integer end excludes by yielding its predecessor. In
+		// big.Int that is unconditional: the int64 floor needed a special case,
+		// `(...-2**63).max` is -9223372036854775809 in MRI, and it is a Bignum.
+		top.Sub(top, big.NewInt(1))
+	}
+	count := n
+	if lo != nil {
+		if top.Cmp(lo) < 0 {
 			return object.NewArrayFromSlice(nil), true
 		}
-		top--
-	}
-	if top < lo {
-		return object.NewArrayFromSlice(nil), true
+		// span is the number of steps from lo to top: exact, and only consulted
+		// when it is small enough to shorten the answer.
+		if span := new(big.Int).Sub(top, lo); span.IsInt64() && span.Int64() < n {
+			count = span.Int64() + 1
+		}
 	}
-	span := uint64(top) - uint64(lo) // exact: the number of steps from lo to top
-	count := n
-	if uint64(n) > span {
-		// n is int64 and n > span, so span+1 cannot overflow int64 here.
-		count = int64(span) + 1
+	// ⛔ A beginless range has nothing to clamp against, so n alone decides the
+	// allocation and `(..1).max(2**40)` is a request for 8 TiB of slice. MRI
+	// bounds it in rb_ary_new2, which refuses a length over ARY_MAX_SIZE with
+	// ArgumentError; refuse here for the same reason, with the same class, so
+	// an untrusted n is a Ruby exception and not an OOM kill.
+	if count > maxRangeMaxN {
+		raise("ArgumentError", "negative array size (or size too big)")
 	}
 	out := make([]object.Value, count)
+	cur := new(big.Int).Set(top)
+	one := big.NewInt(1)
 	for i := int64(0); i < count; i++ {
-		out[i] = object.IntValue(top - i)
+		out[i] = object.NormInt(new(big.Int).Set(cur))
+		cur.Sub(cur, one)
 	}
 	return object.NewArrayFromSlice(out), true
 }
 
+// maxRangeMaxN caps how many elements Range#max(n) will materialise. MRI's own
+// ceiling is ARY_MAX_SIZE (LONG_MAX/sizeof(VALUE)), which no machine can
+// allocate either; this is the point at which rbgo stops trying and raises the
+// class MRI raises.
+const maxRangeMaxN = 1 << 27
+
+// rangeBigEnds returns r's Integer endpoints as big.Ints, with lo nil for a
+// beginless range. ok is false if either present endpoint is not an Integer.
+func rangeBigEnds(r *object.Range) (lo, hi *big.Int, ok bool) {
+	if hi, ok = toBigInt(r.Hi); !ok {
+		return nil, nil, false
+	}
+	if object.IsNil(r.Lo) {
+		return nil, hi, true
+	}
+	if lo, ok = toBigInt(r.Lo); !ok {
+		return nil, nil, false
+	}
+	return lo, hi, true
+}
+
+func toBigInt(v object.Value) (*big.Int, bool) {
+	switch n := v.(type) {
+	case object.Integer:
+		return big.NewInt(int64(n)), true
+	case *object.Bignum:
+		return new(big.Int).Set(n.I), true
+	}
+	return nil, false
+}
+
 func rangeInts(r *object.Range) (lo, hi int64, ok bool) {
 	li, lok := r.Lo.(object.Integer)
 	hi2, hok := r.Hi.(object.Integer)

internal/vm/range_conformance_test.go

@@ -259,8 +259,20 @@ end
 
 		{"max_endless", `(1..).max`, "RangeError"},
 		{"max_n_endless", `(1..).max(2)`, "RangeError"},
-		{"max_n_beginless", `(..1.0).max(2)`, "RangeError"},
-		{"max_excl_beginless_int", `(...1).max`, "TypeError"},
+		// ⛔ These two rows pinned the PRE-4.0 refusals, and ruby/spec marks both
+		// `ruby_version_is ""..."4.0"`. They are not relaxed here, they are
+		// MOVED to what 4.0 refuses instead, so the file still asserts a
+		// refusal for each shape:
+		//
+		//   (..1.0).max(2)  RangeError "beginless"  ->  TypeError, not walkable
+		//   (...1).max      TypeError "non Integer begin"  ->  answers 0
+		//
+		// The second shape has no refusal left at all, so its row names the one
+		// that took its place: an exclusive beginless end that is not Numeric,
+		// which still cannot be walked. `(...1).max == 0` is asserted in
+		// range_max_beginless_test.go, where the answer belongs.
+		{"max_n_beginless_non_integer", `(..1.0).max(2)`, "TypeError"},
+		{"max_excl_beginless_non_numeric", `(..."f").max`, "RangeError"},
 		{"max_excl_float", `(1.5...3.5).max`, "TypeError"},
 		{"max_block_beginless", `(..1).max {|a, b| a }`, "RangeError"},
 

internal/vm/range_max_arithmetic_test.go

@@ -42,11 +42,14 @@ func TestRangeMaxNIsArithmeticNotMaterialised(t *testing.T) {
 		{"a range whose size overflows int64", `p(((-(2**62))..(2**62)).max(2))`,
 			"[4611686018427387904, 4611686018427387903]"},
 
-		// ⛔ AN EXCLUSIVE END AT THE int64 FLOOR has no predecessor to step down
-		// to, so `top--` would underflow. The range is empty for every possible
-		// begin, and the coverage gate is what showed this guard was never
-		// driven -- reachable from Ruby, since -2**63 is an Integer and not a
-		// Bignum.
+		// ⛔ AN EXCLUSIVE END AT THE int64 FLOOR once had no predecessor to step
+		// down to: `top--` underflowed, and the guard that caught it was found
+		// by the coverage gate rather than by anyone's imagination. The
+		// arithmetic is in big.Int now, so -2**63 - 1 simply exists and the
+		// special case is gone -- but the case is kept, because what it asserts
+		// is the ANSWER, not the guard. With a begin of 0 the range is empty
+		// whatever the end's predecessor is; `(...-2**63).max` below is the same
+		// predecessor with nothing to make it empty.
 		{"an exclusive end at the int64 floor", `p((0...(-2**63)).max(2))`, "[]"},
 
 		// Controls: the paths this must NOT have taken over.

internal/vm/range_max_beginless_test.go

@@ -0,0 +1,140 @@
+// Copyright (c) the go-embedded-ruby/ruby authors
+//
+// SPDX-License-Identifier: BSD-3-Clause
+
+package vm
+
+import "testing"
+
+// Ruby 4.0 teaches Range#max to answer for beginless Integer ranges. range.c's
+// range_max reaches its arithmetic with `c = NIL_P(b) ? -1 : OPTIMIZED_CMP(b, e)`
+// -- a nil begin is simply "below the end" -- and guards the Integer-begin
+// demand with `!NIL_P(b) &&`, so a beginless range skips it entirely. The n
+// argument goes through reverse_each, which walks
+// `(NIL_P(beg) || RB_INTEGER_TYPE_P(beg)) && RB_INTEGER_TYPE_P(end)` downward
+// from the end with nothing but n to stop it.
+//
+// Every expectation below was taken from MRI 4.0.7 on this host, through
+// rubydiff, not from reading range.c. The five ruby/spec examples behind
+// `ruby_version_is "4.0"` in core/range/max_spec.rb are the first five rows.
+func TestRangeMaxOnBeginlessRanges(t *testing.T) {
+	for _, tc := range []struct{ name, src, want string }{
+		// The five spec examples.
+		{"exclusive beginless Integer", `p((...1).max)`, "0"},
+		{"n maxima of a beginless range", `p((..1).max(2))`, "[1, 0]"},
+		{"n maxima of an exclusive beginless range", `p((...1).max(2))`, "[0, -1]"},
+		{"n maxima of a very large Integer range", `p((0...2**64).max(2))`,
+			"[18446744073709551615, 18446744073709551614]"},
+		{"a beginless non-Integer range cannot be walked",
+			`begin; (..1.0).max(2); rescue TypeError => e; p e.message; end`,
+			`"can't iterate from NilClass"`},
+
+		// The same refusal for a String end, which is the other half of that
+		// example and reaches it by a different route: not Numeric, so not an
+		// Integer either.
+		{"a beginless String range cannot be walked",
+			`begin; (.."f").max(2); rescue TypeError => e; p e.message; end`,
+			`"can't iterate from NilClass"`},
+
+		// ⛔ The predecessor of an exclusive end is taken in big.Int, so the
+		// int64 floor is not a special case: MRI answers -9223372036854775809,
+		// a Bignum, and so must this.
+		{"an exclusive beginless end at the int64 floor", `p((...(-2**63)).max)`,
+			"-9223372036854775809"},
+		{"n maxima below the int64 floor", `p((..(-2**63)).max(2))`,
+			"[-9223372036854775808, -9223372036854775809]"},
+		{"n maxima below an exclusive int64 floor", `p((...(-2**63)).max(2))`,
+			"[-9223372036854775809, -9223372036854775810]"},
+
+		// Bignum endpoints on a bounded range, which the int64 helper used to
+		// decline and hand to the materialising path -- where a 2**64-element
+		// range is not an answer anyone receives.
+		{"a Bignum end on a bounded range", `p((0..2**64).max(2))`,
+			"[18446744073709551616, 18446744073709551615]"},
+		{"a Bignum end on a beginless range", `p((..(2**64)).max(2))`,
+			"[18446744073709551616, 18446744073709551615]"},
+
+		// n of zero is an empty array, not a refusal: MRI builds an array of
+		// capacity 0 and reverse_each stops immediately.
+		{"zero maxima of a beginless range", `p((..1).max(0))`, "[]"},
+		{"zero maxima of an exclusive beginless range", `p((...1).max(0))`, "[]"},
+
+		// A Float n is converted, as NUM2LONG does.
+		{"a Float count on a beginless range", `p((..1).max(2.7))`, "[1, 0]"},
+
+		// ⛔ THE ORDER OF THE GUARDS IS OBSERVABLE. range_max asks for a block
+		// BEFORE it looks at the n argument, so a beginless range with both
+		// raises rather than answering arithmetically. Answering first would
+		// return [1, 0] here and look perfectly reasonable.
+		{"a block beats the n argument on a beginless range",
+			`begin; (..1).max(2) { |a, b| a <=> b }; rescue RangeError => e; p e.message; end`,
+			`"cannot get the maximum of beginless range with custom comparison method"`},
+		{"a block alone on a beginless range",
+			`begin; (..1).max { |a, b| a <=> b }; rescue RangeError => e; p e.message; end`,
+			`"cannot get the maximum of beginless range with custom comparison method"`},
+
+		// ⛔ An exclusive range whose end is not Numeric takes the same
+		// walk-the-elements route as a block does, so a beginless one raises
+		// the SAME RangeError -- not the TypeError about excluding a non
+		// Integer end, which is what rbgo used to say here.
+		{"an exclusive beginless non-Numeric end",
+			`begin; (..."f").max; rescue RangeError => e; p e.message; end`,
+			`"cannot get the maximum of beginless range with custom comparison method"`},
+
+		// ⛔ A non-Integer BEGIN with an Integer end declines the arithmetic and
+		// falls to the materialising path, which cannot start from a Float.
+		// Found by the coverage gate: rangeBigEnds' second refusal was the only
+		// uncovered line in this change, and it is reachable from Ruby.
+		{"a Float begin with an Integer end",
+			`begin; (1.0..3).max(2); rescue TypeError => e; p e.message; end`,
+			`"can't iterate from Float"`},
+
+		// Controls: what must NOT have moved.
+		{"an endless range still refuses",
+			`begin; (1..).max; rescue RangeError => e; p e.message; end`,
+			`"cannot get the maximum of endless range"`},
+		{"an endless range with n still refuses",
+			`begin; (1..).max(2); rescue RangeError => e; p e.message; end`,
+			`"cannot get the maximum of endless range"`},
+		{"a beginless inclusive range is still its end", `p((..1).max)`, "1"},
+		{"a beginless Float range is still its end", `p((..1.0).max)`, "1.0"},
+		{"an exclusive Float end still refuses",
+			`begin; (...1.0).max; rescue TypeError => e; p e.message; end`,
+			`"cannot exclude non Integer end value"`},
+		{"an exclusive Rational end still refuses",
+			`begin; (...Rational(1)).max; rescue TypeError => e; p e.message; end`,
+			`"cannot exclude non Integer end value"`},
+		{"a non-Integer begin still refuses an excluded end",
+			`begin; (1.0...2).max; rescue TypeError => e; p e.message; end`,
+			`"cannot exclude end value with non Integer begin value"`},
+		{"a beginless minimum is still unobtainable",
+			`begin; (..1).min; rescue RangeError => e; p e.message; end`,
+			`"cannot get the minimum of beginless range"`},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := runSrc(t, tc.src); got != tc.want {
+				t.Errorf("%s\n got %q\nwant %q", tc.src, got, tc.want)
+			}
+		})
+	}
+}
+
+// ⛔ A beginless range has no lower endpoint to clamp n against, so n alone
+// decides the allocation: `(..1).max(2**40)` is a request for 8 TiB of slice,
+// from one short line of untrusted Ruby. MRI bounds it inside rb_ary_new2,
+// which refuses a length above ARY_MAX_SIZE with ArgumentError; rbgo refuses
+// with the same class so the answer is an exception the program can rescue and
+// not a process the kernel kills.
+//
+// The control matters as much as the refusal: a cap that also refused ordinary
+// counts would pass this test while breaking every real use.
+func TestRangeMaxNOnABeginlessRangeRefusesAnUnboundedAllocation(t *testing.T) {
+	const bomb = `begin; (..1).max(2**40); rescue ArgumentError => e; p e.class; end`
+	if got := runSrc(t, bomb); got != "ArgumentError" {
+		t.Errorf("(..1).max(2**40) = %q, want ArgumentError", got)
+	}
+	const ok = `p((..1).max(1000).length)`
+	if got := runSrc(t, ok); got != "1000" {
+		t.Errorf("(..1).max(1000).length = %q, want 1000 -- the cap refuses an ordinary count", got)
+	}
+}