Closed Bug 1826574 Opened 1 year ago Closed 1 year ago

Implement RegExp v flag with set notation

Categories

(Core :: JavaScript Engine, task, P2)

task

Tracking

()

RESOLVED FIXED
116 Branch
Tracking Status
firefox116 --- fixed

People

(Reporter: dminor, Assigned: dminor)

References

(Blocks 2 open bugs)

Details

Attachments

(4 files)

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

Priority: -- → P2
Blocks: regexp-v-flag
No longer blocks: 1519168
Duplicate of this bug: 1830766

Depends on D178941

Status: NEW → ASSIGNED
Pushed by dminor@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8653abdc9503
Enable test262 tests for Unicode Sets; r=iain

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]
Flags: needinfo?(dminor)

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

Flags: needinfo?(dminor)
Pushed by dminor@mozilla.com:
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

(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),

(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

Flags: needinfo?(arai.unmht)

(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.

(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 :)

Flags: needinfo?(arai.unmht)

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

Flags: needinfo?(dminor)
Pushed by dminor@mozilla.com:
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
Regressions: 1837926
Regressions: 1838499
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: