Skip to content

A boxed File::Stat#mode arm answers the oint its dispatch holds - #8470

Merged
matz merged 1 commit into
matz:masterfrom
rubys:fix-poly-stat-mode-oint
Oct 11, 2026
Merged

matz merged 1 commit into
matz:masterfrom
rubys:fix-poly-stat-mode-oint

Conversation

@rubys

@rubys rubys commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Since #8421 (a07741f, "Integer and Float nil out of band", merged with the #8427 stack), a user class with a mode reader that reaches the poly dispatch fails to compile:

class Sub
  attr_reader :mode
  def initialize(m); @mode = m; end
end
a = []
[Sub.new(0)].each { |s| a.push(s) }
puts a[0].mode == 0
r5.rb:10: error: assigning to 'sp_oint' from incompatible type 'sp_int' (aka 'long')

The dispatch's value is an sp_oint, and the File::Stat default arm (emit_call_poly_io_arms) answered sp_stat_mode's bare sp_int. The stat fields were already right, because sp_stat_field returns an oint. mode now takes sp_oint_of(...) when node_is_oint(c, id).

Bisect: 108d210 good, a45a2d4 (merge of #8427) bad; within the stack, a07741f is the first bad commit. Found in roundhouse CI, where the blog's runtime/tep/broadcast.rb (sub.mode == 0 on a subscription) stopped compiling. The full roundhouse blog spinel build passes with this change.

Test: test/poly_mode_oint_beside_stat.rb. It also stores a real File::Stat in the same dispatch, and the output matches CRuby. Local make test: 6920 pass, 0 fail.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Corrected mode results for calls that can return either an integer or nil, preserving the expected output when used alongside file status checks.
  • Tests
    • Added regression coverage for comparing optional mode values with file status modes.

A user class whose `mode` reader can be nil shares the poly dispatch with
File::Stat#mode (the boxed IO arm). Since Integer nil moved out of band
(a07741f, matz#8421) that dispatch's value is an sp_oint, but the stat arm
still answered sp_stat_mode's bare sp_int, so the C did not compile:

    error: incompatible types when assigning to type 'sp_oint' from type 'sp_int'

The stat fields were already right (sp_stat_field answers an oint); mode
now takes the same wrapping when the call's value is an oint.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added the gate: no trailer The head commit carries no Gate trailer label Oct 11, 2026
@coderabbitai

coderabbitai Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: be03f956-6b2d-4363-9901-710d31c0293f

📥 Commits

Reviewing files that changed from the base of the PR and between 91b8722 and 7e5d55c.


📒 Files selected for processing (3)
  • src/codegen_call_io.c
  • test/poly_mode_oint_beside_stat.rb
  • test/poly_mode_oint_beside_stat.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

Walkthrough

The boxed mode call now converts the stat mode result to an optional integer when its result representation requires one. A regression test exercises this behavior alongside Sub#mode.

Changes

Mode dispatch and regression coverage

Layer / File(s) Summary
Boxed mode result and regression coverage
src/codegen_call_io.c, test/poly_mode_oint_beside_stat.rb, test/poly_mode_oint_beside_stat.rb.expected
The boxed mode arm wraps the stat mode result when the call result is an optional integer. The regression test exercises Sub#mode values of 0 and nil alongside File::Stat#mode; the expected output records four results.

Priority: ➖ Normal

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

Change: Bug fix


Merge Risk: ⚪ Minimal · up to 7e5d5

No actionable merge-blocking risk is evident. The change preserves the optional-integer result in mixed mode dispatch, and the regression test covers nil, integer, and stat values.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title directly describes the main change: the boxed File::Stat#mode dispatch arm returns an oint when the dispatch holds one. It is specific and concise, although the wording is somewhat unusu…
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: Docstring Coverage

Explanation

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


  • Fix all pre-merge checks with AI
  • 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 4d023ce into matz:master Oct 11, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gate: no trailer The head commit carries no Gate trailer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants