Bug 146262

Summary:

test262: class and function names should be inferred in assignment

Product:

WebKit

Reporter:

Ryosuke Niwa <rniwa>

Component:

JavaScriptCore

Assignee:

Joseph Pecoraro <joepeck>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

benjamin, buildbot, commit-queue, ggaren, joepeck, keith_miller, kling, mark.lam, msaboff, rniwa, saam, ysuzuki

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
[PATCH] Proposed Fix	none
[PATCH] Start of Fix	none
[PATCH] Proposed Fix	none
[PATCH] Proposed Fix	none
Archive of layout-test-results from ews105 for mac-yosemite-wk2	none

Description Ryosuke Niwa 2015-06-23 16:48:18 PDT

It looks like ES6 spec requires the inferred name.
Since we already compute that for Inspector, we should just return it.
We're missing 14 out of 17 points on https://kangax.github.io/compat-table/es6/ :(

Comment 1 Joseph Pecoraro 2016-09-29 02:18:48 PDT

I've started work on this. Just a basic implementation gets us 650+ test262 tests.

Comment 2 Joseph Pecoraro 2016-09-29 02:24:06 PDT

Created attachment 290191 [details]
[PATCH] Proposed Fix

Comment 3 Joseph Pecoraro 2016-09-29 02:24:57 PDT

Comment on attachment 290191 [details]
[PATCH] Proposed Fix

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

> JSTests/stress/inferred-names.js:34
> +// assert( ({["func"]: ()=>{}}).func.name === "func" );

Not putting r? yet because of this case (arrow function in computed property assignment) that didn't seem to infer name but normal function did. Funny, I didn't see this covered in test262.

Comment 4 Joseph Pecoraro 2016-09-29 02:28:08 PDT

Created attachment 290192 [details]
[PATCH] Start of Fix

Comment 5 Joseph Pecoraro 2016-09-29 02:59:56 PDT

Created attachment 290193 [details]
[PATCH] Proposed Fix

Never got to the bottom of that issue, but filed a separate bug on it. It would seem to be a runtime thing, not a parse time.

Comment 6 Joseph Pecoraro 2016-09-29 03:18:11 PDT

Created attachment 290195 [details]
[PATCH] Proposed Fix

Doing the same for class gets us another ~160

Comment 7 Build Bot 2016-09-29 04:44:19 PDT

Comment on attachment 290195 [details]
[PATCH] Proposed Fix

Attachment 290195 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/2167737

New failing tests:
fast/images/pdf-as-image-with-annotations.html

Comment 8 Build Bot 2016-09-29 04:44:23 PDT

Created attachment 290199 [details]
Archive of layout-test-results from ews105 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

Comment 9 Joseph Pecoraro 2016-09-29 11:34:42 PDT

Comment on attachment 290195 [details]
[PATCH] Proposed Fix

Test failure is unrelated.

Comment 10 Saam Barati 2016-09-29 11:41:45 PDT

Comment on attachment 290195 [details]
[PATCH] Proposed Fix

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

r=me

> Source/JavaScriptCore/parser/ASTBuilder.h:346
> +        if (rhs->isBaseFuncExprNode()) {

Nice, this removes a virtual call

Comment 11 WebKit Commit Bot 2016-09-29 12:04:06 PDT

Comment on attachment 290195 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 290195

Committed r206599: <http://trac.webkit.org/changeset/206599>

Comment 12 WebKit Commit Bot 2016-09-29 12:04:12 PDT

All reviewed patches have been landed.  Closing bug.