|
| 1 | +# Compiler Hang Fix: Infinite Recursion in E.econd |
| 2 | + |
| 3 | +## Problem Description |
| 4 | + |
| 5 | +The ReScript compiler was hanging indefinitely during compilation when processing certain nested conditional expressions. The hang occurred during the JavaScript code generation phase, specifically in the `E.econd` function which optimizes nested conditional expressions. |
| 6 | + |
| 7 | +## Problematic Code Pattern |
| 8 | + |
| 9 | +The issue was triggered by code patterns that create nested conditional expressions with specific structures. Two examples from `Player.res`: |
| 10 | + |
| 11 | +### Example 1: Lines 40-48 |
| 12 | + |
| 13 | +```rescript |
| 14 | +let isYAxis = !(gameObj.direction.y == 0.) |
| 15 | +switch key { |
| 16 | +| Space => (if isYAxis { |
| 17 | + Thundershock.cast(gameObj) |
| 18 | + } else { |
| 19 | + ()->ignore |
| 20 | + }) |
| 21 | +| _ => () |
| 22 | +} |
| 23 | +``` |
| 24 | + |
| 25 | +### Example 2: Lines 51-59 |
| 26 | + |
| 27 | +```rescript |
| 28 | +k->Context.onKeyRelease(key => { |
| 29 | + let isYAxis = !(gameObj.direction.y == 0.) |
| 30 | + switch key { |
| 31 | + | Space => if isYAxis { |
| 32 | + Thundershock.cast(gameObj)->ignore |
| 33 | + } |
| 34 | + | _ => () |
| 35 | + } |
| 36 | +}) |
| 37 | +``` |
| 38 | + |
| 39 | +## Root Cause |
| 40 | + |
| 41 | +The infinite recursion occurred in `compiler/core/js_exp_make.ml` in the `econd` function (which creates optimized conditional expressions). The function attempts to optimize nested conditionals by recursively calling itself: |
| 42 | + |
| 43 | +```ocaml |
| 44 | +| _, Cond (pred1, ifso1, ifnot1), _ |
| 45 | + when Js_analyzer.eq_expression ifnot1 ifnot -> |
| 46 | + econd (and_ pred pred1) ifso1 ifnot |
| 47 | +``` |
| 48 | + |
| 49 | +**The Problem:** |
| 50 | +1. When `ifso1` is a `Seq` (sequence expression) or another `Cond`, the recursive call `econd (and_ pred pred1) ifso1 ifnot` creates a new `Cond` structure |
| 51 | +2. This new structure gets processed by `S.if_` (statement creation), which calls `E.econd` again |
| 52 | +3. If `ifso1` contains nested structures (like `Seq`), the `eq_expression` function can loop infinitely when comparing deeply nested sequences |
| 53 | +4. This creates an infinite cycle: `E.econd` → creates `Cond` → `S.if_` processes it → calls `E.econd` again → repeat |
| 54 | + |
| 55 | +## The Fix |
| 56 | + |
| 57 | +The fix prevents infinite recursion by skipping the optimization when `ifso1` is a `Cond` or `Seq`: |
| 58 | + |
| 59 | +**Location:** `compiler/core/js_exp_make.ml`, lines 1222-1254 |
| 60 | + |
| 61 | +```ocaml |
| 62 | +(match ifso1.expression_desc with |
| 63 | + | Cond _ -> |
| 64 | + (* If ifso1 is a Cond, skip this optimization to prevent infinite recursion *) |
| 65 | + {expression_desc = Cond (pred, ifso, ifnot); comment} |
| 66 | + | _ -> |
| 67 | + (* Also check if ifso1 equals ifnot, which would make the result equivalent *) |
| 68 | + (if Js_analyzer.eq_expression ifso1 ifnot then ( |
| 69 | + {expression_desc = Cond (pred, ifso, ifnot); comment}) |
| 70 | + else ( |
| 71 | + (* If ifso1 is a Seq, it might contain nested structures that cause infinite recursion |
| 72 | + in eq_expression. Skip this optimization to prevent that. *) |
| 73 | + (match ifso1.expression_desc with |
| 74 | + | Seq _ -> |
| 75 | + {expression_desc = Cond (pred, ifso, ifnot); comment} |
| 76 | + | _ -> |
| 77 | + econd (and_ pred pred1) ifso1 ifnot))) |
| 78 | +``` |
| 79 | + |
| 80 | +**Key Changes:** |
| 81 | +1. **Line 1223-1227**: Skip optimization when `ifso1` is a `Cond` - prevents creating structures that match the same pattern |
| 82 | +2. **Line 1237-1241**: Skip optimization when `ifso1` is a `Seq` - prevents `eq_expression` from looping on nested sequences |
| 83 | +3. **Line 1230-1233**: Skip optimization when `ifso1 == ifnot` - prevents creating equivalent structures |
| 84 | + |
| 85 | +Similar guards were added to other patterns in `econd`: |
| 86 | +- When `ifnot1` is a `Cond` (lines 1262-1266, 1277-1281) |
| 87 | +- When `ifso1` is a `Cond` in other patterns (line 1292-1296) |
| 88 | + |
| 89 | +## How to Reproduce |
| 90 | + |
| 91 | +1. Create a ReScript file with nested conditionals that result in `Seq` or `Cond` structures in the `ifso1` position |
| 92 | +2. Compile with the ReScript compiler |
| 93 | +3. The compiler will hang indefinitely during compilation |
| 94 | + |
| 95 | +In my project you can try https://github.com/nojaf/rescript-kaplay/commit/2f34e581f346bff016bcc99126ba9656565f7ca6 |
| 96 | + |
| 97 | +```rescript |
| 98 | +let isYAxis = !(gameObj.direction.y == 0.) |
| 99 | +switch key { |
| 100 | +| Space => (if isYAxis { |
| 101 | + Thundershock.cast(gameObj) // This creates a Seq structure |
| 102 | + } else { |
| 103 | + ()->ignore |
| 104 | + }) |
| 105 | +| _ => () |
| 106 | +} |
| 107 | +``` |
| 108 | + |
| 109 | +Regular v12 |
| 110 | + |
| 111 | +```shell |
| 112 | +✨ Finished Compilation in 0.52s |
| 113 | +(base) nojaf@nojaf-mbp rescript-kaplay % bunx rescript |
| 114 | +[1/3] 🧹 Cleaned previous build due to compiler update |
| 115 | +[1/3] 🧹 Cleaned 0/0 in 0.06s |
| 116 | +[2/3] 🧱 Parsed 116 source files in 0.10s |
| 117 | +[3/3] 🤺 Compiling... ⠐ 109/119 |
| 118 | +``` |
| 119 | +(bsc.exe gets stuck) |
| 120 | + |
| 121 | +This PR |
| 122 | + |
| 123 | +```shell |
| 124 | +(base) nojaf@nojaf-mbp rescript-kaplay % RESCRIPT_BSC_EXE=/Users/nojaf/Projects/rescript/_build/default/compiler/bsc/rescript_compiler_main.exe bunx rescript |
| 125 | +[1/3] 🧹 Cleaned previous build due to compiler update |
| 126 | +[1/3] 🧹 Cleaned 0/0 in 0.05s |
| 127 | +[2/3] 🧱 Parsed 116 source files in 0.10s |
| 128 | +[3/3] 🤺 Compiled 116 modules in 0.33s |
| 129 | +``` |
| 130 | + |
| 131 | +## Files Modified |
| 132 | + |
| 133 | +- `compiler/core/js_exp_make.ml` - Added guards to prevent infinite recursion in `econd` function |
| 134 | + |
| 135 | +## Related Code |
| 136 | + |
| 137 | +The fix is in the `econd` function which optimizes conditional expressions: |
| 138 | + |
| 139 | +```ocaml |
| 140 | +let rec econd ?comment (pred : t) (ifso : t) (ifnot : t) : t = |
| 141 | + (* ... *) |
| 142 | + match (pred.expression_desc, ifso.expression_desc, ifnot.expression_desc) with |
| 143 | + | _, Cond (pred1, ifso1, ifnot1), _ |
| 144 | + when Js_analyzer.eq_expression ifnot1 ifnot -> |
| 145 | + (* Optimization: if b then (if p1 then branch_code0 else branch_code1) else branch_code1 |
| 146 | + is equivalent to: if b && p1 then branch_code0 else branch_code1 *) |
| 147 | + (* FIX: Skip optimization if ifso1 is Cond or Seq to prevent infinite recursion *) |
| 148 | + (match ifso1.expression_desc with |
| 149 | + | Cond _ | Seq _ -> {expression_desc = Cond (pred, ifso, ifnot); comment} |
| 150 | + | _ -> econd (and_ pred pred1) ifso1 ifnot) |
| 151 | +``` |
| 152 | + |
| 153 | +## Testing |
| 154 | + |
| 155 | +- Git clone https://github.com/nojaf/rescript-kaplay/commit/2f34e581f346bff016bcc99126ba9656565f7ca6 |
| 156 | +- Build compiler in branch |
| 157 | +- RESCRIPT_BSC_EXE=/Users/nojaf/Projects/rescript/_build/default/compiler/bsc/rescript_compiler_main.exe bunx rescript |
| 158 | + |
| 159 | +## Notes |
| 160 | + |
| 161 | +- The fix is conservative: it skips the optimization when there's a risk of infinite recursion |
| 162 | +- This is safe because skipping the optimization still produces correct code, just potentially less optimized |
| 163 | +- The fix applies to all similar patterns in `econd` to ensure consistency |
| 164 | +- Sorry for the AI-ness here, I'm a bit out of my league here. |
0 commit comments