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

Support _ syntax sugar in application. #1804

Merged
merged 12 commits into from
Feb 14, 2018

Conversation

cristianoc
Copy link
Contributor

Proof of concept to support ? as syntax sugar in function application.
The implementation is crude, just to illustrate the idea, and check that there are no parsing conflicts. (No printer implementation at the moment).

An application
f(?, 3);
is transformed to
(__x) => f(__x, 3);

This enables the use of pipe in arbitrary positions, as in
let l = [1,2,3] |> List.map (i => i+1, ?) |> List.filter (i => i>0, ?);
or in
let l = (i => i+1) |> List.map(?, [1,2,3]);

Proof of concept to support ? syntax sugar in function application.
The implementation is crude just to illustrate the idea, and check that there are no parsing conflicts.

An application
  f(?, 3);
is transformed to
  (__x) => f(__x, 3)

This enables the use of pipe in arbitrary positions, as in
  let l = [1,2,3] |> List.map (i => i+1, ?) |> List.filter (i => i>0, ?);
or in
  let l = (i => i+1) |> List.map(?, [1,2,3]);
@chenglou
Copy link
Member

chenglou commented Feb 2, 2018

The intermediate allocation seems to be killed as well: https://reasonml.github.io/en/try.html?reason=DYUwLgBAhhC8EG0A+BGANBATBgzEguhAFAQRIB8EAdnJQFIDOAdAIIBObUAnkwLZQAHABRCoASlrQIAagjpqEkmUo1Y9Zu048AZgEtgYEGxHjJMSvKpigA

Which makes this feature. Otherwise, we wouldn't ship it in its current form.

Interestingly, you might be able to use this independently like so: map(? + 1, bla). A bit nicer looking than the current map((+)(1), bla) ever since syntax3

@jordwalke
Copy link
Member

@chenglou which intermediate allocation are you referring to?

@chenglou
Copy link
Member

chenglou commented Feb 2, 2018

n => ... after the pipe

@jchavarri
Copy link
Contributor

jchavarri commented Feb 2, 2018

@chenglou Can't you write

let a = [|1, 2, 3|] 
  |> Js.Array.map((a) => a + 1)
  |> Js.Array.filter((a) => a > 1)

to avoid these allocations today? Or are you thinking about the t first approach in the new StdLib?

(This addition is so cool, btw!)

@chenglou
Copy link
Member

chenglou commented Feb 2, 2018

Yeah. And this generalizes to foo(1, ?, 2)

@SllyQ
Copy link

SllyQ commented Feb 2, 2018

Really like this idea, was thinking could this possibly work with labeled arguments too?
Like transofrming f(~a=?, 3);
to (__x) => f(~a=__x, 3);

@cristianoc
Copy link
Contributor Author

@SllyQ in principle yes, though it looks like for some reason f(~a=?, 3) is already parsed at the moment, and reformats to f(~a?, 3).

In fact, just found out by playing that also f(~a=? :option(int), 3); currently parses and formats to f(~a: option(int)?, 3); which then gives syntax error.

`Some(?)` is tranformed to `__x => Some(__x)`.
This explores the idea of unlocking the treatment of constructors as functions.

Briefly tried to add ? to arbitrary constructors, not just unary ones, but at leas the obvious attempts give parse conflicts.
@TheSpyder
Copy link

TheSpyder commented Feb 3, 2018

~a=? is defining an optional argument in syntax3, it would be confusing to re-use it for this.

What other characters could we use? _ as in Scala is probably out because it overlaps with unused variable. % or $ maybe?

@jaredly
Copy link
Contributor

jaredly commented Feb 6, 2018

I don't think _ is "out", per se -- currently it doesn't parse in a value position, only in a pattern position.
let x = _ + 1 is a syntax error currently.

@SllyQ
Copy link

SllyQ commented Feb 6, 2018

Honestly parsing f(~a=?, 3) to f(~a?, 3) doesn't make sense to me. So I don't think it should be completely out of the question either. But I see how it could be confusing to people that ~a=? would mean two completely different things in function definition and function application.

It seems that the existing parsing of `foo(~x=?)` is not intentional, so it’s been removed.

Syntactically, the existing `foo(~x?)` and `foo(~x=?None)`, are quite similar to `foo(~x=?)` though the latter has a completely different meaning, nothing to do with optional labels.

Alternatively, the underscore “_” symbol is still free in the context where “?” is used here.
@cristianoc
Copy link
Contributor Author

It seems that the existing parsing of foo(~x=?) is not intentional, so it’s been removed.

Syntactically, the existing foo(~x?) and foo(~x=?None), are quite similar to foo(~x=?) though the latter has a completely different meaning, nothing to do with optional labels.

Alternatively, the underscore _ symbol is still free in the context where ? is used here.

@jaredly
Copy link
Contributor

jaredly commented Feb 9, 2018

fwiw I'd probably prefer _ over ?

@chenglou
Copy link
Member

chenglou commented Feb 9, 2018

