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

Warning/error on seq with no body #1470

Closed
sarna opened this issue Jun 28, 2024 · 5 comments
Closed

Warning/error on seq with no body #1470

sarna opened this issue Jun 28, 2024 · 5 comments

Comments

@sarna
Copy link

sarna commented Jun 28, 2024

It'd be nice if Janet warned you (threw an error, printed a warning, ...) when you don't provide the body for seq, for example:

(seq [i :range [0 5]])

Right now it will happily return @[nil nil nil nil nil]. I can't think of a reason why a user wanted to write it this way.

The same thing happens with loop, not sure about other looping constructs.

@sogaiu
Copy link
Contributor

sogaiu commented Jun 29, 2024

I guess if one wanted 5 nils, it would be clearer to express it like:

(seq [i :range [0 5]] nil)

@sogaiu
Copy link
Contributor

sogaiu commented Jul 13, 2024

@sarna I think this commit may have a relevant change:

$ janet
Janet 1.35.2-342a29c7 linux/x64/gcc - '(doc)' for help
repl:1:> (seq [i :range [0 5]])
repl:1:1: compile warning (normal): empty loop body
@[nil nil nil nil nil]

@sogaiu
Copy link
Contributor

sogaiu commented Jul 17, 2024

Although the change introduced makes sense for seq, I'm not sure it does for loop.

I found an example here where an empty-body loop is being used for side-effects:

    (loop [line :iterate (file/read infile :line)
           :until (string/has-prefix? ">THREE " line)])

@sarna
Copy link
Author

sarna commented Jul 17, 2024

With loop I think no warning is okay. seq is explicitly a list comprehension, so I'd expect the result to be used - not really true with loop.

@sogaiu
Copy link
Contributor

sogaiu commented Jul 17, 2024

FWIW, in the above case, the warning (that shows up on stderr) that appears for an empty-body loop broke a "driver" program that was parsing stderr. It had expectations that only certain output would show up on stderr (i.e. that coming from progams being executed).

I think in this case, the warning should actually not happen -- there seem to be legitimate reasons to have empty-bodied loops (in contrast to seq).

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