mruby-regexp: apply /i to the backreference comparison - #7046
Conversation
The `RE_BACKREF` case of `bt_match()` compared the captured text with
`memcmp()` unconditionally, so a backreference stayed case-sensitive even
when the rest of the pattern was folded.
```ruby
/(a)\1/i.match?("aA") # CRuby: true, mruby: false
/(?<n>a)\k<n>/i.match?("aA") # CRuby: true, mruby: false
/(a)(?i)\1/.match?("aA") # CRuby: true, mruby: false
```
Decide the case sensitivity at compile time, per instruction, like every
other ignorecase decision in this engine. `compile_atom()` now records
`c->flags & RE_FLAG_IGNORECASE` in the otherwise unused `offset` field of
the emitted `RE_BACKREF`, and `bt_match()` picks an ASCII case-insensitive
comparison when it is set.
Reading `pat->flags` in the matcher instead would have been wrong for
inline options: `pat->flags` holds the whole-pattern option set, while
`(?i)` and `(?i:...)` live only in the compiler's `c->flags` and are never
written back. That form would fold `/(?-i:(a)\1)/i`, which must not fold,
and would leave `/(a)(?i)\1/` unfolded, which must.
Reusing `offset` is safe for this opcode: `insert_inst()` and
`emit_atom_copy()` relocate offsets only for `RE_JMP`, `RE_SPLIT` and
`RE_SPLITNG`, and `compute_fixed_len()` rejects `RE_BACKREF` through its
`default:` arm. Only `bt_match()` executes the opcode, since a pattern
with a backreference always sets `has_backref` and is dispatched to the
backtracking engine, so the Pike VM needs no change.
Folding stops at ASCII, matching the existing `/i` behavior for literals
and character classes.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis change enables ASCII case-insensitive matching for numeric and named backreferences. The compiler records the active ignore-case state in ChangesCase-insensitive backreferences
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
A backreference ignored the
/iflag: theRE_BACKREFcase ofbt_match()compared the captured text with
memcmp()unconditionally, so\1stayedcase-sensitive even when the rest of the pattern was folded.
Fix
The case sensitivity is now decided at compile time, per instruction, the
same way every other ignorecase decision in this engine is taken.
compile_atom()recordsc->flags & RE_FLAG_IGNORECASEin theoffsetfield of the emitted
RE_BACKREF(unused for this opcode), andbt_match()selects an ASCII case-insensitive comparison when the bit is set.
Reading
pat->flagsfrom the matcher would have been a smaller change butthe wrong one.
pat->flagsis the whole-pattern option set, whereas(?i)and
(?i:...)are a compile-time mechanism on the compiler's ownc->flagsthat is never written back to the pattern. That form would fold
/(?-i:(a)\1)/i, which must not fold, and would leave/(a)(?i)\1/unfolded, which must.
Reusing
offsetneeds no change tore_inst. It is safe for this opcode:insert_inst()andemit_atom_copy()rewrite offsets only forRE_JMP,RE_SPLITandRE_SPLITNG, andcompute_fixed_len()rejectsRE_BACKREFthrough its
default:arm.Only
bt_match()had to change. A pattern with a backreference always setshas_backref, somrb_re_exec()dispatches it to the backtracking engine;the Pike VM has no
RE_BACKREFcase at all.literal_exec()is unreachablehere because
is_literalrequires!has_backref.Case folding stops at ASCII, consistent with the existing
/ihandling forliterals and character classes. Extending
/ito non-ASCII is a separatelimitation and is left untouched.
Tests
mrbgems/mruby-regexp/test/regexp.rbRegexp - backreference under /iassertion for\1/icases added toRegexp - named backreference \k/(a)(?i)\1/,/(a)(?i:\1)/and the/(?-i:(a)\1)/icontrol added to
Regexp - inline options (?i) / (?i:...)All new assertions were run against CRuby 4.0.6 and produce identical
results.
rake testpasses: 1968 total, 0 KO.Summary by CodeRabbit
Bug Fixes
Tests