@TheSpyder @jaredly then let's do _

Cristiano Calcagno added 3 commits February 9, 2018 12:52
Found by trial and error several places where functions are printed, to convert back from (__x) => foo(__x) to foo(_).
Not sure if this is complete.
@chenglou
Copy link
Member

Actually, we might clear out #1405 with this one too. So which one would you prefer now?

let f = switch (_) {
| A => 1
| _ => 2
}
let f = switch (?) {
| A => 1
| _ => 2
}

Cristiano Calcagno added 3 commits February 11, 2018 11:08
This form
  `let unSomeFun = fun | Some(n) => n | None => 0;`
can then be expressed as
  `let unSome = switch _ { | Some(n) => n | None => 0 };`
Add support for pretty printing nested functions such as `x => foo(_);`.
…tion_mark_in_application

# Conflicts:
#	src/reason-parser/reason_parser.mly
@TheSpyder
Copy link

I suggest we get some wider feedback on this, to make sure overloading _ depending on position isn't confusing. I make heavy use of it as a wildcard in pattern matching and sometimes function arguments too.

Using it to solve 1405 seemed weird at first, but looking at the code I can see how it's a natural extension of the original use case. However that does make me wonder how many people will see that they're similar or just be confused.

@andreypopp
Copy link
Contributor

andreypopp commented Feb 12, 2018

Just to mention in this thread — there's Proposal to add partial application to JS which uses ? as placeholders for arguments. Given that Reason syntax tries to appeal to the previous experience with JS syntax probably that needs to be taken into account.

Personally I like _ more. I'm not to worried about _ being overloaded as usually I try to avoid using _ in patterns and rather give some meaningful names like | Error(_err) => ..., that's not possible for "rest of the fields" in record though.

To remove such overloading we could:

  1. Remove the ability to use _ in pattern matching and instead force people into using some _-prefixed names for wildcards (I think it's good for readability).
  2. Introduce ... wildcard for record fields in patterns which has some symmetry to ... in functional record updates:
    | {x, y, ...} => ...
    

@jaredly
Copy link
Contributor

jaredly commented Feb 12, 2018

For the switch case, switch (_) looks fine to me.
@andreypopp I would love to be able to do {x, y, ...}! It always looks a bit awkward to do {x, y, ..._}

@TheSpyder
Copy link

Isn’t spread syntax to put the dots at the start? Is that an option?

@chenglou
Copy link
Member

@andreypopp a little pedantic, but _ works for this case: | Foo(_) even if _ represents either 2 constructor arguments or 1 argument.

Since the ES proposal uses ?, let's also go with it.

@jordwalke
Copy link
Member

The proposal for ? as pipe output, must also specify how to forward pipe output to an optional labeled arg. With _ it would be f(~a=?_) but with ? as pipe output, would it have to be f(~a=??)?

Cristiano Calcagno added 3 commits February 13, 2018 13:08
@cristianoc
Copy link
Contributor Author

To summarize, at this stage there is support for _ immediately as argument of a function application. There are essentially 3 cases, depending on whether labeled/optional arguments are used:

foo(3, _, 7)

foo(3, ~a=_, 7)

foo(3, ~a=?_, 7)

The syntactic sugar corresponds to this:

(__x) => foo(3, __x , 7)

(__x) => foo(3, ~a=__x, 7)

(__x) => foo(3, ~a=?__x, 7)

@rauschma
Copy link

Note: it’s not clear what JS is going to do w.r.t. partial application (or if it will ever be supported), therefore I wouldn’t look to JS in this case.

@rauschma
Copy link

rauschma commented Feb 14, 2018

Scope is an issue. Given:

f(g(_))

What is this translated to?

x => f(g(x))
f(x => g(x))

One possibility is to only allow this kind of partial application for operands of the pipe operator. Then the scope is clear.

@cristianoc
Copy link
Contributor Author

cristianoc commented Feb 14, 2018

@rauschma currently the scope is the direct function application where _ is an argument, so

f(g(_))

is

f(x => g(x))

It would be possible to consider nested scopes, and tie it to the pipe operator. Then one would need to specify what is allowed and what is not. E.g. can it go inside an if-then-else, etc.

A small observation: having a mechanism that is not tied to the pipe operator lets you split things apart:

let foo2 = foo(_, 2);
z |> foo2;

@jaredly
Copy link
Contributor

jaredly commented Feb 14, 2018

Our lambda syntax is already so light-weight, I think sticking with the simple way (just the function being applied) makes sense. b/c if you want more complex things, you can always do x => f(g(x))

@chenglou
Copy link
Member

We did consider tying this to the pipe operator, but it was actually easier to implement the way we did it now (i.e. as a standalone partial app feature). Consider it a nice addition that it works outside of pipes. I do believe it can be confusing though; maybe can discourage them socially for now I think

@chenglou chenglou changed the title [POC][Donotmerge] Support ? syntax sugar in application. Support _ syntax sugar in application. Mar 24, 2018
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.

10 participants