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

Add let.Foo sugar for continuation-passing-style #2140

Closed
wants to merge 24 commits into from

Conversation

jaredly
Copy link
Contributor

@jaredly jaredly commented Aug 14, 2018

This will allow us to get most of the benefits of lwt's ppx or ppx_let without baking in any specific implementation.

I also brought in some of the attribute handling changes in #1826 -- I didn't want to change too much, but it does make handling attributes much nicer.

thanks to @aantron for help with the implementation, and everyone who gave feedback & ideas on the concrete syntax.

Current (stripped-down) proposal

see bottom for original proposal

module Opt = {
  let let_ = Option.flatMap;
  let and_ = Option.pair;
};

let.Opt x = Some(10)
and.Opt y = Some(20)
and.Opt z = Some(30);
Some(x * y + z);

Potential future extensions

  • allow people to write lower-cased let.await and auto-capitalize -- part of the original proposal, but removed for simplicity of initial implementation -- supported in comment by @jordwalke
  • let._ that references a locally defined let_ instead of module-scoped: comment by @texastoland and comment by @hcarty
  • switch.Foo that would map to Foo.switch_(arg, fn): comment by @hcarty
  • relax the requirement of annotating ands: comment by @texastoland

References


original proposed syntax

let await = (value, continuation) => Js.Promise.then_(continuation, value);

let () = {
  let!await name = getName();
  let!await age = getAge(name);
  Promise.resolve({name, age})
}

@cristianoc
Copy link
Contributor

@aantron
Copy link

aantron commented Aug 14, 2018

@jaredly There are a couple reasons to factor out the "monad product" that is done by and, so that it is its own function. Leaving the theoretical ones aside for now, here are the more practical ones:

One is motivated by your let!await example:

let () = {
  let!await name = getName();
  let!await age = getAge(name);
  {name, age}
}

It's probably just a minor oversight, but the second await is actually using Js.Promise.map, not then_, because it is returning unwrapped {name, age} (i.e., not Js.Promise.resolve({name, age})). So, I think the example would have to be:

let await = (value, continuation) => Js.Promise.then_(continuation, value);
let map = (value, continuation) => Js.Promise.map(continuation, value);

let () = {
  let!await name = getName();
  let!map age = getAge(name);
  {name, age}
}

If we want to be able to use and with both then_ and map, and don't take advantage of the "monad product," monad libs will have to define then_, then_2, then_3, map, map2, map3, etc. For some monad-like libs (applicatives), there might also be apply, so there will be apply2, apply3, etc. This causes code bloat and confusion, especially if the user has to ever manually hook all these up to the syntax, or if the user is writing some huge composition and the tuple gets too large.

Also, as can be seen from the option test case, in the PR as now, the user has to manually count how many ands they have, and pick the right suffix. If contributors want to insert or delete and extra and, they have to change the suffix on the first let:

let _ = { 
  let!opt x = Some(10); 
  let!opt2 a = Some(2)     /* <--- If I add an and, do I need to make this opt3? */
  and b = Some(5); 
  Some(a + x * b); 
}; 

That seems like something best left to the compiler.

@jaredly
Copy link
Contributor Author

jaredly commented Aug 14, 2018

  1. Good call, I forgot to wrap in a resolve. let!map would also work

  2. about and

First off, I'm totally willing to explore a solution for and if we find that not having it is prohibitively annoying :) I think this solution is an excellent first step, and then we can later try to figure out how to get product right in the way that is least confusing.

Yes it's less "elegant" than a solution with the monad product, but I think it's also less confusing, and so I'd like to try this way first. We can then later add a and!prod syntax that's totally compatible with this current PR.

If we want to be able to use and with both then_ and map, and don't take advantage of the "monad product," monad libs will have to define then_, then_2, then_3, map, map2, map3, etc. This causes code bloat and confusion

It's boilerplate, but the best kind of boilerplate, in that it's trivial to get right, and probably won't need updating ever, because it lives in a library...

especially if the user has to ever manually hook all these up to the syntax

not sure what you mean by this. let!opt transforms to opt(x, y => z), so it's governed by normal scoping rules. I expect that people will do open Lwt.letSyntax at the top or something. You can also do let!Lwt.await x = y if you want.

