From 08bf5e99acd4be80ee93c33c2c91b0753728fcef Mon Sep 17 00:00:00 2001 From: dawe Date: Fri, 24 May 2024 12:24:45 +0200 Subject: [PATCH 1/7] add tests --- .../ErrorMessages/TailCallAttribute.fs | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/tests/FSharp.Compiler.ComponentTests/ErrorMessages/TailCallAttribute.fs b/tests/FSharp.Compiler.ComponentTests/ErrorMessages/TailCallAttribute.fs index e3ff4102f7c..e1208481308 100644 --- a/tests/FSharp.Compiler.ComponentTests/ErrorMessages/TailCallAttribute.fs +++ b/tests/FSharp.Compiler.ComponentTests/ErrorMessages/TailCallAttribute.fs @@ -1683,3 +1683,42 @@ module M = Message = "The member or function 'traverseSequentials' has the 'TailCallAttribute' attribute, but is not being used in a tail recursive way." } ] + + [] + let ``Don't warn for rec call of async func that evaluates an async parameter in a match!`` () = + """ +namespace N + +module M = + + [] + let rec f (g: bool Async) = async { + match! g with + | false -> () + | true -> return! f g + } + """ + |> FSharp + |> withLangVersion80 + |> compile + |> shouldSucceed + + [] + let ``Don't warn for rec call of async func that evaluates an async parameter in a let!`` () = + """ +namespace N + +module M = + + [] + let rec f (g: bool Async) = async { + let! x = g + match x with + | false -> () + | true -> return! f g + } + """ + |> FSharp + |> withLangVersion80 + |> compile + |> shouldSucceed From 26cce84cb28fd3f94c86dfe9711da871cdf66e49 Mon Sep 17 00:00:00 2001 From: dawe Date: Fri, 24 May 2024 12:25:19 +0200 Subject: [PATCH 2/7] try to detect if we are dealing with an async continuation --- src/Compiler/Checking/TailCallChecks.fs | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/Compiler/Checking/TailCallChecks.fs b/src/Compiler/Checking/TailCallChecks.fs index 97bdc05680b..1f55569e599 100644 --- a/src/Compiler/Checking/TailCallChecks.fs +++ b/src/Compiler/Checking/TailCallChecks.fs @@ -756,7 +756,29 @@ let CheckModuleBinding cenv (isRec: bool) (TBind _ as bind) = | Expr.Lambda(bodyExpr = bodyExpr) -> checkTailCall insideSubBindingOrTry bodyExpr | Expr.DebugPoint(_debugPointAtLeafExpr, expr) -> checkTailCall insideSubBindingOrTry expr | Expr.Let(binding = binding; bodyExpr = bodyExpr) -> - checkTailCall true binding.Expr + let isAsyncContinuation = + match bodyExpr with + | Expr.App(funcExpr = Expr.Val(valRef = valRef)) when + valRef.DisplayNameCore = "MakeAsync" + && valRef.HasDeclaringEntity + && valRef.DeclaringEntity.LogicalName = "AsyncPrimitives" + -> + match valRef.GeneralizedType with + | [ _ ], + TType_fun( + domainType = TType_fun( + domainType = TType_app(tyconRef = funArgDomTypeTyconRef) + rangeType = TType_app(tyconRef = funArgRangeTypeTyconRef)) + rangeType = TType_app(tyconRef = rangeTypeTyConRef)) when + funArgDomTypeTyconRef.DemangledModuleOrNamespaceName = "AsyncActivation`1" + && funArgRangeTypeTyconRef.DemangledModuleOrNamespaceName = "AsyncReturn" + && rangeTypeTyConRef.DemangledModuleOrNamespaceName = "Async`1" + -> + true + | _ -> false + | _ -> false + + checkTailCall (not isAsyncContinuation) binding.Expr let warnForBodyExpr = insideSubBindingOrTry From 1bc19a71d9c0b4bed5312c1884e7c97d0a8bb69e Mon Sep 17 00:00:00 2001 From: dawe Date: Fri, 24 May 2024 12:26:29 +0200 Subject: [PATCH 3/7] remove the name matching checks --- src/Compiler/Checking/TailCallChecks.fs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/Compiler/Checking/TailCallChecks.fs b/src/Compiler/Checking/TailCallChecks.fs index 1f55569e599..da8885a8a81 100644 --- a/src/Compiler/Checking/TailCallChecks.fs +++ b/src/Compiler/Checking/TailCallChecks.fs @@ -758,11 +758,7 @@ let CheckModuleBinding cenv (isRec: bool) (TBind _ as bind) = | Expr.Let(binding = binding; bodyExpr = bodyExpr) -> let isAsyncContinuation = match bodyExpr with - | Expr.App(funcExpr = Expr.Val(valRef = valRef)) when - valRef.DisplayNameCore = "MakeAsync" - && valRef.HasDeclaringEntity - && valRef.DeclaringEntity.LogicalName = "AsyncPrimitives" - -> + | Expr.App(funcExpr = Expr.Val(valRef = valRef)) -> match valRef.GeneralizedType with | [ _ ], TType_fun( From 2fdc22ad3480b68cd7f4bf8c6bfbb536390ec699 Mon Sep 17 00:00:00 2001 From: dawe Date: Fri, 24 May 2024 12:26:44 +0200 Subject: [PATCH 4/7] add release notes --- docs/release-notes/.FSharp.Compiler.Service/8.0.400.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/release-notes/.FSharp.Compiler.Service/8.0.400.md b/docs/release-notes/.FSharp.Compiler.Service/8.0.400.md index 29dac7317ee..421db0b5ed6 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/8.0.400.md +++ b/docs/release-notes/.FSharp.Compiler.Service/8.0.400.md @@ -1,5 +1,6 @@ ### Fixed +* Fix a false positive of the `[]` analysis in combination with async. ([Issue #17237](https://github.com/dotnet/fsharp/issues/17237), [PR #XXX](https://github.com/dotnet/fsharp/pull/XXX)) * Fix internal error when dotting into delegates with multiple type parameters. ([PR #17227](https://github.com/dotnet/fsharp/pull/17227)) * Error for partial implementation of interface with static and non-static abstract members. ([Issue #17138](https://github.com/dotnet/fsharp/issues/17138), [PR #17160](https://github.com/dotnet/fsharp/pull/17160)) * Optimize simple mappings with preludes in computed collections. ([PR #17067](https://github.com/dotnet/fsharp/pull/17067)) From 03ad76ff0ed8f178457450c1d5c6da8dbeecc684 Mon Sep 17 00:00:00 2001 From: dawe Date: Fri, 24 May 2024 12:35:53 +0200 Subject: [PATCH 5/7] update PR number --- docs/release-notes/.FSharp.Compiler.Service/8.0.400.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/release-notes/.FSharp.Compiler.Service/8.0.400.md b/docs/release-notes/.FSharp.Compiler.Service/8.0.400.md index 421db0b5ed6..aa068efeb19 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/8.0.400.md +++ b/docs/release-notes/.FSharp.Compiler.Service/8.0.400.md @@ -1,6 +1,6 @@ ### Fixed -* Fix a false positive of the `[]` analysis in combination with async. ([Issue #17237](https://github.com/dotnet/fsharp/issues/17237), [PR #XXX](https://github.com/dotnet/fsharp/pull/XXX)) +* Fix a false positive of the `[]` analysis in combination with async. ([Issue #17237](https://github.com/dotnet/fsharp/issues/17237), [PR #17241](https://github.com/dotnet/fsharp/pull/17241)) * Fix internal error when dotting into delegates with multiple type parameters. ([PR #17227](https://github.com/dotnet/fsharp/pull/17227)) * Error for partial implementation of interface with static and non-static abstract members. ([Issue #17138](https://github.com/dotnet/fsharp/issues/17138), [PR #17160](https://github.com/dotnet/fsharp/pull/17160)) * Optimize simple mappings with preludes in computed collections. ([PR #17067](https://github.com/dotnet/fsharp/pull/17067)) From 422507985832e3205b905deb2bf35aaab982574b Mon Sep 17 00:00:00 2001 From: dawe Date: Fri, 24 May 2024 17:42:17 +0200 Subject: [PATCH 6/7] be less FSharp.Async specific in the continuation detection --- src/Compiler/Checking/TailCallChecks.fs | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/src/Compiler/Checking/TailCallChecks.fs b/src/Compiler/Checking/TailCallChecks.fs index da8885a8a81..68c89f9daa6 100644 --- a/src/Compiler/Checking/TailCallChecks.fs +++ b/src/Compiler/Checking/TailCallChecks.fs @@ -756,25 +756,18 @@ let CheckModuleBinding cenv (isRec: bool) (TBind _ as bind) = | Expr.Lambda(bodyExpr = bodyExpr) -> checkTailCall insideSubBindingOrTry bodyExpr | Expr.DebugPoint(_debugPointAtLeafExpr, expr) -> checkTailCall insideSubBindingOrTry expr | Expr.Let(binding = binding; bodyExpr = bodyExpr) -> - let isAsyncContinuation = + // detect continuation shapes like MakeAsyns + let isContinuation = match bodyExpr with | Expr.App(funcExpr = Expr.Val(valRef = valRef)) -> match valRef.GeneralizedType with | [ _ ], - TType_fun( - domainType = TType_fun( - domainType = TType_app(tyconRef = funArgDomTypeTyconRef) - rangeType = TType_app(tyconRef = funArgRangeTypeTyconRef)) - rangeType = TType_app(tyconRef = rangeTypeTyConRef)) when - funArgDomTypeTyconRef.DemangledModuleOrNamespaceName = "AsyncActivation`1" - && funArgRangeTypeTyconRef.DemangledModuleOrNamespaceName = "AsyncReturn" - && rangeTypeTyConRef.DemangledModuleOrNamespaceName = "Async`1" - -> + TType_fun(domainType = TType_fun(domainType = TType_app _; rangeType = TType_app _); rangeType = TType_app _) -> true | _ -> false | _ -> false - checkTailCall (not isAsyncContinuation) binding.Expr + checkTailCall (not isContinuation) binding.Expr let warnForBodyExpr = insideSubBindingOrTry From 384cdca808eb85865834b2d86a2199bffeaf6ce2 Mon Sep 17 00:00:00 2001 From: dawe Date: Mon, 24 Jun 2024 22:50:46 +0200 Subject: [PATCH 7/7] Update src/Compiler/Checking/TailCallChecks.fs Co-authored-by: Adam Boniecki <20281641+abonie@users.noreply.github.com> --- src/Compiler/Checking/TailCallChecks.fs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compiler/Checking/TailCallChecks.fs b/src/Compiler/Checking/TailCallChecks.fs index 68c89f9daa6..c373be9eb8a 100644 --- a/src/Compiler/Checking/TailCallChecks.fs +++ b/src/Compiler/Checking/TailCallChecks.fs @@ -756,7 +756,7 @@ let CheckModuleBinding cenv (isRec: bool) (TBind _ as bind) = | Expr.Lambda(bodyExpr = bodyExpr) -> checkTailCall insideSubBindingOrTry bodyExpr | Expr.DebugPoint(_debugPointAtLeafExpr, expr) -> checkTailCall insideSubBindingOrTry expr | Expr.Let(binding = binding; bodyExpr = bodyExpr) -> - // detect continuation shapes like MakeAsyns + // detect continuation shapes like MakeAsync let isContinuation = match bodyExpr with | Expr.App(funcExpr = Expr.Val(valRef = valRef)) ->