Function.prototype.toString should not decompile the AST

• Component: New Bugs
• Status: RESOLVED FIXED

[Geoffrey Garen]

Function.prototype.toString should not decompile the AST

[Geoffrey Garen]

Created attachment 249077 [details]
Patch

[Geoffrey Garen]

Created attachment 249081 [details]
Patch

[Build Bot]

Comment on attachment 249081 [details]
Patch

Attachment 249081 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5132974664187904

New failing tests:
userscripts/window-onerror-for-isolated-world-1.html
userscripts/window-onerror-for-isolated-world-2.html

[Build Bot]

Created attachment 249086 [details]
Archive of layout-test-results from ews100 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-mavericks  Platform: Mac OS X 10.9.5

[Geoffrey Garen]

Created attachment 249121 [details]
Patch

[Geoffrey Garen]

Committed r181810: <http://trac.webkit.org/changeset/181810>

[Darin Adler]

Comment on attachment 249121 [details]
Patch

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

> Source/JavaScriptCore/runtime/FunctionConstructor.cpp:102
> +        builder.append("(");

append('(')

> Source/JavaScriptCore/runtime/FunctionConstructor.cpp:105
> +            builder.append(", ");

appendLiteral(", ")

[Joseph Pecoraro]

Comment on attachment 249121 [details]
Patch

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

> LayoutTests/js/class-syntax-default-constructor-expected.txt:14
> -FAIL A.prototype.constructor should be function B() { super(...arguments); }. Was function A() { }.
> +FAIL A.prototype.constructor should be function () { super(...arguments); }. Was function () { }.

Looks like this was Failing before, and needed to be updated.

> LayoutTests/js/object-literal-computed-methods-expected.txt:11
> +FAIL o.foo.toString() should be function () { return 10; }. Was () { return 10; }.

These are regressions.

> LayoutTests/js/object-literal-methods-expected.txt:11
> +FAIL o.foo.toString() should be function foo() { return 10; }. Was () { return 10; }.

Ditto.

[Joseph Pecoraro]

Comment on attachment 249121 [details]
Patch

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

> LayoutTests/ChangeLog:43
> +        * js/object-literal-methods-expected.txt: These tests fail because object
> +        literal methods do not register their function names appropriately. This
> +        was a pre-existing failure that is now more explicit.

Oh!

[Geoffrey Garen]

Committed r182043: <http://trac.webkit.org/changeset/182043>

[Andreas Kling]

Comment on attachment 249121 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:13
> +        (1) It requires us to keep pieces of the AST live forever. This is an
> +        awkward design and a waste of memory.

This patch does not fix that issue though, right?

[Geoffrey Garen]

> > Source/JavaScriptCore/ChangeLog:13
> > +        (1) It requires us to keep pieces of the AST live forever. This is an
> > +        awkward design and a waste of memory.
> 
> This patch does not fix that issue though, right?

No. We need one more patch to change how we re-parse functions, and then we'll be able to remove the refcounted parts of the AST.

[Michael Saboff]

This change fixed rdar://problem/18828133.