rescript-lang / rescript-compiler

The compiler for ReScript.
https://rescript-lang.org
Other
6.75k stars 449 forks source link

Uncurried breaks multi-argument function types in type parameters #6531

Open TheSpyder opened 11 months ago

TheSpyder commented 11 months ago

Sorry if I used the incorrect terminology in the title. I have a type t<'listener, 'ty> and when a function type is used as the 'listener the 'ty doesn't seem to operate correctly in uncurried mode.

This is another rescript-nodejs scenario, where the 'listener type is used to model the JavaScript event handlers and ensure that the registered function contains the correct argument types for the listener (type definition, type usage).

This all works great when there's only one function argument for 'listener. But when there's two, it fails in uncurried mode.

I have made a playground link to simplify the replication case. Switch to ReScript 11 and the error is one I don't think I can solve by adjusting my code:

This has type:
    t<(string, int) => unit, 'a>
  But it's expected to have type:
    t<(string, int) => unit, t2>
  These two variant types have no intersection
cristianoc commented 11 months ago

There's a real semantic change where 'a => 'b in uncurried model only matches functions of arity 1. While in curried mode it used to match almost any functions (as long as the first arg is not labeled).

TheSpyder commented 11 months ago

Is there a better way to model this concept? 🤔

cristianoc commented 11 months ago

Is there a better way to model this concept? 🤔

Not at the moment. One could consider a generic function type.

TheSpyder commented 11 months ago

Adding a fromString2 works. I've used hacks like this to make generic types more strictly functional, and it's not a common area of the codebase, so that will do for the moment. https://rescript-lang.org/try?version=v11.0.0-rc.7&code=C4TwDgpgBMA8DkAbAlgZ2BAdhATgGinlAD4AoCADwx0wENEoAzHAewFsBlYHZTAcwBcUdD35QAvMRgJaEqfABGBIiCnioAIgCkyACZZgyUBvJVcdBs3ZdRfAExCRvPnOkAKeLWUKAlK-gAxsokEpo6+piGxqSgkDB2pKQA9ElQAO4sOADWqKSIEMAwlMBCcE5iklAArphGBMB2akysnNzObhoYVBo+eQVQvBh8uKWwg641dfFNVq22HYMQwzg9yam6LBComETpmVl9hV3AAIKYugCSkUsj7uV8BIN+lZPA9Y2hszbOdh3HZ5drssekA

tsnobip commented 9 months ago

the other workaround would be to manually curry the type:

let textAndInteger: t<string => int => unit, t2> = fromString("textAndInteger")

But I guess you can't have it all once we've disabled autocurrying.

What's a bit weird is the error message though, it just looks wrong.

TheSpyder commented 9 months ago

That doesn't work, because in the real code it's expecting a one-argument function: https://github.com/TheSpyder/rescript-nodejs/blob/6f5c2b3b26b6f9234e5486420d523e683dda4820/src/EventEmitter.res#L41

But as it turns out, this error was in a test library and not even used, once I try to implement a "text and integer" event listener my fix doesn't work either (once again with a confusing message)

  This has type:
    NodeJs.Event.t<(string, int) => unit, \"EventEmitterTestLib-NodeJs".Emitter1.t>
      (defined as \"Event-NodeJs".t<(string, int) => unit, \"EventEmitterTestLib-NodeJs".Emitter1.t,>)
  But this function argument is expecting:
    NodeJs.Event.t<(string, int) => unit, NodeJs.EventEmitterTestLib.Emitter1.t>
      (defined as
      \"Event-NodeJs".t<(string, int) => unit, NodeJs.EventEmitterTestLib.Emitter1.t>)
  These two variant types have no intersection

Maybe it never worked. I should probably just delete multi-argument event concepts unless someone actually needs a multi-argument event, and then figure out a fix.