fix(runtime): exec/test keep the whole subject, and a global scan keeps the empty match at a match's end (#9429, #9430)

changelog.d/9429-regexp-exec-lastindex-context.md

@@ -0,0 +1,49 @@
+**`exec`/`test` at a non-zero `lastIndex` now evaluate the pattern against the
+whole subject instead of `subject.slice(lastIndex)`** — `^`, `$`, `\b` and both
+lookaround directions get their real context back. No flag beyond `g`/`y` was
+needed to see this:
+
+```js
+const r = /^b/g;      r.lastIndex = 1; r.exec("ab")   // was "b",  now null
+const l = /(?<=a)b/g; l.lastIndex = 1; l.exec("ab")   // was null, now "b"
+```
+
+The engine call sliced the subject at the start offset and then re-based every
+reported range by the same amount. Offsets survived that round trip; assertions
+did not. A slice invents context at its left edge — `^` and `\b` hold at
+offset 0 of the slice, where the subject says they must not — and destroys it —
+`(?<=a)` cannot see the character it needs, and `(?<!a)` therefore holds
+everywhere. Under `/m` it was severe: a line-scanning `while ((m = re.exec(s)))`
+loop saw `^` hold at *every* index, so it walked one character at a time and
+never terminated on its own.
+
+All three engines already expose a positional entry point documented to keep
+the surrounding context — `regex::Regex::captures_at`,
+`fancy_regex::Regex::captures_from_pos` and `regress::Regex::find_from` — and
+each returns absolute offsets, so the re-basing arithmetic is gone rather than
+adjusted. `OwnedExecMatch`'s three constructors no longer take a
+`search_start_byte` at all: with the parameter removed, handing an engine a
+slice again would not compile. The sticky check moves with it, from
+`start() == 0` to `start() == lastIndex`.
+
+**Found while fixing, same function:** `lastIndex > length` was not "no match"
+(RegExpBuiltinExec step 12.a) but a search clamped to the end of the subject —
+`/a*/g` with `lastIndex = 5` on `"ab"` returned an empty match at index 2 where
+Node returns `null`. The bound could not be expressed where it was being
+checked: it is a UTF-16 code-unit comparison, and `utf16_index_to_byte`
+saturates at the payload length, so the byte-offset guard it replaced could
+never fire. That also matters for astral subjects, where the code-unit length
+and the scalar count differ.
+
+Pinned by six runtime tests — one per engine lane, plus the past-the-end bound
+and the `test` routing — and by a fixture byte-compared against Node covering
+`^`, `$`, `\b`, `\B`, lookbehind, negative lookbehind and lookahead at
+`lastIndex` 0 / mid-subject / end / past-end, sticky and global, and seven
+hand-driven `exec` sweeps that have to terminate.
+
+Two of those sweeps need #9408 (landed in #9427) as well as this fix, and are
+the reason to read the pair together: `while ((m = /^/gm.exec("one\r\ntwo")))`
+walks `[0, 4, 5]` — Node's answer — only with both. With #9408 alone the loop
+never terminates, because `^` holds at the slice's left edge at every index;
+with this fix alone it stops early at `[0, 5]`, because `(?m)` still sees LF
+only.

changelog.d/9430-global-scan-empty-match.md

