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

Spec |> await #85

Closed
wants to merge 1 commit into from
Closed

Spec |> await #85

wants to merge 1 commit into from

Conversation

bakkot
Copy link

@bakkot bakkot commented Jan 23, 2018

This specifies the third of the options here (I think).

I believe the inclusion of GetValue actually is observable because it means the running execution context is not suspended if the LHS is an abrupt completion, which is as I'd expect.

I'd be happier if we could prevent

a |> await
b

from parsing as

(a |> await);
(b);

(instead having it just be an error), but I can't see good way of doing that off the top of my head. The reason I want this is because it amounts to a "phantom" NoLineTerminatorHere restriction in AwaitExpressions which only applies in pipelines, which I don't really like.

@mAAdhaTTah
Copy link
Collaborator

mAAdhaTTah commented Feb 10, 2018

I'm working on implementing this in babylon. Is the parsing issue you mention actually a problem? I'm actually wondering if banning |> await identifier and parsing |> await as a single.... step? operation?, such that following |> await with anything other than another |> terminates the pipeline.

So this:

x |> fetch
  |> await y
  |> process

would parse as:

x |> fetch
  |> await;

y |> process;

Would this be too surprising behavior? The other thought was making it a SyntaxError, requiring the next token after |> await to be either |> or ;.

@zenparsing I get what you mean by "ad-hoc" rules 😀.

@littledan
Copy link
Member

@mAAdhaTTah I don't understand how that parse would happen. An expression followed by another expression, with a space in between, is a SyntaxError--ASI is only triggered by newlines.

@mAAdhaTTah
Copy link
Collaborator

@littledan Oh right, ok then. Is lookahead after |> await, requiring the next token to be ; or |>, a feasible solution? Then this:

a |> await
b

would be a SyntaxError, which would avoid an ASI hazard. Otherwise, I think we're stuck with the above behavior.

@littledan
Copy link
Member

Seems like, if we merge this patch, it'll be in a branch that @mAAdhaTTah would maintain. Is that right? If so, should we close this PR?

@mAAdhaTTah
Copy link
Collaborator

@littledan Yeah, I'm going to borrow this change for the F#-style spec, which I'll maintain in our fork, but since we designed the current minimal proposal to ban await, this can't be merged until we make a decision on which proposal we're adopting.

@littledan
Copy link
Member

@mAAdhaTTah Are you planning on writing an explainer in your fork that focuses on the vision of the F# proposal, and linking to this from the explainer in this repository?

@mAAdhaTTah
Copy link
Collaborator

@littledan Yeah, I'm working on that now, but I haven't committed anything yet.

@mAAdhaTTah
Copy link
Collaborator

This is implemented here.

@littledan
Copy link
Member

Great, closing this PR for now then; we can reopen if we select the F# proposal.

@littledan littledan closed this Apr 16, 2018
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants