1306461 - Normalise and partially disable RegExp legacy features

Status: RESOLVED FIXED

(Core :: JavaScript: Standard Library, task, P3)

Description

[Claude Pache]

This bug is about implementing the following ES proposal
https://github.com/tc39/proposal-regexp-legacy-features

Here is the summary of the needed changes. For some of those, there is a low risk of breaking.

Normalisation
-------------
1. RegExp statics (RegExp.$1, etc.) must be configurable and non-enumerable (note: already implemented by Chrome).


Partially disabling RegExp#compile
----------------------------------
2. disable `RegExp.prototype.compile()` for cross-realm calls:
    `RegExp.prototype.compile.call(otherRealm_regexp)` must throw.

3. disable `RegExp.prototype.compile()` on instances of proper subclasses of RegExp:
    `var rx = new (class extends RegExp {})(''); rx.complile('x');` must throw.


Partially disabling RegExp statics
----------------------------------
4. disable RegExp statics after cross-realm matches:
    `RegExp.prototype.exec.call(otherRealm_regexp, '')` updates neither `RegExp.$1` nor `otherRealm_RegExp.$1`.

5. prevent RegExp statics to be accessed from any other object than RegExp:
    `Object.create(RegExp).$1` must throw.

6. disable RegExp statics for matches with instances of proper subclasses of RegExp:
    `var rx = new (class extends RegExp {})(''); rx.exec(''); RegExp.$1` must throw.

Comment 1

[Tooru Fujisawa [:arai]]

about updating RegExp static properties after RegExp#exec, here's the difference between implementations:

code:
  "abc".replace(/(.)/g, function() {
    console.log(RegExp.$1);
    return "";
  });

result:
  Firefox and Safari
    a
    b
    c

  Chrome
    c
    c
    c

this is because of that we're doing match and function call in the same loop, if that optimization is not observable (there's no modification to RegExp prototype and instance, see bug 1264264), to avoid creating a list of match result.

so, if RegExp static properties are added to the spec, the condition about optimizability won't be met in most case (since it's hard to check if RegExp static properties are not accessed inside the function).
I'll check how much performance regression could happen if we split the loop into 2, shortly.

Comment 2

[Tooru Fujisawa [:arai]]

Attachment: Comparison of String#replace between single loop and 2 loops

here's the comparison between the original m-c (before) and modified one that does match and function call separately (after),
with the code that replaces variable-length string.

so, it takes up to x1.7 time with the testcase, when the string gets longer and there are many matched objects.
of course it depends on the length of the list, so if the replace doesn't match so many places, there's almost no difference.

Comment 3

[Mark S. Miller]

> it takes up to x1.7 time with the testcase

This is an opportunity! If you add the optimization so that things are faster once the RegExp statics are deleted, and advertise the fact, it will encourage people to delete the RegExp statics as they should.

Comment 4

[Matthew Gaudet (he/him) [:mgaudet]]

Current Status: Failing the following Test262 tests:

test262/annexB/built-ins/RegExp/prototype/compile/this-cross-realm-instance.js
test262/annexB/built-ins/RegExp/prototype/compile/this-subclass-instance.js
test262/annexB/built-ins/RegExp/legacy-accessors/rightContext/this-subclass-constructor.js
test262/annexB/built-ins/RegExp/legacy-accessors/rightContext/prop-desc.js
test262/annexB/built-ins/RegExp/legacy-accessors/rightContext/this-not-regexp-constructor.js
test262/annexB/built-ins/RegExp/legacy-accessors/rightContext/this-cross-realm-constructor.js
test262/annexB/built-ins/RegExp/legacy-accessors/input/this-subclass-constructor.js
test262/annexB/built-ins/RegExp/legacy-accessors/input/prop-desc.js
test262/annexB/built-ins/RegExp/legacy-accessors/input/this-not-regexp-constructor.js
test262/annexB/built-ins/RegExp/legacy-accessors/input/this-cross-realm-constructor.js
test262/annexB/built-ins/RegExp/legacy-accessors/lastMatch/this-subclass-constructor.js
test262/annexB/built-ins/RegExp/legacy-accessors/lastMatch/this-cross-realm-constructor.js
test262/annexB/built-ins/RegExp/legacy-accessors/lastMatch/this-not-regexp-constructor.js
test262/annexB/built-ins/RegExp/legacy-accessors/lastMatch/prop-desc.js
test262/annexB/built-ins/RegExp/legacy-accessors/lastParen/this-subclass-constructor.js
test262/annexB/built-ins/RegExp/legacy-accessors/lastParen/this-cross-realm-constructor.js
test262/annexB/built-ins/RegExp/legacy-accessors/lastParen/prop-desc.js
test262/annexB/built-ins/RegExp/legacy-accessors/lastParen/this-not-regexp-constructor.js
test262/annexB/built-ins/RegExp/legacy-accessors/leftContext/this-subclass-constructor.js
test262/annexB/built-ins/RegExp/legacy-accessors/leftContext/prop-desc.js
test262/annexB/built-ins/RegExp/legacy-accessors/leftContext/this-cross-realm-constructor.js
test262/annexB/built-ins/RegExp/legacy-accessors/leftContext/this-not-regexp-constructor.js
test262/annexB/built-ins/RegExp/legacy-accessors/index/this-subclass-constructor.js
test262/annexB/built-ins/RegExp/legacy-accessors/index/this-cross-realm-constructor.js
test262/annexB/built-ins/RegExp/legacy-accessors/index/prop-desc.js
test262/annexB/built-ins/RegExp/legacy-accessors/index/this-not-regexp-constructor.js

