fix(vm): arity / type-coerce sweep for each_slice/each_cons/chunk_while by linyiru · Pull Request #330 · linyiru/rubyrs

linyiru · 2026-06-01T18:44:53Z

Summary

Cross-receiver wrong-arity / non-Int-arg sweep for the 9 dispatch sites added in PRs #311 / #312 / #316 / #323:

each_slice / each_cons (1-arg, expected Int): Array / Hash / Range × {block, no-block}
chunk_while (0-arg): Array / Hash / Range × block

Previously all 9 fell through to NoMethodError for any shape outside [Value::Int(n)], contradicting respond_to? which returns true unconditionally — same lockstep contract violation caught in PRs #308 / #316 / #323.

Implementation:

New Vm::arity_error_arg1_int(name, args) → Trap helper in gc.rs (alongside Vm::trap).
6 catch-all arms in iter.rs (block forms × 3 receivers × 2 methods) after each [Value::Int(n)] arm.
6 catch-all arms in array.rs / hash.rs / range.rs (no-block forms).
3 inline chunk_while wrong-arity arms.

CRuby parity (byte-identical class + message):

shape	CRuby surface
0 args / 2+ args	`ArgumentError "wrong number of arguments (given N, expected 1)"`
1 non-Int arg	`TypeError "no implicit conversion of <T> into Integer"`
1 nil arg	`TypeError "no implicit conversion from nil to integer"` (different wording!)
chunk_while + any args	`ArgumentError "given N, expected 0"`

Known divergence (out of scope): Float arg. CRuby coerces (2.5 → 2); rubyrs raises TypeError. A follow-up PR could add Float → Int truncation in the helper.

+1 spec / +12 examples / 0 skipped. Files 151 → 152, examples 954 → 966.

Test plan

All 8 byte-identical CRuby probes (ArgumentError class+wording, TypeError class+wording, Nil/Sym/String distinct wording)
Existing success path unaffected (each_slice(2) { } / .to_a still works)
cargo test -p rubyrs --test ruby_spec passes (12 new examples)
SPEC_STATUS regen passes
No new clippy warnings (2 pre-existing in dispatch.rs)

github-actions · 2026-06-01T18:45:17Z

gapscan PR diff

This PR doesn't touch any file that affects gapscan classification — SUPPORTED_PRISM_NODES / RIDES_ALONG_PRISM_NODES are unchanged. Skipped the base build and the 10 per-target scans.

See docs/gap-reports/ for the dataset and methodology.

Copilot

Pull request overview

This PR fixes dispatch “fall-through to NoMethodError” behavior for each_slice / each_cons (expects exactly 1 Integer arg) and chunk_while (expects 0 args) across Array, Hash, and Range, bringing arity/type error surfaces back in sync with respond_to? and closer to CRuby’s ArgumentError / TypeError class+message wording.

Changes:

Introduces Vm::arity_error_arg1_int helper to centralize CRuby-shaped wrong-arity vs non-Integer argument errors for 1-arg slice/cons methods.
Adds catch-all match arms at the relevant dispatch sites (block + no-block forms) to return the correct traps instead of falling through to NoMethodError.
Adds a new Ruby spec file covering cross-receiver arity/type guard behavior; updates docs/SPEC_STATUS.md accordingly.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
docs/SPEC_STATUS.md	Updates generated spec counts and lists the new spec file.
crates/rubyrs/src/vm/gc.rs	Adds `arity_error_arg1_int` helper for consistent arity/type coercion traps.
crates/rubyrs/src/vm/iter.rs	Adds catch-all error arms for block-form `each_slice`/`each_cons` and wrong-arity `chunk_while`.
crates/rubyrs/src/vm/array.rs	Adds catch-all error arms for no-block `each_slice`/`each_cons`.
crates/rubyrs/src/vm/hash.rs	Adds catch-all error arms for no-block `each_slice`/`each_cons`.
crates/rubyrs/src/vm/range.rs	Adds catch-all error arms for no-block `each_slice`/`each_cons`.
crates/rubyrs/spec/ruby/collection_arity_arg1_int_spec.rb	Adds new specs validating arity/type guard behavior and message wording.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

linyiru · 2026-06-02T00:58:59Z

