Skip to content

reduce(0, :|) over an Array with a Bignum folds boxed - #10183

Merged
matz merged 1 commit into
matz:masterfrom
hasumikin:reduce-seed-bitop-boxed
Oct 25, 2026
Merged

matz merged 1 commit into
matz:masterfrom
hasumikin:reduce-seed-bitop-boxed

Conversation

@hasumikin

@hasumikin hasumikin commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

[28, 2**64, "x"] with its last element popped answered RangeError for reduce(0, :^), where CRuby answers 18446744073709551644.

Background

reduce(init, :op) with no block types its result from the seed. For an Integer seed, two rules make the result boxed:

  • the elements of a poly Array have an erased type
  • a literal Array holds a Bignum, a Rational, a Complex or a boxed value

Both rules apply only to an arithmetic operator. For &, | and ^, the result stayed an sp_int. The fold itself runs boxed, so the unbox at its end raised on a Bignum:

big = 2**64
pa = [28, big, "x"]
pa.pop
p pa.reduce(0, :^)        # RangeError, CRuby 18446744073709551644
p [28, big].reduce(0, :|) # RangeError, CRuby 18446744073709551644

Fix

The two rules now take &, | and ^ too (is_bit_op). The result of such a fold is boxed. A fold whose elements have a known type does not change, so [3, 5].reduce(0, :|) stays an sp_int.

test/reduce_seed_bitop_bignum.rb gives the CRuby answers under raise and promote. It also keeps a fold of machine words and the TypeError for a String element.

make gate (on this branch merged with current master)

scale-test: boxed Hash store work at 2x the methods is 1.93x (limit 2.20)
scale-test: boxed-receiver alias work at 2x the writes is 1.86x (limit 2.20)
scale-test: instance_eval forwarding work at 2x the wrappers is 1.72x (limit 2.50)
scale-test: work at 4x the program is 4.96x (linear 4.00, limit 5.40)
scale-test: work at 4x the program, compiled to C, is 6.30x (limit 6.90)
scale-test: call-shape work at 4x the units, compiled to C, is 4.29x (linear 4.00, limit 4.50)
scale-test: the analysis of a String chain at 4x the links is 4.15x (linear 4.00, limit 4.60)
Tests: 8921 pass, 0 fail, 0 error
gate: ALL GREEN
  • New tests have .expected files that match CRuby 4.0 run with --enable-frozen-string-literal
  • Values past 2^31 are marked # spinel: int64
  • If optcarrot's generated C changed: callgrind numbers, checksum 59662

Summary by CodeRabbit

  • Bug Fixes

    • Fixed bitwise &, |, and ^ reductions with an initial value when arrays contain large integers.
    • Improved handling of reductions over arrays with mixed or unspecified element types, while preserving errors for non-integer values.
  • Tests

    • Added regression coverage for bitwise reductions with large integers, machine-sized integers, and invalid array elements.

`[28, 2**64, "x"]` with its last element popped answered RangeError for
`reduce(0, :^)`, where CRuby answers 18446744073709551644.

## Background

reduce(init, :op) with no block types its result from the seed. For an
Integer seed, two rules make the result boxed:

- the elements of a poly Array have an erased type
- a literal Array holds a Bignum, a Rational, a Complex or a boxed value

Both rules apply only to an arithmetic operator. For `&`, `|` and `^`, the
result stayed an sp_int. The fold itself runs boxed, so the unbox at its
end raised on a Bignum:

```ruby
big = 2**64
pa = [28, big, "x"]
pa.pop
p pa.reduce(0, :^)        # RangeError, CRuby 18446744073709551644
p [28, big].reduce(0, :|) # RangeError, CRuby 18446744073709551644
```

## Fix

The two rules now take `&`, `|` and `^` too (is_bit_op). The result of
such a fold is boxed. A fold whose elements have a known type does not
change, so `[3, 5].reduce(0, :|)` stays an sp_int.

test/reduce_seed_bitop_bignum.rb gives the CRuby answers under raise and
promote. It also keeps a fold of machine words and the TypeError for a
String element.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014j1eT4mbnjKDWvHdDRpp1T
@coderabbitai

coderabbitai Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 01bf4565-3359-438d-9961-9c2afd3f00a0
Commits

Reviewing files that changed from the base of the PR and between 7bcd406 and ae8e333.

Files selected for processing: 3
  • src/analyze_infer_recv.c
  • test/reduce_seed_bitop_bignum.rb
  • test/reduce_seed_bitop_bignum.rb.expected

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

Walkthrough

The change extends boxed-result inference for seeded bitwise reductions over poly arrays. A regression script checks results for Bignum and machine-word values, and checks the error message when a non-Integer element remains.

Changes: Seeded bitwise reductions

Layer / File(s) Summary
Boxed-result inference and regression coverage
src/analyze_infer_recv.c, test/reduce_seed_bitop_bignum.rb, test/reduce_seed_bitop_bignum.rb.expected
Bitwise folds now use boxed-result handling for literal poly arrays containing boxed numeric elements and for poly arrays with erased element types. The regression cases cover Bignum values, machine-word values, and the error from a remaining non-Integer element.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related PRs

  • matz/spinel#10175: Changes boxed Integer bitwise operations with Bignum operands, which are used by the seeded reductions covered here.
Merge Risk: Minimal · up to ae8e3

The change is mergeable; the modified inference paths and regression expectations are consistent.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title names the primary change: reduce(0, :|) over an Array containing a Bignum now folds boxed. The raw summary and objectives confirm the change extends boxing rules to bitwise operators &, |, a…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 …
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title names the primary change: reduce(0, :|) over an Array containing a Bignum now folds boxed. The raw summary and objectives confirm the change extends boxing rules to bitwise operators &, |, and ^ for reduce(init, :op) with an Integer seed. The title uses one specific example operator, so it is somewhat narrower than the full change, but it still refers to the main point and is not misleading.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 unsupported.)

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@matz
matz merged commit 546ad49 into matz:master Oct 25, 2026
5 checks passed
@hasumikin
hasumikin deleted the reduce-seed-bitop-boxed branch October 25, 2026 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants