Repository navigation
Conversation
It falls back to long on Ruby versions that do not define HAVE_RB_LEN_T, so the change also builds there. https://bugs.ruby-lang.org/issues/22400 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#rest_size, #skip, #match?, #skip_until and #exist? passed the length to INT2FIX, which does not fit past FIXNUM_MAX. On x64-mswin64 the result printed as the right number but was not equal to it, and `rest_size + rest_size` came out negative. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟢 Approval recommended
The conversions consistently follow Ruby’s new length API while retaining backward compatibility.
0 open findings
What changed in this PR
Updates StringScanner to use Ruby’s rb_len_t for string lengths and positions while preserving compatibility with older Ruby versions.
Changes:
- Migrates position and length handling to
rb_len_t. - Adds fallback conversion and formatting macros.
- Fixes large length results previously converted with
INT2FIX.
| File | Description |
|---|---|
ext/strscan/strscan.c |
Widens length handling and corrects large integer conversions. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| static rb_len_t | ||
| resolve_capture_index(struct strscanner *p, VALUE specifier) |
There was a problem hiding this comment.
Is this correct? It seems that the "index" is not a length. Should we keep long? (Or we may want to change to size_t but it's out of scope of rb_len_t support.)
There was a problem hiding this comment.
I checked, and Onigmo uses int for it, so long was right. I've changed it back.
They index the match registers, whose count is an int, rather than a String, so rb_len_t does not apply. Only the length of a group name given as a String stays in rb_len_t. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Ruby master now has
rb_len_tfor String and Array lengths and indices (https://bugs.ruby-lang.org/issues/22400). It islongon every platform for now, and the proposal widens it on mswin, wherelongis 32 bits. This switches the scan positions and the lengths derived from them torb_len_t, with a fallback tolongon Ruby withoutHAVE_RB_LEN_T, so nothing changes on released Ruby.The second commit fixes an existing bug where
longis 32 bits.#rest_size,#skip,#skip_until,#match?and#exist?returned a length of 2**30 or more throughINT2FIX, and on x64-mswin64 the result compared unequal to the right number.I ran
rake compileandrake teston mswin with Ruby 3.3 and a 4.1.0dev snapshot that predatesrb_len_t. ruby-head in CI covers theHAVE_RB_LEN_Tside.Generated with Claude Code