janet-lang / janet

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

Make loop `:range` more consistent with `range` #1275

Closed primo-ppcg closed 12 months ago

primo-ppcg commented 12 months ago

Looking at the asymmetry between these forms:

> (range 10)
@[0 1 2 3 4 5 6 7 8 9]
> (loop [i :range [10]] (pp i))
10
11
12
13
14
15
...

That the second produces an infinite loop is a mistake I confess to having made more than once. The :range verb could be modified fairly easily to allow a single term: https://github.com/primo-ppcg/janet/commit/b8e3482619e659f985230756341a9c851dd2c843 Which compiles to the same code as the two or three term forms:

(pp ((disasm |(loop [i :range [10]])) :bytecode))
(pp ((disasm |(loop [i :range [0 10]])) :bytecode))
(pp ((disasm |(loop [i :range [0 10 1]])) :bytecode))
# ->
@[(ldi 0 0) (ldi 1 10) (lt 2 0 1) (jmpno 2 3) (addim 0 0 1) (jmp -3) (retn)]
@[(ldi 0 0) (ldi 1 10) (lt 2 0 1) (jmpno 2 3) (addim 0 0 1) (jmp -3) (retn)]
@[(ldi 0 0) (ldi 1 10) (lt 2 0 1) (jmpno 2 3) (addim 0 0 1) (jmp -3) (retn)]

This change would also affect :range-to, :down, and :down-to.


Other suggestions

A single term form could also be allowed without needing to be a tuple:

(loop [cnt :down 10]
  (pp cnt))
10
9
8
7
6
5
4
3
2
1

This can be achieved by modifying a single line:

@@ /src/boot/boot.janet -416,4 +416,4 @@
 (defn- check-indexed [x]
   (if (indexed? x)
     x
-    (error (string "expected tuple for range, got " x))))
+    [x]))

I'm not sure if that's a good idea, just throwing it out there.

It would also be really nice if :range could also allow a negative step, as range does. I don't know if this could be done without adding some run-time comparisons to the macro expansion, which is definitely undesirable, but if it could I think it would be a good idea. Perhaps the current behavior could be preserved as :up and :up-to.

pepe commented 12 months ago

I like the first modification. About the other suggestion I am not sure 😊.

sogaiu commented 12 months ago

I also like the first item.

Not sure about the rest, except that I also felt a negative step would have been nice on occasion. Uncertain about the practicalities though.

primo-ppcg commented 12 months ago

I also felt a negative step would have been nice on occasion.

For sure. The problem is the following:

(loop [i :range [10 0 -1]])   # step known to be negative at compile time

(fn [x]
  (loop [i :range [10 0 x]])) # step not known to be negative at compile time

It's not really a big issue to check neg? at runtime, however it's also not known which comparator to use, which will compile to much less effecient code. I suspect this is why :down exists in the first place.

I say that it would be really nice, not because I think it can be done, but more in hope that someone smarter might have an idea that would work.