MattX / peroxide

Scheme in Rust
Apache License 2.0
16 stars 3 forks source link

call/cc crashes with no arguments #2

Closed jpellegrini closed 3 years ago

jpellegrini commented 3 years ago

call/cc works when called properly:

>>> (call/cc (lambda (k) (+ 1 (k -1))))
 => -1
>>> 

But it seems that some argument check is missing:

RUST_BACKTRACE=full RUST_BACKTRACE=1 target/debug/main
Handler set!
>>> (call/cc)
thread 'main' panicked at 'index out of bounds: the len is 0 but the index is 0', src/environment.rs:238:13
stack backtrace:
   0: rust_begin_unwind
             at /usr/src/rustc-1.49.0/library/std/src/panicking.rs:495:5
   1: core::panicking::panic_fmt
             at /usr/src/rustc-1.49.0/library/core/src/panicking.rs:92:14
   2: core::panicking::panic_bounds_check
             at /usr/src/rustc-1.49.0/library/core/src/panicking.rs:69:5
   3: <usize as core::slice::index::SliceIndex<[T]>>::index
             at /usr/src/rustc-1.49.0/library/core/src/slice/index.rs:182:10
   4: core::slice::index::<impl core::ops::index::Index<I> for [T]>::index
             at /usr/src/rustc-1.49.0/library/core/src/slice/index.rs:15:9
   5: <alloc::vec::Vec<T> as core::ops::index::Index<I>>::index
             at /usr/src/rustc-1.49.0/library/alloc/src/vec.rs:1939:9
   6: peroxide::environment::ActivationFrame::get
             at ./src/environment.rs:238:13
   7: peroxide::environment::ActivationFrame::get
             at ./src/environment.rs:240:13
   8: peroxide::vm::run_one_instruction
             at ./src/vm.rs:276:26
   9: peroxide::vm::run
             at ./src/vm.rs:200:15
  10: peroxide::Interpreter::compile_run
             at ./src/lib.rs:176:9
  11: peroxide::Interpreter::parse_compile_run
             at ./src/lib.rs:169:9
  12: main::rep
             at ./src/bin/main.rs:141:15
  13: main::handle_one_expr
             at ./src/bin/main.rs:132:13
  14: main::handle_one_expr_wrap
             at ./src/bin/main.rs:79:5
  15: main::do_main
             at ./src/bin/main.rs:68:13
  16: main::main
             at ./src/bin/main.rs:29:11
  17: core::ops::function::FnOnce::call_once
             at /usr/src/rustc-1.49.0/library/core/src/ops/function.rs:227:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
jpellegrini commented 3 years ago

And it seems that not only call/cc, but syntax-rules too, fails in the same way: try ((syntax-rules -1)).

MattX commented 3 years ago

Thanks for letting me know! I'll take a look this week

MattX commented 3 years ago

Argument length check was just broken in general, an easy example is ((lambda (x) x)). The reason was that the bytecode instruction that checks argument count is always the first in a function body, and the first instruction was always skipped due to the program counter being incremented as part of the main loop, even if it was just set to 0 to enter a new function.

Looks like this issue was introduced in commit cc897c89acd18c6811179c8cf6c717a8aeaa0260.

Should be resolved in 4040cc143cff03537d17480e5f169ad5943a25ef, added test cases for all examples discussed in this thread.

jpellegrini commented 3 years ago

Yay! :fireworks: