From b25e828c9b2475676381adb516f45297540804a5 Mon Sep 17 00:00:00 2001 From: laenas Date: Sat, 28 Nov 2020 18:46:29 +0100 Subject: [PATCH 1/4] Resolve issue with implicit yields requiring Zero - even when it is not called --- src/fsharp/CheckComputationExpressions.fs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/fsharp/CheckComputationExpressions.fs b/src/fsharp/CheckComputationExpressions.fs index c8ac4cd1f9a..a30fd5e7e1d 100644 --- a/src/fsharp/CheckComputationExpressions.fs +++ b/src/fsharp/CheckComputationExpressions.fs @@ -950,7 +950,8 @@ let TcComputationExpression cenv env overallTy tpenv (mWhole, interpExpr: Expr, error(Error(FSComp.SR.tcConstructIsAmbiguousInComputationExpression(), m)) | SynExpr.ImplicitZero m -> - if isNil (TryFindIntrinsicOrExtensionMethInfo ResultCollectionSettings.AtMostOneResult cenv env m ad "Zero" builderTy) then error(Error(FSComp.SR.tcRequireBuilderMethod("Zero"), m)) + if (not enableImplicitYield) && + isNil (TryFindIntrinsicOrExtensionMethInfo ResultCollectionSettings.AtMostOneResult cenv env m ad "Zero" builderTy) then error(Error(FSComp.SR.tcRequireBuilderMethod("Zero"), m)) Some (translatedCtxt (mkSynCall "Zero" m [])) | OptionalSequential (JoinOrGroupJoinOrZipClause (_, _, _, _, _, mClause), _) From 9e5664839b29132cd3ec444de88fd3588c9eb2db Mon Sep 17 00:00:00 2001 From: laenas Date: Sun, 29 Nov 2020 10:42:54 +0100 Subject: [PATCH 2/4] Remove dead folder reference from FSPROJ --- .../FSharp.Compiler.ComponentTests.fsproj | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj index 440a4e8cf94..8ffb0c882be 100644 --- a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj +++ b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj @@ -57,8 +57,4 @@ - - - - From ac4d5471c46fe1036a6a55214275f3f3f75daf93 Mon Sep 17 00:00:00 2001 From: laenas Date: Sun, 29 Nov 2020 12:53:31 +0100 Subject: [PATCH 3/4] Add unit tests for Computation Expression Zero requirement behavior --- .../FSharp.Compiler.ComponentTests.fsproj | 1 + .../Language/ComputationExpressionTests.fs | 95 +++++++++++++++++++ 2 files changed, 96 insertions(+) create mode 100644 tests/FSharp.Compiler.ComponentTests/Language/ComputationExpressionTests.fs diff --git a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj index 8ffb0c882be..3f8fc44becd 100644 --- a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj +++ b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj @@ -45,6 +45,7 @@ + diff --git a/tests/FSharp.Compiler.ComponentTests/Language/ComputationExpressionTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/ComputationExpressionTests.fs new file mode 100644 index 00000000000..0b567ee9a9c --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/Language/ComputationExpressionTests.fs @@ -0,0 +1,95 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +namespace FSharp.Compiler.ComponentTests.Language + +open Xunit +open FSharp.Test.Utilities.Compiler + +module ComputationExpressionTests = + [] + let ``A CE not using Zero does not require Zero``() = + FSharp """ +module ComputationExpressionTests +type ListBuilder () = + member __.Combine (a: List<'T>, b) = a @ b + member __.Yield x = List.singleton x + member __.Delay expr = expr () : List<'T> + +let lb = ListBuilder () + +let x = lb {1; 2;} + """ + |> compile + |> shouldSucceed + |> ignore + + [] + let ``A CE explicitly using Zero fails without a defined Zero``() = + FSharp """ +module ComputationExpressionTests +type ListBuilder () = + member __.Combine (a: List<'T>, b) = a @ b + member __.Yield x = List.singleton x + member __.Delay expr = expr () : List<'T> + +let lb = ListBuilder () + +let x = lb {1; 2;()} + """ + |> compile + |> shouldFail + |> withSingleDiagnostic (Error 39, Line 10, Col 18, Line 10, Col 20, "The type 'ListBuilder' does not define the field, constructor or member 'Zero'.") + |> ignore + + [] + let ``A CE explicitly using Zero succeeds with a defined Zero``() = + FSharp """ +module ComputationExpressionTests +type ListBuilder () = + member __.Zero () = [] + member __.Combine (a: List<'T>, b) = a @ b + member __.Yield x = List.singleton x + member __.Delay expr = expr () : List<'T> + +let lb = ListBuilder () + +let x = lb {1; 2;()} + """ + |> compile + |> shouldSucceed + |> ignore + + [] + let ``A CE with a complete if-then expression does not require Zero``() = + FSharp """ +module ComputationExpressionTests +type ListBuilder () = + member __.Combine (a: List<'T>, b) = a @ b + member __.Yield x = List.singleton x + member __.Delay expr = expr () : List<'T> + +let lb = ListBuilder () + +let x = lb {1; 2; if true then 3 else 4;} + """ + |> compile + |> shouldSucceed + |> ignore + + [] + let ``A CE with a missing/empty else branch implicitly requires Zero``() = + FSharp """ +module ComputationExpressionTests +type ListBuilder () = + member __.Combine (a: List<'T>, b) = a @ b + member __.Yield x = List.singleton x + member __.Delay expr = expr () : List<'T> + +let lb = ListBuilder () + +let x = lb {1; 2; if true then 3;} + """ + |> compile + |> shouldFail + |> withSingleDiagnostic (Error 708, Line 10, Col 19, Line 10, Col 31, "This control construct may only be used if the computation expression builder defines a 'Zero' method") + |> ignore \ No newline at end of file From 95729f593d09391624a6dc110d9250dcf72fe239 Mon Sep 17 00:00:00 2001 From: laenas Date: Tue, 1 Dec 2020 19:43:37 +0100 Subject: [PATCH 4/4] Add comment with guidance on the handling of Zero --- src/fsharp/CheckComputationExpressions.fs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/fsharp/CheckComputationExpressions.fs b/src/fsharp/CheckComputationExpressions.fs index a30fd5e7e1d..fa413a52a02 100644 --- a/src/fsharp/CheckComputationExpressions.fs +++ b/src/fsharp/CheckComputationExpressions.fs @@ -949,6 +949,11 @@ let TcComputationExpression cenv env overallTy tpenv (mWhole, interpExpr: Expr, | SynExpr.Paren (_, _, _, m) -> error(Error(FSComp.SR.tcConstructIsAmbiguousInComputationExpression(), m)) + // In some cases the node produced by `mkSynCall "Zero" m []` may be discarded in the case + // of implicit yields - for example "list { 1; 2 }" when each expression checks as an implicit yield. + // If it is not discarded, the syntax node will later be checked and the existence/non-existence of the Zero method + // will be checked/reported appropriately (though the error message won't mention computation expressions + // like our other error messages for missing methods). | SynExpr.ImplicitZero m -> if (not enableImplicitYield) && isNil (TryFindIntrinsicOrExtensionMethInfo ResultCollectionSettings.AtMostOneResult cenv env m ad "Zero" builderTy) then error(Error(FSComp.SR.tcRequireBuilderMethod("Zero"), m))