Skip to content

test: build wide shifts from a variable so the file still compiles - #7117

Merged
matz merged 3 commits into
mruby:masterfrom
takumin:test-int-literal-width
Aug 12, 2026
Merged

test: build wide shifts from a variable so the file still compiles#7117
matz merged 3 commits into
mruby:masterfrom
takumin:test-int-literal-width

Conversation

@takumin

@takumin takumin commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Three test files stop running entirely on a build with a 32 bit mrb_int and
no bigint, and nothing says so.

An integer literal that does not fit mrb_int is rejected while the file is
compiled, so a wide shift written out in full takes its whole file down with
it. The tests are not skipped and not failed. They are absent, and the suite
reports success with fewer tests than it had.

Counted with MRB_INT32 and no bigint, before and after this change:

file literal tests lost
test/t/string.rb ~(-1 << 63) 71
mrbgems/mruby-random/test/random.rb 1 << 62 13
mrbgems/mruby-socket/test/addrinfo.rb 1 << 40 10

Total 1503 tests before, 1732 after, on a build carrying all three gems.

Every one of these lines was already written to cope with a narrow mrb_int:

# test/t/string.rb
assert_nothing_raised { begin; "0123456789".bytesplice(8, ~(-1 << 63), "ab"); rescue ArgumentError, RangeError; end }

# mrbgems/mruby-random/test/random.rb
hi = ((1 << 62) + (1 << 61)) rescue nil

# mrbgems/mruby-socket/test/addrinfo.rb
skip "needs 64-bit mrb_int" unless (1 << 40).kind_of?(Integer)

None of those guards ever ran, because nothing in the file was running yet
when the value was rejected.

Fix

Take the shift width from a variable. The shift becomes a run time operation
that raises RangeError where mrb_int is too narrow, which is exactly what
the rescue and the skip beside it were written for. On a build where the
value fits, the tests run as before.

Note

This is why CI has not caught it: the MRB_INT32 builds that would show the
missing tests are not part of the matrix, and a build that only counts
successes cannot tell a passing file from an absent one.

The same pattern in test additions of mine is fixed in #7113 and #7114.

Tests

  • MRB_INT32 without bigint (stdlib, stdlib-ext, mruby-pack, mruby-sprintf,
    mruby-socket, mruby-io): 1732 tests, 0 failures, against 1503 before
  • default build: 2058 tests plus 105 bintests, 0 failures
  • full-core with MRB_UTF8_STRING: 2249 tests, 0 failures

Summary by CodeRabbit

  • Tests
    • Improved portability of integer-range and string overflow tests on 32-bit environments.
    • Updated networking address-info tests to run only when the runtime supports the required integer width.
    • Preserved existing validation for range and argument errors.

An integer literal that does not fit `mrb_int` is rejected while the file
is compiled, not when the line runs, so `1 << 63` in a test drops the
whole test file on a build with a 32-bit `mrb_int` and no bigint. Nothing
reports it: the tests are not skipped and not failed, they are simply
absent, and the suite goes green with fewer tests than it had.

Three test files pay for this today. Counted on a build with `MRB_INT32`
and no bigint:

* `test/t/string.rb`, from `~(-1 << 63)`: 71 tests
* `mrbgems/mruby-random/test/random.rb`, from `1 << 62`: 13 tests
* `mrbgems/mruby-socket/test/addrinfo.rb`, from `1 << 40`: 10 tests

All three already meant to handle a narrow `mrb_int`. The `rescue nil` in
mruby-random and the `skip` in mruby-socket never got the chance to run,
and the `begin`/`rescue` in `String#bytesplice` covers a line the compiler
had already thrown away.

Take the shift width from a variable in each of them. The shift is then a
run time operation that raises `RangeError` where `mrb_int` is too narrow,
which is what the surrounding `rescue` and `skip` were written for.
@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 9b023dfa-fcdb-4460-8a74-b4d77ee30e5a

📥 Commits

Reviewing files that changed from the base of the PR and between b87cb34 and 1e873be.

📒 Files selected for processing (2)
  • mrbgems/mruby-random/test/random.rb
  • mrbgems/mruby-socket/test/addrinfo.rb
🚧 Files skipped from review as they are similar to previous changes (1)
  • mrbgems/mruby-socket/test/addrinfo.rb

📝 Walkthrough

Walkthrough

The tests now compute wide integer values at runtime. The Addrinfo tests also skip when 64-bit integer support is unavailable. Existing overflow and RangeError assertions remain.

Changes

Integer portability tests

Layer / File(s) Summary
Runtime shift calculations
mrbgems/mruby-random/test/random.rb, test/t/string.rb
Wide integer test values use variable-driven shifts instead of directly folded constants. Existing fallback and exception handling remain.
Addrinfo integer capability guard
mrbgems/mruby-socket/test/addrinfo.rb
Oversized hints use runtime shift calculations. The test rescues unsupported computation and skips without 64-bit integer support while retaining the RangeError assertions.

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

Suggested reviewers: matz

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: computing wide shifts from variables so tests compile on constrained integer builds.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@mrbgems/mruby-random/test/random.rb`:
- Around line 173-177: Update the wide-range test around the shift, capability
probe, and `rand(-hi..hi)` call so support detection does not depend on the
operation’s potentially overflowing result. Use an independent
representation/path check to skip only when wide ranges are unsupported, then
assert that the actual valid-range call returns an Integer instead of treating
nil as unsupported.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c50007ed-cad4-4d0a-9b47-67252235f195

📥 Commits

Reviewing files that changed from the base of the PR and between 19de6ff and b87cb34.

📒 Files selected for processing (3)
  • mrbgems/mruby-random/test/random.rb
  • mrbgems/mruby-socket/test/addrinfo.rb
  • test/t/string.rb

Comment thread mrbgems/mruby-random/test/random.rb Outdated
`Kernel#rand integer range overflow` reaches its wide bounds only where
`mrb_int` is 64 bits wide, and it decided that by calling `rand` on the very
range whose overflow it guards against. The regression it covers returns `nil`
for that range, which the guard read as "this build has no integer range path",
so every assertion behind it was skipped and the suite stayed green with the
bug in place.

Ask what the bounds are instead. `1 << 62` raises `RangeError` where `mrb_int`
is 32 bits and mruby-bigint is absent, and where the gem promotes it the result
is a big integer, which a core method taking an `mrb_int` rejects with the same
error. Neither kind of bound reaches `rand_range_int()`, which is the path the
assertions are written for.

Checked by putting the signed `end - begin` back into `rand_range_int()`: the
wide range assertions now fail there instead of being skipped.
`Addrinfo.getaddrinfo rejects out-of-range integer hints` covers the narrowing
of an `mrb_int` into the C `int` of a `struct addrinfo` field, so it skips where
`mrb_int` is too narrow to hold `1 << 40`. It asked that with `kind_of?(Integer)`,
which a big integer answers true, so on `MRB_INT32` with mruby-bigint the test
ran with a big integer hint. `Addrinfo.getaddrinfo` converts `Integer` hints
only, leaves the field at its default for anything else, and the narrowing under
test never runs: the test failed rather than skipping.

Ask a core method taking an `mrb_int` whether the value is one, and skip unless
it is. This was invisible until the file stopped being dropped at compile time
on a 32-bit `mrb_int`.
@matz
matz merged commit 658596e into mruby:master Aug 12, 2026
20 of 21 checks passed
@takumin
takumin deleted the test-int-literal-width branch August 12, 2026 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants