Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TailCall attribute does not lead to errors when recursive call is in a list comprehension #16649

Closed
Larocceau opened this issue Feb 3, 2024 · 4 comments · Fixed by #16652
Closed

Comments

@Larocceau
Copy link

When using the TailCall attribute, warning FS3569 is not shown when the recursive call is inside a list comprehension

Repro steps

  1. Create a new f# console project.
  2. Add the following code to Program.fs
[<TailCall>]
let rec reverse (input: list<'t>) =
    match input with
    | head :: tail -> [ yield! reverse tail; head ]
    | [] -> []
  1. run dotnet build

Example project with a case where the bug occurs , and a slightly different version where the bug does not occur

Expected behavior

The compiler displays warning FS3569

Actual behavior

The compiler does not show the warning

Known workarounds

Do not use list comprehensions when trying to use the tail call compiler attribute

Related information

  • Operating system: Windows 11 enterprise
  • .NET Core
  • Editing Tools: VSCode
@github-actions github-actions bot added this to the Backlog milestone Feb 3, 2024
@Larocceau
Copy link
Author

@dawedawe since you contributed to this feature this might be of interest to you

@dawedawe
Copy link
Contributor

dawedawe commented Feb 3, 2024

@dawedawe since you contributed to this feature this might be of interest to you

Thanks for the finding, I'll work on it.

@vzarytovskii
Copy link
Member

vzarytovskii commented Feb 5, 2024

Example project with a case where the bug occurs , and a slightly different version where the bug does not occur

// Nearly identical code that is flagged by the compiler;
// only difference is using List.Append instead of a list comprehension

Difference is that list expression will use list collector, and will result in something like

let tailOrNull = input.TailOrNull
let headOrDefault = input.HeadOrDefault
let listCollector = ListCollector<t>()
listCollector.AddMany(reverse(tailOrNull))
listCollector.Add(headOrDefault)
listCollector.Close()

But when the analysis happens, it probably thinks it's atomic or something.

@vzarytovskii
Copy link
Member

vzarytovskii commented Feb 5, 2024

Oh, yeah

[<TailCall>]
let rec recurse (list : list<int>) =
    [ yield! recurse list; () ]

Will result in something like (roughly):

DebugPoint((6,8--6,35), App(toList, [Op(Coerce, App(seq, [App(delay, [Lambda([unitVar], App(append, [.., ..]))])]))]))

Whereas

[<TailCall>]
let rec recurse (list : list<int>) =
    recurse list; ()

into

Sequential(DebugPoint((7,8--7,20), App(.., [..])), DebugPoint((7,22--7,24), ()))

We analyze the latter correctly as sequential expression and produce the warning, and we end up analysing the former as application and decide that it's tail recursive, which we shouldn't.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants