From f4b5b9cc9d785d29f10aaa6eca963efb20162388 Mon Sep 17 00:00:00 2001 From: Mangel Maxime Date: Sun, 1 Nov 2026 23:10:11 +0100 Subject: [PATCH] fix(all): keep reads of imported mutable values in place --- src/Fable.Transforms/FableTransforms.fs | 12 +++++++++++- tests/Js/Main/MiscTests.fs | 5 +++++ tests/Js/Main/MiscTestsHelper.fs | 6 ++++++ tests/Python/MiscTestsHelper.fs | 6 ++++++ tests/Python/TestMisc.fs | 6 ++++++ 5 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/Fable.Transforms/FableTransforms.fs b/src/Fable.Transforms/FableTransforms.fs index 907395faff..70730b2074 100644 --- a/src/Fable.Transforms/FableTransforms.fs +++ b/src/Fable.Transforms/FableTransforms.fs @@ -409,11 +409,21 @@ module private Transforms = | _ -> ValueNone let tryInlineBinding (com: Compiler) (ident: Ident) value letBody = + let readsMutableValue callee = + match callee with + | IdentExpr id -> id.IsMutable + | Import({ Kind = MemberImport memberRef }, _, _) -> + com.TryGetMember(memberRef) |> Option.exists (fun memb -> memb.IsMutable) + | _ -> false + let canInlineBinding = match value with | Import(i, _, _) -> i.IsCompilerGenerated | Call(callee, info, _, _) when List.isEmpty info.Args && List.contains "value" info.Tags -> - canInlineArg com ident.Name callee letBody + if readsMutableValue callee then + canInlineArg com ident.Name value letBody + else + canInlineArg com ident.Name callee letBody // Replace non-recursive lambda bindings | NestedLambda(_args, lambdaBody, _name) -> match lambdaBody with diff --git a/tests/Js/Main/MiscTests.fs b/tests/Js/Main/MiscTests.fs index 94153e6d16..a695d7b7a1 100644 --- a/tests/Js/Main/MiscTests.fs +++ b/tests/Js/Main/MiscTests.fs @@ -1314,6 +1314,11 @@ let tests = Option.defaultWith (fun () -> invalidOp "This is invalid") (Some 3) |> equal 3 throwsError "This is invalid" (fun () -> Option.defaultWith (fun () -> invalidOp "This is invalid") None) + testCase "A read of another file's mutable value is not moved past side effects" <| fun () -> // See #5156 + let before = MiscTestsHelper.trackedCalls + MiscTestsHelper.track 1 |> ignore + MiscTestsHelper.trackedCalls - before |> equal 1 + testCase "Pattern matching optimization works (switch statement)" <| fun () -> let mutable x = "" let i = 4 diff --git a/tests/Js/Main/MiscTestsHelper.fs b/tests/Js/Main/MiscTestsHelper.fs index 3157415696..3f20c76912 100644 --- a/tests/Js/Main/MiscTestsHelper.fs +++ b/tests/Js/Main/MiscTestsHelper.fs @@ -31,3 +31,9 @@ type Vector2<[] 'u> = Vector2 of x: float<'u> * y: float<'u> with static member inline ( + ) (Vector2(ax, ay), Vector2(bx, by)) = Vector2(ax + bx, ay + by) static member inline ( * ) (scalar, Vector2(x, y)) = Vector2(scalar * x, scalar * y) + +let mutable trackedCalls = 0 + +let track x = + trackedCalls <- trackedCalls + 1 + x diff --git a/tests/Python/MiscTestsHelper.fs b/tests/Python/MiscTestsHelper.fs index 8f4352e620..ae7f2ab643 100644 --- a/tests/Python/MiscTestsHelper.fs +++ b/tests/Python/MiscTestsHelper.fs @@ -42,3 +42,9 @@ type CrossModuleDemo = static member propDefault = A "prop" static member methDefault() = A "meth" + +let mutable trackedCalls = 0 + +let track x = + trackedCalls <- trackedCalls + 1 + x diff --git a/tests/Python/TestMisc.fs b/tests/Python/TestMisc.fs index ab47fc7125..2e7d545f41 100644 --- a/tests/Python/TestMisc.fs +++ b/tests/Python/TestMisc.fs @@ -1833,3 +1833,9 @@ let ``test lock storage releases idle entries after exceptions`` () = with _ -> () equal before storage.count #endif + +[] +let ``test A read of another file's mutable value is not moved past side effects`` () = // See #5156 + let before = MiscTestsHelper.trackedCalls + MiscTestsHelper.track 1 |> ignore + MiscTestsHelper.trackedCalls - before |> equal 1