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

Unexpected result for non-tail usage of recur #498

Closed
jeroenvandijk opened this issue Jan 12, 2021 · 10 comments
Closed

Unexpected result for non-tail usage of recur #498

jeroenvandijk opened this issue Jan 12, 2021 · 10 comments

Comments

@jeroenvandijk
Copy link
Contributor

version

Babashka 0.2.6 (or 5aa9031)

platform

Mac OSX

problem

loop / recur allows recur to be called from a non-tail position. In this particular case this gives unexpected result (doesn't recur) with an error message.

repro

(loop []
    (println "hi")
    (recur)
    2)
hi
2

expected behavior

Clojure throws an error:

Syntax error (UnsupportedOperationException) compiling recur at (form-init7929880134577873038.clj:3:5).
Can only recur from tail position

Maybe the clojure behaviour would be most consistent. I'm not sure what the benefits are for not doing this (apart from having to add this check). In this particular case (adding an extra item behind (recur)) caught me by surprise.

@borkdude
Copy link
Collaborator

Note for self:

The last expression in fn, loop, let and do forms are always in tail position; the then and else expression of an if form are also tail positions.

@borkdude
Copy link
Collaborator

A more efficient way to detect this is using a linter: clj-kondo/clj-kondo#1126
In this way the analyzer doesn't have to do extra work. If you aren't already using it, I recommend setting up editor integration with clj-kondo and then hopefully not too far in the future, you will get a warning about this.

@jeroenvandijk
Copy link
Contributor Author

I agree that clj-kondo is a nice tool to catch this. Is it already possible to use clj-kondo as a (cljs) library so the linter could be used in other environments? Embedding Sci on a web page for evaluating user code would be an example use case.

@borkdude
Copy link
Collaborator

@jeroenvandijk Clj-kondo is currently CLJ only, but portions of it could maybe be ported to CLJS. This is a substantial amount of work though. The way I'm using it currently is as a server side service:

https://clj-kondo.michielborkent.nl/

The front-end just sends the code to the backend, the backend lints it and then returns the warnings.

@borkdude
Copy link
Collaborator

To be clear, clj-kondo can be used as a JVM library.

@Cnly
Copy link

Cnly commented Dec 2, 2021

I also hit this problem lately... While linters would be nice, should sci also do something about this? It should be more than a linter warning IMO.

@borkdude
Copy link
Collaborator

borkdude commented Dec 5, 2021

I will first address this in clj-kondo. I am also contemplating how I could address this in SCI without affecting performance too much.

@borkdude
Copy link
Collaborator

It's in clj-kondo now. I'll have a stab at it in SCI soon.

@borkdude
Copy link
Collaborator

In babashka master:

Type:     clojure.lang.ExceptionInfo
Message:  Can only recur from tail position
Location: /tmp/dude.clj:2:3
Phase:    analysis

----- Context ------------------------------------------------------------------
1: (defn foo []
2:   (recur)
     ^--- Can only recur from tail position
3:   1)

borkdude added a commit to babashka/babashka that referenced this issue Jan 23, 2022
borkdude added a commit to babashka/babashka that referenced this issue Jan 23, 2022
borkdude added a commit to babashka/babashka that referenced this issue Jan 23, 2022
@borkdude
Copy link
Collaborator

borkdude commented Mar 5, 2022

Already released a while ago.

@borkdude borkdude closed this as completed Mar 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants