fix(regex): RegExp.prototype.compile error semantics (Annex B) — #5910

[proggeramlug]

Summary

Fixes a coherent single-root subcluster of the #5910 annexB worklist: the
RegExp.prototype.compile (Annex B §B.2.5.1) error-handling cases. compile
was installed as a no-op prototype thunk and its coercion path was lenient,
so several required error behaviours were missed.

Fixes these 5 annexB/built-ins/RegExp/prototype/compile/* cases (all listed in #5910):

case	before	after
this-not-object.js	undefined	TypeError
this-obj-not-regexp.js	undefined	TypeError
flags-to-string-err.js	Symbol → "Symbol()"	TypeError
pattern-to-string-err.js	Symbol → "Symbol()"	TypeError
pattern-regexp-immutable-lastindex.js	no throw	TypeError, source/flags still updated

Root cause & fix (3 small runtime edits, all in perry-runtime)

1. Missing brand check. RegExp.prototype.compile was a no-op thunk, so
   compile.call(<non-regexp>) returned undefined instead of throwing. Replaced
   it with a real regex_proto_compile_thunk that reuses the existing
   regex_instance_or_throw brand check (same one exec/test use) and routes a
   valid receiver to js_regexp_compile_value. The Annex-B no-op is retained only
   for builds compiled without the regex-engine feature.

2. Lenient argument coercion. A Symbol pattern/flags stringified to
   "Symbol(desc)". Added reject_symbol_to_string before ToString, matching
   the abstract ToString operation (§7.1.17) — same guard already used by
   String.prototype.pad*, String.raw, etc.

3. lastIndex reset ignored writability. RegExpInitialize step 12 is
   Set(obj, "lastIndex", 0, true) — a throwing set. The reset is now moved
   after the source/flags update and routed through the existing
   set_last_index_throwing, so a frozen lastIndex throws TypeError while
   .source/.flags are still updated (exactly what the immutable-lastindex case
   asserts).

Descriptors are preserved: compile.length === 2, compile.name === "compile",
and the {writable, !enumerable, configurable} method descriptor are unchanged
(the real thunk goes through the same install_proto_method path as the old no-op).

Validation

Measured with scripts/test262_subset.py --all-features against a from-scratch
base-main build (A/B), test262 pinned SHA 4249661…:

• compile subtree: base 16 pass / 6 fail → patched 20 pass / 2 fail.
• annexB/built-ins/RegExp + built-ins/RegExp (1915 judged): base 1524 pass
  → patched 1528 pass (net +4; the exact 5 target cases flip base-fail → patched-pass,
  no other case regresses).
• cargo fmt clean on all touched files; scripts/check_file_size.sh unaffected
  (largest touched file 387 lines).

Note on this-subclass-instance.js (out of scope, disclosed for transparency)

Under --all-features the differential radar shows one case shifting
base-pass → patched-"runtime-fail": .../compile/this-subclass-instance.js
(features: [legacy-regexp, class], not in the #5910 list). This is a
differential/oracle artifact, not a correctness regression:

• The case asserts subclassInstance.compile() throws TypeError.
• Perry's new (class extends RegExp {})("") does not produce a functioning
  [[RegExpMatcher]] (its .source is undefined — a pre-existing RegExp-
  subclassing gap, unrelated to compile). Given no matcher slot, throwing
  TypeError is the consistent brand-check result — patched Perry now
  satisfies the case's own assert.throws (Perry exit 0).
• The local Node oracle (26.3) instead recompiles the subclass (its instance
  has a real matcher) and so fails the assert — which is why the differential
  flags the divergence. Base "passed" only because its no-op compile failed the
  assert the same way Node does.

i.e. patched Perry is more conformant to what this case asserts; fully fixing it
belongs to the separate RegExp-subclassing gap.

Per the external-contributor guidance, no version bump / CHANGELOG / CLAUDE.md
edits are included.

Refs #5910

Summary by CodeRabbit

• New Features

  • Added functional support for RegExp.prototype.compile() when the regex engine is enabled.
  • RegExp patterns and flags are recompiled in place with standard validation behavior.

• Bug Fixes

  • Invalid Symbol-based pattern or flag values now correctly raise errors.
  • Compilation now properly resets lastIndex and respects observable property behavior.
  • Calls on non-RegExp receivers are rejected as required.

[coderabbitai]

📝 Walkthrough

Walkthrough

RegExp compilation now rejects Symbol coercions, updates compiled state before resetting lastIndex, and installs a brand-checking compile method when the regex engine feature is enabled.

Changes

RegExp compile support

Layer / File(s)	Summary
Compile coercion and state updates crates/perry-runtime/src/regex/compile.rs	Pattern and flag conversion rejects Symbols; compiled source state is updated before lastIndex is reset with throwing semantics.
Compile thunk and prototype installation crates/perry-runtime/src/object/regex_proto_thunks.rs, crates/perry-runtime/src/object/global_this/proto_methods.rs	The regex engine adds a brand-checking RegExp.prototype.compile thunk and installs it conditionally; the no-op fallback remains disabled when the engine is enabled.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant RegExpPrototype
  participant regex_proto_compile_thunk
  participant RegExpInstanceResolver
  participant js_regexp_compile_value
  RegExpPrototype->>regex_proto_compile_thunk: compile(pattern, flags)
  regex_proto_compile_thunk->>RegExpInstanceResolver: resolve receiver
  RegExpInstanceResolver-->>regex_proto_compile_thunk: registered RegExp instance
  regex_proto_compile_thunk->>js_regexp_compile_value: compile pattern and flags
  js_regexp_compile_value-->>RegExpPrototype: return compilation result

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and concisely captures the main change: fixing RegExp.prototype.compile Annex B error semantics.
Description check	✅ Passed	The description covers summary, concrete changes, related issue, and validation, though the template's checklist/test-plan formatting is only partially filled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

crates/perry-runtime/src/regex/compile.rs (1)

67-78: 🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy lift

Stale pointer usage due to unrooted values across potential GC.

The string coercion js_string_coerce(pattern_val) can invoke user code (e.g., custom toString/valueOf methods) and trigger garbage collection. Because flags_val is an unrooted f64 stack local and re is an unrooted raw pointer, they will not be updated if their underlying objects are evacuated, leading to dangling pointer dereferences in subsequent steps. Based on learnings, values representing objects must be rooted via crate::gc::RuntimeHandleScope before any allocating operations.

• crates/perry-runtime/src/regex/compile.rs#L67-L78: Root pattern_val and flags_val with RuntimeHandleScope and reload them before passing to reject_symbol_to_string and js_string_coerce.
• crates/perry-runtime/src/object/regex_proto_thunks.rs#L274-L275: Change js_regexp_compile_value to accept the receiver as an f64 (so it can be safely rooted inside) or re-resolve the raw pointer from IMPLICIT_THIS after the coercions, rather than extracting re as a raw pointer across the call.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-runtime/src/regex/compile.rs` around lines 67 - 78, The regex
compilation flow must root object-valued inputs before coercions that may
allocate or trigger GC. In crates/perry-runtime/src/regex/compile.rs lines
67-78, use RuntimeHandleScope to root pattern_val and flags_val, then reload
both rooted values before reject_symbol_to_string and js_string_coerce. In
crates/perry-runtime/src/object/regex_proto_thunks.rs lines 274-275, update
js_regexp_compile_value to receive the receiver as an f64 and root it
internally, or re-resolve the receiver from IMPLICIT_THIS after coercions; do
not retain an unrooted raw re pointer across the call.

Source: Learnings

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@crates/perry-runtime/src/regex/compile.rs`:
- Around line 67-78: The regex compilation flow must root object-valued inputs
before coercions that may allocate or trigger GC. In
crates/perry-runtime/src/regex/compile.rs lines 67-78, use RuntimeHandleScope to
root pattern_val and flags_val, then reload both rooted values before
reject_symbol_to_string and js_string_coerce. In
crates/perry-runtime/src/object/regex_proto_thunks.rs lines 274-275, update
js_regexp_compile_value to receive the receiver as an f64 and root it
internally, or re-resolve the receiver from IMPLICIT_THIS after coercions; do
not retain an unrooted raw re pointer across the call.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d7311b3d-fa8d-4e31-a2e5-f4feb68dafa7

📥 Commits

Reviewing files that changed from the base of the PR and between a323f6d and 8fb9c25.

📒 Files selected for processing (3)

• crates/perry-runtime/src/object/global_this/proto_methods.rs
• crates/perry-runtime/src/object/regex_proto_thunks.rs
• crates/perry-runtime/src/regex/compile.rs