Bug 150812 – [ES6] Add support for Symbol.replace.

• Component: New Bugs
• Status: RESOLVED DUPLICATE

[Keith Miller]

[ES6] Add support for Symbol.replace.

[Keith Miller]

Created attachment 264619 [details]
Patch

[Keith Miller]

Created attachment 264623 [details]
Patch

[Geoffrey Garen]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Keith Miller]

Hmm, rebasing appears to have broken things. Looking...

[Keith Miller]

Created attachment 265636 [details]
Patch

[WebKit Commit Bot]

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.

[Keith Miller]

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 :/

[Geoffrey Garen]

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.

[Geoffrey Garen]

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.

[Joseph Pecoraro]

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 ***