Fable: Something is wrong with currying/uncurrying

[vbfox]

Description

Well I guess it's currying/uncurrying but essentially functions that should only be called once are getting called multiple times, creating problems.

I might try to dig into it, especially to extract a smaller repro but at least I have one

Repro code

REPL link

type Dispatch<'msg> = 'msg -> unit
type SetState<'model, 'msg> = 'model -> Dispatch<'msg> -> unit
type Program<'model, 'msg> = {
    setState: SetState<'model, 'msg>
}

let map (mapSetState: SetState<'model, 'msg> -> SetState<'model, 'msg>) (program: Program<'model, 'msg>) =
    {
        setState = mapSetState (program.setState)
    }

let witChanges (program: Program<'model,'msg>) =
    let doMap (setState: SetState<'model, 'msg>) =
        printfn("doMap")
        setState

    program |> map doMap

let testProgram: Program<string, string> = {
    setState = (fun m d -> printfn("setState"))
}

let changedProgram = witChanges testProgram

changedProgram.setState "Foo" (fun _ -> ())
changedProgram.setState "Bar" (fun _ -> ())
changedProgram.setState "Baz" (fun _ -> ())

Expected and actual results

The expected behavior can be observed on .NET :

setState
setState
setState

Fable capture the function and call it multiple times instead:

setState
doMap
setState
doMap
setState

Related information

• Fable version: 2.3.3
• Operating system: Windows 10

[alfonsogarciacaro]

Probably related to #1659. The main problem of this is to fix the behavior we need to disable an optimization which removes a lot of closures in pipes.

[vbfox]

Probably.

But I feel that a fix is needed, there is too much risk in a compiler that has correctness problems, no optimization is worth that (At least no optimization enabled by default).

I noticed it in an optimization context so it slowed down the app to a crawl (not always only when hiding it for long enough) but It could have happened anywhere, depending on the domain covered by the fable app it could do anything.

Our main web app at work (not in fable, maybe one day) is in the finance domain, having a compiler at risk of from creating double financial transactions would be something removing all confidence in a tech.

[alfonsogarciacaro]

Actually I just realized that #1659 was fixed by this commit (which seems to have indeed disabled some optimizations, like the printfn statements which drive me crazy). This issue is actually related to currying/uncurrying as you said from the beginning 😅

Unfortunately, because of the way Fable 2 works, it's not possible to disable uncurrying anymore. I guess it's possible to cache the intermediate results to avoid the multiple optimization, but you have found a corner case when currying, uncurrying and partial applications at runtime are being combined and I'm having a hard time to find where exactly the cache would be needed :confused:

let map (mapSetState: SetState<'model, 'msg> -> SetState<'model, 'msg>) (program: Program<'model, 'msg>) =
    {
        setState = mapSetState (program.setState)
    }

let witChanges (program: Program<'model,'msg>) =
    let doMap (setState: SetState<'model, 'msg>) =
        printfn("doMap")
        setState

    program |> map doMap

Translates to:

export function map(mapSetState, program) {
  return new Program$00602(uncurry(2, partialApply(2, mapSetState, [program.setState])));
}
export function witChanges(program$$1) {
  const doMap = function doMap(setState) {
    toConsole(printf("doMap"));
    return curry(2, setState);
  };

  return map(uncurry(3, doMap), program$$1);
}

[matthid]

So you could say:

• The problem is in return map(uncurry(3, doMap), program$$1); because uncurry removes the possibility to partially apply doMap (which needs to be done later)
• The problem is partialApply because it doesn't actually invoke the thing.

I guess we somehow need to "mark" curried funktions or translate them to a different representation to properly partially apply them, instead of delaying the execution?

Context:

https://github.com/fable-compiler/Fable/blob/c8f5d904e700e30d4b056ca78db279ad93e63cd4/src/fable-library/Util.ts#L547-L641

[matthid]

Somewhat smaller code sample:

type TestFunc = int -> unit
let map (mapSetState: int -> TestFunc)  =
    mapSetState 1

let doMap (i:int) : TestFunc =
    printfn "doMap %d" i 
    (fun j -> printf "logic %d - %d" i j) 

let setState = map (doMap)

setState 1
setState 2
setState 3

One workaround seems to be to inline map (at least in this minified version). Needless to say that this isn't always possible

[matthid]

@alfonsogarciacaro What about https://github.com/fable-compiler/Fable/pull/1837?
I'm not sure

• if this is allowed according to the JS spec
• might lead to other issues

Obviously the concrete implementation should be tested with various arities and edge cases and should be written more "type-safe" if possible. Also the concrete "field" we use on the "Function" object is up for discussion as well (if we feel this is a good idea).

[vbfox]

Regarding the JS spec you're doing what is totally normal in JS.

A function and a class are the same thing as every function is also a constructor. Members on the function are what would be considered static members in a language like C# so no problem adding some.

[matthid]

@vbfox static feels wrong. Does it behave as I think it does or will the member be potentially shared over two lambdas?

[vbfox]

Ah sorry I didn't see that my analogy with static members would cause confusion.

Essentially no problem with that, it would be 'static' if the function was used as a class constructor (There is a proposal for the keyword on classes and it already works in babel/typescript down-compiling to more or less what you do there) but types/functions are first class objects in javascript they don't have the magic property of existing only once like in .Net world.

Functions can have properties attached to them, they aren't shared with other function just the instance that is created (And here you create a new one each time with the => keyword)

[alfonsogarciacaro]

Sorry @matthid! I missed the last notifications and during the flight back I finally found a solution only to discover after landing it was actually the same as you proposed 😅 I'm very impressed you fixed this without having much experience with Fable internals. But if you don't mind, we can keep my implementation as I found other cases (basically when you curry/uncurry a function by passing it through another id-like function) where Fable was failing and I finally managed to make them work.

I've taken the test from your PR anyways just to be sure. Thanks a lot for all your help you two!

[matthid]

@alfonsogarciacaro No problem. I looked at the solution and indeed very similar. I think it might have the same problem with when different arity-numbers are in place. Sadly I don't know enough about the compiler to reproduce this via regular code but I might be able to try to reproduce it with plain uncurry and curry calls (But I won't be able to confirm if that is ever a problem in practice).

What I mean is: I think it is possible now that uncurry or curry return functions not expected by the compiler, as they don't match the arity argument or something like that.

[vbfox]

Nice changes all over 🎉 :shipit:

[alfonsogarciacaro]

@matthid In principle the arity shouldn't be an issue. It's only passed as an argument to give a hint to the JS runtime about the new function it needs to build, but the important thing is that the function moves from curried to uncurried and vice versa, the soundness of the arity is checked by the F# compiler.

Anyways, if you can find an example where the current implementation breaks, please post and we'll try to fix it :)