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

Incorrectly accepts `async ({await}) => 1` #2

Open
bakkot opened this issue Nov 7, 2019 · 4 comments

Comments

@bakkot
Copy link

@bakkot bakkot commented Nov 7, 2019

When CoverCallExpressionAndAsyncArrowHead is refined to async ArrowFormalParameters, the ArrowFormalParameters nonterminal is given the flag [+Await], and await is only a valid IdentifierReference when the flag is -Await. So async ({await}) => 1 should be a parse error. But it isn't. Instead the await is treated as an identifier.

@bakkot

This comment has been minimized.

Copy link
Author

@bakkot bakkot commented Nov 7, 2019

(Also, congrats on the parser!)

@pvdz

This comment has been minimized.

Copy link
Owner

@pvdz pvdz commented Nov 8, 2019

Thanks for reporting it. I'll be sure to make this work soon :)

@pvdz

This comment has been minimized.

Copy link
Owner

@pvdz pvdz commented Nov 11, 2019

This turns out more difficult than I thought.

The problem is a sibling case:

async({await});
async({await}) => x;

The call is valid unless module goal, the arrow is never valid.

The problem here lies in the way that I'm handling the "are we parsing async code"-state. In a nutshell either we are or we aren't parsing async code. This is fine for most cases, except this one, where we are parsing code that "might be async".

The problem is in _parseGroupToplevels, where we receive the asyncToken param which tells us whether the group was prefixed with async. At this point we won't know whether we are parsing the call or the arrow, yet, as we can only know this once we find the arrow. That's a general arrow problem, and async has some additional edge cases like this.

While parsing the parser mainly tracks whether or not the parsed content might be destructible, must be destructible, cannot be destructible, or can only be destructible through assignment.

If the code is async then a variable named await will trigger an error regardless, so that error is not conditional (nor is there a way to return this in its destructibility). If the code is not async then it won't trigger an error, whether call or arrow. Unfortunately for the case in this issue we need some sort of "maybe async" state, which is the only one of its kind... :/

Just writing this out to get a better picture of the problem, and explain why it's not a nobrainer to solve :)

@pvdz

This comment has been minimized.

Copy link
Owner

@pvdz pvdz commented Nov 11, 2019

Actually, async(await)=>x is properly handled so I did already solve this. And that's through the PIGGY_BACK_SAW_AWAIT_KEYWORD state. So the fix here is to make sure object/array parsers also properly reflect these await/yield states. Okay :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.