Comment 5

[Serah Nderi]

Hi all,
I'm currently working on this as part of my Igalia Coding Experience. I’ll be pushing related commits to GitLab/Igalia and coordinating review through Phabricator once ready.

Thanks,
Serah Nderi

Comment 6

[Serah Nderi]

Attachment: Bug 1306461 - RegExp: Add Realm and LegacyFeaturesEnabled slots in RegExpAlloc, r=iain!

Comment 7

[Serah Nderi]

Attachment: Bug 1306461 - RegExp: Add Realm and LegacyFeaturesEnabled slots in RegExpAlloc, r=iain!

Comment 8

[Serah Nderi]

Attachment: Bug 1306461 - Normalise and partially disable RegExp legacy features, r=iain!

Comment 9

[Serah Nderi]

Attachment: Bug 1306461 - Fixing the macros to make the diff a bit smaller, r=iain!

Comment 10

[Serah Nderi]

Attachment: Bug 1306461 - Normalise and partially disable RegExp legacy features, r=iain!

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/9b7cbea13fbf
https://hg.mozilla.org/integration/autoland/rev/b1e96c949cbd
Normalise and partially disable RegExp legacy features, r=iain

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/d835cf9f440e
https://hg.mozilla.org/integration/autoland/rev/41cab394a465
Revert "Bug 1306461 - Normalise and partially disable RegExp legacy features, r=iain" for causing SM failures on Value.h

Comment 13

[Cristina Horotan [:chorotan]]

Backed out for causing SM failures on Value.h

Backout link

Push with failures

Failure log

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/aa2685ed07d7
https://hg.mozilla.org/integration/autoland/rev/16c284b959b5
Normalise and partially disable RegExp legacy features, r=iain

Comment 15

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/16c284b959b5

Comment 16

[Hamish Willee]

FF148 MDN docs can be tracked in https://github.com/mdn/content/issues/42746

I don't understand this change (at all). Is there any developer facing change, and if so, does it need to be captured in our docs and compatibility data? If so, can you spell it out for me, or I will hand this to a more JS experienced writer.

Would you like to propose a release note?

---

There is some info in https://bugzilla.mozilla.org/show_bug.cgi?id=1306461#c0 .

Taking just one thing from that

2. disable `RegExp.prototype.compile()` for cross-realm calls:
    `RegExp.prototype.compile.call(otherRealm_regexp)` must throw.

3. disable `RegExp.prototype.compile()` on instances of proper subclasses of RegExp:
    `var rx = new (class extends RegExp {})(''); rx.complile('x');` must throw.

If you look at https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/compile we could perhaps add exceptions for the throw case if needed. But not sure how we'd represent the compatibility data. It also doesn't feel all that important given this is marked as deprecated.

Comment 17

[Serah Nderi]

Hey Hamish, I had written a blog post on the implementation of the proposal: https://blogs.igalia.com/compilers/2026/01/20/legacy-regexp-features-in-javascript/ .

Here, I explain the change(s) I introduced to the codebase and the reasoning behind them. Now, as for if there's a developer facing this change and if it'd be needed to add to the developer docs, that I'm unsure but since the proposal says : "The design of our semantics is very simple: any attempt to use a feature when it is disabled will throw a TypeError." in https://github.com/tc39/proposal-regexp-legacy-features/blob/918a4b09723b34e4f857f10b4576028a8a02e97d/web-breaking-hazards.md

the deprecated RegExp.prototype.compile() documentation could potentially be updated to show the new throw cases. I wanted to provide this context, but an experienced JS writer might have a differing opinion on whether it's worth documenting given the deprecation status.