Editorial: Eliminate CoveredFoo SDOs#2350
Merged
ljharb merged 5 commits intotc39:masterfrom Mar 24, 2021
Merged
Conversation
bakkot
approved these changes
Mar 20, 2021
Member
bakkot
left a comment
bakkot
left a comment
There was a problem hiding this comment.
This is a nice simplification, thanks.
| 1. Let _sourceText_ be the source text matched by |ArrowFunction|. | ||
| 1. Let _parameters_ be CoveredFormalsList of |ArrowParameters|. | ||
| 1. [id="step-arrowfunction-evaluation-functioncreate"] Let _closure_ be OrdinaryFunctionCreate(%Function.prototype%, _sourceText_, _parameters_, |ConciseBody|, ~lexical-this~, _scope_). | ||
| 1. [id="step-arrowfunction-evaluation-functioncreate"] Let _closure_ be OrdinaryFunctionCreate(%Function.prototype%, _sourceText_, |ArrowParameters|, |ConciseBody|, ~lexical-this~, _scope_). |
Member
There was a problem hiding this comment.
This is the only change which is suspicious, but as far as I can tell it all works: although this has the effect of changing the [[FormalParameters]] slot of arrow functions from containing an |ArrowFormalParameters| to containing a |CoverParenthesizedExpressionAndArrowParameterList|, it appears to be the case that all consumers of [[FormalParameters]] are already equipped to handle |CoverParenthesizedExpressionAndArrowParameterList| correctly.
syg
approved these changes
Mar 24, 2021
michaelficarra
approved these changes
Mar 24, 2021
bakkot
added
the
ready to merge
Here in InstantiateArrowFunctionExpression,
I assert that we can replace the status quo's
CoveredFormalsList of |ArrowParameters|
with just
|ArrowParameters|
(and then eliminate the _parameters_ alias).
case analysis on the 2 forms of |ArrowParameters|:
case 1:
When |ArrowParameters| is an instance of
ArrowParameters : BindingIdentifier
then CoveredFormalsList simply returns |ArrowParameters| itself,
so the above replacement is obviously correct.
case 2:
When |ArrowParameters| is an instance of
ArrowParameters : CoverParenthesizedExpressionAndArrowParameterList
then CoveredFormalsList will return an instance of
ArrowFormalParameters
(whose particulars are not important here),
which then gets passed to OrdinaryFunctionCreate's ParameterList.
But if you go to OrdinaryFunctionCreate and follow what happens
to ParameterList, you find that, ultimately, it's submitted to:
- ExpectedArgumentCount
- BoundNames
- IsSimpleParameterList
- ContainsExpression
- IteratorBindingInitialization
And all of these have an explicit definition for:
ArrowParameters : CoverParenthesizedExpressionAndArrowParameterList
of roughly the form:
1. Let _formals_ be the CoveredFormalsList of |CoverParenthesizedExpressionAndArrowParameterList|.
1. Return <SDO> of _formals_.
That is, with respect to the OrdinaryFunctionCreate's call-tree,
if you don't call CoveredFormalsList near the root,
it will be called near the leaves.
So in this case too, it's okay to perform the given replacement.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When the first two of these were introduced (in ES6), they were useful because they encapsulated a sizeable amount of pseudocode. But after successive rewordings (in PRs #692, #971, #890), the underlying pseudocode is almost as brief as invoking the SDO. So I think their benefit (in terms of conciseness or encapsulation) is now outweighed by their cost (in terms of indirection); eliminating them will make it slightly easier to understand how the spec uses cover grammars.
(For comparison, note that we don't bother to define similar SDOs for:
the |AssignmentPattern| that is covered by |LeftHandSideExpression|or
the |AssignmentPattern| that is covered by |DestructuringAssignmentTarget|We just use those phrases directly in algorithms.)
This PR undefines the following ids, because there didn't seem to be a good place to make them oldids: