Summary: | [ES6] Add support for Symbol.replace. | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Keith Miller <keith_miller> | ||||||||||||||
Component: | New Bugs | Assignee: | Keith Miller <keith_miller> | ||||||||||||||
Status: | RESOLVED DUPLICATE | ||||||||||||||||
Severity: | Normal | CC: | buildbot, commit-queue, joepeck, keith_miller, rniwa | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Bug Depends on: | 151359 | ||||||||||||||||
Bug Blocks: | |||||||||||||||||
Attachments: |
|
Description
Keith Miller
2015-11-02 13:09:49 PST
Created attachment 264619 [details]
Patch
Created attachment 264623 [details]
Patch
Comment on attachment 264623 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=264623&action=review r=me > Source/JavaScriptCore/ChangeLog:12 > + In a future patch we need to update the code to reflectthe ES6 spec. reflect the Comment on attachment 264623 [details] Patch Attachment 264623 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/373567 New failing tests: fast/profiler/nested-start-and-stop-profiler.html Created attachment 264629 [details]
Archive of layout-test-results from ews106 for mac-mavericks-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Comment on attachment 264623 [details] Patch Attachment 264623 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/373585 New failing tests: fast/profiler/nested-start-and-stop-profiler.html Created attachment 264630 [details]
Archive of layout-test-results from ews112 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 264623 [details] Patch Attachment 264623 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/373626 New failing tests: storage/indexeddb/modern/idbobjectstore-get-failures.html fast/profiler/nested-start-and-stop-profiler.html Created attachment 264632 [details]
Archive of layout-test-results from ews102 for mac-mavericks
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102 Port: mac-mavericks Platform: Mac OS X 10.9.5
Hmm, rebasing appears to have broken things. Looking... Created attachment 265636 [details]
Patch
Attachment 265636 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:4052: One line control clauses should not use braces. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:4054: One line control clauses should not use braces. [whitespace/braces] [4]
Total errors found: 2 in 23 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 265636 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=265636&action=review > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:4054 > + } I was unsure what do do here. The NEXT/LAST_OPCODE macros use continue in them so they cannot be wrapped in a do-while loop yet they are multiline so they need braces. There might be a cleaner way to do this but I can't think of it :/ Comment on attachment 265636 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=265636&action=review The new feature code looks good to me. I don't get the fix to the pre-existing profiler bug in the DFG though. >> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:4054 >> + } > > I was unsure what do do here. The NEXT/LAST_OPCODE macros use continue in them so they cannot be wrapped in a do-while loop yet they are multiline so they need braces. There might be a cleaner way to do this but I can't think of it :/ I don't get this :(. The important question is whether we expect to execute the ret that follows the call. Is there really a case where A calls B and inlines B, and yet B will execute a ret? Also, it would be nice to use something more descriptive than a bool to say what happened in handleCall. I'm still not entirely clear one what we're trying to find out from handleCall, though. Comment on attachment 265636 [details]
Patch
Here's what we want:
(1) handleCall should return an enum that tells you whether the call is terminal, which is just a fancy word for a tail call at the level of generated code -- i.e., a tail call from a non-inlined caller or an inlined caller in tail position.
(2) Clients of handleCall that might issue a terminal call (which means op_tail_call and op_tail_call_varargs) should check for terminal, and if terminal, stop all bytecode parsing. This prevents stray bytecodes that are dead from entering the graph, which would trigger assertions and other weirdness.
(3) ByteCodeParser::parseBlock should no longer be permissive in its assertion, at the top, that we never parse past a terminal.
(4) ByteCodeParser::parseBlock for op_ret should no longer special-case the condition forbidden by (3), since it won't happen anymore.
These changes are complex enough that they should go into a separate patch.
Symbol.replace support now exists. I'm guessing it came in as part of: [ES] Implement RegExp.prototype.@@replace and use it for String.prototype.replace https://bugs.webkit.org/show_bug.cgi?id=156562 https://trac.webkit.org/changeset/200117 I'm going to close this bug as a dup. *** This bug has been marked as a duplicate of bug 156562 *** |