+        match &args[0] {
+            Value::Nil => self.trap(RubyError::TypeError {
+                msg: "no implicit conversion from nil to integer".to_string(),
+            }),
+            other => self.trap(RubyError::TypeError {
+                msg: format!("no implicit conversion of {} into Integer", other.type_name()),
+            }),


Adopted in bafda4d. Switched to type_name_for_coerce — true/false now render correctly (CRuby byte-identical: 'no implicit conversion of true into Integer').

linyiru · 2026-06-02T00:59:00Z

+                    // Wrong-arity / non-Int for Array#each_slice
+                    // no-block form (block-form gap mirrored by
+                    // iter.rs catch-all).
+                    ("each_slice", _) => {
+                        return Err(self.arity_error_arg1_int(name, args));
+                    }


Adopted in bafda4d. Switched to usize::try_from(*n).unwrap_or(usize::MAX), matching the take/drop/first/last pattern in array.rs and the Hash/Range siblings from PR #316/#323.

linyiru · 2026-06-02T00:59:01Z

+                    // Wrong-arity / non-Int for Array#each_cons no-block form.
+                    ("each_cons", _) => {
+                        return Err(self.arity_error_arg1_int(name, args));
+                    }


Adopted in bafda4d. Same wasm32 saturation fix for each_cons no-block arm.

linyiru · 2026-06-02T00:59:02Z

+
+describe "chunk_while arity guards (0-arg method)" do
+  it "raises ArgumentError on Array#chunk_while(1)" do
+    klass, msg = caught_pair { [1].chunk_while(2) { |_a, _b| true } }


Adopted in bafda4d. Changed all 3 chunk_while specs to pass (1) as the literal arg — matches the description's "(1)" reading (we both intended 'one argument given'; the test value just made it ambiguous).

linyiru · 2026-06-02T00:59:04Z

+  end
+
+  it "raises ArgumentError on Hash#chunk_while(1)" do
+    klass, msg = caught_pair { {a: 1}.chunk_while(2) { |_a, _b| true } }


Adopted in bafda4d. Changed all 3 chunk_while specs to pass (1) as the literal arg — matches the description's "(1)" reading (we both intended 'one argument given'; the test value just made it ambiguous).

linyiru · 2026-06-02T00:59:05Z

+  end
+
+  it "raises ArgumentError on Range#chunk_while(1)" do
+    klass, msg = caught_pair { (1..3).chunk_while(2) { |_a, _b| true } }


Adopted in bafda4d. Changed all 3 chunk_while specs to pass (1) as the literal arg — matches the description's "(1)" reading (we both intended 'one argument given'; the test value just made it ambiguous).

- `arity_error_arg1_int` now uses `type_name_for_coerce` so `true`/`false` render as `true`/`false` instead of `Boolean`, matching CRuby and the rest of the VM's coercion-error sites. - Array#each_slice / #each_cons no-block success arms switched from `*n as usize` to `usize::try_from(*n).unwrap_or(usize::MAX)` — wasm32-safe, matching the take/drop/first/last pattern in the same file and the Hash/Range siblings from PR #316/#323. - chunk_while spec: changed test args from `(2)` to `(1)` so the literal arg matches the description's "(1)" reading (Copilot interpreted "(1)" as the passed value, not the arity). Verified: `[1].each_slice(true) { }` → "no implicit conversion of true into Integer" (CRuby byte-identical). Refs PR #330 review (Copilot).

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

…g1_int `type_name_for_coerce` falls back to "Object" for Block / CurriedProc / BoundMethod, but CRuby's coercion error message uses the class names "Proc" and "Method": - `[1].each_slice(->{})` → CRuby: "no implicit conversion of Proc into Integer"; rubyrs pre-fix: "... of Object ...". - `[1].each_slice([].method(:each))` → CRuby: "... Method ..."; rubyrs pre-fix: "... Object ...". Override Block/CurriedProc → "Proc" and BoundMethod → "Method" inline before falling back. Bool stays correctly mapped via `type_name_for_coerce` ("true"/"false", not "TrueClass"/"FalseClass" — `class_name_for_error` would break that). +1 spec example. Refs PR #330 review (code-review).

…oMethodError) Cycle-1 wired up the chunk_while wrong-arity catch-alls in iter.rs (block-form dispatch), but missed the no-block dispatch paths in array.rs / hash.rs / range.rs. Without a block, `[1].chunk_while(1)` fell through to NoMethodError — the same lockstep contract violation this PR was meant to fix everywhere. Add `("chunk_while", many) if !many.is_empty()` arms to all three no-block dispatchers. Byte-identical to CRuby: `ArgumentError: wrong number of arguments (given N, expected 0)`. +3 specs (one per receiver). Refs PR #330 review (code-review).

