Skip to content

Editorial: set [[SourceText]] for async arrow functions#1548

Merged
ljharb merged 1 commit intotc39:masterfrom
jmdyck:async_arrow_SourceText
Jun 2, 2019
Merged

Editorial: set [[SourceText]] for async arrow functions#1548
ljharb merged 1 commit intotc39:masterfrom
jmdyck:async_arrow_SourceText

Conversation

@jmdyck
Copy link
Collaborator

@jmdyck jmdyck commented May 25, 2019

These lines didn't appear in #697.
@ljharb says that was likely an oversight:

@ljharb ljharb requested review from a team, michaelficarra and zenparsing May 26, 2019 14:50
michaelficarra
michaelficarra approved these changes May 29, 2019
View reviewed changes
Copy link
Member

@michaelficarra michaelficarra left a comment

Choose a reason for hiding this comment

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

Haha there's too many ways to create functions! I got async FDs, async FEs, and async methods, but I missed async arrows. LGTM.

@jmdyck
Copy link
Collaborator Author

jmdyck commented May 29, 2019

After looking some more, I'm thinking that the NamedEvaluation of a ClassExpression should also have a Set ...[[SourceText]]. If you agree, I can add that to this PR.

@ljharb
Copy link
Member

ljharb commented May 29, 2019

Seems right to me - are there any other cases?

@michaelficarra
Copy link
Member

michaelficarra commented May 29, 2019

@jmdyck That NamedEvaluation algorithm definition for ClassExpression is a bit strange. Compare it to all of the others: they just defer to the evaluation algorithm and then call SetFunctionName. We should probably modify NamedEvaluation for ClassExpression to do the same. I'd do it in a separate PR.

@michaelficarra michaelficarra mentioned this pull request May 29, 2019
Closed
@jmdyck
Copy link
Collaborator Author

jmdyck commented May 29, 2019

Yeah, I noticed that strangeness, but I don't know enough to question it yet.

@ljharb ljharb self-assigned this May 29, 2019
@jmdyck
Copy link
Collaborator Author

jmdyck commented Jun 1, 2019

are there any other cases?

No, I don't think so.

@jmdyck @ljharb
Editorial: set [[SourceText]] for async arrow functions (tc39#1548)
fea0b84 
These lines didn't appear in tc39#697.
`@ljharb` says that was likely an oversight:
tc39#1458 (comment)
@ljharb ljharb force-pushed the async_arrow_SourceText branch from ea88962 to fea0b84 Compare June 2, 2019 04:53
@ljharb ljharb merged commit fea0b84 into tc39:master Jun 2, 2019
@jmdyck jmdyck deleted the async_arrow_SourceText branch June 2, 2019 14:57
@jmdyck
Copy link
Collaborator Author

jmdyck commented Jun 4, 2019

(Just noticed that @anba included these additions in the first commit of PR #1490. @michaelficarra: That commit also handles the NamedEvaluation of a ClassExpression, discussed above.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ljharb ljharb ljharb approved these changes

@michaelficarra michaelficarra michaelficarra approved these changes

+1 more reviewer

@zenparsing zenparsing zenparsing approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@ljharb ljharb

Labels

editorial change spec bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jmdyck @ljharb @michaelficarra @zenparsing