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

Tail-calls in recursive functions are not optimized if used with pipeline operators #1770

Closed
ymtszw opened this issue Aug 29, 2018 · 3 comments

Comments

@ymtszw
Copy link

commented Aug 29, 2018

From slack discussion: https://elmlang.slack.com/archives/C13L7S5GR/p1535488054000100

In Elm 0.19. Reproducible in Ellie.

SSCCE:

module Main exposing (main)

import Browser
import Html

main : Program () () ()
main =
    Browser.sandbox
        { init = ()
        , update = \_ m -> m
        , view = \_ -> Html.text (tailCallWithPipeline 12345 "something")
        }

-- TCO Bug SSCCE

tailCallWithPipeline : Int -> String -> String
tailCallWithPipeline count acc =
    if count <= 0 then
        acc

    else
        acc
            |> tailCallWithPipeline (count - 1)


tailCallWithoutPipeline : Int -> String -> String
tailCallWithoutPipeline count acc =
    if count <= 0 then
        acc

    else
        tailCallWithoutPipeline (count - 1) acc
<html>
<head></head>
<body>
  <main></main>
  <script>
    var app = Elm.Main.init({ node: document.querySelector('main') })
  </script>
</body>
</html>

Here, tailCallWithPipeline and tailCallWithoutPipeline are semantically identical recursive functions, the only difference is a use of pipeline operator |>.
However, executing tailCallWithPipeline ends up in runtime error: RangeError: Maximum call stack size exceeded, whereas tailCallWithoutPipeline does not (correctly optimized and run fine).

Both worked fine in 0.18.

@atdixon

This comment has been minimized.

Copy link

commented Jan 9, 2019

Here is another (smaller/repl-based) example:

> sum_ n i = if n == 0 then i else sum_ (n - 1) (n + i)
> sum_ 1000000 0
500000500000 : Int

HOWEVER, not if slightly modified using pipe operator:

> sum_ n i = if n == 0 then i else sum_ (n - 1) <| n + i
> sum_ 1000000 0
RangeError: Maximum call stack size exceeded

RavenDream added a commit to reserve-protocol/elm-html-string that referenced this issue Feb 3, 2019

RavenDream added a commit to reserve-protocol/elm-html-string that referenced this issue Feb 3, 2019

@evancz

This comment has been minimized.

Copy link
Member

commented Jul 19, 2019

In changing how code was generated in 0.19.0 to make sure the generated assets were really small, we moved some AST shuffling later into the optimization process. The old version tried to flatten out <| and |> uses earlier in the pipeline.

The resulting behavior is that tail-call elimination is only performed when the recursive call is in "tail call position" (i.e. called directly at the conclusion of some some branch of logic)

This particular optimization enters into the realm of feature because it can change program behavior, so it's definitely important to keep this stable. It looks like this behavior is going to remain the same from 0.19.0 to 0.19.1, and I think the best path is to just require the "tail call position" without any special exceptions for now.

@evancz evancz closed this Jul 19, 2019

@atdixon

This comment has been minimized.

Copy link

commented Jul 19, 2019

Is the change in behavior anything other than a "RangeError: Maximum call stack size exceeded"? I think it would be safe to say no client would be relying on this behavior and furthermore the behavior is likely widely variant across systems.

From the operator documentation, we have

-- (arg |> func) is the same as (func arg)

But what seems to be suggested in your note is that this is not an equivalency. I may be missing something, but this is rather surprising behavior. A common refactoring relies on the expectation of this equivalency; that (arg |> func) and (func arg) are referentially transparent. But perform such refactoring in tail position and you're in for trouble?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.