…le family Before this sweep, all 9 dispatch sites added in PRs #311 / #312 / #316 / #323 fell through to NoMethodError for any shape outside `[Value::Int(n)]`: - `arr.each_slice(2, 3) { }` → NoMethodError (CRuby: ArgumentError "wrong number of arguments (given 2, expected 1)") - `arr.each_slice("2") { }` → NoMethodError (CRuby: TypeError "no implicit conversion of String into Integer") - `arr.each_slice` → NoMethodError (CRuby: ArgumentError "given 0, expected 1") - `arr.chunk_while(2) { }` → NoMethodError (CRuby: ArgumentError "given 1, expected 0") This contradicted `respond_to?` which returns true uncondi- tionally for all 9 method names across Array / Hash / Range — the same lockstep contract violation pattern caught in PRs #308 / #316 / #323. Add `Vm::arity_error_arg1_int` helper (gc.rs alongside `Vm::trap`) and 14 catch-all match arms (one after each existing `[Value::Int(n)]` arm) plus 3 chunk_while arms inline. Error class and wording byte-identical to CRuby for Int / non-Int / Nil / multi-arg / zero-arg shapes. Known divergence (out of scope): Float arg. CRuby coerces (2.5 → 2); rubyrs raises TypeError. A follow-up PR could add Float → Int truncation in the helper. +1 spec / +12 examples. Files 151 → 152, examples 954 → 966.

- `arity_error_arg1_int` now uses `type_name_for_coerce` so `true`/`false` render as `true`/`false` instead of `Boolean`, matching CRuby and the rest of the VM's coercion-error sites. - Array#each_slice / #each_cons no-block success arms switched from `*n as usize` to `usize::try_from(*n).unwrap_or(usize::MAX)` — wasm32-safe, matching the take/drop/first/last pattern in the same file and the Hash/Range siblings from PR #316/#323. - chunk_while spec: changed test args from `(2)` to `(1)` so the literal arg matches the description's "(1)" reading (Copilot interpreted "(1)" as the passed value, not the arity). Verified: `[1].each_slice(true) { }` → "no implicit conversion of true into Integer" (CRuby byte-identical). Refs PR #330 review (Copilot).

…g1_int `type_name_for_coerce` falls back to "Object" for Block / CurriedProc / BoundMethod, but CRuby's coercion error message uses the class names "Proc" and "Method": - `[1].each_slice(->{})` → CRuby: "no implicit conversion of Proc into Integer"; rubyrs pre-fix: "... of Object ...". - `[1].each_slice([].method(:each))` → CRuby: "... Method ..."; rubyrs pre-fix: "... Object ...". Override Block/CurriedProc → "Proc" and BoundMethod → "Method" inline before falling back. Bool stays correctly mapped via `type_name_for_coerce` ("true"/"false", not "TrueClass"/"FalseClass" — `class_name_for_error` would break that). +1 spec example. Refs PR #330 review (code-review).

…oMethodError) Cycle-1 wired up the chunk_while wrong-arity catch-alls in iter.rs (block-form dispatch), but missed the no-block dispatch paths in array.rs / hash.rs / range.rs. Without a block, `[1].chunk_while(1)` fell through to NoMethodError — the same lockstep contract violation this PR was meant to fix everywhere. Add `("chunk_while", many) if !many.is_empty()` arms to all three no-block dispatchers. Byte-identical to CRuby: `ArgumentError: wrong number of arguments (given N, expected 0)`. +3 specs (one per receiver). Refs PR #330 review (code-review).

Closes the divergence acknowledged in PR #330's commit message: CRuby truncates `arr.each_slice(2.5)` to 2 (Integer cast). rubyrs previously raised TypeError. NaN / ±Infinity raise RangeError with CRuby's exact wording ("float NaN / Inf / -Inf out of range of integer" — note the short labels, NOT `float_domain_label`'s "Infinity"). Implementation: - New `Vm::float_to_int_arg(f) -> Result<i64, Trap>` helper in gc.rs (alongside `arity_error_arg1_int`). - 12 Float-arm catch-alls (each_slice/each_cons × block/no-block × Array/Hash/Range) placed BEFORE the existing `[Value::Int(n)]` arms. Each converts and self-recurses with a synthesised Int arg so the existing Int arm owns the rest of the logic (no body duplication). - The float-truncates-to-zero case (e.g. `each_slice(0.5)` or `each_slice(-1.5)`) reaches the Int arm with n=0/-1 and the `*n <= 0` guard fires ArgumentError "invalid slice size: 0" — CRuby parity (CRuby's message omits the ": N" suffix; we keep the existing rubyrs wording for sibling consistency). Documented divergence (out of scope): finite floats outside i64 range (e.g. `1e30`) silently saturate via `as i64`; CRuby raises RangeError with a `%g`-formatted message ("float 1e+30 out of range of integer"). Exact %g formatting parity isn't worth the effort for this pathological input. +1 spec / +10 examples (covers truncate, no-block to_a path, NaN / +Inf / -Inf across all three receivers).

