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

Editorial: missing syntax error (involving CoverCallExpressionAndAsyncArrowHead) #2607

Open
jmdyck opened this issue Jan 3, 2022 · 3 comments

Comments

@jmdyck
Copy link
Collaborator

jmdyck commented Jan 3, 2022

In test262-parser-tests, fail/4e885526e8dfaa12.js indicates that an ES parser should raise an (early) syntax error for:

f({x = 0})

I agree that it should, but I think the spec fails to require this.


Presumably the intended syntax error hinges on the fact that {x = 0} is parsed as an instance of the production:

PropertyDefinition : CoverInitializedName

and 13.2.5.1 has an Early Error (EE) rule associated with that production that says:

It is a Syntax Error if any source text is matched by this production.

However, that rule is preceded by a paragraph that gives various conditions under which that rule is not applied, and one of those conditions is "when initially parsing a ... CoverCallExpressionAndAsyncArrowHead". And indeed, the whole expression is a CallExpression, and is parsed via the production:

CallExpression : CoverCallExpressionAndAsyncArrowHead

so it seems we're obliged to not apply the EE rule, and so not raise the Syntax Error.


The "initially parsing" sentence goes back to ES6, but the CoverCallEtc part was added by PR #775 in commit 993e8e4, where it's definitely intentional. However, I think only the AsyncArrowHead side of things was being considered. That is, for the production:

AsyncArrowFunction : CoverCallExpressionAndAsyncArrowHead

there's an EE rule that requires the CoverCallEtc to cover an AsyncArrowHead. So in that case, it's fine to not apply EE rules on the initial parse of the CoverCallEtc, because it will be re-parsed (as an AsyncArrowHead), which will then apply the EE rules that are appropriate for an AsyncArrowHead.

But consider the CallExpression side of things. By symmetry, you might expect the production:

CallExpression : CoverCallExpressionAndAsyncArrowHead

to have a corresponding EE rule that requires CoverCallEtc to cover something, making it okay to not apply EE rules on the initial parse of CoverCallEtc. But there is no such rule.

In the whole spec, the only semantics associated with that production is an Evaluation rule. Its first step is:

1. Let _expr_ be the |CallMemberExpression| that is covered by |CoverCallExpressionAndAsyncArrowHead|.

so clearly there's an assumption that CoverCallEtc covers a CallMemberExpression, but there's no rule to guarantee it.

(I think the best solution is to add the 'missing' EE rule.)


Back when CoverCallEtc was introduced (with Async Functions in PR #692),it probably wasn't necessary to have such a rule, because the productions:

CoverCallExpressionAndAsyncArrowHead : MemberExpression Arguments

and:

CallMemberExpression : MemberExpression Arguments

have the same right-hand side, so any source text matched by the former would necessarily also be matched by the latter.

But while that's true as far as the EBNF is concerned, and even most EE rules, it's not true when you have EE rules whose application depends on whether you're inside a CoverCallEtc.

@waldemarhorwat
Copy link

Towards the end of 13.2 the CoverCallExpressionAndAsyncArrowHead is refined to

ParenthesizedExpression[Yield, Await] :
( Expression[+In, ?Yield, ?Await] )

I see what you mean about the missing "must cover" rule in 13.2. According to 5.1.4, one should be there: "In such cases a more restrictive supplemental grammar is provided that further restricts the acceptable token sequences. Typically, an early error rule will then state that, in certain contexts, "P must cover an N", where P is a Parse Node (an instance of the generalized production) and N is a nonterminal from the supplemental grammar"

@jmdyck
Copy link
Collaborator Author

jmdyck commented Feb 3, 2022

Towards the end of 13.2 the CoverCallExpressionAndAsyncArrowHead is refined to

ParenthesizedExpression : ( Expression )

I see what you mean about the missing "must cover" rule in 13.2.

You're smooshing a couple of things there:

  • 13.2 refines CoverParenthesizedExpressionAndArrowParameterList to ParenthesizedExpression.
  • 13.3 refines CoverCallExpressionAndAsyncArrowHead to CallMemberExpression.

So 13.2 isn't missing a "must cover" rule; it's there in 13.2.9.1. Rather, the missing "must cover" rule is missing from 13.3.

(It is a bit odd that we have both "interpretation is refined" prose and "must cover" early error rules saying basically the same thing.)

@jmdyck
Copy link
Collaborator Author

jmdyck commented Nov 29, 2023

This bug might be resolved by PR #3227, which removes the paragraph containing the sentence:

In addition, [these early error rules] are not applied when initially parsing a [...] CoverCallExpressionAndAsyncArrowHead.

After that removal, I think the 'missing' early error rule (where CoverCallExpressionAndAsyncArrowHead must cover a CallMemberExpression) can be added or not, without affecting the correctness of the spec (but I think adding it would make more sense).

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

No branches or pull requests

2 participants