Conversation
|
I again checked the sizes for lablgtk, but in this case they even seem to slightly decrease compared to 5.1.0. |
lthls
left a comment
There was a problem hiding this comment.
This PR fixes two bugs: a major one about delaying evaluation of optional arguments, and a minor one about evaluation order being inconsistent with normal applications.
But I still think the handling of optional arguments followed by missing arguments is wrong: the reference test should produce the output from #10653, not the one from this PR. Recovering efficient partial applications should only be done later, when information about the function's actual arity is known (typically in the middle-end).
I would be willing to approve this if the test contains a comment explaining that the produced result is not yet correct. Then we could work on replacing the current optimisation by a more correct one in another PR.
|
@lthls As I commented in #10653, I beg to differ here. |
|
By the way, I just discovered something strange about n-ary functions. let f ?(a=print_endline"a") ?(b=print_endline"b") = function c -> cis a function of 3 arguments, and the defaults are computed after receiving let f ?(a=print_endline"a") ?(b=print_endline"b") = fun c -> cis a function of two arguments returning a function, and the defaults are computed after receiving |
|
This looks like the "syntactic arity" convention (https://github.com/ocaml/RFCs/blob/master/rfcs/n_ary_functions.md) that was recently implemented. Like all conventions, it can look strange sometimes, but I think that knowing (syntactically) how the compiler is going to handle a curried function definition is useful. I don't know if syntactic arity can help with the issue discussed here and in #10653, as I haven't been able to follow the discussion. |
lthls
left a comment
There was a problem hiding this comment.
I think we should go ahead and merge this. We'll change the reference output of the test later if we fix the other bug.
|
@lthls Thanks for the approval. I intend to add some comments explaining what this code does before merging (it took me a while to remember the details, as it was written 20 years ago). The behavior may be open to debate, but it is the intended one. @xavierleroy Syntactic arity alone does not solve the problem, but if we combine it with the requirement that any n-ary abstraction must be terminated by a non-optional argument, which is the case in any reasonable code (except combinators used to build functions by combining sets of optional arguments), then there is no discrepancy with #10653, since side-effects are then guaranteed to only occur after a non-optional argument. That is why I suggested introducing a warning for that. |
Alternative to #10653, preserving the delaying of partial applications of optional arguments to avoid code blow up.