Same `*n as usize` truncation concern raised across the each_slice/each_cons sweep (PRs #316/#323/#330). Switch to `usize::try_from(*n).unwrap_or(usize::MAX)` so 32-bit usize targets saturate instead of silently truncating large positive i64 repeat counts. Refs PR #345 review (Copilot).

Extends the lockstep-contract sweep from PRs #330 / #338 / #340 / #345 to the 0..1-Int-arg family on Array. Adds: - `Vm::arity_error_arg0_or_1_int` helper in gc.rs — variant of `arity_error_arg1_int` with CRuby's "expected 0..1" wording for methods that accept either 0 or 1 Integer argument. - 4 Float arms (one per method) before the existing `[Value::Int(n)]` arms — self-recurse with the truncated Int. - 4 catch-all arms `("...", _)` routed through the new helper. Surface gaps fixed (all byte-identical to CRuby): - `arr.pop("1")` / `.shift(:x)` / `.first("1")` / `.last(nil)` → previously ArgumentError "wrong number of arguments" or (for first/last) NoMethodError; now TypeError with correct wording. - `arr.pop(2.5)` / `.shift(2.5)` / `.first(2.5)` / `.last(2.5)` → previously ArgumentError; now coerces to Int(2) per CRuby semantics. - `arr.pop(NaN)` etc. → RangeError "float NaN out of range of integer". This unblocks the previously-pending "method-not-implemented" note in array_pop_spec.rb / array_shift_spec.rb for the `#to_int`-style coerce path (the comment explicitly cited the catch-all `(many)` arm that this PR replaces). +1 spec / +12 examples. Files 157 → 158, examples 1017 → 1029.

Copilot AI review requested due to automatic review settings June 1, 2026 18:44

Copilot started reviewing on behalf of linyiru June 1, 2026 18:45 View session

Copilot AI reviewed Jun 1, 2026

View reviewed changes

linyiru requested a review from Copilot June 2, 2026 00:59

Copilot started reviewing on behalf of linyiru June 2, 2026 00:59 View session

Copilot AI reviewed Jun 2, 2026

View reviewed changes

linyiru added 4 commits June 1, 2026 22:34

linyiru force-pushed the feat/each-slice-arity-sweep branch from 246e8a7 to 5f7f924 Compare June 2, 2026 03:35

linyiru merged commit cdc0180 into master Jun 2, 2026
7 of 10 checks passed

linyiru mentioned this pull request Jun 2, 2026

feat(vm): Float coerce for each_slice/each_cons #338

Merged

6 tasks

linyiru mentioned this pull request Jun 2, 2026

fix(vm): arity / Float-coerce sweep for take/drop (Array/Hash) #340

Merged

5 tasks

linyiru mentioned this pull request Jun 2, 2026

fix(vm): arity / Float-coerce sweep for Array#pop/#shift/#first/#last #349

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(vm): arity / type-coerce sweep for each_slice/each_cons/chunk_while#330

fix(vm): arity / type-coerce sweep for each_slice/each_cons/chunk_while#330
linyiru merged 4 commits into
masterfrom
feat/each-slice-arity-sweep

linyiru commented Jun 1, 2026

Uh oh!

github-actions Bot commented Jun 1, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

linyiru Jun 2, 2026

Uh oh!

linyiru Jun 2, 2026

Uh oh!

linyiru Jun 2, 2026

Uh oh!

linyiru Jun 2, 2026

Uh oh!

linyiru Jun 2, 2026

Uh oh!

linyiru Jun 2, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

linyiru commented Jun 1, 2026

Summary

Test plan

Uh oh!

github-actions Bot commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

gapscan PR diff

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

linyiru Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

linyiru Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

linyiru Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

linyiru Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

linyiru Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

linyiru Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Jun 1, 2026 •

edited

Loading