From e9aaa25ef81262564a122b2a3e44aa03bd5d746d Mon Sep 17 00:00:00 2001 From: dawe Date: Mon, 5 Feb 2024 18:06:57 +0100 Subject: [PATCH 1/4] Add test for recursive call in list comprehension --- .../ErrorMessages/TailCallAttribute.fs | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/FSharp.Compiler.ComponentTests/ErrorMessages/TailCallAttribute.fs b/tests/FSharp.Compiler.ComponentTests/ErrorMessages/TailCallAttribute.fs index 9bad5b4d0af..56572879715 100644 --- a/tests/FSharp.Compiler.ComponentTests/ErrorMessages/TailCallAttribute.fs +++ b/tests/FSharp.Compiler.ComponentTests/ErrorMessages/TailCallAttribute.fs @@ -1512,3 +1512,29 @@ module Microsoft.FSharp.Core Message = "The member or function 'f' has the 'TailCallAttribute' attribute, but is not being used in a tail recursive way." } ] + + [] + let ``Warn for recursive call in list comprehension`` () = + """ +namespace N + + module M = + + [] + let rec reverse (input: list<'t>) = + match input with + | head :: tail -> [ yield! reverse tail; head ] + | [] -> [] + """ + |> FSharp + |> compile + |> shouldFail + |> withResults [ + { Error = Warning 3569 + Range = { StartLine = 9 + StartColumn = 40 + EndLine = 9 + EndColumn = 52 } + Message = + "The member or function 'reverse' has the 'TailCallAttribute' attribute, but is not being used in a tail recursive way." } + ] From 591333fc8050ab0c04086db46987fcdf9ad35f02 Mon Sep 17 00:00:00 2001 From: dawe Date: Mon, 5 Feb 2024 18:09:14 +0100 Subject: [PATCH 2/4] Change the default to TailCall.No when checking the args of a Coerce --- src/Compiler/Checking/TailCallChecks.fs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Compiler/Checking/TailCallChecks.fs b/src/Compiler/Checking/TailCallChecks.fs index cd8dfd2b77c..c26683e3da9 100644 --- a/src/Compiler/Checking/TailCallChecks.fs +++ b/src/Compiler/Checking/TailCallChecks.fs @@ -539,12 +539,10 @@ and CheckExprOp cenv (op, tyargs, args, m) ctxt : unit = | TOp.ValFieldSet _rf, _, [ _arg1; _arg2 ] -> () | TOp.Coerce, [ tgtTy; srcTy ], [ x ] -> - let tailCall = TailCall.YesFromExpr cenv.g x - if TypeDefinitelySubsumesTypeNoCoercion 0 g cenv.amap m tgtTy srcTy then - CheckExpr cenv x ctxt tailCall + CheckExpr cenv x ctxt TailCall.No else - CheckExprNoByrefs cenv tailCall x + CheckExprNoByrefs cenv TailCall.No x | TOp.Reraise, [ _ty1 ], [] -> () From b27f0e28bc0642974e59fa10c210e3fa26fa8535 Mon Sep 17 00:00:00 2001 From: dawe Date: Mon, 5 Feb 2024 18:14:48 +0100 Subject: [PATCH 3/4] add release-notes entry --- docs/release-notes/.FSharp.Compiler.Service/8.0.300.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md b/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md index b8dc1de0de9..69f26805706 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md +++ b/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md @@ -1,5 +1,6 @@ ### Fixed +* Fix missing warning for recursive calls in list comprehensions. ([PR #TODO](https://github.com/dotnet/fsharp/pull/TODO)) * Code generated files with > 64K methods and generated symbols crash when loaded. Use infered sequence points for debugging. ([Issue #16399](https://github.com/dotnet/fsharp/issues/16399), [#PR 16514](https://github.com/dotnet/fsharp/pull/16514)) * `nameof Module` expressions and patterns are processed to link files in `--test:GraphBasedChecking`. ([PR #16550](https://github.com/dotnet/fsharp/pull/16550)) * Graph Based Checking doesn't throw on invalid parsed input so it can be used for IDE scenarios ([PR #16575](https://github.com/dotnet/fsharp/pull/16575), [PR #16588](https://github.com/dotnet/fsharp/pull/16588), [PR #16643](https://github.com/dotnet/fsharp/pull/16643)) From b9540b3c4a12595f8b9080f8ef82499095396aa6 Mon Sep 17 00:00:00 2001 From: dawe Date: Mon, 5 Feb 2024 18:21:18 +0100 Subject: [PATCH 4/4] add PR number to release notes --- docs/release-notes/.FSharp.Compiler.Service/8.0.300.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md b/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md index 69f26805706..5dbecd12ffa 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md +++ b/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md @@ -1,6 +1,6 @@ ### Fixed -* Fix missing warning for recursive calls in list comprehensions. ([PR #TODO](https://github.com/dotnet/fsharp/pull/TODO)) +* Fix missing warning for recursive calls in list comprehensions. ([PR #16652](https://github.com/dotnet/fsharp/pull/16652)) * Code generated files with > 64K methods and generated symbols crash when loaded. Use infered sequence points for debugging. ([Issue #16399](https://github.com/dotnet/fsharp/issues/16399), [#PR 16514](https://github.com/dotnet/fsharp/pull/16514)) * `nameof Module` expressions and patterns are processed to link files in `--test:GraphBasedChecking`. ([PR #16550](https://github.com/dotnet/fsharp/pull/16550)) * Graph Based Checking doesn't throw on invalid parsed input so it can be used for IDE scenarios ([PR #16575](https://github.com/dotnet/fsharp/pull/16575), [PR #16588](https://github.com/dotnet/fsharp/pull/16588), [PR #16643](https://github.com/dotnet/fsharp/pull/16643))