Rescript-compiler: make uncurry to curry more forgiving

[bobzhang]

see commit dad35f4

when a function which has arity 0 is treated as curried function, treat it as arity 1

[baldurh]

I believe this has been blocking us from updating since 4.0.7. That is, we began seeing errors in our clients when we updated from 4.0.6 to 4.0.7
@Siggnja

[baldurh]

I created a small repro repo at https://github.com/baldurh/bs-platform-arity-test

[yawaramin]

@baldurh what happens if you change bar to let bar(fn) = fn(.);?

[baldurh]

@yawaramin Sure, that works fine. But my main concern is that we have a large codebase with hundreds of reason-react components which are used both from JS and other Reason code and we have no way of surfacing these issues at compile time. We would always be in the dark in regards to where these bugs might pop up.

What worries me a bit is that 0 arity functions are not treated equally depending on where they are coming from. If they are passed from Reason code they work as expected but now, if you pass them from JS they will break.

Even if we’d be successful in "fixing" our code by forcing uncurrying in all our Reason components which are being used from the JS side, there’s no guarantee these bugs won’t creep in later.

In fact, currently there seems to be no concept of 0 arity in Bucklescript since this:

let foo = () => Js.log("foo");

compiles to this:

function foo(param) {
  console.log("foo");
  return /* () */0;
}

There’s this unused parameter there to force it to be treated as 1 arity function (which honestly I feel is cheating a little bit). Also, if you’d have a minimizer that removes unused parameters you’d be in trouble again (which we don’t since our linter would never allow JS code like this).

[yawaramin]

I think it would clarify things to look at the OCaml syntax, because the Reason syntax has a special syntactic sugar for functions with arity 1 where the parameter is the unit value:

OCaml: let foo = fun () -> Js.log "foo"

Reason: let foo = () => Js.log("foo");

The unit value is actually being passed in here when the function gets called. That's why it gets compiled down to param.

Anyway, I think the current issue and your pain point are actually discussing two different things. The way I'm understanding it, this issue is about being able to silently treat an _uncurried_ function let foo = (.) => (); as a _curried_ one let foo = () => (); at any callsite. Your pain point is that you need a conversion in the other direction. And based on a quick thought experiment that conversion may be difficult to do correctly. E.g., this function: let foo = u => u; ... is polymorphic and actually returns its argument, so we can't just discard the argument i.e. treat it as arity zero.

[baldurh]

Right, ok maybe I misunderstood this GitHub issue and assumed it was about the same issue I (and many others I might add) have. I’m sorry if I did. We can take this discussion to #3370 instead?

Other than _purity_ I’ve yet to hear the downside to having a safeguard in place which makes sure this situation—which is undetectable at compile time—won’t crash your app.

As I now understand it, there really are no 0 arity functions in ocaml/reason, right? But is there any harm in having a single line of code that deals with that special case of _foreign_ functions? Maybe this whole issue is more complex than I realise? Will handling arity 0 functions in the curry_1 function introduce other issues?

[bobzhang]

@baldurh we will loose such check in next release. In retrospect, it is my mistake to make it too strict

[bobzhang]

there really are no 0 arity functions in ocaml/reason, right?

Yes, that's why we made an extension, namely fn(.) so that arity is guaranteed, it is guaranteed to be working when interop with JS code base when you use (.), but we understand people may make mistakes, since make it more forgiving in this special case, won't make any harm( the only harm may be some more maintainance on our side). So we are going to make such change

[bobzhang]

fixed in #3385

[baldurh]

Thanks @bobzhang :) Can’t wait for the release!