@@ -0,0 +1,57 @@
+**A global regex scan no longer drops the empty match that sits where the
+previous match ended** — `"a".match(/a*/g)` is `["a",""]`, `"a".replace(/a*/g,
+"<>")` is `"<><>"`, and the same for `matchAll` and every `replace` form.
+
+```js
+"a".match(/a*/g)         // was ["a"]        now ["a",""]
+"aXa".match(/a*/g)       // was ["a","a"]    now ["a","","a",""]
+"ab".match(/b*/g)        // was ["","b"]     now ["","b",""]
+"a".replace(/a*/g, "<>") // was "<>"         now "<><>"
+```
+
+ECMAScript's `RegExp.prototype [ @@match ]` loop keeps a zero-width match at
+the previous match's end and *then* advances one code unit
+(`AdvanceStringIndex`). Rust's iterators do the opposite: both
+`regex_automata`'s `Searcher::try_advance` and `fancy_regex`'s
+`Matches::next_with` — the latter documented as "adapted from the `regex`
+crate … ignores empty matches immediately after a match" — discard it and
+re-search one character to the right. Every global operation was built on
+those iterators, so every one inherited the rule.
+
+**The reported symptom understated it.** The rule fires wherever an empty
+match lands on a previous match's end, not only at the end of the subject, so
+interior matches were lost too: `"aXa".match(/a*/g)` was missing *two* of
+Node's four elements, and `"a1b22".match(/\d*/g)` three of five.
+
+One `global_scan` module now holds the ECMAScript loop, and every global site
+goes through it: `String#match`, `matchAll`, `replace`/`replaceAll` with a
+string replacement, with a `$<name>` replacement, and with a callback — on
+both the linear `regex` lane and the `fancy_regex` lookaround/backreference
+lane. `regress`, the third engine, already stepped one position past a
+zero-width match, which is the ECMAScript rule; its iterators are used
+unchanged, and a test pins that lane as the control. `Regex::replace_all` is
+gone from the string-replacement path for the same reason — it runs the
+crate's iterator internally.
+
+The scan takes a starting byte offset rather than a slice, which also gives
+`matchAll` the #9429 treatment: it used to search
+`&subject[lastIndex..]`, so a `matchAll` on a regex with a non-zero
+`lastIndex` evaluated `^`, `\b` and lookbehind against the wrong left edge.
+
+**`test_parity_regex_replace_fn_lookahead` diverged from Node because of
+this**, exactly as #9430 recorded — and the runner could not see it, because
+that test is scored against a stored `expected/…txt` holding `OK` rather than
+against Node. Its `/[a-z]+|(?=\.)/g` assertion asked for `["ab","cd"]`, which
+is the Rust iterator's answer; Node has always produced `["ab","","cd"]` and
+thrown. The assertion now reads Node's answer, so both runtimes print `OK`.
+
+**Found while fixing, NOT fixed here:** `split` by a pattern only fancy-regex
+can compile does not run `RegExp.prototype [ @@split ]` at all — the fallback
+walks `find_iter` and slices between matches. It therefore emits a trailing
+`""` the spec's `q < size` bound never reaches (`"a,b,".split(/(?<=,)/)` →
+`["a,","b,",""]` vs Node's `["a,","b,"]`) and splices no captured groups
+(`"aXbXc".split(/((?<=a)X)/)` → `["a","bXc"]` vs Node's `["a","X","bXc"]`).
+That is a lane gap rather than a scan gap — the `regex` lane runs the spec
+algorithm in `spec_regex_split` and is correct — so it is excluded from this
+fixture with a comment, and the runtime test `fancy_lookbehind_split`
+currently pins the wrong answer.

crates/perry-runtime/src/regex.rs

@@ -36,6 +36,8 @@ mod escape;
 #[cfg(feature = "regex-engine")]
 mod exec_array;
 #[cfg(feature = "regex-engine")]
+mod global_scan;
+#[cfg(feature = "regex-engine")]
 mod grammar;
 #[cfg(feature = "regex-engine")]
 mod lazy;
@@ -1418,14 +1420,18 @@ unsafe fn replace_regex_str_fancy(
     repl_str: &str,
 ) -> *mut StringHeader {
     let has_named_groups = fre.capture_names().any(|n| n.is_some());
-    let mut captures_list: Vec<fancy_regex::Captures> = Vec::new();
-    let mut iter = fre.captures_iter(str_data);
-    while let Some(Ok(caps)) = iter.next() {
-        captures_list.push(caps);
-        if !global {
-            break;
+    // #9430: the ECMAScript scan for the global form. fancy-regex's own
+    // iterator drops a zero-width match that lands where the previous match
+    // ended, so `"a".replace(/(?<=x)?a*/g, …)`-shaped patterns lost their
+    // trailing (and every interior) empty replacement.
+    let captures_list: Vec<fancy_regex::Captures> = if global {
+        global_scan::fancy_captures(fre, str_data, 0)
+    } else {
+        match fre.captures(str_data) {
+            Ok(Some(caps)) => vec![caps],
+            Ok(None) | Err(_) => Vec::new(),
         }
-    }
+    };
     let mut result = String::new();
     let mut last_end = 0usize;
     for caps in &captures_list {
@@ -1507,19 +1513,29 @@ pub extern "C" fn js_string_replace_regex(
         // Route through a JS-aware expander (closure form) so `$&` / `` $` `` /
         // `$'` — which the regex crate's native `$` syntax doesn't support —
         // are substituted per match. `$$`, `$n`, and `$<name>` are handled too.
-        let result = if global {
-            regex
-                .replace_all(str_data, |caps: &regex::Captures| {
-                    expand_js_replacement(repl_str, caps, str_data, has_named_groups)
-                })
-                .to_string()
+        // #9430: `Regex::replace_all` runs the crate's own match iterator,
+        // whose empty-match rule is not ECMAScript's. Drive the ECMAScript
+        // scan and splice the replacements here instead; the non-global form
+        // is the same loop over a one-element list.
+        let captures_list: Vec<regex::Captures> = if global {
+            global_scan::std_captures(regex, str_data, 0)
         } else {
-            regex
-                .replace(str_data, |caps: &regex::Captures| {
-                    expand_js_replacement(repl_str, caps, str_data, has_named_groups)
-                })
-                .to_string()
+            regex.captures(str_data).into_iter().collect()
         };
+        let mut result = String::with_capacity(str_data.len());
+        let mut last_end = 0usize;
+        for caps in &captures_list {
+            let full = caps.get(0).expect("capture zero is the full match");
+            result.push_str(&str_data[last_end..full.start()]);
+            result.push_str(&expand_js_replacement(
+                repl_str,
+                caps,
+                str_data,
+                has_named_groups,
+            ));
+            last_end = full.end();
+        }
+        result.push_str(&str_data[last_end..]);
 
         finish_replace_bytes(result.as_bytes())
     }

crates/perry-runtime/src/regex/exec.rs

@@ -44,61 +44,73 @@ pub extern "C" fn js_regexp_exec(
         let has_indices = (*re).has_indices;
         let use_last_index = global || sticky;
         let last_index = if use_last_index { last_index_read } else { 0 };
-        let search_start_byte = if use_last_index && last_index > 0 {
-            super::exec_array::utf16_index_to_byte(str_data, last_index)
-        } else {
-            0
-        };
 
-        if search_start_byte > str_data.len() {
-            if use_last_index {
-                set_last_index_throwing(re, 0);
-            }
+        // Spec RegExpBuiltinExec step 12.a: `lastIndex > length` is "no match"
+        // outright — NOT a search clamped to the end of the subject. The bound
+        // is in UTF-16 code units, the same unit `lastIndex` is stored in;
+        // comparing byte offsets can't express it because
+        // `utf16_index_to_byte` saturates at `str_data.len()`.
+        if use_last_index && last_index > (*s).utf16_len as usize {
+            set_last_index_throwing(re, 0);
             LAST_EXEC_INDEX.with(|idx| *idx.borrow_mut() = -1.0);
             LAST_EXEC_GROUPS.with(|g| *g.borrow_mut() = ptr::null_mut());
             return ptr::null_mut();
         }
-        let search_str = &str_data[search_start_byte..];
 
+        let search_start_byte = if use_last_index && last_index > 0 {
+            super::exec_array::utf16_index_to_byte(str_data, last_index)
+        } else {
+            0
+        };
+
+        // #9429: search FROM `search_start_byte` in the whole subject rather
+        // than searching a `&str_data[search_start_byte..]` slice. Every
+        // zero-width assertion — `^`, `$`, `\b`, and both lookaround
+        // directions — is defined against the real subject, and a slice both
+        // invents context at its left edge (`^`/`\b` hold where they must not)
+        // and destroys it (`(?<=a)` fails where it must hold). All three
+        // engines expose a positional entry point with exactly these
+        // semantics, documented as such: `regex::Regex::captures_at`,
+        // `fancy_regex::Regex::captures_from_pos` and
+        // `regress::Regex::find_from`. Their reported offsets are absolute, so
+        // nothing downstream re-bases them.
         let owned = if let Some(repeat_matcher) = lookup_repeat_matcher(re) {
             repeat_matcher
                 .regex
-                .find(search_str)
-                .filter(|matched| !sticky || matched.start() == 0)
+                .find_from(str_data, search_start_byte)
+                .next()
+                .filter(|matched| !sticky || matched.start() == search_start_byte)
                 .map(|matched| {
                     if use_last_index {
                         set_last_index_throwing(
                             re,
-                            super::exec_array::byte_index_to_utf16_index(
-                                str_data,
-                                search_start_byte + matched.end(),
-                            ),
+                            super::exec_array::byte_index_to_utf16_index(str_data, matched.end()),
                         );
                     }
                     OwnedExecMatch::from_repeat_matcher(
                         str_data,
-                        search_start_byte,
                         &repeat_matcher,
                         &matched,
                         has_indices,
                     )
                 })
         } else if let Some(fre) = lookup_fancy_regex(re) {
-            match fre.captures(search_str) {
-                Ok(Some(caps)) if !sticky || caps.get(0).is_some_and(|full| full.start() == 0) => {
+            match fre.captures_from_pos(str_data, search_start_byte) {
+                Ok(Some(caps))
+                    if !sticky
+                        || caps
+                            .get(0)
+                            .is_some_and(|full| full.start() == search_start_byte) =>
+                {
                     let full = caps.get(0).expect("capture zero is the full match");
                     if use_last_index {
                         set_last_index_throwing(
                             re,
-                            super::exec_array::byte_index_to_utf16_index(
-                                str_data,
-                                search_start_byte + full.end(),
-                            ),
+                            super::exec_array::byte_index_to_utf16_index(str_data, full.end()),
                         );
                     }
                     Some(OwnedExecMatch::from_fancy(
                         str_data,
-                        search_start_byte,
                         &fre,
                         &caps,
                         has_indices,
@@ -108,26 +120,22 @@ pub extern "C" fn js_regexp_exec(
             }
         } else {
             regex
-                .captures(search_str)
-                .filter(|caps| !sticky || caps.get(0).is_some_and(|full| full.start() == 0))
+                .captures_at(str_data, search_start_byte)
+                .filter(|caps| {
+                    !sticky
+                        || caps
+                            .get(0)
+                            .is_some_and(|full| full.start() == search_start_byte)
+                })
                 .map(|caps| {
                     let full = caps.get(0).expect("capture zero is the full match");
                     if use_last_index {
                         set_last_index_throwing(
                             re,
-                            super::exec_array::byte_index_to_utf16_index(
-                                str_data,
-                                search_start_byte + full.end(),
-                            ),
+                            super::exec_array::byte_index_to_utf16_index(str_data, full.end()),
                         );
                     }
-                    OwnedExecMatch::from_standard(
-                        str_data,
-                        search_start_byte,
-                        regex,
-                        &caps,
-                        has_indices,
-                    )
+                    OwnedExecMatch::from_standard(str_data, regex, &caps, has_indices)
                 })
         };