or if the user is writing some huge composition and the tuple gets too large.

If they're writing some huge composition, you could build up the product yourself.

let a = b
and c = d
and e = f;
let!await ((a, c), e) = Lwt.await(Lwt.prod(Lwt.prod(a, c), e);

It doesn't seem to me like having massive compositions will be common enough for us to make things more complicated to support it, at least for this initial pass.

If I add an and, do I need to make this opt3?

Yup :) again, for this first pass I'm not too concerned about supporting support complex uses. We can add the and syntax afterwards if we find that it's super annoying not to have it.


All this to say:

  • I think this PR is quite straightforward for novices to understand (including potential error messages), and I'm super happy about that
  • adding and! syntax would make things more confusing
  • I'd like to see how things go without it, but I've also left the door open for implementing an and! syntax in a follow-up PR

@aantron
Copy link

aantron commented Aug 14, 2018

I'm all for merging all or parts of this soon, in the interest of not holding up evolution, but just saying :)

I do think the and part probably needs more exploration and experience to get right.

We could keep and without and!prod, by having

let await = {bind: (value, continuation) => ..., pair: (value, value') => ...}

and having let!await … and … take the right functions out of that record. Not saying that's a good idea, it's just a suggestion for keeping the syntax less visually complex (at a cost to library authors), because it doesn't require annotations on and. We would have some trouble switching to this later, if we think it's a good idea, if we commit this PR as is right now.

especially if the user has to ever manually hook all these up to the syntax

I mean when a library gets things slightly wrong, like when the author didn't realize they have a monad (or applicative, or whatever) and didn't make it compatible with the syntax, and the user has to write some glue code.

Another common case of this might be composed monads, like say there is an async monad that forgot to define an async+result monad, and now the user has to do it. This sort of thing comes up all the time in my OCaml experience.

Also, on the subject of boilerplate, since result has two binds (on the value and the error), and two maps, that is that much more boilerplate to define if we have to bake in the products. Maybe the failure case is not important enough for making it compatible with the syntax, I don't know.

If they're writing some huge composition, you could build up the product yourself.

Yes. But the point where you have to do this is not predictable to the user. And I can imagine large and-compositions would eventually happen with parsers and observables.

@aantron
Copy link

aantron commented Aug 14, 2018

Right now, I'm thinking we might want something like prod, then optional prod3, etc., because the extra products help with optimization. Not sure how to best implement that, though.

@aantron
Copy link

aantron commented Aug 14, 2018

...and another option could be to define only bind/await/whatever, and have Reason translate let!await … and … always into a sequence of binds, unless the bind definition has some annotation attached, that gives hints about the names of the product functions. That completely removes the need for any boilerplate, both on the part of the library author, and on the part of the user adjusting opt2 to opt3 and so on. Boilerplate becomes optional, only for optimization.

@jaredly
Copy link
Contributor Author

jaredly commented Aug 14, 2018

and having let!await … and … take the right functions out of that record.

Yeah, that does make the syntax simpler, but at the cost of making the transform more confusing. I like the simplicity of having the let!opt literally be a function reference that you can e.g. cmd-click, and error reporting is super straightforward.

large and-compositions would eventually happen with parsers and observables.

tbh if you're anding a ton of observables, you're in for a world of hurt 😂


but yeah, I think an and!foo syntax is a solid direction to go, and will be fully compatible with this PR.

@jaredly
Copy link
Contributor Author

jaredly commented Aug 14, 2018

...and another option could be to define only bind/await/whatever, and have Reason translate let!await … and … always into a sequence of binds

yuuup that gets into rather more magic than I'd want

@jaredly
Copy link
Contributor Author

jaredly commented Aug 14, 2018

I could imagine adding a "super-magic-monad" syntax later, but I want to keep this as straightforward and understandable to newcomers as possible, even though that means sacrificing some monady goodness.

@hcarty
Copy link
Contributor

hcarty commented Aug 14, 2018

Somewhat tangential to the main point of this PR - can we avoid overloading the meaning of ! as done here? It would be nice to either keep ! consistent as meaning "this is an explicit override/shadow/redefinition" or change the other uses of keyword! to use some other syntax. Like open^ Mod;, no seriously open Mod;, etc.

@jaredly
Copy link
Contributor Author

jaredly commented Aug 14, 2018

I chose let! to mirror F#, and I think it looks nice. I'd be fine with no seriously open Mod :P

@jaredly
Copy link
Contributor Author

jaredly commented Aug 14, 2018

or wait, what about like @override open Mod? That's both clear and doesn't require any parser changes

@hcarty
Copy link
Contributor

hcarty commented Aug 14, 2018

@jaredly I like it - keyword! predates annotations and extension points on the OCaml side but the could work well here. There may need to be some special handling though since the compiler ignores unused/typo'd @annotations on its own.

It could be open%override, which would avoid the typo issue.

@jaredly jaredly mentioned this pull request Aug 14, 2018
Copy link
Contributor

@IwanKaramazow IwanKaramazow left a comment

Choose a reason for hiding this comment

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

  • Can you add tests to make sure whitespace is interleaved correct?
let _ = {
  let!opt x = Some(10);

  let!opt2 a = Some(2)
  and b = Some(5);

  Some(a + x * b);
};
  • Can you add tests with extensions?
let!await%lwt

};

let _ = {
let!opt x = Some(10);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you also add tests for the sugar outside of let bindings, e.g. structure_items, Pstr_eval?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let bindings are the only place it's allowed. structure_items will throw an error (because there's no continuation, it doesn't really make sense).
I think I also want to disallow extensions... although I could be convinced

uncurried : bool
}
end
open T
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we avoid the open T here?

@aantron
Copy link

aantron commented Aug 14, 2018

I want to keep this as straightforward and understandable to newcomers as possible

I think a far more straightforward thing to do is to give and no special treatment, and just expand let!foo … and … as if it was a series of lets.

Even simpler, we can reject and, and fail with an error message that links people to an issue for discussing how and should be handled.

@jaredly
Copy link
Contributor Author

jaredly commented Aug 14, 2018

yeah, that makes sense

@jaredly
Copy link
Contributor Author

jaredly commented Aug 15, 2018

Ok, whitespace interleaving tested!

@texastoland
Copy link

texastoland commented Aug 29, 2018

@jaredly my apology for the barrage. Yes that's what I originally understood and back to understanding. I'll summarize.

My concern (#2140 (comment)) is = is confusing because it's literally unequal e.g. map (#2140 (comment)) or ordinary async-less callbacks (#2140 (comment)). A more minor complaint is let.f looks like a module of let and not let at all. I think ppx_let is a really neat solution but they couldn't do anything about syntax.

Your idea is a really neat generalization of ppx_let. Without and it's basically LiveScript (FP CoffeeScript fork) backcalls (#2140 (comment)). Backcalls are a more literal desugaring but otherwise the same as let->. <= would parse unambiguously next to let but would look odd because it's ligature is less-than-or-equals (Haskell etc. use skinny arrows).

It looks like the implementation still includes the let_ and and_ interface? I think just let would be problematic for hosting multiple functions e.g. bind and map.

@jordwalke's separate concern is how to handle async exceptions. I won't have time to write it up formally today but after a good sleep I agree. The desguaring could catch unhandled exceptions and re-raise them one level up. It'd be even nicer if every computation were stored in a result (or Error monad lite) with a function to handle errors inline. That's high level but I haven't tried anything real yet.

@texastoland
Copy link

texastoland commented Aug 29, 2018

I wanted to point out another use case for this proposal. PureScript and Scala use do notation for list comprehensions. The PureScript example could be written using operators

/* example */
let factors = n => {
  0 -- n >>= a =>      /* for a in 0 to our number         */
  0 -- a >>= b =>      /* for b in 0 to a                  */
  a * b == n >>? () => /* if the product equals our number */
  return((a, b))       /* make a pair                      */
};

/* [[6,4],[8,3],[12,2],[24,1]] */
24 |> factors |> Js.log

Using ppx_let

let factors = n => {
  let%bind a = 0 -- n
  let%bind b = 0 -- a
  let%bind () = guard(a * b == n)
  (a, b)
};

Using this proposal (I think)

let yield = return
let for_ = bind
let if_ = (>>?)

let factors = n => {      /* alternatively:           */
  let.for_ a = 0 -- n     /* let a <= for_(0 -- n)    */
  let.for_ b = 0 -- a     /* let b <= for_(0 -- a)    */
  let.if_ () = a * b == n /* let _ <= if_(a * b == n) */
  yield((a, b))
};

Given

open Belt.Array

/* ppx_let */
let return = x => [|x|]
let bind = (x, f) =>
  reduce(x, [||], (acc, a) => concat(acc, f(a)))

/* MonadZero */
let guard = t => t ? [|()|] : [||]

/* infix */
let (>>=) = bind
let (>>?) = t => bind(guard(t))
let (--) = range

Playground

@jordwalke
Copy link
Member

@texastoland : You mentioned you haven't thought too much about the error handling. Aside from a nicer (imho) syntax than ppx-let, that error handling is the main reason I see this PR being better than ppx-let. If you have a proposal, please include how error propagation would work. Without it, a feature request is not really complete. The real problems come up when when handling errors and/or exceptions and it's that use case that I think this PR's justification becomes apparent.

@texastoland
Copy link

texastoland commented Aug 30, 2018

that error handling is the main reason I see this PR being better than ppx-let

I don't see that being done here?

https://github.com/facebook/reason/blob/b1a5ca72e733adf93fa43cfc58aaf6ea9bf23330/src/reason-parser/reason_parser.mly#L952-L961

I think it's a separate issue from the syntax being unintuitive (more importantly unequational) though.

The desguaring could catch unhandled exceptions and re-raise them one level up. It'd be even nicer if every computation were stored in a result (or Error monad lite) with a function to handle errors inline.

I can write this up pretty trivially but I'd like to understand the status quo.

@jordwalke
Copy link
Member

I don't see that being done here?

Did you read the entirety of my Sketch? (It included an example in the linked Sketch that explains why this PR is helpful).

@texastoland
Copy link

texastoland commented Aug 30, 2018

Ah yes you articulated the problem awesome! I meant the current proposal doesn't address it either unless I misunderstood both the thread and this PR's changes. Regardless I agree and I'll publish a link concretely how I see it being desugared separate from any syntax concern.

@SanderSpies
Copy link
Contributor

SanderSpies commented Aug 30, 2018

Hopefully not adding too much noise to this discussion, but I thought this alternative syntax might be interesting:

let (x) = identifier(expr, $x);

The idea here is to look somewhat similar to a regexp search and replace.

It also allows you to put the callback argument in different places:

let (x) = identifier($x, expr); 

Edit: this syntax won't work as is - but I believe it's still an interesting direction to explore.

@jaredly
Copy link
Contributor Author

jaredly commented Aug 30, 2018

@SanderSpies that looks similar to @chenglou's proposal (comment) -- which was

let-> x = identifier(_, expr);

For me, the ability to put the callback argument in different places substantially hurts "grokkability", because now I have to hunt for where the callback function is being inserted, which in a function that accepts multiple handlers (e.g. an "on success" callback and an "on error" callback) could drastically change the execution semantics of the following code.

Your example also drops the let-> marker, which means, for me, it will be nearly impossible to look at a bunch of code and determine "what are the let bindings that will have very different execution semantics"

The code sample (taken from reason-language-server, which there is using a variant of ppx_let) that I'm using to evaluate different syntax proposals is this: (and I'm having to guess a little bit what you mean by $x)

let (uri, position) = try(Protocol.rPositionParams(params), $(uri, position));
let (text, verison, isClean) =
  try(maybeHash(state.documentText, uri) |> orError("No document text found"), $(text, verison, isClean));
let package = try(State.getPackage(uri, state), $package);
let offset =
  try(PartialParser.positionToOffset(text, position) |> orError("invalid offset"), $offset);
let {extra} = try(State.getDefinitionData(uri, state, ~package), ${extra});
let position = Utils.cmtLocFromVscode(position);

{
  let (commas, labelsUsed, lident, i) =
    opt(PartialParser.findFunctionCall(text, offset - 1), $(commas, labelsUsed, lident, i));
  let lastPos = i + String.length(lident) - 1;
  let pos =
    opt(PartialParser.offsetToPosition(text, lastPos) |?>> Utils.cmtLocFromVscode, $pos);
  let (_, loc) = opt(References.locForPos(~extra, pos), $(_, loc));
  let typ =
    opt(switch (loc) {
    | Typed(t, _) => Some(t)
    | _ => None
    }, $typ);
  let rec loop = t => ...
  let (args, rest) = loop(typ);
  let args = opt(args == [] ? None : Some(args), $args);
  Some(Ok(...));
} |? Ok((state, Json.Null));

How long does it take to spot all the $s? 😫

And here's that code in the let.foo style

let.try (uri, position) = Protocol.rPositionParams(params);
let.try (text, verison, isClean) =
  maybeHash(state.documentText, uri) |> orError("No document text found");
let.try package = State.getPackage(uri, state);
let.try offset =
  PartialParser.positionToOffset(text, position) |> orError("invalid offset");
let.try {extra} = State.getDefinitionData(uri, state, ~package);
let position = Utils.cmtLocFromVscode(position);

{
  let.opt (commas, labelsUsed, lident, i) =
    PartialParser.findFunctionCall(text, offset - 1);
  let lastPos = i + String.length(lident) - 1;
  let.opt pos =
    PartialParser.offsetToPosition(text, lastPos) |?>> Utils.cmtLocFromVscode;
  let.opt (_, loc) = References.locForPos(~extra, pos);
  let.opt typ =
    switch (loc) {
    | Typed(t, _) => Some(t)
    | _ => None
    };
  let rec loop = t => ...;
  let (args, rest) = loop(typ);
  let.opt args = args == [] ? None : Some(args);
  Some(Ok(...));
}
|? Ok((state, Json.Null));

@SanderSpies
Copy link
Contributor

@jaredly I guess you are right, in this example it's somewhat hard to spot the replace locations.

Apologies for adding noise to the conversation, as it seems that Cheng's syntax version was in the same spirit and even better. The underscore has a different meaning for me, so that might be why I missed it.

@andreypopp
Copy link
Contributor

Tangentially related, with ppx_let I often find myself writing a series of let%bind () = ..; bindings:

let%bind () = Fs.unlink(p);
let%bind () = Fs.symlink(~target, p);

With the current proposal it would look like:

let.async () = Fs.unlink(p);
let.async () = Fs.symlink(~target, p);

Which looks great! But maybe there's a room for a special syntax:

async Fs.unlink(p);
async Fs.symlink(~target, p);

or something like that, which looks less noisy.

@jaredly
Copy link
Contributor Author

jaredly commented Sep 4, 2018

So, thinking about this some more, and talking to @texastoland, I'm now thinking about it this way: If we want to do "plain functions", then the name of the function needs to go to the right of the =. If we do a module, then it can go next to the let.
e.g. either we do something like let-> x = async(thing), or we do let.Async x = thing where Async is a module with a let_ (we could also auto-capitalize, allowing let.async, as discussed).

Having "plain functions" where the name is next to the let will inevitably be very confusing, as users will use all kinds of "callback functions" -- not just ones that "make sense" as monad transformers. e.g. let.readFile contents = "data.json" would be perfectly valid under the "plain functions let.foo" proposal, and is quite confusing to parse.

@texastoland
Copy link

texastoland commented Sep 4, 2018

let.Async x = thing where Async is a module with a let_

My idea was let.Async x <- await(thing) where Async is a module with an await. This is the least magic (<- could be replaced by another symbol except = or a right arrow). await(thing) is a typical function call (as opposed to the function and argument on opposite sides of =), there's no = to lie about equality as I've demonstrated in numerous examples, and @jaredly pointed out to me let.Async syntax resembles existing record.Module.field.

A friendlier option with more precedent would be to enclose the scope in a keyword (Haskell do, Scala for, F# named after module) and get rid of PPX-like lets altogether. I've already outlined a formal RFC (including error handling) with all my concerns and suggestions from this PR but it'll take a little bit to write up completely with examples. Tl;dr I like this PR's implementation but the syntax and modular interface are problematic for all use cases.

@andreypopp
Copy link
Contributor

andreypopp commented Sep 4, 2018

There's yet another advantage of using a module to represent semantics of CPS bindings.

Consider this piece of code with ppx_let:

let f = () => {
  let%bind value = ...;
}

which will be turned by refmt into:

let f = () => {
  let%bind value = ...;
  ()
}

(note the trailing () inserted by refmt) which will automatically lead to type error and this will be confusing! Instead if we keep all CPS semantics within a module we could have a monadic unit there or a return / pure. In this case the code:

let f = () => {
  let.Async value = ...;
}

could be transformed into:

let f = () => {
  let.Async value = ...;
  Async.return();
}

which won't lead to a type error.

This will require to supply:

let return : 'a => t('a)

from authors of modules which work with such syntax.

@jaredly
Copy link
Contributor Author

jaredly commented Sep 5, 2018

Hrmmmm I think I want to avoid magically adding a Something.return() anywhere.

@jaredly
Copy link
Contributor Author

jaredly commented Sep 5, 2018

I made a ppx that implements the let.Module sugar https://github.com/jaredly/let-anything to exercise the idea a bit more.

@jaredly
Copy link
Contributor Author

jaredly commented Sep 12, 2018

Ok, I've been using let%Anything for a week and am pretty convinced it's the way to go -- let-> and friends are too "loose/permissive", and I think will just result in confusion, and won't adequately meet the need that started this whole thing -- namely providing a nice, standardized "async/await"-like functionality for dealing with promises.
In light of that, and some feedback I've gotten about "how do I handle exceptions?", I'm also going to add a try transformer that maps to TheModule.try_.
Stay tuned for the refresh.

@texastoland
Copy link

texastoland commented Sep 12, 2018

Tonight and tomorrow I'm writing up a thorough explanation in RFC form of @jaredly's implementation that answers "why not generalized CPS with arbitrary functions", "what about monads" (and how it relates to Haskell's do notation or idiom brackets), "what about F#'s builders", "what about async exceptions", "what could we improve incrementally" (mostly discussed above), etc. I hope it helps for clarification and more focused feedback since this thread is already lengthy.

@zploskey
Copy link
Contributor

Did you get around to drafting up an RFC, @texastoland?

@texastoland
Copy link

texastoland commented Oct 14, 2018

I outlined it and was working on code examples when I got wrapped up. Jared's let%Anything is a very close approximation of F# computation expressions. There are improvements left for the future but it's a nice start. I'm traveling but I'll try to post this week. Thanks for the reminder @zploskey!

@flash-gordon
Copy link

Is there any further plan for this PR? I would like to stick to a certain strategy when dealing with async code in Reason. Thanks.

@jordwalke
Copy link
Member

I like this. It would be cool to build the syntax parsing changes which simply defines an extension point, then have the actual ast manipulation done as a separate ppx plugin so people can try it out and give feedback. That way we avoid breaking changes. I would really like to try this out in some of my projects.

@andreypopp
Copy link
Contributor

It would be much more easier (for me at least) to try the syntax (easier to implement too!) if it would desugar into ppx_let:

let.Async x = V; E

into

[%bind.Async let x = V in E]

@fakenickels
Copy link

fakenickels commented Dec 13, 2018

Any news around this?

@texastoland
Copy link

texastoland commented Dec 13, 2018

ocaml/ocaml#1947 got merged and Bob blogged about merging upstream changes. @jordwalke speculated about Reason implementing it ahead of BuckleScript. I assume the maintainers will wait for revisions (🤞🏽 see issue) to the official syntax.

@texastoland
Copy link

Can this be closed now or repurposed to backport the syntax like in Dune?

@fakenickels
Copy link

As I think Reason will integrate the official let+ syntax now I think would be good to point about this in this PR and close it to not let people confused

@jaredly
Copy link
Contributor Author

jaredly commented Dec 5, 2019

closing in favor of #2487

@jaredly jaredly closed this Dec 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.