From 72a3c3aef580537ec273da8fe965f7b1e9698c51 Mon Sep 17 00:00:00 2001 From: Copilot Date: Fri, 29 May 2026 18:31:49 +0200 Subject: [PATCH 1/5] Sprint 02: RED tests for #19457 (nested let! in plain let in CE) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Language/ComputationExpressionTests.fs | 155 +++++++++++++++++- 1 file changed, 153 insertions(+), 2 deletions(-) diff --git a/tests/FSharp.Compiler.ComponentTests/Language/ComputationExpressionTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/ComputationExpressionTests.fs index f34388a5494..db20f381f55 100644 --- a/tests/FSharp.Compiler.ComponentTests/Language/ComputationExpressionTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Language/ComputationExpressionTests.fs @@ -2400,9 +2400,9 @@ let foo() = |> typecheck |> shouldSucceed - // https://github.com/dotnet/fsharp/issues/19456 + // https://github.com/dotnet/fsharp/issues/19457 [] - let ``Issue 19456 - let bang nested in plain let binding inside task CE should raise FS0750`` () = + let ``Issue 19457 - let bang nested in plain let binding inside task CE should compile`` () = FSharp """ open System.Threading.Tasks @@ -2413,6 +2413,157 @@ let y() = b return a } + """ + |> asLibrary + |> typecheck + |> shouldSucceed + + // https://github.com/dotnet/fsharp/issues/19457 + [] + let ``Issue 19457 - let bang nested in plain let returns awaited value not Task`` () = + FSharp """ +module Test +open System.Threading.Tasks +let y() = + task { + let a = + let! b = Task.FromResult(42) + b + return a + } +[] +let main _ = + let r = y().Result + if r <> 42 then failwithf "expected 42, got %d" r + 0 + """ + |> compileExeAndRun + |> shouldSucceed + + // https://github.com/dotnet/fsharp/issues/19457 + [] + let ``Issue 19457 - do bang nested in plain let inside task CE compiles and runs`` () = + FSharp """ +module Test +open System.Threading.Tasks +let mutable x = 0 +let test() = + task { + let a = + do! Task.Delay(0) + x <- 1 + 42 + return a + } +[] +let main _ = + let r = test().Result + if r <> 42 then failwithf "expected 42, got %d" r + if x <> 1 then failwithf "expected x=1, got %d" x + 0 + """ + |> compileExeAndRun + |> shouldSucceed + + // https://github.com/dotnet/fsharp/issues/19457 + [] + let ``Issue 19457 - multiple sequential let bang nested in plain let inside task CE`` () = + FSharp """ +module Test +open System.Threading.Tasks +let test() = + task { + let result = + let! a = Task.FromResult(1) + let! b = Task.FromResult(2) + a + b + return result + } +[] +let main _ = + let r = test().Result + if r <> 3 then failwithf "expected 3, got %d" r + 0 + """ + |> compileExeAndRun + |> shouldSucceed + + // https://github.com/dotnet/fsharp/issues/19457 + [] + let ``Issue 19457 - let bang nested in plain let inside async CE`` () = + FSharp """ +module Test +let test() = + async { + let a = + let! b = async { return 42 } + b + return a + } +[] +let main _ = + let r = Async.RunSynchronously(test()) + if r <> 42 then failwithf "expected 42, got %d" r + 0 + """ + |> compileExeAndRun + |> shouldSucceed + + // https://github.com/dotnet/fsharp/issues/19457 + [] + let ``Issue 19457 - let in let in let bang deep nesting inside task CE`` () = + FSharp """ +module Test +open System.Threading.Tasks +let test() = + task { + let a = + let c = 10 + let! b = Task.FromResult(c) + b + return a + } +[] +let main _ = + let r = test().Result + if r <> 10 then failwithf "expected 10, got %d" r + 0 + """ + |> compileExeAndRun + |> shouldSucceed + + // https://github.com/dotnet/fsharp/issues/19457 + [] + let ``Issue 19457 - outer return uses awaited inner value`` () = + FSharp """ +module Test +open System.Threading.Tasks +let test() = + task { + let a = + let! b = Task.FromResult(42) + b + return a + 1 + } +[] +let main _ = + let r = test().Result + if r <> 43 then failwithf "expected 43, got %d" r + 0 + """ + |> compileExeAndRun + |> shouldSucceed + + // https://github.com/dotnet/fsharp/issues/19457 - regression guard: + // let! outside any CE must still raise FS0750. + [] + let ``Issue 19457 - let bang outside any CE still raises FS0750`` () = + FSharp """ +module Test +open System.Threading.Tasks +let bad() = + let! x = Task.FromResult(1) + x """ |> asLibrary |> typecheck From 4e7d7406164712aca448d0c0c4cb3e01a5065cb1 Mon Sep 17 00:00:00 2001 From: Copilot Date: Fri, 29 May 2026 20:32:14 +0200 Subject: [PATCH 2/5] Sprint 03: fix #19457 - lift CE constructs from plain let RHS in CE When a plain 'let p = rhs in body' appears inside a computation expression, and 'rhs' itself is a chain of CE-only constructs (e.g. 'let! x = ...; x' or 'do! ...; rest'), rewrite the expression so those constructs are lifted into the enclosing CE, where they desugar correctly. This makes the example from issue #19457 compile. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../.FSharp.Compiler.Service/11.0.100.md | 1 + .../CheckComputationExpressions.fs | 176 ++++++++++++++---- 2 files changed, 137 insertions(+), 40 deletions(-) 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 43601b6e34c..feed8aa2b86 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 FS0750 "This construct may only be used within computation expressions" incorrectly raised for `let!`/`do!` appearing in the RHS of a plain `let` binding inside a computation expression. Such CE-only constructs are now lifted into the enclosing CE so the example from the issue compiles. ([Issue #19457](https://github.com/dotnet/fsharp/issues/19457)) * 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)) * Fix FS0421 "The address of the variable cannot be used at this point" incorrectly raised for the discard pattern `let _ = &expr` when `let x = &expr` compiles. ([Issue #18841](https://github.com/dotnet/fsharp/issues/18841), [PR #19811](https://github.com/dotnet/fsharp/pull/19811)) * Honor `--nowarn` and `--warnaserror` for warnings emitted during command-line option parsing ([Issue #19576](https://github.com/dotnet/fsharp/issues/19576), [PR #19776](https://github.com/dotnet/fsharp/pull/19776)) diff --git a/src/Compiler/Checking/Expressions/CheckComputationExpressions.fs b/src/Compiler/Checking/Expressions/CheckComputationExpressions.fs index 3ab446a6687..202ab72fa88 100644 --- a/src/Compiler/Checking/Expressions/CheckComputationExpressions.fs +++ b/src/Compiler/Checking/Expressions/CheckComputationExpressions.fs @@ -1009,6 +1009,40 @@ let requireBuilderMethod methodName ceenv m1 m2 = if not (hasBuilderMethod ceenv m1 methodName) then error (Error(FSComp.SR.tcRequireBuilderMethod methodName, m2)) +/// Detect whether an expression syntactically contains computation-expression-only constructs +/// (such as let!, use!, do!, return, return!, yield, yield!, match!, while!) that can only +/// be legally type-checked inside a CE translation. Used to decide whether to "lift" these +/// constructs out of the RHS of a plain 'let' binding inside a CE so that the surrounding +/// CE translator can process them. We only recurse through positions where the lifted +/// construct would syntactically remain in CE-evaluation position (Sequential tail, +/// LetOrUse body); branches that open a new scope (lambdas, match clauses, if branches, +/// nested CEs) are intentionally not traversed. +let rec private exprContainsCEOnlyConstruct expr = + match expr with + | LetOrUse(_, true, _) -> true + | SynExpr.DoBang _ + | SynExpr.MatchBang _ + | SynExpr.WhileBang _ + | SynExpr.YieldOrReturnFrom _ + | SynExpr.YieldOrReturn _ -> true + | LetOrUse({ Body = body; IsRecursive = false }, false, false) -> exprContainsCEOnlyConstruct body + | SynExpr.Sequential(expr1 = e1; expr2 = e2) -> exprContainsCEOnlyConstruct e1 || exprContainsCEOnlyConstruct e2 + | _ -> false + +/// Walk the binding RHS of a plain 'let p = rhs in body' that appears inside a computation expression, +/// threading the binding 'let p = in body' to the value-position of 'rhs' so that any +/// CE-only constructs (let!, do!, etc.) appearing as a prefix of 'rhs' end up lifted into the +/// enclosing CE, where the CE translator can desugar them properly. See issue dotnet/fsharp#19457. +let rec private liftCEFromBindingRhs (rhs: SynExpr) (k: SynExpr -> SynExpr) : SynExpr = + match rhs with + | SynExpr.LetOrUse data when not data.IsRecursive -> + SynExpr.LetOrUse + { data with + Body = liftCEFromBindingRhs data.Body k + } + | SynExpr.Sequential(sp, isTrueSeq, e1, e2, m, trivia) -> SynExpr.Sequential(sp, isTrueSeq, e1, liftCEFromBindingRhs e2 k, m, trivia) + | _ -> k rhs + /// /// Try translate the syntax sugar /// @@ -1841,51 +1875,113 @@ let rec TryTranslateComputationExpression false, false) -> - // For 'query' check immediately - if ceenv.isQuery then - match (List.map (BindingNormalization.NormalizeBinding ValOrMemberBinding cenv ceenv.env) binds) with - | [ NormalizedBinding(_, SynBindingKind.Normal, false, false, _, _, _, _, _, _, _, _) ] when not isRec -> () - | normalizedBindings -> - let failAt m = - error (Error(FSComp.SR.tcNonSimpleLetBindingInQuery (), m)) + // https://github.com/dotnet/fsharp/issues/19457 + // If the (single) plain 'let p = rhs in body' binding's RHS is itself a chain of CE-only + // constructs (e.g. 'let! x = ...; x' or 'do! ...; rest'), rewrite the expression so that + // those constructs are lifted into the enclosing CE, where they desugar correctly. + // We only apply this when not in a query, when the binding is non-recursive and non-bang, + // and when there is exactly one binding (the case the issue is about). Otherwise we fall + // through to the standard plain-let handling. + let liftedRewrite = + if ceenv.isQuery || isRec then + None + else + match binds with + | [ SynBinding( + accessibility = a + kind = bk + isInline = isInline + isMutable = isMutable + attributes = attrs + xmlDoc = xmlDoc + valData = valData + headPat = headPat + returnInfo = returnInfo + expr = rhsExpr + range = bindRange + debugPoint = debugPoint + trivia = bindTrivia) ] when exprContainsCEOnlyConstruct rhsExpr -> + let rewritten = + liftCEFromBindingRhs rhsExpr (fun finalValue -> + let newBinding = + SynBinding( + a, + bk, + isInline, + isMutable, + attrs, + xmlDoc, + valData, + headPat, + returnInfo, + finalValue, + bindRange, + debugPoint, + bindTrivia + ) - match normalizedBindings with - | NormalizedBinding(mBinding = mBinding) :: _ -> failAt mBinding - | _ -> failAt m + SynExpr.LetOrUse + { + IsRecursive = isRec + IsFromSource = isFromSource + Bindings = [ newBinding ] + Body = innerComp + Range = m + Trivia = trivia + }) - // Add the variables to the query variable space, on demand - let varSpace = - addVarsToVarSpace varSpace (fun mQueryOp env -> - // Normalize the bindings before detecting the bound variables - match (List.map (BindingNormalization.NormalizeBinding ValOrMemberBinding cenv env) binds) with - | [ NormalizedBinding(kind = SynBindingKind.Normal; shouldInline = false; isMutable = false; pat = pat) ] -> - // successful case - use _holder = TemporarilySuspendReportingTypecheckResultsToSink cenv.tcSink + Some rewritten + | _ -> None - let _, _, vspecs, envinner, _ = - TcMatchPattern cenv (NewInferenceType cenv.g) env ceenv.tpenv pat None TcTrueMatchClause.No + match liftedRewrite with + | Some rewritten -> Some(TranslateComputationExpression ceenv firstTry q varSpace rewritten translatedCtxt) + | None -> - vspecs, envinner - | _ -> - // error case - error (Error(FSComp.SR.tcCustomOperationMayNotBeUsedInConjunctionWithNonSimpleLetBindings (), mQueryOp))) + // For 'query' check immediately + if ceenv.isQuery then + match (List.map (BindingNormalization.NormalizeBinding ValOrMemberBinding cenv ceenv.env) binds) with + | [ NormalizedBinding(_, SynBindingKind.Normal, false, false, _, _, _, _, _, _, _, _) ] when not isRec -> () + | normalizedBindings -> + let failAt m = + error (Error(FSComp.SR.tcNonSimpleLetBindingInQuery (), m)) - Some( - TranslateComputationExpression ceenv CompExprTranslationPass.Initial q varSpace innerComp (fun holeFill -> - translatedCtxt ( - SynExpr.LetOrUse - { - IsRecursive = isRec - //isUse = false, - IsFromSource = isFromSource - //isBang = false, - Bindings = binds - Body = holeFill - Range = m - Trivia = trivia - } - )) - ) + match normalizedBindings with + | NormalizedBinding(mBinding = mBinding) :: _ -> failAt mBinding + | _ -> failAt m + + // Add the variables to the query variable space, on demand + let varSpace = + addVarsToVarSpace varSpace (fun mQueryOp env -> + // Normalize the bindings before detecting the bound variables + match (List.map (BindingNormalization.NormalizeBinding ValOrMemberBinding cenv env) binds) with + | [ NormalizedBinding(kind = SynBindingKind.Normal; shouldInline = false; isMutable = false; pat = pat) ] -> + // successful case + use _holder = TemporarilySuspendReportingTypecheckResultsToSink cenv.tcSink + + let _, _, vspecs, envinner, _ = + TcMatchPattern cenv (NewInferenceType cenv.g) env ceenv.tpenv pat None TcTrueMatchClause.No + + vspecs, envinner + | _ -> + // error case + error (Error(FSComp.SR.tcCustomOperationMayNotBeUsedInConjunctionWithNonSimpleLetBindings (), mQueryOp))) + + Some( + TranslateComputationExpression ceenv CompExprTranslationPass.Initial q varSpace innerComp (fun holeFill -> + translatedCtxt ( + SynExpr.LetOrUse + { + IsRecursive = isRec + //isUse = false, + IsFromSource = isFromSource + //isBang = false, + Bindings = binds + Body = holeFill + Range = m + Trivia = trivia + } + )) + ) // 'use x = expr in expr' | LetOrUse({ From 5d182040a88c41dabbe6302fd8a37978a732b6e8 Mon Sep 17 00:00:00 2001 From: Copilot Date: Fri, 29 May 2026 22:03:44 +0200 Subject: [PATCH 3/5] Sprint 04: fantomas + release notes for #19457 Verification-only sprint: - fantomas --check clean on CheckComputationExpressions.fs and ComputationExpressionTests.fs - release notes entry for #19457 already added in Sprint 03 (docs/release-notes/.FSharp.Compiler.Service/11.0.100.md) - ComponentTests Release: 13922 passed, 0 failed, 775 skipped - SurfaceAreaTest on net10.0: passed (no public API surface change) - PR description draft at .tools/ralph/pr_description.md Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> From 6503962a3a9cfec601f79efd9ba93178d196219f Mon Sep 17 00:00:00 2001 From: Copilot Date: Fri, 29 May 2026 22:42:39 +0200 Subject: [PATCH 4/5] Add PR link to release notes for #19457 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/release-notes/.FSharp.Compiler.Service/11.0.100.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 feed8aa2b86..49d522f8705 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -1,6 +1,6 @@ ### Fixed -* Fix FS0750 "This construct may only be used within computation expressions" incorrectly raised for `let!`/`do!` appearing in the RHS of a plain `let` binding inside a computation expression. Such CE-only constructs are now lifted into the enclosing CE so the example from the issue compiles. ([Issue #19457](https://github.com/dotnet/fsharp/issues/19457)) +* Fix FS0750 "This construct may only be used within computation expressions" incorrectly raised for `let!`/`do!` appearing in the RHS of a plain `let` binding inside a computation expression. Such CE-only constructs are now lifted into the enclosing CE so the example from the issue compiles. ([Issue #19457](https://github.com/dotnet/fsharp/issues/19457), [PR #19868](https://github.com/dotnet/fsharp/pull/19868)) * 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)) * Fix FS0421 "The address of the variable cannot be used at this point" incorrectly raised for the discard pattern `let _ = &expr` when `let x = &expr` compiles. ([Issue #18841](https://github.com/dotnet/fsharp/issues/18841), [PR #19811](https://github.com/dotnet/fsharp/pull/19811)) * Honor `--nowarn` and `--warnaserror` for warnings emitted during command-line option parsing ([Issue #19576](https://github.com/dotnet/fsharp/issues/19576), [PR #19776](https://github.com/dotnet/fsharp/pull/19776)) From f68f5c6af47399b642dc272f76f014ca705577b3 Mon Sep 17 00:00:00 2001 From: Copilot Date: Tue, 2 Jun 2026 11:34:57 +0200 Subject: [PATCH 5/5] Fix infinite recursion in CE lift for tail-position CE constructs The previous exprContainsCEOnlyConstruct could trigger the lift for CE constructs (match!, do!, while!, return, yield) at the tail position of the RHS, where liftCEFromBindingRhs applies its continuation k. Since k wraps the construct back into the same let-binding form, this caused TranslateComputationExpression to re-match and re-lift indefinitely. Fix: split detection into two functions: - containsCEConstructAtAnyPosition: checks if any CE construct exists (used for e1 of Sequential which stays in place after lifting) - exprContainsCEOnlyConstruct: only triggers the lift when the CE construct is in a position liftCEFromBindingRhs can handle (inside a LetOrUse node it threads through, or in e1 of Sequential) Add regression test for match! at tail of plain let RHS. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../CheckComputationExpressions.fs | 36 +++++++++++++------ .../Language/ComputationExpressionTests.fs | 22 ++++++++++++ 2 files changed, 48 insertions(+), 10 deletions(-) diff --git a/src/Compiler/Checking/Expressions/CheckComputationExpressions.fs b/src/Compiler/Checking/Expressions/CheckComputationExpressions.fs index 202ab72fa88..76725160c30 100644 --- a/src/Compiler/Checking/Expressions/CheckComputationExpressions.fs +++ b/src/Compiler/Checking/Expressions/CheckComputationExpressions.fs @@ -1009,15 +1009,9 @@ let requireBuilderMethod methodName ceenv m1 m2 = if not (hasBuilderMethod ceenv m1 methodName) then error (Error(FSComp.SR.tcRequireBuilderMethod methodName, m2)) -/// Detect whether an expression syntactically contains computation-expression-only constructs -/// (such as let!, use!, do!, return, return!, yield, yield!, match!, while!) that can only -/// be legally type-checked inside a CE translation. Used to decide whether to "lift" these -/// constructs out of the RHS of a plain 'let' binding inside a CE so that the surrounding -/// CE translator can process them. We only recurse through positions where the lifted -/// construct would syntactically remain in CE-evaluation position (Sequential tail, -/// LetOrUse body); branches that open a new scope (lambdas, match clauses, if branches, -/// nested CEs) are intentionally not traversed. -let rec private exprContainsCEOnlyConstruct expr = +/// Check if an expression is or contains a CE-only construct at any position. +/// Used for checking e1 of Sequential where the expression stays in place after lifting. +let rec private containsCEConstructAtAnyPosition expr = match expr with | LetOrUse(_, true, _) -> true | SynExpr.DoBang _ @@ -1025,8 +1019,30 @@ let rec private exprContainsCEOnlyConstruct expr = | SynExpr.WhileBang _ | SynExpr.YieldOrReturnFrom _ | SynExpr.YieldOrReturn _ -> true + | LetOrUse({ Body = body; IsRecursive = false }, false, false) -> containsCEConstructAtAnyPosition body + | SynExpr.Sequential(expr1 = e1; expr2 = e2) -> containsCEConstructAtAnyPosition e1 || containsCEConstructAtAnyPosition e2 + | _ -> false + +/// Detect whether an expression syntactically contains computation-expression-only constructs +/// in positions that liftCEFromBindingRhs can successfully handle. This prevents infinite +/// recursion: liftCEFromBindingRhs threads through LetOrUse and Sequential(e2), applying +/// its continuation k at the tail. If the tail IS a CE construct (e.g. match!, do!), wrapping +/// it with k reproduces the original expression, causing unbounded recursion. Therefore we +/// only detect CE constructs that are: +/// 1. LetOrUse with bang (let!/use!) - liftCEFromBindingRhs threads through these +/// 2. In e1 of Sequential - liftCEFromBindingRhs keeps e1 in place, so CE constructs there are safe +/// Branches that open a new scope (lambdas, match clauses, if branches, nested CEs) are +/// intentionally not traversed. +let rec private exprContainsCEOnlyConstruct expr = + match expr with + // let!/use! bang bindings: liftCEFromBindingRhs handles SynExpr.LetOrUse by recursing into Body, + // so these are always in a liftable position. + | LetOrUse(_, true, _) -> true + // Sequential: e1 stays in place (any CE construct there is safe since liftCEFromBindingRhs + // only recurses into e2). For e2, apply the same tail-position rules recursively. + | SynExpr.Sequential(expr1 = e1; expr2 = e2) -> containsCEConstructAtAnyPosition e1 || exprContainsCEOnlyConstruct e2 + // Plain let: recurse into body (same tail-position rules apply to the body) | LetOrUse({ Body = body; IsRecursive = false }, false, false) -> exprContainsCEOnlyConstruct body - | SynExpr.Sequential(expr1 = e1; expr2 = e2) -> exprContainsCEOnlyConstruct e1 || exprContainsCEOnlyConstruct e2 | _ -> false /// Walk the binding RHS of a plain 'let p = rhs in body' that appears inside a computation expression, diff --git a/tests/FSharp.Compiler.ComponentTests/Language/ComputationExpressionTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/ComputationExpressionTests.fs index db20f381f55..46677566ec0 100644 --- a/tests/FSharp.Compiler.ComponentTests/Language/ComputationExpressionTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Language/ComputationExpressionTests.fs @@ -2554,6 +2554,28 @@ let main _ = |> compileExeAndRun |> shouldSucceed + // https://github.com/dotnet/fsharp/issues/19457 - regression guard: + // match! directly in the RHS of a plain let (at tail position) should NOT trigger + // the lift (which would cause infinite recursion). It should produce FS0750 instead. + [] + let ``Issue 19457 - match bang at tail of plain let RHS does not cause infinite recursion`` () = + FSharp """ +module Test +open System.Threading.Tasks +let test() = + task { + let result = + match! Task.FromResult(Some 42) with + | Some x -> x + | None -> 0 + return result + } + """ + |> asLibrary + |> typecheck + |> shouldFail + |> withErrorCode 750 + // https://github.com/dotnet/fsharp/issues/19457 - regression guard: // let! outside any CE must still raise FS0750. []