fable-compiler / Fable

F# to JavaScript, TypeScript, Python, Rust and Dart Compiler
http://fable.io/
MIT License
2.93k stars 301 forks source link

REPL regression #996

Closed ncave closed 7 years ago

ncave commented 7 years ago

The recent curried lambda change is breaking the REPL.

Previously it generated this (which works):

  const x8 = function (st_9) {
      return u_tycon_repr(st_9);
    }(st_1);

Now it generates this (which does not work):

  const x8 = ($var44 => $var45 => (0, _CurriedLambda2.default)(function (st_9) {
    return u_tycon_repr(st_9);
  })($var44, $var45))(st_1);

I'm not sure why does it generate those additional var parameters.

Shouldn't it just generate this (which seems to work):

  const x8 = (0, _CurriedLambda2.default)(function (st_9) {
    return u_tycon_repr(st_9);
  })(st_1);
alfonsogarciacaro commented 7 years ago

Thanks for catching this, @ncave! Probably it's related to the type attached to the curried lambda which triggers the wrapping in ensureArity. Do you think it'd be possible to isolate the problem in a unit test? Could you point to the source code in FCS? Cheers!

ncave commented 7 years ago

This will repro the code generation issue (wrapping), but not why it's failing in the original code:

module Test

let inline tup2 f1 f2 x =
  let a = f1 x
  let b = f2 x
  (a,b)

let f x = 
    match x with
    | _ -> (fun _ -> x)

let test x = tup2 id f x
ncave commented 7 years ago

@alfonsogarciacaro The actual reason why it's failing seems to be that the extra wrapping is postponing (changing) the execution order of the code, which is sequentially reading from IO. When the extra wrapping is removed, it works as expected.

alfonsogarciacaro commented 7 years ago

Ah, ok! Hmm, that's very unfortunate. It's one of the tradeoffs I had to make with the uncurrying optimization. It used to be a warning about that, I don't remember why I commented it out 😕

ncave commented 7 years ago

But in this case it seems to have all the arguments it needs (which is one).

ncave commented 7 years ago

@alfonsogarciacaro Here is a slightly bigger repro that can be turned into a test if needed:

module Test

let inline tup2 f1 f2 x =
  let a = f1 x
  let b = f2 x
  (a,b)

let mutable m = 0

let f x =
    m <- m + x
    (fun _ -> x)

let test x =
  let a, b = tup2 id f x
  printfn "%A" m

test 5

This should print 5 (and it does, under dotnet), but instead it prints 0.

alfonsogarciacaro commented 7 years ago

Ok, after trying different approaches, finally I found one that makes your test pass and doesn't break any other. I have a bit of a feeling that I'm writing patches on top of patches to solve all the edge cases happening after the uncurrying optimization. I just hope there are not too many left and that the improved interop with JS is worth the effort :pray:

ncave commented 7 years ago

@alfonsogarciacaro Thanks, it works!