janet-lang / janet

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

Should `(empty? nil)` return true instead of throwing an error? #1381

Closed amano-kenji closed 8 months ago

amano-kenji commented 8 months ago

This caused an error in my program.

error: expected iterable type, got nil
  in empty? [boot.janet] on line 117, column 50
  in _thunk [repl] (tailcall) on line 4, column 1
pepe commented 8 months ago

I would not say so. At least, because I have code where I rely on such a behavior :-).

amano-kenji commented 8 months ago

Then, I think (doc empty?) should be made clearer.

sogaiu commented 8 months ago

The current implementation of empty? relies on next. I don't know whether it's considered an implementation detail or not, but it seems that empty? will likely produce a value for an argument if it's the kind of thing that next can be passed (and result in ordinary operation).

I don't know if all such values are intended as arguments, but FWIW next has an associated bytecode which calls out to janet_next_impl and the expectation there seems to be only things that are "iterable" (which can be observed from the error string in the original post of this issue).

So what's "iterable"? The code above suggests it is something that passes one of the following predicates:

AFAIU, there isn't currently a built-in predicate that expresses this notion.

Note that nil does not yield a truthy value for any of those predicates:

$ janet
Janet 1.33.0-e85a8417 linux/x64/gcc - '(doc)' for help
repl:1:> (dictionary? nil)
false
repl:2:> (bytes? nil)
false
repl:3:> (indexed? nil)
false
repl:4:> (abstract? nil)
false
repl:5:> (fiber? nil)
false

From another angle, I would not have expected empty? to have worked on nil, because empty? to me sounds like it would be a question to ask of something that could be "not empty" or "empty", depending on circumstances. nil doesn't seem like that kind of thing to me.

It's true that in a language like Common Lisp where nil and () (the empty list) are considered the same, the question sort of makes sense, but I think that is more of an exceptional situation.

Having said that, first works on nil with no error (^^; Though this also seems like a bit of an exceptional behavior...at least to me.

amano-kenji commented 8 months ago

In plain english, null is empty.

sogaiu commented 8 months ago

I don't think the docstrings in Janet (let alone many other programming languages) are oriented toward just plain English -- they often assume a certain amount of programming background. I have found Janet's docstrings to be this way and I don't believe it likely that they are going to change radically in such a direction. Thus, I find the statement "In plain english, null is empty" to be unmoving and unconvincing.

As to the title of the issue, my opinion, for what little it might be worth, would be "no". I don't think "(empty? nil) should return true instead of throwing an error" for the reason that @pepe mentioned but also based on what I wrote above.

However, it may yet be possible to improve the documentation, though I think that deserves a separate issue.

Since you find the docstring to be unsatisfactory, may be you would be a good candidate for generating a more satisfactory one. It seems that you would probably be good at recognizing what is satisfactory to you. If you have some concrete suggestions as to how, perhaps those could show up as a PR (or discussed in another issue). May be others would agree that your idea is an improvement and it might get merged to the benefit of all.

amano-kenji commented 8 months ago

Closing this to create another issue.