diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md index c843ddb3df1..82b406ad2fd 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -1,5 +1,6 @@ ### Fixed +* Fix optimizer eliminating side-effectful receiver of unit-typed member access (e.g. inside `task { (effectful()).UnitProp }`). ([Issue #13099](https://github.com/dotnet/fsharp/issues/13099), [PR #19885](https://github.com/dotnet/fsharp/pull/19885)) * Suppress hover/symbol resolution for wildcard `_` patterns inside `member _.…` bodies that incorrectly showed `val _: T` tooltip. ([PR #19760](https://github.com/dotnet/fsharp/pull/19760)) * Deduplicate format specifier locations in computation expressions so editor tooling no longer reports duplicate entries for the same `%` specifier. ([Issue #16419](https://github.com/dotnet/fsharp/issues/16419), [PR #19791](https://github.com/dotnet/fsharp/pull/19791)) * Reject non-function bindings for single-case and partial active pattern names with FS1209, matching the existing multi-case behavior. ([PR #19763](https://github.com/dotnet/fsharp/pull/19763)) diff --git a/src/Compiler/Optimize/Optimizer.fs b/src/Compiler/Optimize/Optimizer.fs index 3cbb574598c..99328d04314 100644 --- a/src/Compiler/Optimize/Optimizer.fs +++ b/src/Compiler/Optimize/Optimizer.fs @@ -1715,6 +1715,24 @@ let TryEliminateBinding cenv _env bind e2 _m = | _ -> None let (DebugPoints(e2, recreate0)) = e2 + + // Protect side-effectful member receiver temporaries from being dropped by + // state-machine lowering. After the optimizer inlines a unit-returning property + // access (e.g. task { (f()).End }), it produces: + // let this = receiver in () + // where 'this' has IsMemberThisVal = true. LowerStateMachines.BindResumableCodeDefinitions + // treats any IsMemberThisVal binding as a resumable-code definition and removes it from + // the expression tree. If the variable is unused the side-effectful receiver is silently + // lost. Converting to Expr.Sequential here makes the expression invisible to that pass. + // See issue #13099. + let xUnusedInBody () = + let fvs = accFreeInExpr (CollectLocalsWithStackGuard()) e2 emptyFreeVars + not (Zset.contains vspec1 fvs.FreeLocals) + + if vspec1.IsMemberThisVal && ExprHasEffect g e1 && xUnusedInBody () then + Some (mkSequential e1.Range e1 e2 |> recreate0) + else + match e2 with // Immediate consumption of value as itself 'let x = e in x' diff --git a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj index acfeca79f35..ac12744ef95 100644 --- a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj +++ b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj @@ -268,6 +268,7 @@ + diff --git a/tests/FSharp.Compiler.ComponentTests/Optimizations/TaskCEUnitPropertyAccess.fs b/tests/FSharp.Compiler.ComponentTests/Optimizations/TaskCEUnitPropertyAccess.fs new file mode 100644 index 00000000000..9c2d3692afc --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/Optimizations/TaskCEUnitPropertyAccess.fs @@ -0,0 +1,186 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +namespace Optimizations + +open Xunit +open FSharp.Test.Compiler + +// Regression tests for https://github.com/dotnet/fsharp/issues/13099 +// The Release-mode optimizer was eliminating a side-effectful receiver expression +// when its result flowed into a unit-typed member access (e.g. `.End`) inside `task { }`. +// +// In Debug, the call site is preserved and the side-effectful exception is observed. +// In Release, the optimizer reduces the unit-typed property access to the unit constant +// and drops the receiver, swallowing the exception. +// +// Each test compiles a snippet under both Debug (optimize=false) and Release +// (optimize=true) and asserts the program completes without an unhandled exception. +// +// Note: snippets execute *in-process* via reflection. They MUST NOT call `exit` +// because that would terminate the test runner. Use `failwith` (or simply let an +// expected exception propagate) to signal test failure. +module TaskCEUnitPropertyAccess = + + let private run (optimize: bool) (source: string) = + Fsx source + |> asExe + |> withOptimization optimize + |> compileExeAndRun + |> shouldSucceed + |> ignore + + [] // Debug + [] // Release + [] + let ``TaskCE_UnitPropertyAccess_PreservesReceiverSideEffects`` (optimize: bool) = + run optimize """ +type SomeOutputType() = + member x.End = () + +let someFunctionWithReturnType () = + failwith "This should be raised" + SomeOutputType() + +let theTaskAtHand () = + task { (someFunctionWithReturnType ()).End } + +try + theTaskAtHand().Wait() + failwith "Expected exception was not raised; the optimizer dropped the side-effectful receiver." +with +| :? System.AggregateException as ex when ex.InnerException.Message = "This should be raised" -> () +""" + + [] + [] + [] + let ``UnitPropertyAccess_OnSideEffectfulReceiver_OutsideTaskCE_Raises`` (optimize: bool) = + run optimize """ +type SomeOutputType() = + member x.End = () + +let someFunctionWithReturnType () = + failwith "boom" + SomeOutputType() + +let test () = (someFunctionWithReturnType ()).End + +try + test () + failwith "Expected exception was not raised; the optimizer dropped the side-effectful receiver." +with +| ex when ex.Message = "boom" -> () +""" + + [] + [] + [] + let ``TaskCE_UnitMethodCall_PreservesReceiverSideEffects`` (optimize: bool) = + run optimize """ +type SomeOutputType() = + member x.Finish() = () + +let someFunctionWithReturnType () = + failwith "boom" + SomeOutputType() + +let theTaskAtHand () = + task { (someFunctionWithReturnType ()).Finish() } + +try + theTaskAtHand().Wait() + failwith "Expected exception was not raised; the optimizer dropped the side-effectful receiver." +with +| :? System.AggregateException as ex when ex.InnerException.Message = "boom" -> () +""" + + [] + [] + [] + let ``TaskCE_UnitPropertyAccess_RunsBothReceiverAndGetterEffects`` (optimize: bool) = + run optimize """ +let mutable counter = 0 + +type Tracker() = + member x.Done = counter <- counter + 1 + +let makeTracker () = + counter <- counter + 1 + Tracker() + +let theTaskAtHand () = + task { (makeTracker ()).Done } + +theTaskAtHand().Wait() +if counter <> 2 then + failwithf "Expected counter=2 (receiver + getter side effects) but got %d" counter +""" + + [] + [] + [] + let ``TaskCE_NonUnitPropertyAccess_OnSideEffectfulReceiver_Raises`` (optimize: bool) = + run optimize """ +type HasValue() = + member x.Value = 42 + +let makeHasValue () = + failwith "boom" + HasValue() + +let theTaskAtHand () = + task { let _ = (makeHasValue ()).Value in () } + +try + theTaskAtHand().Wait() + failwith "Expected exception was not raised." +with +| :? System.AggregateException as ex when ex.InnerException.Message = "boom" -> () +""" + + [] + [] + [] + let ``AsyncCE_UnitPropertyAccess_PreservesReceiverSideEffects`` (optimize: bool) = + run optimize """ +type SomeOutputType() = + member x.End = () + +let someFunctionWithReturnType () = + failwith "boom" + SomeOutputType() + +let theAsyncAtHand () = + async { (someFunctionWithReturnType ()).End } + +try + theAsyncAtHand () |> Async.RunSynchronously + failwith "Expected exception was not raised." +with +| ex when ex.Message = "boom" -> () +""" + + [] + [] + [] + let ``TaskCE_NestedUnitPropertyAccess_PreservesReceiverSideEffects`` (optimize: bool) = + run optimize """ +type Inner() = + member x.End = () + +type Outer() = + member x.Inner = Inner() + +let makeOuter () = + failwith "boom" + Outer() + +let theTaskAtHand () = + task { (makeOuter ()).Inner.End } + +try + theTaskAtHand().Wait() + failwith "Expected exception was not raised; the optimizer dropped the side-effectful receiver." +with +| :? System.AggregateException as ex when ex.InnerException.Message = "boom" -> () +""" diff --git a/tests/ILVerify/ilverify_FSharp.Compiler.Service_Debug_net10.0.bsl b/tests/ILVerify/ilverify_FSharp.Compiler.Service_Debug_net10.0.bsl index 51b95b60067..424ee9bee24 100644 --- a/tests/ILVerify/ilverify_FSharp.Compiler.Service_Debug_net10.0.bsl +++ b/tests/ILVerify/ilverify_FSharp.Compiler.Service_Debug_net10.0.bsl @@ -6,6 +6,6 @@ [IL]: Error [ReturnPtrToStack]: : Internal.Utilities.Text.Lexing.LexBuffer`1::get_LexemeView()][offset 0x00000019] Return type is ByRef, TypedReference, ArgHandle, or ArgIterator. [IL]: Error [ReturnPtrToStack]: : FSharp.Compiler.CodeAnalysis.ItemKeyStore::ReadKeyString([System.Reflection.Metadata]System.Reflection.Metadata.BlobReader&)][offset 0x00000026] Return type is ByRef, TypedReference, ArgHandle, or ArgIterator. [IL]: Error [ReturnPtrToStack]: : FSharp.Compiler.CodeAnalysis.ItemKeyStore::ReadFirstKeyString()][offset 0x00000070] Return type is ByRef, TypedReference, ArgHandle, or ArgIterator. -[IL]: Error [StackUnexpected]: : FSharp.Compiler.CodeAnalysis.ItemKeyStoreBuilder::writeRange([FSharp.Compiler.Service]FSharp.Compiler.Text.Range)][offset 0x00000011][found address of '[FSharp.Compiler.Service]FSharp.Compiler.Text.Range'][expected Native Int] Unexpected type on the stack. +[IL]: Error [StackUnexpected]: : FSharp.Compiler.CodeAnalysis.ItemKeyStoreBuilder::writeRange([FSharp.Compiler.Service]FSharp.Compiler.Text.Range)][offset 0x0000000F][found address of '[FSharp.Compiler.Service]FSharp.Compiler.Text.Range'][expected Native Int] Unexpected type on the stack. [IL]: Error [ExpectedNumericType]: : FSharp.Compiler.EditorServices.SemanticClassificationKeyStoreBuilder::WriteAll([FSharp.Compiler.Service]FSharp.Compiler.EditorServices.SemanticClassificationItem[])][offset 0x0000001D][found address of '[FSharp.Compiler.Service]FSharp.Compiler.EditorServices.SemanticClassificationItem'] Expected numeric type on the stack. [IL]: Error [UnmanagedPointer]: : FSharp.Compiler.Interactive.Shell+Utilities+pointerToNativeInt::Invoke(object)][offset 0x00000007] Unmanaged pointers are not a verifiable type. diff --git a/tests/ILVerify/ilverify_FSharp.Compiler.Service_Debug_netstandard2.0.bsl b/tests/ILVerify/ilverify_FSharp.Compiler.Service_Debug_netstandard2.0.bsl index 51b95b60067..424ee9bee24 100644 --- a/tests/ILVerify/ilverify_FSharp.Compiler.Service_Debug_netstandard2.0.bsl +++ b/tests/ILVerify/ilverify_FSharp.Compiler.Service_Debug_netstandard2.0.bsl @@ -6,6 +6,6 @@ [IL]: Error [ReturnPtrToStack]: : Internal.Utilities.Text.Lexing.LexBuffer`1::get_LexemeView()][offset 0x00000019] Return type is ByRef, TypedReference, ArgHandle, or ArgIterator. [IL]: Error [ReturnPtrToStack]: : FSharp.Compiler.CodeAnalysis.ItemKeyStore::ReadKeyString([System.Reflection.Metadata]System.Reflection.Metadata.BlobReader&)][offset 0x00000026] Return type is ByRef, TypedReference, ArgHandle, or ArgIterator. [IL]: Error [ReturnPtrToStack]: : FSharp.Compiler.CodeAnalysis.ItemKeyStore::ReadFirstKeyString()][offset 0x00000070] Return type is ByRef, TypedReference, ArgHandle, or ArgIterator. -[IL]: Error [StackUnexpected]: : FSharp.Compiler.CodeAnalysis.ItemKeyStoreBuilder::writeRange([FSharp.Compiler.Service]FSharp.Compiler.Text.Range)][offset 0x00000011][found address of '[FSharp.Compiler.Service]FSharp.Compiler.Text.Range'][expected Native Int] Unexpected type on the stack. +[IL]: Error [StackUnexpected]: : FSharp.Compiler.CodeAnalysis.ItemKeyStoreBuilder::writeRange([FSharp.Compiler.Service]FSharp.Compiler.Text.Range)][offset 0x0000000F][found address of '[FSharp.Compiler.Service]FSharp.Compiler.Text.Range'][expected Native Int] Unexpected type on the stack. [IL]: Error [ExpectedNumericType]: : FSharp.Compiler.EditorServices.SemanticClassificationKeyStoreBuilder::WriteAll([FSharp.Compiler.Service]FSharp.Compiler.EditorServices.SemanticClassificationItem[])][offset 0x0000001D][found address of '[FSharp.Compiler.Service]FSharp.Compiler.EditorServices.SemanticClassificationItem'] Expected numeric type on the stack. [IL]: Error [UnmanagedPointer]: : FSharp.Compiler.Interactive.Shell+Utilities+pointerToNativeInt::Invoke(object)][offset 0x00000007] Unmanaged pointers are not a verifiable type.