1826574 - Implement RegExp v flag with set notation

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P2)

Description

[Dan Minor [:dminor]]

RegExp v flag with set notation is in stage 3, we should implement it.

Comment 2

[Dan Minor [:dminor]]

Attachment: Bug 1826574 - Enable test262 tests for Unicode Sets; r=iain

Comment 3

[Dan Minor [:dminor]]

Attachment: Bug 1826574 - Add support for Unicode Sets; r=iain

Depends on D178941

Comment 4

[Dan Minor [:dminor]]

Attachment: Bug 1826574 - Update expectation for debug/Debugger-onNativeCall-06.js; r=iain

Depends on D178942

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8653abdc9503
Enable test262 tests for Unicode Sets; r=iain

Comment 6

[Cristian Tuns]

Backed out for causing SM bustages in test262/*

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | test262/built-ins/RegExp/unicodeSets/generated/character-class-escape-difference-character-class.js | (args: "--dll /builds/worker/fetches/injector/libbreakpadinjector.so") [0.0 s]

Comment 7

[Dan Minor [:dminor]]

I accidentally landed only the first patch in the set :(

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5ece69e22ffd
Enable test262 tests for Unicode Sets; r=iain
https://hg.mozilla.org/integration/autoland/rev/47a50e1c3e11
Add support for Unicode Sets; r=iain
https://hg.mozilla.org/integration/autoland/rev/e886a7d39892
Update expectation for debug/Debugger-onNativeCall-06.js; r=iain

Comment 9

[Noemi Erli[:noemi_erli]]

Backed out 3 changesets (Bug 1826574) for causing failures in browser_script_command_execute_basic.js CLOSED TREE

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&collapsedPushes=842382&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&searchStr=linux%2C18.04%2Cx64%2Cwebrender%2Cdebug%2Cmochitests%2Cwith%2Chttp%2F3%2Cserver%2Ctest-linux1804-64-qr%2Fdebug-mochitest-devtools-chrome-http3%2Cdt3&revision=e886a7d39892474dd39d5d6a7b6b063be23d7ab5&selectedTaskRun=HDjIqQQFRYyg8tF08-kvoQ.0

Log: https://treeherder.mozilla.org/logviewer?job_id=417536082&repo=autoland&lineNumber=2907
https://treeherder.mozilla.org/logviewer?job_id=417538017&repo=autoland&lineNumber=128136

Backout: https://hg.mozilla.org/integration/autoland/rev/89f599e3ab6aeda059cb121a4baa443cff361a23

Comment 10

[Nicolas Chevobbe [:nchevobbe]]

(In reply to Noemi Erli[:noemi_erli] from comment #9)

Backed out 3 changesets (Bug 1826574) for causing failures in browser_script_command_execute_basic.js CLOSED TREE

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&collapsedPushes=842382&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&searchStr=linux%2C18.04%2Cx64%2Cwebrender%2Cdebug%2Cmochitests%2Cwith%2Chttp%2F3%2Cserver%2Ctest-linux1804-64-qr%2Fdebug-mochitest-devtools-chrome-http3%2Cdt3&revision=e886a7d39892474dd39d5d6a7b6b063be23d7ab5&selectedTaskRun=HDjIqQQFRYyg8tF08-kvoQ.0

Log: https://treeherder.mozilla.org/logviewer?job_id=417536082&repo=autoland&lineNumber=2907
https://treeherder.mozilla.org/logviewer?job_id=417538017&repo=autoland&lineNumber=128136

Backout: https://hg.mozilla.org/integration/autoland/rev/89f599e3ab6aeda059cb121a4baa443cff361a23

This is a devtools test failing here. eager evaluation of /a/giy.flags now returns undefined.
From what I can tell, nativeIsEagerlyEvaluateable (https://searchfox.org/mozilla-central/rev/8dd0d2bebe4c897152da6c86d937e4be80bbaa54/devtools/server/actors/webconsole/eval-with-debugger.js#472), which is called from Debugger.onNativeCall, is triggered with unicodeSets, and since it's not in the allow list of RegExp getter, makes the eager evaluation to bail.

The following changes make the test pass again:

diff --git a/devtools/server/actors/webconsole/eager-ecma-allowlist.js b/devtools/server/actors/webconsole/eager-ecma-allowlist.js
--- a/devtools/server/actors/webconsole/eager-ecma-allowlist.js
+++ b/devtools/server/actors/webconsole/eager-ecma-allowlist.js
@@ -217,6 +217,7 @@ const getterAllowList = [
   getter(RegExp.prototype, "source"),
   getter(RegExp.prototype, "sticky"),
   getter(RegExp.prototype, "unicode"),
+  getter(RegExp.prototype, "unicodeSets"),
   getter(RegExp, Symbol.species),
   getter(Set.prototype, "size"),
   getter(Set, Symbol.species),

Comment 11

[Nicolas Chevobbe [:nchevobbe]]

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #10)

From what I can tell, nativeIsEagerlyEvaluateable (https://searchfox.org/mozilla-central/rev/8dd0d2bebe4c897152da6c86d937e4be80bbaa54/devtools/server/actors/webconsole/eval-with-debugger.js#472), which is called from Debugger.onNativeCall, is triggered with unicodeSets, and since it's not in the allow list of RegExp getter, makes the eager evaluation to bail.

arai, is this expected that evaluating /a/giy.flags would trigger onNativeCall with all existing RegExp getters (flags,hasIndices,global,ignoreCase,multiline,dotAll,unicode,unicodeSets,sticky,unicode,unicodeSets) ?

That's a bit surprising to me, I'd expect to only have the getters we're trying to execute

Comment 12

[Mathias Bynens]

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #11)

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #10)

From what I can tell, nativeIsEagerlyEvaluateable (https://searchfox.org/mozilla-central/rev/8dd0d2bebe4c897152da6c86d937e4be80bbaa54/devtools/server/actors/webconsole/eval-with-debugger.js#472), which is called from Debugger.onNativeCall, is triggered with unicodeSets, and since it's not in the allow list of RegExp getter, makes the eager evaluation to bail.

arai, is this expected that evaluating /a/giy.flags would trigger onNativeCall with all existing RegExp getters (flags,hasIndices,global,ignoreCase,multiline,dotAll,unicode,unicodeSets,sticky,unicode,unicodeSets) ?

That's a bit surprising to me, I'd expect to only have the getters we're trying to execute

I'm not arai but I think I can answer this one. The spec for the flags getter (https://tc39.es/ecma262/#sec-get-regexp.prototype.flags) mandates that each getter is accessed in order to construct the flags string.

Comment 13

[Nicolas Chevobbe [:nchevobbe]]

(In reply to Mathias Bynens from comment #12)

I'm not arai but I think I can answer this one. The spec for the flags getter (https://tc39.es/ecma262/#sec-get-regexp.prototype.flags) mandates that each getter is accessed in order to construct the flags string.

thanks for the answer, mystery solved :)

Comment 14

[Dan Minor [:dminor]]

Thanks for the extra info :) I'll reland this after the soft freeze is over.

Comment 15

[Dan Minor [:dminor]]

Attachment: Bug 1826574 - Add unicodeSets getter to eager-ecma-allowlist.js; r=nchevobbe!

Depends on D178943

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2c8973e3bfd5
Enable test262 tests for Unicode Sets; r=iain
https://hg.mozilla.org/integration/autoland/rev/adab81d2c9eb
Add support for Unicode Sets; r=iain
https://hg.mozilla.org/integration/autoland/rev/05729e7c441b
Update expectation for debug/Debugger-onNativeCall-06.js; r=iain
https://hg.mozilla.org/integration/autoland/rev/cefcde9bc1ca
Add unicodeSets getter to eager-ecma-allowlist.js; r=nchevobbe,devtools-reviewers

Comment 17

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/2c8973e3bfd5
https://hg.mozilla.org/mozilla-central/rev/adab81d2c9eb
https://hg.mozilla.org/mozilla-central/rev/05729e7c441b
https://hg.mozilla.org/mozilla-central/rev/cefcde9bc1ca