janet-lang / janet

A dynamic language and bytecode vm
https://janet-lang.org
MIT License
3.57k stars 229 forks source link

`if-let` breaks tail call optimization. #1401

Closed amano-kenji closed 9 months ago

amano-kenji commented 9 months ago

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.

sogaiu commented 9 months ago

@amano-kenji May be you can see if https://github.com/janet-lang/janet/commit/9e0daaee095cea5837574f2181072dd32346787a fixed things for you?

amano-kenji commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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

amano-kenji commented 9 months ago

Closing this to create another issue.