From 90ca5929667e6f0bb6ed5f3d5fb9abb18a858e98 Mon Sep 17 00:00:00 2001 From: Justin Donaldson Date: Wed, 4 Mar 2026 16:13:30 -0800 Subject: [PATCH 1/2] [lua] Add regression test for pcall return wrapping Tests that return inside try-catch still uses do...end wrapper (since pcall callback has appended return _hx_pcall_default), while tail returns use bare return. --- tests/unit/src/unit/issues/Issue7350.hx | 33 +++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 tests/unit/src/unit/issues/Issue7350.hx diff --git a/tests/unit/src/unit/issues/Issue7350.hx b/tests/unit/src/unit/issues/Issue7350.hx new file mode 100644 index 00000000000..f46bc59530f --- /dev/null +++ b/tests/unit/src/unit/issues/Issue7350.hx @@ -0,0 +1,33 @@ +package unit.issues; + +class Issue7350 extends Test { + // Immediate return in try: generates `do return 42 end` before + // `return _hx_pcall_default` — without do...end this is a Lua syntax error + // (two returns at the same block level). + function immediateReturnInTry():Int { + try { + return 42; + } catch (e:Dynamic) { + return -1; + } + } + + // Multiple returns at the try-block level: each needs do...end wrapping + // to avoid syntax errors before the appended _hx_pcall_default. + function multiReturnInTry(x:Int):Int { + try { + if (x > 10) return 100; + if (x > 0) return x * 2; + return 0; + } catch (e:Dynamic) { + return -1; + } + } + + function test() { + eq(42, immediateReturnInTry()); + eq(100, multiReturnInTry(20)); + eq(10, multiReturnInTry(5)); + eq(0, multiReturnInTry(-1)); + } +} From c9d27568e4353fad141aec364d91725427dfb50d Mon Sep 17 00:00:00 2001 From: Justin Donaldson Date: Thu, 19 Mar 2026 10:35:35 -0700 Subject: [PATCH 2/2] Rewrite Issue7350 test to exercise pcall return propagation edge cases Address review feedback: old test had returns inside if-blocks where do...end wrapping was redundant and tests passed either way. New test covers null/false/zero return propagation through pcall, fall-through vs return distinction via _hx_pcall_default sentinel, nested try-catch, try-catch in loops, and exception handling paths. --- tests/unit/src/unit/issues/Issue7350.hx | 100 ++++++++++++++++++++---- 1 file changed, 85 insertions(+), 15 deletions(-) diff --git a/tests/unit/src/unit/issues/Issue7350.hx b/tests/unit/src/unit/issues/Issue7350.hx index f46bc59530f..61302f46243 100644 --- a/tests/unit/src/unit/issues/Issue7350.hx +++ b/tests/unit/src/unit/issues/Issue7350.hx @@ -1,33 +1,103 @@ package unit.issues; class Issue7350 extends Test { - // Immediate return in try: generates `do return 42 end` before - // `return _hx_pcall_default` — without do...end this is a Lua syntax error - // (two returns at the same block level). - function immediateReturnInTry():Int { + // Return null through try-catch: null must propagate correctly, + // not be confused with _hx_pcall_default sentinel (Lua-specific). + function returnNullFromTry():Dynamic { try { - return 42; + return null; } catch (e:Dynamic) { - return -1; + return "error"; + } + } + + // Return false through try-catch: falsy values must propagate. + function returnFalseFromTry():Bool { + try { + return false; + } catch (e:Dynamic) { + return true; } } - // Multiple returns at the try-block level: each needs do...end wrapping - // to avoid syntax errors before the appended _hx_pcall_default. - function multiReturnInTry(x:Int):Int { + // Return zero through try-catch. + function returnZeroFromTry():Int { try { - if (x > 10) return 100; - if (x > 0) return x * 2; return 0; } catch (e:Dynamic) { return -1; } } + // Try body falls through without returning: code after try-catch + // must execute (pcall sentinel must NOT trigger a return). + function fallThroughTry():Int { + var x = 1; + try { + x = 2; + } catch (e:Dynamic) { + x = 3; + } + return x + 10; + } + + // Exception in try: catch block should handle it and return. + function returnFromCatch():String { + try { + throw "boom"; + } catch (e:Dynamic) { + return "caught"; + } + } + + // Nested try-catch: inner return must propagate through outer. + function nestedTryCatch():Int { + try { + try { + return 99; + } catch (e:Dynamic) { + return -1; + } + } catch (e:Dynamic) { + return -2; + } + } + + // Try-catch in a loop: return exits the function, not just the loop. + function returnFromTryInLoop():Int { + for (i in 0...5) { + try { + if (i == 3) + return i; + } catch (e:Dynamic) { + return -1; + } + } + return -2; + } + + // Try-catch where try falls through and catch is not entered: + // subsequent code must see side effects from the try body. + function sideEffectsInTry():String { + var buf = new StringBuf(); + buf.add("a"); + try { + buf.add("b"); + } catch (e:Dynamic) { + buf.add("x"); + } + buf.add("c"); + return buf.toString(); + } + function test() { - eq(42, immediateReturnInTry()); - eq(100, multiReturnInTry(20)); - eq(10, multiReturnInTry(5)); - eq(0, multiReturnInTry(-1)); + eq(null, returnNullFromTry()); + eq(false, returnFalseFromTry()); + eq(0, returnZeroFromTry()); + eq(12, fallThroughTry()); + eq("caught", returnFromCatch()); + eq(99, nestedTryCatch()); + eq(3, returnFromTryInLoop()); + eq("abc", sideEffectsInTry()); } }