Skip to content

Commit 65b720e

Browse files
sisshiki1969claude
andauthored
Interpolated regexp modifiers: options via a leading operand, not (?imx) (#908)
An interpolated regexp carried its literal-syntax modifier letters into the run-time-assembled source as a `(?imxn…)` prefix. Two bugs: - `n`/`u`/`e`/`s` are encoding selectors, not Onigmo group options, so `/#{x}/n` built `(?n)…` and Onigmo raised "undefined group option" — aborting every interpolated regexp with an encoding modifier (11 ruby/spec language errors). - Even the valid `i`/`m`/`x` leaked into `Regexp#source`: `/a#{x}/im` reported `"(?im-x:a…)"` instead of `"a…"`. `gen_regexp` now decodes the modifiers into the same option word `const_regexp` builds (Onigmo `i`/`m`/`x` bits + `NOENCODING` / `KCODE_*` encoding bits) and emits it as a leading Fixnum operand of the `ConcatRegexp` instruction — the sole producer/consumer pair, so the operand layout is private. `concatenate_regexp` peels it off, applies the Onigmo options for real, and fixes the declared encoding from the KCODE bits (mirroring `const_regexp`). No inline prefix is emitted, so the source stays verbatim. ruby/spec language: 38 → 27 failing (−11). The two remaining `/s` interpolation cases now fail the same way as the pre-existing non-interpolated `/s` match (Shift_JIS byte-offset mapping through the lossy-UTF-8 matcher view — a separate capture-encoding limitation), not the former crash. Claude-Session: https://claude.ai/code/session_013XLHRVwyWsn5Pp8zgNcZaK Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9406ad2 commit 65b720e

3 files changed

Lines changed: 92 additions & 7 deletions

File tree

monoruby/src/builtins/regexp.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2231,4 +2231,26 @@ mod tests {
22312231
.map { |r| r.source.encoding.to_s }"#,
22322232
);
22332233
}
2234+
2235+
#[test]
2236+
fn regexp_interpolation_encoding_modifiers() {
2237+
// An interpolated regexp with an `n`/`u`/`e`/`s` encoding modifier
2238+
// must not feed the letter into an Onigmo group option (`(?n)` —
2239+
// which used to raise "undefined group option"); the modifier fixes
2240+
// the declared encoding while the interpolated bytes form the source.
2241+
run_test(
2242+
r#"x = "a"
2243+
[ /#{x}/n, /#{x}/u, /#{x}/e, /#{x}/s, /#{x}/ ]
2244+
.map { |r| r.encoding.to_s }"#,
2245+
);
2246+
// `i`/`m`/`x` still lower to an inline `(?imx)` group and the
2247+
// interpolated fragments still form the source verbatim.
2248+
run_test(
2249+
r#"x = "b"
2250+
r = /a#{x}c/imx
2251+
[r.source, (r =~ "xABCx"), r.match("aBc")[0]]"#,
2252+
);
2253+
// A modifier combined with `i`, and with a leading empty fragment.
2254+
run_test(r#"x = "Z"; (/#{x}/i =~ "qzq")"#);
2255+
}
22342256
}

monoruby/src/bytecodegen/expression.rs

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1945,11 +1945,45 @@ impl<'a> BytecodeGen<'a> {
19451945
fn gen_regexp(&mut self, ret: BcReg, nodes: Vec<Node>, option: String, loc: Loc) -> Result<()> {
19461946
let mut len = nodes.len();
19471947
let arg = self.sp();
1948-
if !option.is_empty() {
1949-
let r = self.push().into();
1950-
self.emit_literal(r, Value::string(format!("(?{option})")));
1951-
len += 1;
1948+
// The interpolated-regexp source is assembled at run time by
1949+
// `runtime::concatenate_regexp`. Decode the literal-syntax modifier
1950+
// letters into the same option word `const_regexp` builds, and pass
1951+
// it as a leading Fixnum operand rather than as an inline `(?imx)`
1952+
// source prefix. Two reasons:
1953+
// - `n`/`u`/`e`/`s` are encoding selectors, *not* valid Onigmo
1954+
// group options — `(?n)` makes Onigmo raise "undefined group
1955+
// option". They must set the regexp's `KCODE_*` / `NOENCODING`
1956+
// bits instead.
1957+
// - Even the valid `i`/`m`/`x` must not sit in the source: CRuby's
1958+
// `Regexp#source` for `/a#{x}/im` is `"a…"`, not `"(?im-x:a…)"`.
1959+
// Passing them as real Onigmo option bits keeps `#source` clean.
1960+
// `concatenate_regexp` (the sole consumer of `ConcatRegexp`) peels
1961+
// this operand off and hands the bits to the regexp builder.
1962+
let mut opt: i64 = 0;
1963+
if option.contains('i') {
1964+
opt |= onigmo_regex::ONIG_OPTION_IGNORECASE as i64;
1965+
}
1966+
if option.contains('x') {
1967+
opt |= onigmo_regex::ONIG_OPTION_EXTEND as i64;
1968+
}
1969+
if option.contains('m') {
1970+
opt |= onigmo_regex::ONIG_OPTION_MULTILINE as i64;
1971+
}
1972+
if option.contains('n') {
1973+
opt |= RegexpInner::NOENCODING as i64;
1974+
} else if option.contains('u') {
1975+
opt |= RegexpInner::KCODE_UTF8 as i64;
1976+
} else if option.contains('e') {
1977+
opt |= RegexpInner::KCODE_EUCJP as i64;
1978+
} else if option.contains('s') {
1979+
opt |= RegexpInner::KCODE_SJIS as i64;
19521980
}
1981+
// Always the first operand (even when 0) so the run-time layout is
1982+
// uniform: operand 0 is the option word, the rest are source
1983+
// fragments.
1984+
let r = self.push().into();
1985+
self.emit_literal(r, Value::integer(opt));
1986+
len += 1;
19531987
for expr in nodes {
19541988
self.push_expr(expr)?;
19551989
}

monoruby/src/codegen/runtime.rs

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -748,6 +748,17 @@ pub(super) extern "C" fn concatenate_regexp(
748748
len: usize,
749749
) -> Option<Value> {
750750
use crate::value::rvalue::Encoding;
751+
// Operand 0 is the literal-syntax option word (Onigmo `i`/`m`/`x` bits
752+
// plus the `NOENCODING` / `KCODE_*` encoding-selector bits for
753+
// `n`/`u`/`e`/`s`), emitted by `gen_regexp` as a Fixnum (0 = none).
754+
// `gen_regexp` is the sole producer of `ConcatRegexp`, so this leading
755+
// operand is always present. Operands are read *downward* from `arg`
756+
// (`concatenate_string_inner` walks `arg.sub(i)`), so operand 0 is at
757+
// `*arg` and the source fragments start one slot below.
758+
// SAFETY: `arg` points at operand 0 of `len` (>= 1) operand `Value`s.
759+
let option = unsafe { (*arg).try_fixnum().unwrap_or(0) } as u32;
760+
let arg = unsafe { arg.sub(1) };
761+
let len = len - 1;
751762
// Build the interpolated source as a String first, so each operand's
752763
// bytes and encoding combine under the same rules as `"#{}"` — a
753764
// non-ASCII embedded String (e.g. EUC-JP) then upgrades the regexp's
@@ -766,16 +777,34 @@ pub(super) extern "C" fn concatenate_regexp(
766777
// best-effort UTF-8 view while the raw bytes + encoding drive
767778
// `Regexp#source` / `#encoding`.
768779
let reg_str = String::from_utf8_lossy(&bytes).into_owned();
769-
let onig_enc = if enc == Encoding::Ascii8 {
780+
// Split the encoding-selector bits out of the option word into the
781+
// KCODE the resolver reads (mirroring `const_regexp`); the Onigmo
782+
// `i`/`m`/`x` bits stay in `option` and `with_option_kcode_source`
783+
// strips the Ruby-only bits before handing the mask to Onigmo. With an
784+
// encoding modifier the declared encoding is fixed by it regardless of
785+
// the interpolated content's encoding; without one it is derived from
786+
// that content as before.
787+
let kcode = if option & RegexpInner::KCODE_MASK != 0 {
788+
Some(option & RegexpInner::KCODE_MASK)
789+
} else {
790+
None
791+
};
792+
let onig_enc = if option & RegexpInner::NOENCODING != 0 {
793+
// `/n`: ASCII / BINARY matching.
794+
onigmo_regex::OnigmoEncoding::ASCII
795+
} else if kcode.is_some() {
796+
// `/u` `/e` `/s`: match against the best-effort UTF-8 view.
797+
onigmo_regex::OnigmoEncoding::UTF8
798+
} else if enc == Encoding::Ascii8 {
770799
onigmo_regex::OnigmoEncoding::ASCII
771800
} else {
772801
onigmo_regex::OnigmoEncoding::UTF8
773802
};
774803
let inner = match RegexpInner::with_option_kcode_source(
775804
reg_str,
776-
0,
805+
option,
777806
onig_enc,
778-
None,
807+
kcode,
779808
Some(enc),
780809
Some(bytes),
781810
) {

0 commit comments

Comments
 (0)