From bd6b31ab151ae6457221402e54fe6f7f50b89753 Mon Sep 17 00:00:00 2001 From: Dag Brattli Date: Wed, 11 Mar 2026 06:54:00 +0100 Subject: [PATCH 1/3] [Beam] Fix try/catch warnings and reraise unbound variable bug - Skip exception wrapping (#{message => ...}) in catch blocks that don't reference the exception variable, eliminating unused term warnings - Strip side-effect-free expressions (literals, variables, pure BIFs like self/0, node/0) from non-final block positions to avoid "no effect" warnings - Fix reraise() generating unbound MatchValue variable by adding CatchReasonVar to context so Throw uses the raw Erlang reason variable - Add Extended case to containsIdentRef/isCapturedInClosure utilities Co-Authored-By: Claude Opus 4.6 --- src/Fable.Transforms/Beam/ErlangPrinter.fs | 35 +++-- src/Fable.Transforms/Beam/Fable2Beam.Util.fs | 12 ++ src/Fable.Transforms/Beam/Fable2Beam.fs | 141 +++++++++++-------- tests/Beam/TryCatchTests.fs | 58 ++++++++ 4 files changed, 170 insertions(+), 76 deletions(-) diff --git a/src/Fable.Transforms/Beam/ErlangPrinter.fs b/src/Fable.Transforms/Beam/ErlangPrinter.fs index 2156c749b..8276e9a73 100644 --- a/src/Fable.Transforms/Beam/ErlangPrinter.fs +++ b/src/Fable.Transforms/Beam/ErlangPrinter.fs @@ -3,20 +3,25 @@ module Fable.Transforms.ErlangPrinter open Fable.AST.Beam open Fable.Transforms -/// Strip bare `ok` atoms from non-final positions in expression lists. -/// These are stray unit values (F# unit → Erlang `ok`) that have no side effects. -let private stripStrayOk (exprs: ErlExpr list) = +/// Strip expressions with no side effects from non-final positions in expression lists. +/// This removes stray unit values (F# unit → Erlang `ok`), standalone variables, +/// literals, and known pure BIF calls (e.g. `self()`, `node()`) that would otherwise +/// produce "has no effect" warnings from the Erlang compiler. +let private stripNoEffect (exprs: ErlExpr list) = match exprs with | [] | [ _ ] -> exprs | _ -> - let isOkAtom = + let isNoEffect = function - | Literal(AtomLit(Atom "ok")) -> true + | Literal _ -> true | Emit("ok", []) -> true + | Variable _ -> true + | Call(None, ("self" | "node"), []) -> true + | Call(Some "erlang", ("self" | "node"), []) -> true | _ -> false - let nonFinal = exprs.[.. exprs.Length - 2] |> List.filter (not << isOkAtom) + let nonFinal = exprs.[.. exprs.Length - 2] |> List.filter (not << isNoEffect) nonFinal @ [ exprs.[exprs.Length - 1] ] module Output = @@ -258,7 +263,7 @@ module Output = sb.Append(") ->") |> ignore sb.AppendLine() |> ignore - let body = stripStrayOk clause.Body + let body = stripNoEffect clause.Body body |> List.iteri (fun j bodyExpr -> @@ -296,7 +301,7 @@ module Output = sb.Append(") ->") |> ignore sb.AppendLine() |> ignore - let body = stripStrayOk clause.Body + let body = stripNoEffect clause.Body body |> List.iteri (fun j bodyExpr -> @@ -340,7 +345,7 @@ module Output = sb.Append(" ->") |> ignore sb.AppendLine() |> ignore - let caseBody = stripStrayOk clause.Body + let caseBody = stripNoEffect clause.Body caseBody |> List.iteri (fun j bodyExpr -> @@ -371,7 +376,7 @@ module Output = // Wrap multi-expression blocks in begin...end to avoid // comma-separated expressions being misinterpreted as // separate function call arguments - let filtered = stripStrayOk exprs + let filtered = stripNoEffect exprs let needsBeginEnd = filtered.Length > 1 if needsBeginEnd then @@ -406,7 +411,7 @@ module Output = | TryCatch(body, catchVar, catchBody, after) -> sb.AppendLine("try") |> ignore - let tryBody = stripStrayOk body + let tryBody = stripNoEffect body tryBody |> List.iteri (fun i bodyExpr -> @@ -425,7 +430,7 @@ module Output = writeIndent () sb.AppendLine($" _:%s{catchVar} ->") |> ignore - let catchBody' = stripStrayOk catchBody + let catchBody' = stripNoEffect catchBody catchBody' |> List.iteri (fun i bodyExpr -> @@ -498,7 +503,7 @@ module Output = sb.Append(" ->") |> ignore sb.AppendLine() |> ignore - let caseBody = stripStrayOk clause.Body + let caseBody = stripNoEffect clause.Body caseBody |> List.iteri (fun j bodyExpr -> @@ -525,7 +530,7 @@ module Output = sb.Append(" ->") |> ignore sb.AppendLine() |> ignore - let afterBody = stripStrayOk bodyExprs + let afterBody = stripNoEffect bodyExprs afterBody |> List.iteri (fun j bodyExpr -> @@ -573,7 +578,7 @@ module Output = sb.Append(" ->") |> ignore sb.AppendLine() |> ignore - let topBody = stripStrayOk clause.Body + let topBody = stripNoEffect clause.Body topBody |> List.iteri (fun i bodyExpr -> diff --git a/src/Fable.Transforms/Beam/Fable2Beam.Util.fs b/src/Fable.Transforms/Beam/Fable2Beam.Util.fs index 3934a696b..ca940b82c 100644 --- a/src/Fable.Transforms/Beam/Fable2Beam.Util.fs +++ b/src/Fable.Transforms/Beam/Fable2Beam.Util.fs @@ -74,6 +74,12 @@ let rec containsIdentRef (name: string) (expr: Expr) : bool = || match baseCall with | Some e -> containsIdentRef name e | None -> false + | Extended(kind, _) -> + match kind with + | Throw(Some e, _) -> containsIdentRef name e + | Throw(None, _) + | Debugger + | Curry _ -> false | _ -> false /// Check if an identifier is captured inside a closure (Lambda/Delegate) within the expression. @@ -128,6 +134,12 @@ let isCapturedInClosure (name: string) (expr: Expr) : bool = || match baseCall with | Some e -> check inClosure e | None -> false + | Extended(kind, _) -> + match kind with + | Throw(Some e, _) -> check inClosure e + | Throw(None, _) + | Debugger + | Curry _ -> false | _ -> false check false expr diff --git a/src/Fable.Transforms/Beam/Fable2Beam.fs b/src/Fable.Transforms/Beam/Fable2Beam.fs index 645f3672c..7094e59cf 100644 --- a/src/Fable.Transforms/Beam/Fable2Beam.fs +++ b/src/Fable.Transforms/Beam/Fable2Beam.fs @@ -52,6 +52,7 @@ type Context = CtorFieldExprs: Map // field name -> Erlang expr during constructor ClassFieldPrefix: bool // When true, NewRecord uses "field_" prefix for map keys (for explicit val field class ctors) CtorParamNames: Set // Constructor parameter names (stored as class fields) + CatchReasonVar: (string * string) option // (catch ident name, Erlang reason var name) for reraise support } /// Check if an entity ref refers to an interface type @@ -759,7 +760,12 @@ let rec transformExpr (com: IBeamCompiler) (ctx: Context) (expr: Expr) : Beam.Er let reasonVar = $"Exn_reason_%d{ctr}" let identVar = capitalizeFirst ident.Name - let ctx' = { ctx with LocalVars = ctx.LocalVars.Add(ident.Name) } + let ctx' = + { ctx with + LocalVars = ctx.LocalVars.Add(ident.Name) + CatchReasonVar = Some(ident.Name, reasonVar) + } + let erlCatchBody = transformExpr com ctx' catchExpr let catchBodyExprs = @@ -767,80 +773,86 @@ let rec transformExpr (com: IBeamCompiler) (ctx: Context) (expr: Expr) : Beam.Er | Beam.ErlExpr.Block es -> es | e -> [ e ] - let reasonRef = Beam.ErlExpr.Variable reasonVar + // Only generate the exception wrapping/binding when the catch body + // actually references the exception identifier. This avoids unused + // term warnings from the Erlang compiler. + if containsIdentRef ident.Name catchExpr then + let reasonRef = Beam.ErlExpr.Variable reasonVar - let formatExpr = - Beam.ErlExpr.Call( - None, - "iolist_to_binary", - [ - Beam.ErlExpr.Call( - Some "io_lib", - "format", - [ - Beam.ErlExpr.Call( - None, - "binary_to_list", - [ Beam.ErlExpr.Literal(Beam.ErlLiteral.StringLit "~p") ] - ) - Beam.ErlExpr.List [ reasonRef ] - ] - ) - ] - ) - - let messageExpr = - Beam.ErlExpr.Case( - reasonRef, - [ - { - Pattern = Beam.PWildcard - Guard = [ Beam.ErlExpr.Call(None, "is_binary", [ reasonRef ]) ] - Body = [ reasonRef ] - } - { - Pattern = Beam.PWildcard - Guard = [] - Body = [ formatExpr ] - } - ] - ) + let formatExpr = + Beam.ErlExpr.Call( + None, + "iolist_to_binary", + [ + Beam.ErlExpr.Call( + Some "io_lib", + "format", + [ + Beam.ErlExpr.Call( + None, + "binary_to_list", + [ Beam.ErlExpr.Literal(Beam.ErlLiteral.StringLit "~p") ] + ) + Beam.ErlExpr.List [ reasonRef ] + ] + ) + ] + ) - // If reason is already a map (F# exception) or reference (class inheriting exn), use it directly. - // Otherwise wrap in #{message => ...} for .Message access. - let bindIdent = - Beam.ErlExpr.Match( - Beam.PVar identVar, + let messageExpr = Beam.ErlExpr.Case( reasonRef, [ { Pattern = Beam.PWildcard - Guard = [ Beam.ErlExpr.Call(None, "is_map", [ reasonRef ]) ] - Body = [ reasonRef ] - } - { - Pattern = Beam.PWildcard - Guard = [ Beam.ErlExpr.Call(None, "is_reference", [ reasonRef ]) ] + Guard = [ Beam.ErlExpr.Call(None, "is_binary", [ reasonRef ]) ] Body = [ reasonRef ] } { Pattern = Beam.PWildcard Guard = [] - Body = - [ - Beam.ErlExpr.Map - [ - Beam.ErlExpr.Literal(Beam.ErlLiteral.AtomLit(Beam.Atom "message")), - messageExpr - ] - ] + Body = [ formatExpr ] } ] ) - ) - Beam.ErlExpr.TryCatch(bodyExprs, reasonVar, [ bindIdent ] @ catchBodyExprs, afterExprs) + // If reason is already a map (F# exception) or reference (class inheriting exn), use it directly. + // Otherwise wrap in #{message => ...} for .Message access. + let bindIdent = + Beam.ErlExpr.Match( + Beam.PVar identVar, + Beam.ErlExpr.Case( + reasonRef, + [ + { + Pattern = Beam.PWildcard + Guard = [ Beam.ErlExpr.Call(None, "is_map", [ reasonRef ]) ] + Body = [ reasonRef ] + } + { + Pattern = Beam.PWildcard + Guard = [ Beam.ErlExpr.Call(None, "is_reference", [ reasonRef ]) ] + Body = [ reasonRef ] + } + { + Pattern = Beam.PWildcard + Guard = [] + Body = + [ + Beam.ErlExpr.Map + [ + Beam.ErlExpr.Literal(Beam.ErlLiteral.AtomLit(Beam.Atom "message")), + messageExpr + ] + ] + } + ] + ) + ) + + Beam.ErlExpr.TryCatch(bodyExprs, reasonVar, [ bindIdent ] @ catchBodyExprs, afterExprs) + else + Beam.ErlExpr.TryCatch(bodyExprs, reasonVar, catchBodyExprs, afterExprs) | None, [] -> // No catch handler and no finalizer erlBody @@ -1156,8 +1168,14 @@ let rec transformExpr (com: IBeamCompiler) (ctx: Context) (expr: Expr) : Beam.Er | Extended(kind, _range) -> match kind with | Throw(Some exprArg, _typ) -> - let erlExpr = transformExpr com ctx exprArg - Beam.ErlExpr.Call(Some "erlang", "error", [ erlExpr ]) + // For reraise: if the thrown expression references the catch ident, + // use the raw Erlang reason variable to preserve the original exception. + match exprArg, ctx.CatchReasonVar with + | IdentExpr ident, Some(catchIdentName, reasonVar) when ident.Name = catchIdentName -> + Beam.ErlExpr.Call(Some "erlang", "error", [ Beam.ErlExpr.Variable reasonVar ]) + | _ -> + let erlExpr = transformExpr com ctx exprArg + Beam.ErlExpr.Call(Some "erlang", "error", [ erlExpr ]) | Throw(None, _typ) -> // Re-raise (should not normally happen outside catch context) Beam.ErlExpr.Call( @@ -3355,6 +3373,7 @@ let transformFile (com: Fable.Compiler) (file: File) : Beam.ErlModule = CtorFieldExprs = Map.empty ClassFieldPrefix = false CtorParamNames = Set.empty + CatchReasonVar = None } let ctorNameRegistry = System.Collections.Generic.Dictionary() diff --git a/tests/Beam/TryCatchTests.fs b/tests/Beam/TryCatchTests.fs index 66db821de..2d6ab7d4b 100644 --- a/tests/Beam/TryCatchTests.fs +++ b/tests/Beam/TryCatchTests.fs @@ -54,6 +54,64 @@ let ``test doesntThrow works`` () = let ``test throwsAnyError with division by zero works`` () = throwsAnyError (fun () -> 1 / 0) +[] +let ``test try-catch with wildcard ignoring exception produces no warnings`` () = + // Regression: catch with wildcard `_` previously generated unused #{message => ...} map + let result = + try + failwith "ignored" + 0 + with _ -> + 42 + result |> equal 42 + +[] +let ``test reraise preserves exception`` () = + // Regression: reraise() generated unbound MatchValue variable + let msg = + try + try + failwith "original" + "" + with _ -> + reraise () + with e -> + e.Message + msg |> equal "original" + +[] +let ``test reraise in try-catch with side effect`` () = + // Matches the Seq.fs pattern: try ... with _ -> sideEffect(); reraise() + let mutable sideEffectRan = false + let msg = + try + try + failwith "boom" + "" + with _ -> + sideEffectRan <- true + reraise () + with e -> + e.Message + sideEffectRan |> equal true + msg |> equal "boom" + +[] +let ``test try-catch result used in match`` () = + // Regression: try/catch result fed into pattern match (the Seq.fs generateWhileSome pattern) + let compute (x: int) : int option = + if x > 0 then Some(x * 2) else None + let result = + match + try + compute 5 + with _ -> + reraise () + with + | None -> -1 + | Some v -> v + result |> equal 10 + [] let ``test nested try-catch works`` () = let result = From 678ae9ba35761df03ba920c9cd5dc272c951b624 Mon Sep 17 00:00:00 2001 From: Dag Brattli Date: Wed, 11 Mar 2026 06:56:11 +0100 Subject: [PATCH 2/3] [Beam] Add changelog entries for try/catch and reraise fixes Co-Authored-By: Claude Opus 4.6 --- src/Fable.Cli/CHANGELOG.md | 6 ++++++ src/Fable.Compiler/CHANGELOG.md | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/src/Fable.Cli/CHANGELOG.md b/src/Fable.Cli/CHANGELOG.md index cd14566ed..641ce89e6 100644 --- a/src/Fable.Cli/CHANGELOG.md +++ b/src/Fable.Cli/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Fixed + +* [Beam] Fix unused term warning in try/catch when exception variable is not referenced (by @dbrattli) +* [Beam] Fix "no effect" warning for pure BIF calls (`self/0`, `node/0`) in non-final block positions (by @dbrattli) +* [Beam] Fix `reraise()` generating unbound `MatchValue` variable — use raw Erlang reason variable for re-throw (by @dbrattli) + ## 5.0.0-rc.3 - 2026-03-10 ### Added diff --git a/src/Fable.Compiler/CHANGELOG.md b/src/Fable.Compiler/CHANGELOG.md index 59a3a4f8a..22041a5a4 100644 --- a/src/Fable.Compiler/CHANGELOG.md +++ b/src/Fable.Compiler/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Fixed + +* [Beam] Fix unused term warning in try/catch when exception variable is not referenced (by @dbrattli) +* [Beam] Fix "no effect" warning for pure BIF calls (`self/0`, `node/0`) in non-final block positions (by @dbrattli) +* [Beam] Fix `reraise()` generating unbound `MatchValue` variable — use raw Erlang reason variable for re-throw (by @dbrattli) + ## 5.0.0-rc.10 - 2026-03-10 ### Added From c52d2358521b1b6933e5eea8109333b949cea3c4 Mon Sep 17 00:00:00 2001 From: Dag Brattli Date: Wed, 11 Mar 2026 20:11:30 +0100 Subject: [PATCH 3/3] [Beam] Fix Seq.toList on strings by routing through fable_utils:to_list fable_utils:to_list already handles binaries (strings) via unicode:characters_to_list, but Seq.toList was falling through to the compiled seq:to_list which doesn't handle raw BEAM types. Add explicit Replacement for Seq.toList to use fable_utils:to_list. Co-Authored-By: Claude Opus 4.6 --- src/Fable.Transforms/Beam/Replacements.fs | 4 ++++ tests/Beam/SeqTests.fs | 6 ++++++ 2 files changed, 10 insertions(+) diff --git a/src/Fable.Transforms/Beam/Replacements.fs b/src/Fable.Transforms/Beam/Replacements.fs index 1e8b3a0b8..3610e545a 100644 --- a/src/Fable.Transforms/Beam/Replacements.fs +++ b/src/Fable.Transforms/Beam/Replacements.fs @@ -1887,6 +1887,10 @@ let private seqModule = match info.CompiledName, args with | "Cast", [ arg ] -> Some arg + | "ToList", [ arg ] -> + // Use fable_utils:to_list which handles binaries (strings), plain lists, + // ref-wrapped arrays, enumerator maps, and lazy seqs. + emitExpr r t [ arg ] "fable_utils:to_list($0)" |> Some | "ToArray", [ arg ] -> // seq:to_array already returns a ref-wrapped array, don't double-wrap Helper.LibCall(com, "seq", "to_array", t, [ arg ], info.SignatureArgTypes, ?loc = r) diff --git a/tests/Beam/SeqTests.fs b/tests/Beam/SeqTests.fs index 2eb0b3622..ac5a08b4a 100644 --- a/tests/Beam/SeqTests.fs +++ b/tests/Beam/SeqTests.fs @@ -1034,3 +1034,9 @@ let ``test Seq.except works with various types`` () = Seq.except [(1, 2)] [(1, 2)] |> Seq.isEmpty |> equal true Seq.except [|49|] [|7; 49|] |> Seq.last |> equal 7 Seq.except [{ Bar= "test" }] [{ Bar = "test" }] |> Seq.isEmpty |> equal true + +[] +let ``test Seq.toList on string works`` () = + let text = "ABC" + let chars = text |> Seq.toList + chars |> List.length |> equal 3