-
-
Notifications
You must be signed in to change notification settings - Fork 25
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
feat(compiler): wrap partial application in anonymous functions #92
Conversation
From what I can tell, the typespec generation doesn't use the same parts of the Typedtree as the code generation? I am not sure yet if I can detect there how many arguments a let binding takes immediately vs how many are a part of the returned anonymous function, I know how to do that with the |
I could probably rewrite the exports and function spec generation to do the same thing as code generation, not sure yet if this is the only way... |
let f = abs_diff_times 3 in | ||
f (-1) 2 | ||
|
||
let another_partial _ = Io.format "Hello" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if you do
let top_level_partial () = Io.format "hello"
or
let top_level_partial = Io.format "hello"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2nd one "cannot be generalized"
1st one has the same result as with _
, so
-spec top_level_partial(ok, list(_)) -> ok.
instead of
-spec top_level_partial() -> fun(list(_) -> ok).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I just noticed my failing test is wrong, it should say what I typed above instead of just -spec name(_) -> ok.
let lhs = List.map Pat.bind outer_names in | ||
let idents = List.map Expr.ident outer_names in | ||
let rhs = Expr.apply name (args @ idents) in | ||
Expr.fun_ ~cases:[ FunDecl.case ~lhs ~guard:None ~rhs ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand this correctly, you're
- computed the arity of this particular function type, using
Uncurry.uncurry_tarrow/2
- checking if we're applying this function with the as many arguments as the arity indicated by the type from 1)
- if we are missing arguments, create a function that will take the missing arguments and pass them on to the function from 1)
I think this will break for named and default arguments, so we need to have another look at this.
This can be problematic because fun's in Erlang can have a single arity, but multiple clauses. This means that a function f ?(x = 1) y = x + y
would translate to 2 erlang functions:
f(Y) -> f(1, Y).
f(X, Y) -> X + Y.
And a function with named arguments like f ~x ~y = x / y
would need to be kept track until is fully applied before we can call it.
For example, what does let div_one = (f ~y: 1)
translate to?
Div_one = fun (X) -> f(1, X) end,
% or
Div_one = fun (X) -> f(X, 1) end,
I think we may need to take a step back and rethink this. It's turning out to be a tad more complex than I thought id' be at first 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will break for named and default arguments,
which are not supported right now anyway, but I understand we should think ahead...
Gleam supports labeled arguments, and I think those are probably fine to figure out at Caramel compilation time. Not sure about default arguments, they indeed seem tricky. What values are allowed? Just simple literals? If so then maybe they are just in the typedtree and could be fetched from there and printed when needed...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means that a function
f ?(x = 1) y = x + y
would translate to 2 erlang functions:
I would propose only generate the full arguments version in the resulting Erlang, so only support optional / labeled arguments when called in from Caramel, not other langs.
For example, what does
let div_one = (f ~y:1)
translate to?
This depends on the way f was declared, because of the order, and the rules of Ocaml erasure would need to be preserved. So assuming it is f x ~y = x + y
which in erlang is just f(X, Y), then
Div_one = fun (X) -> f(X, 1) end,
and if f ~y x = x + y
then fun (X) -> f(1, X) end,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or even generate the erlang functions with smaller arity for optional arguments, but only for erlang/elixir interoperability (don't use them from caramel because it's unnecessarily complicating things), and only if there is just one optional or have some rules like Elixir that they can be ommited inclusively starting from the end only.
In any case I think we can figure it out and if not, caramel is not 1.0 anyway :)
Not to sound pessimistic but isn't one way to deal with this complexity by just not implementing and providing currying? Currying, at least imho, is a common source of frustrating and somewhat confusing error messages in OCaml code. I've more often caused a compilation error accidentally currying things than I have intentionally using currying. It's trivial to write a let binding which is a manual curry. |
@AeroNotix I agree "disallowing" currying is a way forward. But I think there still would be complexity in the compiler that needs to be introduced, possibly similar to the code present in this PR, in order to detect partial application and show an error. Or is there a way to configure the OCaml compiler to disallow it at the type checking step? |
Hi! 👋🏽 since I'm currently not working on and have no plans to continue working on this version of the compiler, I'll close this PR. Thanks a lot for your contribution 🙏🏽 |
"this version" - implies there is another version. Is that the case? |
relates to #44
relates to #90
specs are still wrong