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

if-let breaks tail call optimization. #1401

Closed
amano-kenji opened this issue Feb 12, 2024 · 6 comments
Closed

if-let breaks tail call optimization. #1401

amano-kenji opened this issue Feb 12, 2024 · 6 comments

Comments

@amano-kenji
Copy link
Contributor

amano-kenji commented Feb 12, 2024

if-let macro and all other macros that depend on if-let macro break tail call optimization.

I don't know what other macros break tail-call optimization.

@amano-kenji amano-kenji changed the title Tail call optimization doesn't apply to tail calls that don't start at the root of the fiber call stack. Tail call optimization doesn't apply to tail calls outside tail call position. Feb 12, 2024
@amano-kenji amano-kenji changed the title Tail call optimization doesn't apply to tail calls outside tail call position. Tail call optimization doesn't apply to recursive functions outside tail call position. Feb 12, 2024
@amano-kenji amano-kenji changed the title Tail call optimization doesn't apply to recursive functions outside tail call position. Tail call optimization doesn't apply to tail call functions outside tail call position. Feb 12, 2024
@amano-kenji amano-kenji changed the title Tail call optimization doesn't apply to tail call functions outside tail call position. Tail call optimization doesn't apply to tail call functions outside fiber tail call positions. Feb 12, 2024
@amano-kenji amano-kenji changed the title Tail call optimization doesn't apply to tail call functions outside fiber tail call positions. Tail call optimization applies only to fiber tail call positions. Feb 13, 2024
@amano-kenji amano-kenji changed the title Tail call optimization applies only to fiber tail call positions. Macros break tail call optimization. Feb 14, 2024
@amano-kenji amano-kenji changed the title Macros break tail call optimization. if-let breaks tail call optimization. Feb 14, 2024
bakpakin added a commit that referenced this issue Feb 15, 2024
Changes to avoid multiple macro expansions of the "false" branch caused
a regression in this functionality.
@sogaiu
Copy link
Contributor

sogaiu commented Feb 15, 2024

@amano-kenji May be you can see if 9e0daae fixed things for you?

@amano-kenji
Copy link
Contributor Author

It fixes the tail call optimization, but the fiber call stack for false branch points to the top of if-let instead of the line where debug/stacktrace is called.

For the true branch, the call stack points to the line where debug/stacktrace is called.

Other than that, it's fine. It's only a minor issue.

@amano-kenji
Copy link
Contributor Author

This is my test code.

(defn recurse
  [branch]
  (if-let [ha branch]
    (do
      (ev/sleep 1)
      (debug/stacktrace (fiber/current) "print stack for true branch" "")
      (recurse branch))
    (do
      (ev/sleep 1)
      (debug/stacktrace (fiber/current) "print stack for false branch" "")
      (recurse branch))))

(recurse false)

@sogaiu
Copy link
Contributor

sogaiu commented Feb 15, 2024

Ok, to spell out some details...with the following in recurse.janet:

(defn recurse
  [branch]
  (if-let [ha branch]
    (do
      (ev/sleep 1)
      (debug/stacktrace (fiber/current) "print stack for true branch" "")
      (recurse branch))
    (do
      (ev/sleep 1)
      (debug/stacktrace (fiber/current) "print stack for false branch" "")
      (recurse branch))))

(defn main
  [& args]
  (def arg
    (case (get args 1 "true")
      "true" true
      "false" false
      nil))
  (recurse arg))

For the invocation janet recurse.janet true I get:

alive: print stack for true branch
  in debug/stacktrace [src/core/debug.c] on line 388
  in recurse [recurse.janet] (tailcall) on line 6, column 7
alive: print stack for true branch
  in debug/stacktrace [src/core/debug.c] on line 388
  in recurse [recurse.janet] (tailcall) on line 6, column 7
...

where line 6, column 7 refers to the code:

(debug/stacktrace (fiber/current) "print stack for true branch" "")

which is fine, but, for the invocation janet recurse.janet false I get:

alive: print stack for false branch
  in debug/stacktrace [src/core/debug.c] on line 388
  in recurse [recurse.janet] (tailcall) on line 3, column 3
alive: print stack for false branch
  in debug/stacktrace [src/core/debug.c] on line 388
  in recurse [recurse.janet] (tailcall) on line 3, column 3
...

where line 3, column 3 refers to the code:

(if-let [ha branch]

which I guess would be nicer if this was actually pointing at:

(debug/stacktrace (fiber/current) "print stack for false branch" "")

(line 10, column 7).

@sogaiu
Copy link
Contributor

sogaiu commented Feb 16, 2024

I guess the location information reporting situation mentioned in this comment is a different issue.

@amano-kenji
Copy link
Contributor Author

Closing this to create another issue.

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

No branches or pull requests

3 participants