mruby-regexp: share the pattern-skipping rules between the two prescans - #7165

Two scans read a regexp pattern before the parser does:

• preprocess_pattern() removes (?#...) comment groups, and under /x also whitespace and # line comments;
• has_named_group() answers whether the pattern declares a (?<name>...) group, which is what lets compile_atom() demote a plain (...) written before the declaration that demotes it.

Both are hunting for a (? opener, so both have to agree with the parser about when a ( is really an opener and not a byte hiding behind an escape or inside a character class. Each carried its own copy of those rules, and the copies had drifted apart.

Three rules matched: the escape pair, the class from [ through ] (with ^ and a leading literal ] handled as compile_charclass() handles them), and the POSIX bracket, whose ] must not close the class. The fourth did not. preprocess_pattern() treats \u{...} as one escape and copies the brace group whole, because the free-spacing pass would otherwise strip the spaces separating the list's codepoints and collapse \u{61 62} into the single codepoint \u{6162}. has_named_group() skipped \u as a plain two-byte escape and then walked into the braces.

This is observable, in diagnostics

The divergence cannot change whether a pattern compiles: a well-formed \u{...} list contains only hex digits and separator whitespace, so it can hide neither a class opener nor a (?<, and every pattern that exposes the difference is malformed and rejected either way. But it changes which error is reported. dont_capture is computed from has_named_group() before parsing begins, while the Unicode list is validated during parsing, so a (?< mistaken for a declaration switches on the demotion that rejects a numbered backreference earlier in the pattern, before the parser ever reaches the malformed list:

Regexp.new("\\1\\u{(?<a>")

before: numbered backref/call is not allowed. (use name): /\1\u{(?<a>/
after:  invalid Unicode list: /\1\u{(?<a>/
CRuby:  invalid Unicode list: /\1\u{(?<a>/

Measured by differential testing between a build of this branch and a build of its base, over patterns assembled from the tokens that exercise these rules (\u{, }, [, ], ^, (?<, (, ), a, space, \1, [:alpha:]), every sequence of one to four of them, each also with a \1 prefix, 43,356 distinct patterns:

• 0 patterns change whether they compile.
• 643 change which error is reported, in three message pairs, and in both directions.
• Of those 643, the old code's message matched CRuby 4.0.6's on 1; the new one matches on 604. So unifying the walks moves the error reporting toward CRuby rather than away from it.

Every one of those patterns was already an error; no valid pattern is affected.

The free-spacing pass is untouched

preprocess_pattern() now copies verbatim the span the shared helper returns, which is by construction the same span the inline code copied. Differentially: over 6,990 distinct patterns built the same way under Regexp::EXTENDED from a token set extended with 61 62, # and a newline, and matching every pattern that compiled against twelve subjects, no row where either side compiled differs at all. The 39 rows that do differ are error-message changes of the kind above.

The change

This moves the rules into one helper, skip_uninterpreted(), which steps over whichever construct starts at the current byte and reports the class state back through an in_class flag the caller owns. preprocess_pattern() copies the returned span verbatim; has_named_group() jumps over it, gaining the \u{...} rule it had been missing.

has_comment_group() is deliberately left alone. It is a third walk, but a naive memchr prefilter that visits every ( and so returns TRUE iff the bytes (?# occur anywhere; its only possible error is a false positive costing one unnecessary preprocess_pattern() call. It is not a copy of these rules, and folding it in would only make the fast path slower.

Tests

The new rows are read by both walks at once: each places a (?< and a following plain group behind a construct the rules must cross. A rule dropped from the free-spacing pass strips a space it should have kept inside a class; the same rule dropped from the named-group scan turns the bracketed (?< into a phantom named group and demotes the plain group. A final row pins the diagnostic above, which is the one behaviour this change deliberately alters.

rake -m test: mrbtest Total: 2076, OK: 2047, KO: 0, Crash: 0, Warning: 0, Skip: 29 (2075 without this branch, so the block is the +1); bintest Total: 105, OK: 105, KO: 0. With re_compile.c alone reverted and the tests in place, that block fails, so it does test the change.

Summary by CodeRabbit

• Bug Fixes

  • Improved regular expression parsing for escaped characters, character classes, POSIX bracket expressions, and Unicode escape lists.
  • Fixed extended-mode (/x) handling so spaces and patterns are interpreted consistently.
  • Improved detection of named capture groups in complex expressions.
  • Malformed Unicode escape lists now report errors more reliably.

• Tests

  • Added regression coverage for these parsing scenarios and their error handling.