diff --git a/compiler/core/js_exp_make.ml b/compiler/core/js_exp_make.ml index 2703e1d8ca..0e48847f40 100644 --- a/compiler/core/js_exp_make.ml +++ b/compiler/core/js_exp_make.ml @@ -1166,7 +1166,7 @@ let rec econd ?comment (pred : t) (ifso : t) (ifnot : t) : t = | Bool true, _, _ -> ifso | _, Bool true, Bool false -> pred | _, Cond (pred1, ifso1, ifnot1), _ - when Js_analyzer.eq_expression ifnot1 ifnot -> + when Js_analyzer.eq_expression ifnot1 ifnot -> ( (* {[ if b then (if p1 then branch_code0 else branch_code1) else branch_code1 @@ -1176,16 +1176,31 @@ let rec econd ?comment (pred : t) (ifso : t) (ifnot : t) : t = if b && p1 then branch_code0 else branch_code1 ]} *) - econd (and_ pred pred1) ifso1 ifnot + (* Prevent infinite recursion: if ifso1 is a Cond or Seq, skip this optimization *) + match ifso1.expression_desc with + | Cond _ | Seq _ -> {expression_desc = Cond (pred, ifso, ifnot); comment} + | _ -> + if Js_analyzer.eq_expression ifso1 ifnot then + {expression_desc = Cond (pred, ifso, ifnot); comment} + else econd (and_ pred pred1) ifso1 ifnot) | _, Cond (pred1, ifso1, ifnot1), _ when Js_analyzer.eq_expression ifso1 ifnot - -> - econd (and_ pred (not pred1)) ifnot1 ifnot + -> ( + (* Prevent infinite recursion: if ifnot1 is a Cond, skip this optimization *) + match ifnot1.expression_desc with + | Cond _ -> {expression_desc = Cond (pred, ifso, ifnot); comment} + | _ -> econd (and_ pred (not pred1)) ifnot1 ifnot) | _, _, Cond (pred1, ifso1, ifnot1) when Js_analyzer.eq_expression ifso ifso1 - -> - econd (or_ pred pred1) ifso ifnot1 + -> ( + (* Prevent infinite recursion: if ifnot1 is a Cond, skip this optimization *) + match ifnot1.expression_desc with + | Cond _ -> {expression_desc = Cond (pred, ifso, ifnot); comment} + | _ -> econd (or_ pred pred1) ifso ifnot1) | _, _, Cond (pred1, ifso1, ifnot1) when Js_analyzer.eq_expression ifso ifnot1 - -> - econd (or_ pred (not pred1)) ifso ifso1 + -> ( + (* Prevent infinite recursion: if ifso1 is a Cond, skip this optimization *) + match ifso1.expression_desc with + | Cond _ -> {expression_desc = Cond (pred, ifso, ifnot); comment} + | _ -> econd (or_ pred (not pred1)) ifso ifso1) | Js_not e, _, _ when not_empty_branch ifnot -> econd ?comment e ifnot ifso | ( _, Seq (a, {expression_desc = Undefined _}), diff --git a/debug_compiler_hang.md b/debug_compiler_hang.md new file mode 100644 index 0000000000..347b7726f9 --- /dev/null +++ b/debug_compiler_hang.md @@ -0,0 +1,164 @@ +# Compiler Hang Fix: Infinite Recursion in E.econd + +## Problem Description + +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. + +## Problematic Code Pattern + +The issue was triggered by code patterns that create nested conditional expressions with specific structures. Two examples from `Player.res`: + +### Example 1: Lines 40-48 + +```rescript +let isYAxis = !(gameObj.direction.y == 0.) +switch key { +| Space => (if isYAxis { + Thundershock.cast(gameObj) + } else { + ()->ignore + }) +| _ => () +} +``` + +### Example 2: Lines 51-59 + +```rescript +k->Context.onKeyRelease(key => { + let isYAxis = !(gameObj.direction.y == 0.) + switch key { + | Space => if isYAxis { + Thundershock.cast(gameObj)->ignore + } + | _ => () + } +}) +``` + +## Root Cause + +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: + +```ocaml +| _, Cond (pred1, ifso1, ifnot1), _ + when Js_analyzer.eq_expression ifnot1 ifnot -> + econd (and_ pred pred1) ifso1 ifnot +``` + +**The Problem:** +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 +2. This new structure gets processed by `S.if_` (statement creation), which calls `E.econd` again +3. If `ifso1` contains nested structures (like `Seq`), the `eq_expression` function can loop infinitely when comparing deeply nested sequences +4. This creates an infinite cycle: `E.econd` → creates `Cond` → `S.if_` processes it → calls `E.econd` again → repeat + +## The Fix + +The fix prevents infinite recursion by skipping the optimization when `ifso1` is a `Cond` or `Seq`: + +**Location:** `compiler/core/js_exp_make.ml`, lines 1222-1254 + +```ocaml +(match ifso1.expression_desc with + | Cond _ -> + (* If ifso1 is a Cond, skip this optimization to prevent infinite recursion *) + {expression_desc = Cond (pred, ifso, ifnot); comment} + | _ -> + (* Also check if ifso1 equals ifnot, which would make the result equivalent *) + (if Js_analyzer.eq_expression ifso1 ifnot then ( + {expression_desc = Cond (pred, ifso, ifnot); comment}) + else ( + (* If ifso1 is a Seq, it might contain nested structures that cause infinite recursion + in eq_expression. Skip this optimization to prevent that. *) + (match ifso1.expression_desc with + | Seq _ -> + {expression_desc = Cond (pred, ifso, ifnot); comment} + | _ -> + econd (and_ pred pred1) ifso1 ifnot))) +``` + +**Key Changes:** +1. **Line 1223-1227**: Skip optimization when `ifso1` is a `Cond` - prevents creating structures that match the same pattern +2. **Line 1237-1241**: Skip optimization when `ifso1` is a `Seq` - prevents `eq_expression` from looping on nested sequences +3. **Line 1230-1233**: Skip optimization when `ifso1 == ifnot` - prevents creating equivalent structures + +Similar guards were added to other patterns in `econd`: +- When `ifnot1` is a `Cond` (lines 1262-1266, 1277-1281) +- When `ifso1` is a `Cond` in other patterns (line 1292-1296) + +## How to Reproduce + +1. Create a ReScript file with nested conditionals that result in `Seq` or `Cond` structures in the `ifso1` position +2. Compile with the ReScript compiler +3. The compiler will hang indefinitely during compilation + +In my project you can try https://github.com/nojaf/rescript-kaplay/commit/2f34e581f346bff016bcc99126ba9656565f7ca6 + +```rescript +let isYAxis = !(gameObj.direction.y == 0.) +switch key { +| Space => (if isYAxis { + Thundershock.cast(gameObj) // This creates a Seq structure + } else { + ()->ignore + }) +| _ => () +} +``` + +Regular v12 + +```shell +✨ Finished Compilation in 0.52s +(base) nojaf@nojaf-mbp rescript-kaplay % bunx rescript +[1/3] 🧹 Cleaned previous build due to compiler update +[1/3] 🧹 Cleaned 0/0 in 0.06s +[2/3] 🧱 Parsed 116 source files in 0.10s +[3/3] 🤺 Compiling... ⠐ 109/119 +``` +(bsc.exe gets stuck) + +This PR + +```shell +(base) nojaf@nojaf-mbp rescript-kaplay % RESCRIPT_BSC_EXE=/Users/nojaf/Projects/rescript/_build/default/compiler/bsc/rescript_compiler_main.exe bunx rescript +[1/3] 🧹 Cleaned previous build due to compiler update +[1/3] 🧹 Cleaned 0/0 in 0.05s +[2/3] 🧱 Parsed 116 source files in 0.10s +[3/3] 🤺 Compiled 116 modules in 0.33s +``` + +## Files Modified + +- `compiler/core/js_exp_make.ml` - Added guards to prevent infinite recursion in `econd` function + +## Related Code + +The fix is in the `econd` function which optimizes conditional expressions: + +```ocaml +let rec econd ?comment (pred : t) (ifso : t) (ifnot : t) : t = + (* ... *) + match (pred.expression_desc, ifso.expression_desc, ifnot.expression_desc) with + | _, Cond (pred1, ifso1, ifnot1), _ + when Js_analyzer.eq_expression ifnot1 ifnot -> + (* Optimization: if b then (if p1 then branch_code0 else branch_code1) else branch_code1 + is equivalent to: if b && p1 then branch_code0 else branch_code1 *) + (* FIX: Skip optimization if ifso1 is Cond or Seq to prevent infinite recursion *) + (match ifso1.expression_desc with + | Cond _ | Seq _ -> {expression_desc = Cond (pred, ifso, ifnot); comment} + | _ -> econd (and_ pred pred1) ifso1 ifnot) +``` + +## Testing + +- Git clone https://github.com/nojaf/rescript-kaplay/commit/2f34e581f346bff016bcc99126ba9656565f7ca6 +- Build compiler in branch +- RESCRIPT_BSC_EXE=/Users/nojaf/Projects/rescript/_build/default/compiler/bsc/rescript_compiler_main.exe bunx rescript + +## Notes + +- The fix is conservative: it skips the optimization when there's a risk of infinite recursion +- This is safe because skipping the optimization still produces correct code, just potentially less optimized +- The fix applies to all similar patterns in `econd` to ensure consistency +- Sorry for the AI-ness here, I'm a bit out of my league here. \ No newline at end of file diff --git a/packages/@rescript/runtime/lib/es6/Primitive_float.js b/packages/@rescript/runtime/lib/es6/Primitive_float.js index b87b31cbf9..33a1a18de9 100644 --- a/packages/@rescript/runtime/lib/es6/Primitive_float.js +++ b/packages/@rescript/runtime/lib/es6/Primitive_float.js @@ -6,7 +6,9 @@ function compare(x, y) { return 0; } else if (x < y) { return -1; - } else if (x > y || x === x) { + } else if (x > y) { + return 1; + } else if (x === x) { return 1; } else if (y === y) { return -1; diff --git a/packages/@rescript/runtime/lib/js/Primitive_float.js b/packages/@rescript/runtime/lib/js/Primitive_float.js index 05bf928667..e04d75cc15 100644 --- a/packages/@rescript/runtime/lib/js/Primitive_float.js +++ b/packages/@rescript/runtime/lib/js/Primitive_float.js @@ -6,7 +6,9 @@ function compare(x, y) { return 0; } else if (x < y) { return -1; - } else if (x > y || x === x) { + } else if (x > y) { + return 1; + } else if (x === x) { return 1; } else if (y === y) { return -1; diff --git a/tests/tests/src/reasonReactRouter.mjs b/tests/tests/src/reasonReactRouter.mjs index 2dcbad15d8..e07ed19b42 100644 --- a/tests/tests/src/reasonReactRouter.mjs +++ b/tests/tests/src/reasonReactRouter.mjs @@ -79,20 +79,28 @@ function search() { function push(path) { let match = globalThis.history; let match$1 = globalThis.window; - if (match !== undefined && match$1 !== undefined) { - Primitive_option.valFromOption(match).pushState(null, "", path); - Primitive_option.valFromOption(match$1).dispatchEvent(safeMakeEvent("popstate")); - return; + if (match !== undefined) { + if (match$1 !== undefined) { + Primitive_option.valFromOption(match).pushState(null, "", path); + Primitive_option.valFromOption(match$1).dispatchEvent(safeMakeEvent("popstate")); + return; + } else { + return; + } } } function replace(path) { let match = globalThis.history; let match$1 = globalThis.window; - if (match !== undefined && match$1 !== undefined) { - Primitive_option.valFromOption(match).replaceState(null, "", path); - Primitive_option.valFromOption(match$1).dispatchEvent(safeMakeEvent("popstate")); - return; + if (match !== undefined) { + if (match$1 !== undefined) { + Primitive_option.valFromOption(match).replaceState(null, "", path); + Primitive_option.valFromOption(match$1).dispatchEvent(safeMakeEvent("popstate")); + return; + } else { + return; + } } } diff --git a/tests/tests/src/stdlib/Stdlib_IteratorTests.mjs b/tests/tests/src/stdlib/Stdlib_IteratorTests.mjs index d315168d28..d1ae4caefe 100644 --- a/tests/tests/src/stdlib/Stdlib_IteratorTests.mjs +++ b/tests/tests/src/stdlib/Stdlib_IteratorTests.mjs @@ -48,9 +48,13 @@ let asyncResult = { }; await Stdlib_AsyncIterator.forEach(asyncIterator, v => { - if (v !== undefined && v[0] === "second") { - asyncResult.contents = "second"; - return; + if (v !== undefined) { + if (v[0] === "second") { + asyncResult.contents = "second"; + return; + } else { + return; + } } }); diff --git a/tests/tests/src/ticker.mjs b/tests/tests/src/ticker.mjs index f1c5c7dee0..aece94b660 100644 --- a/tests/tests/src/ticker.mjs +++ b/tests/tests/src/ticker.mjs @@ -182,8 +182,10 @@ function process_quote(ticker_map, new_ticker, new_value) { let match$1 = match._0; let match$2 = match$1.lhs.value; let match$3 = match$1.rhs.value; - let value = match$2 !== undefined && match$3 !== undefined ? ( - match$1.op === "PLUS" ? match$2 + match$3 : match$2 - match$3 + let value = match$2 !== undefined ? ( + match$3 !== undefined ? ( + match$1.op === "PLUS" ? match$2 + match$3 : match$2 - match$3 + ) : undefined ) : undefined; ticker.value = value; });