Bug 130877 - Support spread operand in |new| expressions
Summary: Support spread operand in |new| expressions
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Oliver Hunt
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-03-27 17:05 PDT by Oliver Hunt
Modified: 2014-03-30 18:34 PDT (History)
1 user (show)

See Also:


Attachments
Patch (32.83 KB, patch)
2014-03-27 17:10 PDT, Oliver Hunt
msaboff: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Hunt 2014-03-27 17:05:00 PDT
Support spread operand in |new| expressions
Comment 1 Oliver Hunt 2014-03-27 17:10:21 PDT
Created attachment 228010 [details]
Patch
Comment 2 Michael Saboff 2014-03-27 18:05:26 PDT
Comment on attachment 228010 [details]
Patch

r=me
Any reason you didn't split this into the refactor and then the spread?
Comment 3 Oliver Hunt 2014-03-27 18:09:27 PDT
(In reply to comment #2)
> (From update of attachment 228010 [details])
> r=me
> Any reason you didn't split this into the refactor and then the spread?

Because the only point of the refactor was so i could share code for op_construct_varargs i think the only thing that really counted as a refactor was emjtcompilevarargs in byecodegenerator
Comment 4 Oliver Hunt 2014-03-27 18:10:52 PDT
Committed r166392: <http://trac.webkit.org/changeset/166392>
Comment 5 Simon Fraser (smfr) 2014-03-28 11:10:18 PDT
This broke the CLOOP build.
Comment 6 Filip Pizlo 2014-03-30 18:34:13 PDT
Comment on attachment 228010 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=228010&action=review

> LayoutTests/js/regress/script-tests/call-spread.js:34
> +
> +function testFunction(a, b)
> +{
> +    "use strict"
> +    result |= 0;
> +    return a * 1000 + b * 100 + arguments[2] * 10 + arguments.length ^ (result & 1024);
> +}
> +
> +var arrayArguments = [2, 3, 4]
> +var result = 0;
> +for (var i = 0; i < 1000000; i++)
> +    result += testFunction(...arrayArguments);
> +
> +for (var i = 0; i < 1000000; i++)
> +    result += testFunction(...[1, 2, result, 4]);
> +
> +function test2() {
> +    for (var i = 0; i < 1000000; i++)
> +        result += testFunction(...arguments);
> +}
> +
> +test2(1,2,3,4)
> +
> +
> +function test3() {
> +    aliasedArguments = arguments;
> +    for (var i = 0; i < 1000000; i++)
> +        result += testFunction(...aliasedArguments);
> +}
> +
> +test3(1,2,result,4)
> +
> +if (result != -856444619779264)
> +    throw "Result was " + result + " expected -856444619779264";

Is there a reason why this test needs to run for 30 seconds?

> LayoutTests/js/regress/script-tests/new-spread.js:34
> +
> +function testFunction(a, b)
> +{
> +    "use strict"
> +    result |= 0;
> +    result += a * 1000 + b * 100 + arguments[2] * 10 + arguments.length ^ (result & 1024);
> +}
> +
> +var arrayArguments = [2, 3, 4]
> +var result = 0;
> +for (var i = 0; i < 1000000; i++)
> +    new testFunction(...arrayArguments);
> +
> +for (var i = 0; i < 1000000; i++)
> +    new testFunction(...[1, 2, result, 4]);
> +
> +function test2() {
> +    for (var i = 0; i < 1000000; i++)
> +        new testFunction(...arguments);
> +}
> +
> +test2(1,2,3,4)
> +
> +
> +function test3() {
> +    aliasedArguments = arguments;
> +    for (var i = 0; i < 1000000; i++)
> +        new testFunction(...aliasedArguments);
> +}
> +
> +test3(1,2,result,4)
> +
> +if (result != -2371153088)
> +    throw "Result was " + result + " expected -2371153088";

Is there a reason why this test next to run for 40 seconds?