Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "name" property for classes as part of ClassDefinitionEvaluation #1372

Merged
merged 1 commit into from
Feb 6, 2019

Conversation

anba
Copy link
Contributor

@anba anba commented Dec 17, 2018

SetFunctionName is currently called after ClassDefinitionEvaluation, which means the class name is not available in static class field initializers (tc39/proposal-class-fields#85) and may cause issues for a @frozen decorator (tc39/proposal-decorators#168).

This PR moves the initialization of the "name" property into ClassDefinitionEvaluation, which should avoid both aforementioned issues. I've split this PR into three parts for easier reviewing:

  • The first commit 7bb51dd adds calls to a new runtime semantic NamedEvaluation to evaluate anonymous function definitions (as identified by IsAnonymousFunctionDefinition).

  • 3d752d7 adds the actual definitions for NamedEvaluation.

  • And the last commit 03fa5ba moves the "name" property assignment into ClassDefinitionEvaluation.

This PR doesn't result in any observable changes for non-class anonymous function definitions. For class definitions (with explicit and inferred names), the "name" property is now defined earlier, which is observable when calling [[OwnPropertyKeys]]. I don't expect to see any web-compatibility issues for this change given that all major engines already return the property keys of classes in a different order:

function printNames(kind, c) {
    c.foo = 0;
    print(`${kind}: ${Object.getOwnPropertyNames(c)}`);
}

// SM: prototype,m,foo,length,name
// JSC: length,name,prototype,m,foo
// V8: length,prototype,m,name,foo
// CH: prototype,length,name,m,foo
//
// Spec: length,prototype,m,name,foo
// Proposed: length,prototype,name,m,foo
printNames("class", class C { static m(){} });

// For comparison a normal function (Strict mode enabled to avoid non-standard arguments and caller.)
// SM: foo,prototype,length,name
// JSC: length,name,foo,prototype
// V8: length,name,prototype,foo
// CH: prototype,length,name,foo
//
// Spec: length,prototype,name,foo
printNames("function", function f(){ "use strict" });

@ljharb ljharb requested review from zenparsing, ljharb, bterlson and a team December 17, 2018 22:41
@ljharb ljharb added the normative change Affects behavior required to correctly evaluate some ECMAScript source text label Dec 17, 2018
@ljharb ljharb added needs consensus This needs committee consensus before it can be eligible to be merged. needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 labels Jan 2, 2019
Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grammar's not my strong suit, so I'll defer to others for that part of the review.

spec.html Show resolved Hide resolved
spec.html Show resolved Hide resolved
spec.html Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Show resolved Hide resolved
spec.html Show resolved Hide resolved
@ljharb ljharb added has consensus This has committee consensus. and removed needs consensus This needs committee consensus before it can be eligible to be merged. labels Jan 29, 2019
@ljharb ljharb requested review from waldemarhorwat and a team January 29, 2019 19:09
@littledan
Copy link
Member

We got consensus on this change in the January 2019 TC39 meeting! Thanks everybody.

@anba Would you be interested in writing tests for this change?

@anba
Copy link
Contributor Author

anba commented Jan 30, 2019

Created tc39/test262#2057

leobalter pushed a commit to tc39/test262 that referenced this pull request Jan 30, 2019
@ljharb ljharb added has test262 tests and removed needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 labels Jan 30, 2019
@targos targos mentioned this pull request Feb 4, 2019
14 tasks
@ljharb ljharb assigned ljharb and unassigned bterlson Feb 6, 2019
…Evaluation (tc39#1372)

 - Add definition for NamedEvaluation
 - Add NamedEvaluation operation when evaluating anonymous functions
 - Call SetFunctionName as part of ClassDefinitionEvaluation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has consensus This has committee consensus. has test262 tests normative change Affects behavior required to correctly evaluate some ECMAScript source text
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants