Fable hangs on active pattern usage

[MaxWilson]

Description

In some cases, Fable hangs at compile time even as dotnet.exe grows to a multi-gigabyte memory footprint.

Repro code

I don't have a standalone repro unfortunately, but I do have a minimal delta in the git repo here. Branch repro/fableHangs fails to compile; branch repro/fableHangsWorkaround succeeds. The only difference between them is that the repro has a few extra cases in its active patterns. Deleting the following lines makes fable no longer hang:

        | Word(AnyCase("att" | "attack"), IntNoWhitespace(ac, Advantage(Roll(dmg, rest)))) -> Some(Branch(adv 0, [Crit, doubleDice dmg; AtLeast ac, dmg]), rest)
        | Word(AnyCase("att" | "attack"), IntNoWhitespace(ac, Disadvantage(Roll(dmg, rest)))) -> Some(Branch(disadv 0, [Crit, doubleDice dmg; AtLeast ac, dmg]), rest)
        | Word(AnyCase("att" | "attack"), IntNoWhitespace(ac, Advantage(NumericBonus(toHit, WS(Roll(dmg, rest)))))) -> Some(Branch(adv toHit, [Crit, doubleDice dmg; AtLeast ac, dmg]), rest)
        | Word(AnyCase("att" | "attack"), IntNoWhitespace(ac, Disadvantage(NumericBonus(toHit, WS(Roll(dmg, rest)))))) -> Some(Branch(disadv toHit, [Crit, doubleDice dmg; AtLeast ac, dmg]), rest)

Expected and actual results

Expected: fable either compiles or emits an error.
Actual: fable grows to a multi-GB footprint (dotnet.exe is around 19GB) and then hangs.

Known workarounds

In this case, "delete those four lines" is an acceptable workaround. Those cases represent edge cases, and simply failing to match those edge cases is not a big problem for me. Filing the issue for awareness, but I am not blocked.

Related information

Fable version: 2.1.12
Operating system: Windows 10

[alfonsogarciacaro]

Hmm, it looks like there's some infinite recursion happening here. Not sure what's the problem but I see you're suppressing warning 40 in that file. I think in those cases, the F# compiler injects some code to check the value is correctly initialized. Although we have some support for that (see #237) it's probably flaky and may not work in complicated cases like yours.

Would it be possible to refactor the code to avoid the warning? ...without the "nowarn" cheat ;)

[MaxWilson]

It is not possible to refactor to avoid the warning--recursion is a necessary part of left-recursive packrat parsing, because I need to define the recursive active pattern in such a way that it can be cached and called repeatedly until a fixpoint is reached. (That is what the pack function does.)

Note also that the F# compiler does not hang during regular F# compilation of this file (e.g. for unit tests), only in Fable.

[MaxWilson]

I also want to note that in this particular case, deleting those four lines is an acceptable workaround--they represent little-used grammar productions that I can live without.

[MaxWilson]

I actually found a better workaround for my use case. The issue is that the previous workaround was not robust: I still ran into situations where the compiler would apparently hang. I wound up doing two things: I was ultimately able to follow Alfonso's suggestion to remove warning 40 by using stateful variables for my recursive pattern definitions instead of relying on compiler magic, which was enough to prevent dotnet.exe from blowing up to 19GB, but there were still perf issues (10 minutes to compile instead of 45 seconds). So the second thing I ultimately did was to flatten my active patterns by defining helper active patterns.

Snippet illustrating both techniques:

    let mutable (|SimpleRoll|_|) = uninitialized<_>
    let (|SumOfSimpleRolls|_|) = packrec <| fun (|SumOfSimpleRolls|_|) -> function
        | SumOfSimpleRolls(lhs, OWS(Char('+', OWS(SimpleRoll(r, rest))))) -> Some(lhs@[r], rest)
        | SumOfSimpleRolls(lhs, OWS(Char('+', OWS(Int(n, rest))))) -> Some(lhs@[StaticValue n], rest)
        | SumOfSimpleRolls(lhs, OWS(Char('-', OWS(Int(n, rest))))) -> Some(lhs@[StaticValue -n], rest)
        | SimpleRoll(roll, rest) -> Some([roll], rest)
        | _ -> None
    (|SimpleRoll|_|) <-
        let (|LongSimpleRoll|_|) = function OWS(IntNoWhitespace(n, Char ('d', IntNoWhitespace(d, Char ('k', IntNoWhitespace(m, rest)))))) -> Some (Combine(Sum, Best(m, (Repeat(n, Dice(1, d))))), rest) | _ -> None
        let (|MidSimpleRoll|_|) = function | OWS(IntNoWhitespace(n, Char ('d', IntNoWhitespace(d, rest)))) -> Some (Dice(n, d), rest) | _ -> None
        pack <| function
        | LongSimpleRoll(roll, ctx) -> Some(roll, ctx)
        | MidSimpleRoll(roll, ctx) -> Some(roll, ctx)
        | OWS(IntNoWhitespace(n, Char ('d', rest))) -> Some (Dice(n, 6), rest)
        | OWS(Char ('d', IntNoWhitespace(d, Char('a', rest)))) -> Some (Combine(Max, Repeat(2, Dice(1,d))), rest)
        | OWS(Char ('d', IntNoWhitespace(d, Char('d', rest)))) -> Some (Combine(Min, Repeat(2, Dice(1,d))), rest)
        | OWS(Char ('d', IntNoWhitespace(d, rest))) -> Some (Dice(1,d), rest)
        | OWS(IntNoWhitespace(n, rest)) -> Some(StaticValue n, rest)
        | _ -> None

Additional details at this commit: https://github.com/MaxWilson/ShiningSword/commit/1c5693518beaa0dc03a08c784fff03cc720f13fc

[alfonsogarciacaro]

Good to hear you managed to fix it @MaxWilson, thanks for sharing!

[MaxWilson]

Should I close this issue, or leave it open since technically a workaround isn't a fix?

[alfonsogarciacaro]

We can leave it open in case there's some day time to reproduce and try to fix this :)

[cmeeren]

Sounds like what I've previously reported at https://github.com/Microsoft/visualfsharp/issues/4691

[alfonsogarciacaro]

Closing as there's already an issue to track this in the fsharp repo, thanks @cmeeren!