Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 72 additions & 1 deletion crates/shen-rust/src/primitives.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,69 @@ pub fn register_hot_overrides(interp: &mut Interp) {

interp.register_native("read-file-as-string", 1, hot_read_file_as_string);
interp.register_aot_direct("read-file-as-string", hot_read_file_as_string);

// pr — print a string to a stream. The canonical kernel `pr` gates the
// *whole* write on `*hush*` (see kernel/klambda/writer.kl), so under
// `-q` / `(set *hush* true)` even a write to an explicitly-opened file
// stream is silently dropped, producing zero-byte files (issue #2).
// That gate is only meant to silence interactive console chatter, so we
// consult `*hush*` ONLY when the target is the standard output stream;
// writes to any other (file) stream always occur. See #2.
interp.register_native("pr", 2, hot_pr);
interp.register_aot_direct("pr", hot_pr);
}

/// pr — write the string `args[0]` to the output stream `args[1]`, honouring
/// `*hush*` only for the standard output stream (issue #2). Returns the
/// original string, matching the kernel contract.
fn hot_pr(interp: &mut Interp, args: &[Value]) -> ShenResult<Value> {
let s = match args[0].as_str() {

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add an arity guard before indexing args here? Normal apply paths should pass two arguments, but this hot override is also registered on the direct path, so a malformed/direct call would currently panic on args[0] or args[1] instead of returning a ShenError.

Some(s) => s.to_string(),
None => {
return Err(ShenError::new(format!(
"pr: arg 1 not a string: {:?}",
args[0]
)))
}
};
let target = match args[1].as_stream() {
Some(t) => t,
None => {
return Err(ShenError::new(format!(
"pr: arg 2 not a stream: {:?}",
args[1]
)))
}
};

// Suppress the write only when `*hush*` is set AND the target is the
// standard output stream. File (and other) streams always get written.
let hush_sym = interp.intern("*hush*");
let hush = matches!(
interp.env.get_global(hush_sym).and_then(|v| v.as_bool()),
Some(true)
);
if hush {
let stout_sym = interp.intern("*stoutput*");
let is_stdout = interp
.env
.get_global(stout_sym)
.and_then(|v| v.as_stream())
.is_some_and(|out| Rc::ptr_eq(&out, &target));
if is_stdout {
return Ok(args[0]);
}
}

let mut stream = target.borrow_mut();
match &mut *stream {
Stream::Out(w) => {
w.write_all(s.as_bytes())
.map_err(|e| ShenError::new(format!("pr: {e}")))?;
}
_ => return Err(ShenError::new("pr: not an output stream")),
}
Ok(args[0])
}

// --- hot-override bodies (DirectFn-compatible plain fns) ---
Expand Down Expand Up @@ -376,8 +439,13 @@ fn register_core(interp: &mut Interp) {
});

