Summary: | [ES6] Allow RegExp constructor to take pattern from an existing RegExp with new flags | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Michael Saboff <msaboff> | ||||||||||||||
Component: | JavaScriptCore | Assignee: | Michael Saboff <msaboff> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | buildbot, commit-queue, keith_miller, mark.lam, rniwa, saam, webkit-bug-importer | ||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Attachments: |
|
Description
Michael Saboff
2016-03-10 12:12:48 PST
Created attachment 273597 [details]
Patch
Comment on attachment 273597 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=273597&action=review r=me with comments > Source/JavaScriptCore/runtime/RegExpConstructor.cpp:268 > + if (exec->hadException()) should be vm.exception() > Source/JavaScriptCore/runtime/RegExpConstructor.cpp:271 > + return vm.throwException(exec, createSyntaxError(exec, ASCIILiteral("Invalid flags supplied to RegExp constructor."))); Do we have tests for this exception? > Source/JavaScriptCore/runtime/RegExpConstructor.cpp:284 > + regExp = RegExp::create(vm, regExp->pattern(), flags); Maybe this should have an OOM exception check? Comment on attachment 273597 [details] Patch Attachment 273597 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/956056 New failing tests: sputnik/Conformance/15_Native_Objects/15.10_RegExp/15.10.4/S15.10.4.1_A8_T8.html sputnik/Conformance/15_Native_Objects/15.10_RegExp/15.10.4/S15.10.4.1_A2_T1.html sputnik/Conformance/15_Native_Objects/15.10_RegExp/15.10.3/S15.10.3.1_A2_T2.html sputnik/Conformance/15_Native_Objects/15.10_RegExp/15.10.3/S15.10.3.1_A2_T1.html sputnik/Conformance/15_Native_Objects/15.10_RegExp/15.10.4/S15.10.4.1_A2_T2.html Created attachment 273606 [details]
Archive of layout-test-results from ews103 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 273597 [details] Patch Attachment 273597 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/956064 New failing tests: sputnik/Conformance/15_Native_Objects/15.10_RegExp/15.10.4/S15.10.4.1_A8_T8.html sputnik/Conformance/15_Native_Objects/15.10_RegExp/15.10.4/S15.10.4.1_A2_T1.html sputnik/Conformance/15_Native_Objects/15.10_RegExp/15.10.3/S15.10.3.1_A2_T2.html sputnik/Conformance/15_Native_Objects/15.10_RegExp/15.10.3/S15.10.3.1_A2_T1.html sputnik/Conformance/15_Native_Objects/15.10_RegExp/15.10.4/S15.10.4.1_A2_T2.html Created attachment 273607 [details]
Archive of layout-test-results from ews107 for mac-yosemite-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 273597 [details] Patch Attachment 273597 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/956072 New failing tests: sputnik/Conformance/15_Native_Objects/15.10_RegExp/15.10.4/S15.10.4.1_A8_T8.html sputnik/Conformance/15_Native_Objects/15.10_RegExp/15.10.4/S15.10.4.1_A2_T1.html sputnik/Conformance/15_Native_Objects/15.10_RegExp/15.10.3/S15.10.3.1_A2_T2.html sputnik/Conformance/15_Native_Objects/15.10_RegExp/15.10.3/S15.10.3.1_A2_T1.html sputnik/Conformance/15_Native_Objects/15.10_RegExp/15.10.4/S15.10.4.1_A2_T2.html Created attachment 273609 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.10.5
(In reply to comment #3) > Comment on attachment 273597 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=273597&action=review > > r=me with comments > > > Source/JavaScriptCore/runtime/RegExpConstructor.cpp:268 > > + if (exec->hadException()) > > should be vm.exception() I made this change throughout the function. > > Source/JavaScriptCore/runtime/RegExpConstructor.cpp:271 > > + return vm.throwException(exec, createSyntaxError(exec, ASCIILiteral("Invalid flags supplied to RegExp constructor."))); > > Do we have tests for this exception? Added. > > Source/JavaScriptCore/runtime/RegExpConstructor.cpp:284 > > + regExp = RegExp::create(vm, regExp->pattern(), flags); > > Maybe this should have an OOM exception check? Added. I'll fix the sputnik tests and land. Comment on attachment 273597 [details] Patch Attachment 273597 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/956089 New failing tests: sputnik/Conformance/15_Native_Objects/15.10_RegExp/15.10.4/S15.10.4.1_A8_T8.html sputnik/Conformance/15_Native_Objects/15.10_RegExp/15.10.4/S15.10.4.1_A2_T1.html sputnik/Conformance/15_Native_Objects/15.10_RegExp/15.10.3/S15.10.3.1_A2_T2.html sputnik/Conformance/15_Native_Objects/15.10_RegExp/15.10.3/S15.10.3.1_A2_T1.html sputnik/Conformance/15_Native_Objects/15.10_RegExp/15.10.4/S15.10.4.1_A2_T2.html Created attachment 273613 [details]
Archive of layout-test-results from ews114 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 273616 [details]
Patch for Landing - Made suggested changes, removed obsolete sputnik tests
Committed r197962: <http://trac.webkit.org/changeset/197962> Comment on attachment 273616 [details] Patch for Landing - Made suggested changes, removed obsolete sputnik tests View in context: https://bugs.webkit.org/attachment.cgi?id=273616&action=review > Source/JavaScriptCore/runtime/RegExpConstructor.cpp:265 > + RegExpFlags flags = NoFlags; > + bool haveFlags = false; I think it would be elegant to use Optional<RegExpFlags> rather than a separate boolean. Might return to do that refactoring later. > Source/JavaScriptCore/runtime/RegExpConstructor.cpp:269 > + return 0; nullptr |