Apply operators with explicit currying #81
Conversation
| one = 2 |> (fn x y -> x + y) <| 1 | ||
|
|
||
| two = 3 |> second _ 2 | ||
| three = second _ 4 <| 5 |
There was a problem hiding this comment.
It looks like these aren't named after the numbers they evaluate to anymore - in that case I think we can just get rid of the definitions altogether since they're not what's being tested. We can probably keep one just to make sure the precedence of =, |>, and <| stays correct
src/parser/desugar.rs
Outdated
| call.args.insert(0, arg); | ||
| Ast::FunctionCall(call) | ||
| }, | ||
| Ast::Lambda(lambda @ ast::Lambda{..}) => { |
There was a problem hiding this comment.
The @ ast::Lambda{..} clause doesn't look necessary here
src/parser/desugar.rs
Outdated
| return Ast::lambda(lambda.args, None, Ast::FunctionCall(fcall), location) | ||
| } else { | ||
| let arg_to_replace = lambda.args.remove(0); | ||
| replace_lambda_body(&mut fcall, new_arg, arg_to_replace); |
There was a problem hiding this comment.
These two lines look identical to the ones in the then case, maybe they can be merged
src/parser/desugar.rs
Outdated
|
|
||
| for idx in idxs { | ||
| match &new_arg { | ||
| Ast::Literal(lit) => { |
There was a problem hiding this comment.
Honestly, I'm having second thoughts with this optimization. Applying it to all lambdas complicates things somewhat and makes me think that this is maybe better left to the optimizer. I thought it might be salvagable by changing the check to see if the argument is only used once instead, but that could still change evaluation order of something like (print 1; 5) |> foo (print 2; 10) _.
I think it'd be simpler if we just kept it as immediately calling the lambda so 3 |> print _ = (fn $1 -> print $1) 3
There was a problem hiding this comment.
I do agree, I think the potential savings down the line in terms of time to optimize isn't worth the complexity. What if i roll back the slotting and keep the seperate desugar.rs file?
|
Good point, I don't think we have any. Let me add some real quick! Edit: Seems we do, but it is some simple addition examples |
|
Ah, I see it now. Haven't merged my local branch with it yet. Lets add one more test to it like (actually, on second thought, does EDIT: looks like not, I'd guess a check is needed somewhere near line 49 in desugar_apply_operator. We can save this for another PR if you want |
|
I think I can look into that now, might as well :) |
|
There was an easy fix for it, but it needs to be aware if its inside a match, otherwise the same problems with typeconstructors occur. For now I've just added the new test using (+). When #76 is fixed, explicit currying for inline operators could be added in without very cumbersome checks :) |
|
Thanks! The prefix operator in the test is another good one, I don't think I had any tests for operators used in a prefix position at all before |
Closes #75
I've taken the opportunity to also put the desugaring functions in another file, as it was getting a little long.
The slotting works with literals and variables, as I believe there might be unintended consequences if I were to slot for example an impure function call slotting into
fn x -> x + x. This is both for the sake of not putting clone on everything on the AST, and for uninteded execution.There could certainly be some improvements, but am unsure what that could be. Will change up the stuff you see can be simplified :)