// --- vectors (absvectors) ---
// Cap oversized requests so an absurd size raises a CATCHABLE Shen error
// instead of OOM-aborting the whole process (e.g. `(absvector 100000000000)`
// from the reader-fuzz corpus). 2^24 (~16.7M slots) is ~800x the largest
// vector the kernel itself allocates; mirrors shen-go and shen-cl (#3).
const MAX_ABSVECTOR: i64 = 1 << 24;
interp.register_native("absvector", 1, |_, args| match args[0].as_int() {
Some(n) if n >= 0 => {
Some(n) if (0..=MAX_ABSVECTOR).contains(&n) => {
let len = n as usize;
let cells = vec![Value::sym(crate::symbol::SymId(0)); len];
// Will be overwritten with an "uninitialized" sentinel in
Expand All @@ -386,6 +454,9 @@ fn register_core(interp: &mut Interp) {
// WellKnown ordering — harmless for the kernel).
Ok(Value::absvector(cells))
}
Some(n) if n > MAX_ABSVECTOR => Err(ShenError::new(format!(
"absvector size {n} out of range (0..{MAX_ABSVECTOR})"
))),
_ => Err(ShenError::new(format!("absvector: bad arg: {:?}", args[0]))),
});
interp.register_native("<-address", 2, |_, args| {
Expand Down
38 changes: 20 additions & 18 deletions crates/shen-rust/tests/cli_launcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,17 @@
//! Mirror of shen-go's `cmd/shen/main_test.go`: spawn the real release
//! binary and exercise the launcher protocol — `eval -e EXPR`, `eval -l
//! FILE`, `script FILE`, `--version` / `--help`, piping EOF, the `-q`
//! file-write divergence, and adversarial input.
//! file-write behavior, and adversarial input.
//!
//! * `-q` (which sets `*hush*`): `*hush*` only silences chatter on the
//! standard output stream. A `pr` to an explicitly-opened file stream is
//! always written, with or without `-q` — matching shen-cl/shen-go. We
//! assert the file IS written in BOTH cases (issue #2 fixed a prior bug
//! where `-q` dropped the write, producing zero-byte files).
//!
//! DIVERGENCES from shen-go, locked in with comments (the spec requires the
//! port's CORRECT documented behavior, not faked parity):
//!
//! * `-q` (which sets `*hush*`): on shen-rust `-q` SILENCES `pr` writes to
//! file streams (zero-byte files), whereas shen-cl/shen-go route `pr` to
//! files regardless. We assert the file IS written WITHOUT `-q`, and is
//! EMPTY (or the process still succeeds) WITH `-q`.
//! * Adversarial `eval -e`: shen-rust prints the error to stderr and the
//! launcher exits SUCCESS (it does not map an eval error to a nonzero
//! code the way shen-go does). We assert the error message is reported
Expand Down Expand Up @@ -235,12 +237,13 @@ fn piped_stdin_eof_exits_cleanly() {
);
}

/// The `-q` / `*hush*` divergence, locked in as shen-rust's CORRECT
/// documented behavior. WITHOUT `-q`, `pr` to a file stream writes the
/// payload. WITH `-q`, `*hush*` silences `pr` to file streams (zero-byte
/// file) — this is the cross-impl divergence the spec calls out.
/// Regression for issue #2: `-q` (`*hush*`) must NOT silence `pr` writes to
/// file streams. `*hush*` only suppresses chatter on the standard output
/// stream; an explicitly-opened file stream is always written, with or
/// without `-q` — matching shen-cl / shen-go. (Previously shen-rust dropped
/// the write under `-q`, producing zero-byte files.)
#[test]
fn quiet_silences_pr_to_file_but_default_writes() {
fn quiet_does_not_silence_pr_to_file() {
let dir = std::env::temp_dir().join(format!("shen-rust-cli-hush-{}", std::process::id()));
std::fs::create_dir_all(&dir).ok();

Expand All @@ -260,8 +263,8 @@ fn quiet_silences_pr_to_file_but_default_writes() {
"without -q, pr must write the payload to the file"
);

// --- WITH -q: *hush* silences the pr write (documented shen-rust
// divergence). The process still succeeds; the file is empty. ---
// --- WITH -q: the file is STILL written (issue #2 regression). `*hush*`
// only silences the standard output stream, never a file stream. ---
let path_b = dir.join("out_q.txt");
let expr_b = format!(
r#"(let S (open "{}" out) (do (pr "payload" S) (close S)))"#,
Expand All @@ -270,12 +273,11 @@ fn quiet_silences_pr_to_file_but_default_writes() {
let o = run(&["eval", "-q", "-e", &expr_b], None, TIMEOUT);
assert_no_backtrace(&o, "eval -q");
assert!(o.success, "eval -q failed:\n{}", o.combined);
let data_b = std::fs::read(&path_b).unwrap_or_default();
assert!(
data_b.is_empty(),
"shen-rust -q (*hush*) must silence pr to files; got {} bytes: {:?}",
data_b.len(),
String::from_utf8_lossy(&data_b)
let data_b = std::fs::read(&path_b).expect("output file should exist with -q (issue #2)");
assert_eq!(
String::from_utf8_lossy(&data_b),
"payload",
"with -q, *hush* must NOT silence pr to a file stream (issue #2)"
);

std::fs::remove_file(&path_a).ok();
Expand Down
10 changes: 10 additions & 0 deletions crates/shen-rust/tests/primitives_coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,16 @@ fn absvector_out_of_range_errors() {
// Reading past the end is a catchable error, not a panic.
assert!(run(&mut i, "(<-address (absvector 2) 5)").is_err());
assert!(run(&mut i, "(address-> (absvector 2) 5 1)").is_err());
// An absurd size is a catchable error (capped), not an OOM abort.
assert!(run(&mut i, "(absvector 100000000000)").is_err());
assert!(run(
&mut i,
"(trap-error (absvector 100000000000) (lambda E true))"
)
.is_ok());
// The cap boundary and legitimate sizes still succeed.
assert!(run(&mut i, "(absvector 16777216)").is_ok());
assert!(run(&mut i, "(absvector 8)").is_ok());
}

// ---------------------------------------------------------------------------
Expand Down