diff --git a/src/builtins/concurrency.rs b/src/builtins/concurrency.rs index 55aa445a..56f7f236 100644 --- a/src/builtins/concurrency.rs +++ b/src/builtins/concurrency.rs @@ -187,56 +187,18 @@ pub fn call_channel(vm: &mut Vm, name: &str, args: &[Value]) -> Result = - Vec::with_capacity(ops.len()); - for op in &ops { - let pair2 = pair.clone(); - let waker = Box::new(move || { - let (lock, cvar) = &*pair2; - *lock.lock() = true; - cvar.notify_one(); - }); - match op { - SelectOp::Receive(ch) if !ch.is_closed() => { - registrations.push(ch.register_recv_waker_guard(waker)); - } - SelectOp::Send(ch, _) if !ch.is_closed() => { - registrations.push(ch.register_send_waker_guard(waker)); - } - // Closed channels: no registration needed — a - // subsequent `try_select_sweep` iteration observes - // the closed state directly. - SelectOp::Receive(_) | SelectOp::Send(_, _) => {} - } - } - loop { - if let Some(result) = try_select_sweep(&ops)? { - // Dropping `registrations` (at scope exit) runs - // each guard's Drop → `remove_*_waker`. Idempotent - // for entries whose waker already fired. - drop(registrations); - return Ok(result); - } - let (lock, cvar) = &*pair; - let mut notified = lock.lock(); - if !*notified { - cvar.wait_for(&mut notified, std::time::Duration::from_secs(1)); - } - *notified = false; - } + // Main thread: same wake-graph-driven protocol as + // `channel.receive` / `channel.send`. Returns a + // "deadlock on main thread" error if the wake graph proves + // no counterparty can ever make any arm ready (previously + // this path spun on a 1s condvar poll forever). + main_thread_wait_for_select(&ops, vm) } "recv_timeout" => { // channel.recv_timeout(ch, dur) -> Result(a, String) @@ -988,9 +950,19 @@ fn main_thread_is_starved(vm: &Vm, target: &crate::scheduler::MainTarget) -> boo // Join with no scheduler: the joinee never ran, so there's // no result coming. crate::scheduler::MainTarget::Join(_) => true, - // Select with no scheduler: no channel timers tracked through - // SelectEdge ids; deadlock. - crate::scheduler::MainTarget::Select(_) => true, + // Select with no scheduler: same reasoning as Recv/Send, per + // arm. If ANY arm's channel has a pending timer close, the + // timer thread's `ch.close()` will fire that arm's waker and + // unblock the select — not starved. This is the + // `channel.select([Recv(ch), Recv(channel.timeout(..))])` case + // with no spawned tasks (so no scheduler is ever created): the + // timeout arm must still wake. Only when NO arm has an external + // timer waiting is the select a genuine deadlock. + crate::scheduler::MainTarget::Select(edges) => !edges.iter().any(|e| match e { + crate::scheduler::SelectEdge::Recv(ch) | crate::scheduler::SelectEdge::Send(ch) => { + ch.has_pending_timer_close() + } + }), } } @@ -1398,6 +1370,150 @@ fn main_thread_wait_for_receive( } } +/// Block the main thread on a `channel.select` over `ops` until one +/// arm becomes ready, or the wake graph proves no scheduled task can +/// drive ANY arm forward (deadlock). +/// +/// Phase 4: same wake-graph-driven protocol as +/// `main_thread_wait_for_receive` / `_send`, generalized to a set of +/// select arms via `MainTarget::Select`. Each arm registers a +/// recv/send waker (held as a `WakerRegistration` guard so a losing +/// sibling is deregistered on return and its `waiting_*` counter does +/// not leak a phantom peer); a single shared condvar is poked by any +/// waker firing or by the wake-graph signal callback. On each wake we +/// re-run `try_select_sweep` (lost-wakeup guard) and consult +/// `main_thread_is_starved`; a confirmed-stable starvation fires a +/// "deadlock on main thread" error rather than spinning forever. +/// +/// Before this path existed, this branch spun on `cvar.wait_for(1s)` +/// indefinitely — a `channel.select` over a set with no possible +/// counterparty never returned. `channel.receive` / `channel.send` on +/// the same dead set DO report a deadlock; this brings select in line +/// (and matches docs/concurrency.md, which claims select "detects a +/// deadlock and reports an error"). +/// +/// NOTE: `channel.recv_timeout` deliberately keeps its own inline +/// condvar loop and does NOT call this — its private timer channel is +/// `pending_timer_close`, so the timer thread's `close()` guarantees +/// termination even with no scheduler attached, where this function's +/// no-scheduler `MainTarget::Select` starvation check would (correctly, +/// for a plain select) report deadlock. +fn main_thread_wait_for_select(ops: &[SelectOp], vm: &Vm) -> Result { + // Build the wake-graph target: one edge per arm. Closed channels + // are still included — `is_main_starved`'s Select arm treats a + // closed channel as fuel (not starved), and `try_select_sweep` + // observes the closed state directly on the next pass. + let edges: Vec = ops + .iter() + .map(|op| match op { + SelectOp::Receive(ch) => crate::scheduler::SelectEdge::Recv(ch.clone()), + SelectOp::Send(ch, _) => crate::scheduler::SelectEdge::Send(ch.clone()), + }) + .collect(); + let target = crate::scheduler::MainTarget::Select(edges); + + let pair = Arc::new((Mutex::new(false), Condvar::new())); + // Install the wake-graph signal callback + park MAIN on the select + // edge set so parked counterparties' BFS finds MAIN as a wake + // destination. Unpark on exit (the `unpark_main` closure below). + let _signal_guard = install_main_signal(vm, &pair); + if let Some(sched) = vm.current_scheduler() { + sched.park_main(&target); + } + let unpark_main = |vm: &Vm| { + if let Some(sched) = vm.current_scheduler() { + sched.unpark_main(); + } + }; + + // Per-arm waker registrations. Each iteration re-registers every + // open arm and drops the prior guards first, so a stale waker is + // deregistered before a fresh one is minted (no `waiting_*` leak — + // same rationale as the receive/send single-waker paths, but here + // the guards are a `Vec` over the arm set). + let mut registrations: Vec = Vec::with_capacity(ops.len()); + // Re-check helper: returns Some(result) when an arm is ready, + // dropping the registrations FIRST then unparking MAIN — same + // drop/unpark ordering as the receive/send recheck closures. + let try_finish = + |registrations: &mut Vec| -> Result, VmError> { + if let Some(result) = try_select_sweep(ops)? { + registrations.clear(); + unpark_main(vm); + return Ok(Some(result)); + } + Ok(None) + }; + loop { + if let Some(result) = try_finish(&mut registrations)? { + return Ok(result); + } + // Drop the previous iteration's guards before minting new ones + // so old wakers are deregistered first. + registrations.clear(); + for op in ops { + let pair2 = pair.clone(); + let waker = Box::new(move || { + let (lock, cvar) = &*pair2; + *lock.lock() = true; + cvar.notify_one(); + }); + match op { + SelectOp::Receive(ch) if !ch.is_closed() => { + registrations.push(ch.register_recv_waker_guard(waker)); + } + SelectOp::Send(ch, _) if !ch.is_closed() => { + registrations.push(ch.register_send_waker_guard(waker)); + } + // Closed channels: no registration — `try_select_sweep` + // observes the closed state directly. + SelectOp::Receive(_) | SelectOp::Send(_, _) => {} + } + } + // Re-check after registering to close the lost-wakeup window + // between the sweep above and the registrations. + if let Some(result) = try_finish(&mut registrations)? { + return Ok(result); + } + // Pre-wait starvation check: if the wake graph already proves + // no arm can ever be made ready, this is a candidate deadlock. + if main_thread_is_starved(vm, &target) { + // Catch an arm that raced ready between the re-check above + // and this BFS. + if let Some(result) = try_finish(&mut registrations)? { + return Ok(result); + } + // Confirm the starvation is STABLE before firing — a single + // snapshot can be transiently starved under contention. + let confirmed = confirm_main_starved(&pair, || main_thread_is_starved(vm, &target)); + // An arm that raced ready during the confirm window wins + // over the deadlock verdict. + if let Some(result) = try_finish(&mut registrations)? { + return Ok(result); + } + if confirmed { + registrations.clear(); + unpark_main(vm); + return Err(VmError::new( + "deadlock on main thread: channel select with no counterparty".into(), + )); + } + // Progress signalled or starvation cleared: re-evaluate. + continue; + } + // Indefinite wait — woken by any arm's waker (channel state + // change) or the wake-graph signal callback. No 100ms tick. + { + let (lock, cvar) = &*pair; + let mut notified = lock.lock(); + while !*notified { + cvar.wait(&mut notified); + } + *notified = false; + } + } +} + /// Block the main thread until `handle` produces a result or the wake /// graph proves no scheduled task can drive the joinee forward /// (deadlock). diff --git a/src/cli/add.rs b/src/cli/add.rs index 215c67c0..142918ba 100644 --- a/src/cli/add.rs +++ b/src/cli/add.rs @@ -277,6 +277,13 @@ fn run_add_command(args: &[String]) -> Result<(), Box> { ) .into()); } + if silt::manifest::is_reserved_keyword(&name) { + return Err(format!( + "silt add: dependency name `{name}` is a reserved silt keyword; \ + pick a different name" + ) + .into()); + } let already_present = manifest .dependencies .keys() diff --git a/src/cli/help.rs b/src/cli/help.rs index 3aafa71f..657d688e 100644 --- a/src/cli/help.rs +++ b/src/cli/help.rs @@ -29,13 +29,23 @@ pub(crate) fn usage_text() -> String { // other rows instead of being pushed right by 12 characters. const SIG_WIDTH: usize = 58; let line = |sig: &str, desc: &str| format!(" {sig: String { + let mut d = String::from(base); if !cfg!(feature = "watch") { d.push_str(" [--watch requires feature: watch]"); } d }; + let run_desc = watch_caveat("Run a program"); + let check_desc = watch_caveat("Type-check without running"); + let test_desc = watch_caveat("Run test functions"); + let disasm_desc = watch_caveat("Show bytecode disassembly"); let mut out = String::new(); out.push_str("silt — a statically-typed, expression-based language\n"); out.push('\n'); @@ -46,9 +56,9 @@ pub(crate) fn usage_text() -> String { )); out.push_str(&line( "silt check [--format json] [--watch] ", - "Type-check without running", + &check_desc, )); - out.push_str(&line(test_usage_banner(), "Run test functions")); + out.push_str(&line(test_usage_banner(), &test_desc)); out.push_str(&line("silt fmt [--check] [files...]", "Format source code")); out.push_str(&line("silt repl", "Interactive REPL [feature: repl]")); out.push_str(&line( @@ -59,10 +69,7 @@ pub(crate) fn usage_text() -> String { "silt lsp", "Start the language server [feature: lsp]", )); - out.push_str(&line( - "silt disasm [--watch] []", - "Show bytecode disassembly", - )); + out.push_str(&line("silt disasm [--watch] []", &disasm_desc)); out.push_str(&line( "silt self-update [--dry-run] [--force]", "Update the silt binary to the latest release", diff --git a/src/cli/pipeline.rs b/src/cli/pipeline.rs index b082cf9b..2dd191e1 100644 --- a/src/cli/pipeline.rs +++ b/src/cli/pipeline.rs @@ -451,7 +451,7 @@ pub(crate) fn is_unknown_module_warning(err: &SourceError) -> bool { /// Returns true iff `err` is the typechecker's "module 'X' is not /// imported" error. The compiler emits the same diagnostic (with -/// identical wording, see `src/compiler/mod.rs:1923`, `:2029`, `:2782`) +/// identical wording, see `src/compiler/mod.rs:2561`, `:2657`, `:3476`) /// as a hard compile error that actually blocks bytecode emission, so /// the CLI pipeline drops the typechecker's copy to avoid rendering the /// same sentence twice. See `reportable_type_errors` for the call site. diff --git a/src/cli/watch.rs b/src/cli/watch.rs index 75bdb6ff..57b2b0a3 100644 --- a/src/cli/watch.rs +++ b/src/cli/watch.rs @@ -24,7 +24,7 @@ use crate::cli::package::find_project_root; pub(crate) fn maybe_handle_watch(args: &[String]) -> bool { #[cfg(feature = "watch")] { - if args.iter().any(|a| a == "--watch" || a == "-w") { + if has_watch_flag_before_separator(args) { handle_watch(args); return true; } @@ -32,7 +32,7 @@ pub(crate) fn maybe_handle_watch(args: &[String]) -> bool { #[cfg(not(feature = "watch"))] { - if args.iter().any(|a| a == "--watch" || a == "-w") { + if has_watch_flag_before_separator(args) { eprintln!( "The 'watch' feature is not enabled. Rebuild with: cargo build --features watch" ); @@ -43,13 +43,48 @@ pub(crate) fn maybe_handle_watch(args: &[String]) -> bool { false } +/// Index (into `args`) of the first standalone `--` token in `args[1..]`, +/// or `args.len()` when there is none. The `--` separator marks the end of +/// silt's own CLI flags: everything after it is verbatim program args (see +/// `cli::run::dispatch` and `cli::check::dispatch`). We skip `args[0]` (the +/// binary path) so a literal `--` program name can't be misread — argv[0] +/// is never a separator. +fn separator_index(args: &[String]) -> usize { + args.iter() + .enumerate() + .skip(1) + .find(|(_, a)| a.as_str() == "--") + .map(|(i, _)| i) + .unwrap_or(args.len()) +} + +/// True iff `--watch` / `-w` appears as a silt CLI flag — i.e. BEFORE the +/// first standalone `--` separator. A `-w` (or even `--watch`) that the user +/// passes as a program argument (`silt run prog.silt -- -w`) lives after the +/// separator and must NOT trigger watch mode; it is forwarded to the program +/// via `io.args()` instead. Scanning the entire arg vector (the pre-fix +/// behavior) hijacked such program args and forced watch mode. +fn has_watch_flag_before_separator(args: &[String]) -> bool { + let sep = separator_index(args); + args[..sep].iter().any(|a| a == "--watch" || a == "-w") +} + #[cfg(feature = "watch")] fn handle_watch(args: &[String]) { - let filtered: Vec = args[1..] + // Strip `--watch` / `-w` ONLY from the CLI-flag region (before the + // first standalone `--`). Everything from the separator onward is + // forwarded verbatim into the re-invoked subprocess, so a `-w` / + // `--watch` the user passes as a program arg (`silt run prog.silt + // -- -w`) survives to `io.args()` instead of being silently dropped. + // `separator_index` works on the full `args` (it skips `args[0]`); the + // post-`--` tail starts at `sep` and is spliced through unchanged. + let sep = separator_index(args); + let mut filtered: Vec = args[1..sep] .iter() .filter(|a| *a != "--watch" && *a != "-w") .cloned() .collect(); + filtered.extend(args[sep..].iter().cloned()); // BEFORE entering the watcher, dry-validate the underlying subcommand // so we don't spawn a watcher for a command that's going to fail diff --git a/src/compiler/mod.rs b/src/compiler/mod.rs index d0f9471b..c340b651 100644 --- a/src/compiler/mod.rs +++ b/src/compiler/mod.rs @@ -35,8 +35,9 @@ fn encode_type_expr(resolver: &crate::types::canonical::Resolver, te: &TypeExpr) TypeExprKind::Named(n) => { let s = resolve(*n); match s.as_str() { - "Int" | "Float" | "ExtFloat" | "String" | "Bool" | "Date" | "Time" - | "DateTime" => s, + "Int" | "Float" | "ExtFloat" | "String" | "Bool" | "Date" | "Time" | "DateTime" => { + s + } _ if s.chars().next().map(|c| c.is_uppercase()).unwrap_or(false) => { format!("Record:{s}") } @@ -438,7 +439,7 @@ pub struct Compiler { /// non-curated submodule functions (registered in the typechecker /// env / VM dispatcher but absent from `module::builtin_module_functions`) /// remain callable through the alias. Mirrors the typechecker's - /// round-58 prefix-mirror (see src/typechecker/mod.rs:1107) on the + /// round-58 prefix-mirror (see src/typechecker/mod.rs:2941) on the /// compiler side; without it `l.sum` failed with /// "undefined global: l.sum" at runtime even though `list.sum` worked. imported_builtin_module_aliases: HashMap, diff --git a/src/formatter.rs b/src/formatter.rs index f2d596bd..48dcb24d 100644 --- a/src/formatter.rs +++ b/src/formatter.rs @@ -3643,9 +3643,7 @@ fn format_fn_with_comments(f: &FnDecl, depth: usize) -> String { // line). Without this, `fn foo(a, b -- note\n)` mis-attaches // the comment to the first param. Mirrors the call / collection // emitters. - let is_last_on_line = !param_lines[i + 1..] - .iter() - .any(|l| *l == Some(p_line)); + let is_last_on_line = !param_lines[i + 1..].iter().any(|l| *l == Some(p_line)); let trailing = if is_last_on_line { take_trailing_for_line(p_line) .map(|c| format!(" {c}")) diff --git a/src/lsp/completion.rs b/src/lsp/completion.rs index 5ba0d375..64b32f1f 100644 --- a/src/lsp/completion.rs +++ b/src/lsp/completion.rs @@ -415,7 +415,7 @@ fn auto_derived_methods_for(canon_name: &str) -> &'static [&'static str] { // - Tuple/Map/Set → Equal/Hash/Display only (no Compare). // // Trait method names are sourced from `builtin_trait_decls` - // (src/typechecker/mod.rs:6926): + // (src/typechecker/mod.rs:7560): // Display → display, Compare → compare, Equal → equal, Hash → hash. // // `Bytes` is a stdlib alias for `List(Int)` and thus collapses onto diff --git a/src/lsp/conversions.rs b/src/lsp/conversions.rs index 910746d0..3f32c231 100644 --- a/src/lsp/conversions.rs +++ b/src/lsp/conversions.rs @@ -15,7 +15,7 @@ use crate::lexer::Span; /// /// LSP positions count characters in **UTF-16 code units** (per the spec, /// and what nearly every client uses as the default encoding). The lexer -/// increments `span.col` once per Unicode codepoint (src/lexer.rs:247), +/// increments `span.col` once per Unicode codepoint (src/lexer.rs:311), /// which is NOT the same as a UTF-16 unit count for characters outside /// the BMP (e.g. `😀` is 1 codepoint but 2 UTF-16 units). /// diff --git a/src/lsp/rename.rs b/src/lsp/rename.rs index 98a1b4ea..1812611a 100644 --- a/src/lsp/rename.rs +++ b/src/lsp/rename.rs @@ -235,14 +235,20 @@ pub fn is_user_renameable(name: &str) -> bool { true } -/// Basic identifier shape check. Matches silt's lexer: starts with a -/// letter or `_`, followed by any mix of alphanumerics and `_`. +/// Basic identifier shape check. Matches silt's lexer: starts with an +/// ASCII letter or `_`, followed by any mix of alphanumerics and `_`. +/// +/// The lexer's identifier-start set is ASCII-only (`'a'..='z' | 'A'..='Z' +/// | '_'` at `lexer.rs`), so the first-char check must use +/// `is_ascii_alphabetic` — `char::is_alphabetic` would accept Unicode +/// letters (`é`, `名`, …) that the lexer rejects, letting rename rewrite +/// source into something that no longer lexes on the next `silt run`. fn is_valid_silt_ident(name: &str) -> bool { let mut chars = name.chars(); let Some(first) = chars.next() else { return false; }; - if !first.is_alphabetic() && first != '_' { + if !first.is_ascii_alphabetic() && first != '_' { return false; } for c in chars { diff --git a/src/lsp/workspace.rs b/src/lsp/workspace.rs index 0151d2a2..d72b200e 100644 --- a/src/lsp/workspace.rs +++ b/src/lsp/workspace.rs @@ -281,7 +281,7 @@ fn collect_references_in_expr(expr: &Expr, name: Symbol, out: &mut Vec) { out.push(expr.span); } // Round-71 DX-2 fix: do NOT match on FieldAccess by symbol equality. - // `FieldAccess.span` is the receiver's span (parser.rs:2369-2370), + // `FieldAccess.span` is the receiver's span (parser.rs:2598/2674), // not the field's, so pushing it here would corrupt the receiver // identifier on rename. Field names live in a separate namespace // from let/fn names; symbol-collision matching across the two diff --git a/src/manifest.rs b/src/manifest.rs index 61796473..068db2de 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -214,6 +214,20 @@ impl Manifest { path: absolute, }); } + // Reserved-keyword check: a package named after a keyword + // (`loop`, `match`, …) lexes as that keyword, so `import loop` + // can never parse. Same import-shadowing footgun as the builtin + // collision above; share the predicate so init/add/load agree. + if is_reserved_keyword(&raw.package.name) { + return Err(ManifestError::Validation { + message: format!( + "package name `{}` is a reserved silt keyword; \ + pick a different name", + raw.package.name + ), + path: absolute, + }); + } validate_version(&raw.package.version, &absolute)?; let mut dependencies = BTreeMap::new(); @@ -228,6 +242,15 @@ impl Manifest { path: absolute, }); } + if is_reserved_keyword(&raw_name) { + return Err(ManifestError::Validation { + message: format!( + "dependency name `{raw_name}` is a reserved silt keyword; \ + pick a different name" + ), + path: absolute, + }); + } let dep = convert_dependency(&raw_name, raw_dep, &absolute)?; let sym = intern::intern(&raw_name); dependencies.insert(sym, dep); @@ -348,9 +371,29 @@ pub fn validate_package_name(name: &str) -> Result<(), String> { pick a different name" )); } + if is_reserved_keyword(name) { + return Err(format!( + "package name `{name}` is a reserved silt keyword; \ + pick a different name" + )); + } Ok(()) } +/// Is `name` a reserved silt keyword? Consults both `lexer::KEYWORDS` +/// (keyword-shaped tokens like `loop`, `match`, `type`) and +/// `lexer::KEYWORD_LITERALS` (`true`, `false`, lexed as `Token::Bool`). +/// +/// A package/dependency named after a reserved word lexes as that keyword +/// rather than a `Token::Ident`, so `import loop` can never parse — the +/// same import-shadowing footgun that round-75 fixed for builtin module +/// names. Rejecting up front (init / add / Manifest::load) keeps the +/// failure where the user can act on it instead of surfacing as a +/// confusing parser diagnostic far from the offending name. +pub fn is_reserved_keyword(name: &str) -> bool { + crate::lexer::KEYWORDS.contains(&name) || crate::lexer::KEYWORD_LITERALS.contains(&name) +} + fn validate_identifier(name: &str, role: &str, manifest_path: &Path) -> Result<(), ManifestError> { if is_silt_identifier(name) { return Ok(()); diff --git a/src/parser.rs b/src/parser.rs index 30e3e77f..8a49877d 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -3706,7 +3706,7 @@ impl Parser { /// `..[-]N` exits stay in lock-step. /// /// i64::MIN safety: silt's lexer rejects `9223372036854775808` at - /// lex time (see src/lexer.rs:599-603), so `Token::Int(n)` is always + /// lex time (see src/lexer.rs:626-627), so `Token::Int(n)` is always /// in `[0, i64::MAX]`. The negated tail `-m` therefore never /// underflows, and the caller's `-n` for the head is likewise safe. /// We still spell the negation as a plain unary minus to match the diff --git a/src/scheduler.rs b/src/scheduler.rs index ad5043a0..27f34635 100644 --- a/src/scheduler.rs +++ b/src/scheduler.rs @@ -27,8 +27,8 @@ pub mod test_hooks; pub mod test_support; pub mod wake_graph; -pub use wake_graph::MainTarget; -use wake_graph::{NodeId, ParkEdge, SelectEdge, WakeGraph}; +pub use wake_graph::{MainTarget, SelectEdge}; +use wake_graph::{NodeId, ParkEdge, WakeGraph}; /// Callback invoked on every wake-graph state change. Type-aliased to /// keep `SchedulerInner::main_waiters` legible — clippy's diff --git a/src/typechecker/inference.rs b/src/typechecker/inference.rs index e709bff3..5ee0de8b 100644 --- a/src/typechecker/inference.rs +++ b/src/typechecker/inference.rs @@ -3189,11 +3189,11 @@ impl TypeChecker { } // Primitive types — check method table for trait methods. // ExtFloat is auto-derived (see `register_auto_derived_impls_for` - // in `src/typechecker/mod.rs:6542`); Channel and Fn are not + // in `src/typechecker/mod.rs:7896`); Channel and Fn are not // auto-derived but user-defined trait impls register entries // under the canonical names "Channel" / "Fn" via - // `type_name_for_method_dispatch` (see `src/typechecker/mod.rs:1832`, - // `src/typechecker/mod.rs:1840`), so dispatch must route those + // `type_name_for_impl` (see `src/typechecker/mod.rs:2045`, + // `src/typechecker/mod.rs:2056`), so dispatch must route those // receivers through the same `method_table` lookup. The // `"Fn"` key matches `canonical_name(Type::Fun)`, // `head_symbol_of_canon`, and `dispatch_name_for_value` diff --git a/src/typechecker/mod.rs b/src/typechecker/mod.rs index 970ecd11..41ed23b8 100644 --- a/src/typechecker/mod.rs +++ b/src/typechecker/mod.rs @@ -5496,9 +5496,8 @@ impl TypeChecker { // and routes them through `user_decl_type_names` already, so the // `pkg == builtin_pkg` arm covers them; an imported type there // still carries its real producer package and is excluded. - let owned_for_synth = |pkg: Symbol| -> bool { - pkg == builtin_pkg || current_pkg == Some(pkg) - }; + let owned_for_synth = + |pkg: Symbol| -> bool { pkg == builtin_pkg || current_pkg == Some(pkg) }; let mut builtin_enum_names: Vec = self .enums .iter() diff --git a/src/vm/dispatch.rs b/src/vm/dispatch.rs index bc716270..2ca9edc6 100644 --- a/src/vm/dispatch.rs +++ b/src/vm/dispatch.rs @@ -329,8 +329,8 @@ impl Vm { // longer exists. The only theoretical fall-through is a // `Value::Variant` with no `__type_of__` registration // (src/vm/mod.rs ~:940), which is not constructible from a - // valid program. `PartialEq for Value` (src/value.rs - // 1586/1610) compares records and variants structurally, + // valid program. `impl PartialEq for Value` (src/value.rs + // 1662) compares records and variants structurally, // so this arm stays sound even on that malformed input. Some(Ok(Value::Bool(*receiver == extra_args[0]))) } @@ -365,7 +365,7 @@ impl Vm { (Value::String(a), Value::String(b)) => a.cmp(b), (Value::Bool(a), Value::Bool(b)) => a.cmp(b), // List vs List: the typechecker auto-derives Compare for - // List (see src/typechecker/mod.rs:3386), so a value of + // List (see src/typechecker/mod.rs:7796), so a value of // `List(T)` flowing through a `Compare` bound must // resolve here. Defer to the existing element-wise // ordering on `Value::cmp`, which already handles @@ -387,14 +387,14 @@ impl Vm { // longer exists. The only theoretical fall-through is a // `Value::Variant` with no `__type_of__` registration // (src/vm/mod.rs ~:940), which is not constructible from a - // valid program. `Value::cmp` (src/value.rs 1728/1742) + // valid program. `fn cmp` (src/value.rs 1782) // orders records and variants structurally, so this arm // stays sound even on that malformed input. (Value::Variant(..), Value::Variant(..)) | (Value::Record(..), Value::Record(..)) => receiver.cmp(other), // // Unit vs Unit: typechecker auto-derives Compare for `()` - // (src/typechecker/mod.rs:3383). All units are equal. + // (src/typechecker/mod.rs:7793). All units are equal. (Value::Unit, Value::Unit) => std::cmp::Ordering::Equal, _ => { return Some(Err(VmError::new(format!( @@ -419,7 +419,7 @@ impl Vm { // auto-derived primitives fall through to here. // // `Value` already implements `std::hash::Hash` with a - // canonical bit-hash for floats (see src/value.rs:1759). + // canonical bit-hash for floats (see src/value.rs:2161). // We reuse that impl via `DefaultHasher` so the result // matches `HashMap` keying. if !extra_args.is_empty() { @@ -443,7 +443,7 @@ impl Vm { // `Value::Variant` with no `__type_of__` registration // (src/vm/mod.rs ~:940), which is not constructible from a // valid program. `impl Hash for Value` (src/value.rs - // 2020/2027) hashes records and variants structurally, so + // 2134) hashes records and variants structurally, so // this arm stays sound even on that malformed input. match receiver { Value::Int(_) @@ -453,7 +453,7 @@ impl Vm { | Value::String(_) | Value::List(_) // Range hashes via the same `impl Hash for Value` - // (src/value.rs:1791); typechecker registers Hash for + // (src/value.rs:2134); typechecker registers Hash for // every `List(T)` that flows through a `Hash` bound, // and `1..5` reaches dispatch as `Value::Range`. | Value::Range(..) diff --git a/src/vm/execute.rs b/src/vm/execute.rs index c05d7cb0..c4f52e7c 100644 --- a/src/vm/execute.rs +++ b/src/vm/execute.rs @@ -87,9 +87,10 @@ fn language_eq(a: &Value, b: &Value) -> bool { let names_ok = na.as_str() == "" || nb.as_str() == "" || na == nb; names_ok && fa.len() == fb.len() - && fa.iter().zip(fb.iter()).all(|((ka, va), (kb, vb))| { - ka == kb && language_eq(va, vb) - }) + && fa + .iter() + .zip(fb.iter()) + .all(|((ka, va), (kb, vb))| ka == kb && language_eq(va, vb)) } (Value::Variant(na, xa), Value::Variant(nb, xb)) => { na == nb @@ -101,6 +102,41 @@ fn language_eq(a: &Value, b: &Value) -> bool { } } +/// Gate the language-level `==` / `!=` operators against function-shaped +/// values, returning the canonical surface name (`"Fn"`) to name in the +/// error if the value cannot participate in equality, or `None` if it can. +/// +/// silt does NOT statically enforce inferred trait bounds on polymorphic +/// templates: `pending_numeric_checks` (src/typechecker/inference.rs) +/// skips operands whose type is still a `Var`, on the documented promise +/// that "the VM catches it at runtime with a clean operator-domain +/// diagnostic." Ordering honors that promise (`compare()`'s catch-all in +/// src/vm/arithmetic.rs errors on function-shaped values) and so does +/// string-interpolation Display (the `Op::DisplayValue` gate). Equality was +/// the lone bypass: a polymorphic `fn eq(x: a, y: a) -> Bool { x == y }` +/// called with two functions used to silently return a `Bool` — `PartialEq +/// for Value` does `Arc::ptr_eq` on closures and name-equality on builtins +/// (src/value.rs) — instead of erroring like the concrete `f == g`, which +/// `is_valid_compare_operand` rejects at compile time (`Type::Fun` falls in +/// its `_ => false` arm). +/// +/// The rejected set is exactly the function-shaped values, all of which the +/// typechecker types as `Type::Fun` — the only type its equality gate +/// rejects. Channel / Handle / TcpListener / TcpStream are deliberately NOT +/// rejected: they are equatable by identity at runtime and the typechecker +/// accepts them (`Type::Channel` and `Type::Generic(..)` in +/// `is_valid_compare_operand`), keeping the runtime and compile-time layers +/// in parity. Collection keying / dedup uses `PartialEq for Value` directly, +/// not this operator path, so its function-identity equality is untouched +/// (see tests/round74_hash_eq_ord_contract_tests.rs). Locked by +/// tests/round96_eq_fn_runtime_tests.rs. +fn equality_operand_violation(val: &Value) -> Option<&'static str> { + match val { + Value::VmClosure(_) | Value::BuiltinFn(_) | Value::VariantConstructor(..) => Some("Fn"), + _ => None, + } +} + /// Kind of higher-order builtin iteration, used by `iterate_builtin` to /// determine how to interpret the accumulator and what to do with each /// callback result. @@ -1284,6 +1320,17 @@ impl Vm { let b = self.pop()?; let a = self.pop()?; self.check_same_type(&a, &b)?; + // Reject function-shaped operands at the execution site: the + // typechecker skips this bound on still-polymorphic operands + // and relies on the VM to catch it (see + // `equality_operand_violation`). `check_same_type` has + // already proven `a` and `b` share a discriminant, so gating + // on `a` alone covers both. + if let Some(name) = equality_operand_violation(&a) { + return Err(VmError::new(format!( + "type '{name}' does not implement Equal" + ))); + } // The language-level `==` operator follows IEEE-754 for // `ExtFloat` (so NaN != NaN). `PartialEq for Value` on // `ExtFloat` uses `to_bits()` equality so that NaN is @@ -1296,6 +1343,11 @@ impl Vm { let b = self.pop()?; let a = self.pop()?; self.check_same_type(&a, &b)?; + if let Some(name) = equality_operand_violation(&a) { + return Err(VmError::new(format!( + "type '{name}' does not implement Equal" + ))); + } self.push(Value::Bool(!language_eq(&a, &b))); } Op::Lt => self.compare(|ord| ord.is_lt())?, @@ -1341,9 +1393,9 @@ impl Vm { Op::And => { // Round-75 VM-2: the compiler always lowers `BinOp::And` // to a `JumpIfFalse` short-circuit (see - // `src/compiler/mod.rs:2010`); the + // `src/compiler/mod.rs:2326`); the // `BinOp::And | BinOp::Or => unreachable!()` at - // `compiler/mod.rs:2047` confirms no other emission + // `compiler/mod.rs:2358` confirms no other emission // path exists. Match the LoopSetup precedent // (execute.rs:Op::LoopSetup) and crash loudly on // accidental re-emission rather than silently @@ -1359,7 +1411,7 @@ impl Vm { } Op::Or => { // See Op::And above — same rationale. Compiler emits - // `JumpIfTrue` short-circuit at compiler/mod.rs:2021; + // `JumpIfTrue` short-circuit at compiler/mod.rs:2337; // `Op::Or` is reserved-but-never-emitted. unreachable!( "compiler always lowers BinOp::And/Or to JumpIfFalse/JumpIfTrue \ @@ -1371,33 +1423,51 @@ impl Vm { match &val { Value::String(_) => self.push(val), // Mirror the typechecker's string-interpolation Display - // gate (inference.rs ~2654): function-shaped values and - // channels have no Display impl. For a *concrete* operand - // the typechecker rejects these at compile time; but for a + // gate (inference.rs ~2654): the typechecker's + // `type_name_for_impl` reduces a *concrete* operand to a + // canonical name and rejects interpolation when that name + // is absent from the Display `trait_impl_set` — which + // excludes every first-class value that has no Display + // impl: function-shaped values (`Fn`), `Channel`, + // `Handle` (task handles), `TcpListener` / `TcpStream` + // (opaque resources left explicitly unprintable, see + // mod.rs ~7878), and the descriptor values + // (`TypeDescriptor` / `PrimitiveDescriptor`). For a // polymorphic type variable the operand type is still // `Var` at the interpolation site (type_name_for_impl -> - // None), so the gate is skipped and the value reaches - // here. Erroring at the execution site closes the + // None), so the compile-time gate is skipped and the value + // reaches here. Erroring at the execution site closes the // silent-wrong-behavior hole and matches how a polymorphic // `x > y` on incomparable values (e.g. two Channels) errors // at runtime rather than silently producing a result. - // These are the only first-class values whose surface type - // (`Fn` / `Channel`) the Display gate rejects yet can flow - // through an unbounded type variable. Parity is locked by + // + // The rejected set is sourced from the single predicate + // `value_implements_display` (below) so the runtime gate + // and the surface-name reporting cannot drift from the + // `type_name` oracle. Parity is locked by // tests/round95_interp_display_runtime_tests.rs. - Value::VmClosure(_) | Value::BuiltinFn(_) | Value::VariantConstructor(..) => { - return Err(VmError::new( - "type 'Fn' does not implement Display \ - (required for string interpolation)" - .to_string(), - )); - } - Value::Channel(_) => { - return Err(VmError::new( - "type 'Channel' does not implement Display \ + _ if !Self::value_implements_display(&val) => { + // Report the canonical surface name so the runtime + // message matches the typechecker's compile-time one + // (which names `Type::Fun` -> "Fn", not the per-shape + // `BuiltinFn` / `VariantConstructor` tags). Function- + // shaped values, Channel, Handle and the Tcp resources + // collapse to their canonical name via + // `dispatch_name_for_value`; the descriptor values + // (whose canonical name is the *carried* type name) + // fall back to their `type_name` so the diagnostic + // names the descriptor kind, not the reflected type. + let name = match &val { + Value::TypeDescriptor(_) | Value::PrimitiveDescriptor(_) => { + self.type_name(&val).to_string() + } + _ => crate::types::canonical::dispatch_name_for_value(&val) + .unwrap_or_else(|| self.type_name(&val).to_string()), + }; + return Err(VmError::new(format!( + "type '{name}' does not implement Display \ (required for string interpolation)" - .to_string(), - )); + ))); } _ => { let s = self.display_value(&val); diff --git a/src/vm/mod.rs b/src/vm/mod.rs index 8316104a..3f68f10f 100644 --- a/src/vm/mod.rs +++ b/src/vm/mod.rs @@ -847,6 +847,48 @@ impl Vm { } } + /// Whether a runtime value's type has a Display impl — the single + /// runtime-side oracle for the string-interpolation Display gate + /// (`Op::DisplayValue`, src/vm/execute.rs). + /// + /// This mirrors the typechecker's compile-time gate: the typechecker + /// reduces a *concrete* operand to its canonical name + /// (`type_name_for_impl` -> `crate::types::canonical::canonicalize`) + /// and rejects interpolation when that name is absent from the Display + /// `trait_impl_set`. The auto-derive lists (src/typechecker/mod.rs + /// ~7787-7876) stamp Display onto every printable built-in plus user + /// records / variants; the values that are deliberately left out are + /// the first-class no-Display types enumerated below: + /// + /// - function-shaped values (`Fn` — closures, builtins, variant + /// constructors): no Display impl; + /// - `Channel` / `Handle`: opaque concurrency primitives; + /// - `TcpListener` / `TcpStream`: opaque network resources, left + /// explicitly unprintable (src/typechecker/mod.rs ~7878); + /// - `TypeDescriptor` / `PrimitiveDescriptor`: reflective handles + /// with no surface Display. + /// + /// For a *polymorphic* operand the operand type is still a type + /// variable at the interpolation site, so the compile-time gate is + /// skipped (`type_name_for_impl` returns `None`); `Op::DisplayValue` + /// consults this predicate at the execution site to reject the same + /// set rather than silently rendering a debug string. Locked by + /// tests/round95_interp_display_runtime_tests.rs. + pub fn value_implements_display(val: &Value) -> bool { + !matches!( + val, + Value::VmClosure(_) + | Value::BuiltinFn(_) + | Value::VariantConstructor(..) + | Value::Channel(_) + | Value::Handle(_) + | Value::TcpListener(_) + | Value::TcpStream(_) + | Value::TypeDescriptor(_) + | Value::PrimitiveDescriptor(_) + ) + } + /// Human-readable type name for error messages. Renders descriptor /// and function-shaped values in surface-syntax terms rather than /// leaking internal `Value` variant names. diff --git a/src/vm/runtime.rs b/src/vm/runtime.rs index 6ca919f0..78c4f7b3 100644 --- a/src/vm/runtime.rs +++ b/src/vm/runtime.rs @@ -447,7 +447,7 @@ mod tests { // If a prior test poisoned the lock, recover — env-var state // is still safe to clean up below. let lock = ENV_LOCK.lock().unwrap_or_else(|p| p.into_inner()); - // SAFETY: see scheduler.rs:1614 — Rust 1.80+ uses thread-local + // SAFETY: see scheduler.rs:1830 — Rust 1.80+ uses thread-local // env caches and these tests do not spawn concurrent env readers. unsafe { std::env::remove_var("SILT_IO_POOL_SIZE") }; EnvGuard { _lock: lock } diff --git a/src/vm/tests.rs b/src/vm/tests.rs index f04b2568..a374f87c 100644 --- a/src/vm/tests.rs +++ b/src/vm/tests.rs @@ -599,7 +599,7 @@ fn test_leq_float() { /// Round-75 VM-2: Op::And dispatch is now `unreachable!()` — the /// compiler always lowers `BinOp::And` to a `JumpIfFalse` short- -/// circuit (see `compiler/mod.rs:2010`). Hand-emitting Op::And here +/// circuit (see `compiler/mod.rs:2326`). Hand-emitting Op::And here /// must crash the VM with the unreachable! panic, matching the /// LoopSetup precedent. #[test] @@ -619,7 +619,7 @@ fn test_and_op_is_unreachable() { /// Round-75 VM-2: symmetric to `test_and_op_is_unreachable` — the /// compiler always lowers `BinOp::Or` to a `JumpIfTrue` short- -/// circuit at `compiler/mod.rs:2021`, so direct emission of Op::Or +/// circuit at `compiler/mod.rs:2337`, so direct emission of Op::Or /// must crash with the unreachable! panic. #[test] #[should_panic( diff --git a/tests/aliased_import_runtime_tests.rs b/tests/aliased_import_runtime_tests.rs index e8bb2b95..7d5af73e 100644 --- a/tests/aliased_import_runtime_tests.rs +++ b/tests/aliased_import_runtime_tests.rs @@ -21,7 +21,7 @@ //! Fix: track alias → canonical-builtin-module in the compiler and //! rewrite `l.sum(…)` as `CallBuiltin("list.sum", …)` in //! `extract_builtin_name`, matching the typechecker's prefix-mirror -//! invariant (src/typechecker/mod.rs:1107). +//! invariant (src/typechecker/mod.rs:2941). //! //! These tests exercise the runtime via the `silt` CLI so they fail //! before the compiler-side fix and pass after. diff --git a/tests/auto_derive_dead_arm_proof_tests.rs b/tests/auto_derive_dead_arm_proof_tests.rs index 1b2bbfd1..d4242f0e 100644 --- a/tests/auto_derive_dead_arm_proof_tests.rs +++ b/tests/auto_derive_dead_arm_proof_tests.rs @@ -568,9 +568,92 @@ fn dead_arm_prose_does_not_regress() { // 3. Sanity: the corrected prose is actually present (so this // lock can't be satisfied by an empty/renamed source). assert!( - dispatch_src.contains("Defensive fallback") - || dispatch_src.contains("defensive fallback"), + dispatch_src.contains("Defensive fallback") || dispatch_src.contains("defensive fallback"), "expected the corrected 'defensive fallback' prose in \ src/vm/dispatch.rs" ); } + +/// Cross-reference lock: every `src/value.rs ` line citation in the +/// dead-arm comments of `src/vm/dispatch.rs` must point at a line in +/// `src/value.rs` that actually contains the item the comment names. +/// +/// Round-95 introduced five stale citations here — e.g. the comment +/// said `impl PartialEq for Value` lived at value.rs 1586/1610 when it +/// is actually at 1662, and `fn cmp` was cited at 1728/1742 when it is +/// at 1782. Those numbers were wrong the moment they were written and +/// nothing caught it. This test asserts each cited line resolves to its +/// named item, so the references cannot silently drift again. +#[test] +fn dispatch_value_rs_citations_resolve() { + let dispatch_src = include_str!("../src/vm/dispatch.rs"); + let value_src = include_str!("../src/value.rs"); + let value_lines: Vec<&str> = value_src.lines().collect(); + + // (substring that must appear in the dispatch comment immediately + // before/around the citation, identifier the target value.rs line + // must contain). Each pair is checked against EVERY `src/value.rs + // ` citation found in dispatch.rs; the citation whose comment + // mentions the cue string must land on a line containing `needle`. + // + // We resolve by scanning dispatch.rs for the literal citation text + // and confirming the value.rs line at contains `needle`. + let checks: &[(&str, &str)] = &[ + // arm 1: PartialEq for Value + ( + "impl PartialEq for Value` (src/value.rs", + "impl PartialEq for Value", + ), + // arm 2: fn cmp + ("fn cmp` (src/value.rs", "fn cmp"), + // float canonical bit-hash citation + ( + "canonical bit-hash for floats (see src/value.rs:", + "Value::Float(f) =>", + ), + // arm 3: impl Hash for Value (full reference, with line on next line) + ("impl Hash for Value` (src/value.rs", "impl Hash for Value"), + // Range-hash inline citation + ("impl Hash for Value`\n", "impl Hash for Value"), + ]; + + // Pull out every "src/value.rs" occurrence with its number, + // ignoring whether the separator is `:` or ` ` (some citations wrap + // the number onto the next comment line). + fn cited_line_after(text: &str, anchor: &str) -> Option { + let idx = text.find(anchor)?; + let rest = &text[idx + anchor.len()..]; + // Skip non-digit comment scaffolding (spaces, `:`, `//`, newlines). + let digits: String = rest + .chars() + .skip_while(|c| !c.is_ascii_digit()) + .take_while(|c| c.is_ascii_digit()) + .collect(); + digits.parse::().ok() + } + + for (anchor, needle) in checks { + let n = cited_line_after(dispatch_src, anchor).unwrap_or_else(|| { + panic!( + "could not find a numeric value.rs citation after the \ + anchor {anchor:?} in src/vm/dispatch.rs — the dead-arm \ + cross-reference comments changed shape; update this lock" + ) + }); + assert!( + n >= 1 && n <= value_lines.len(), + "dispatch.rs cites src/value.rs:{n} (anchor {anchor:?}) but \ + value.rs only has {} lines", + value_lines.len() + ); + let target = value_lines[n - 1]; + assert!( + target.contains(needle), + "stale cross-reference in src/vm/dispatch.rs: comment near \ + {anchor:?} cites src/value.rs:{n}, but that line is \ + {target:?} — it does NOT contain the named item {needle:?}. \ + Find the real line (grep `{needle}` in src/value.rs) and \ + correct the citation." + ); + } +} diff --git a/tests/builtin_module_constant_value_access_runtime_tests.rs b/tests/builtin_module_constant_value_access_runtime_tests.rs new file mode 100644 index 00000000..c93bedb5 --- /dev/null +++ b/tests/builtin_module_constant_value_access_runtime_tests.rs @@ -0,0 +1,134 @@ +//! Round-95 follow-up RUNTIME parity lock — every constant in the +//! `module::builtin_module_constants` registry must be seeded as a +//! first-class VALUE global by `Vm::register_builtins` +//! (`src/vm/dispatch.rs`), reachable at runtime without crashing with +//! `error[runtime]: undefined global: .`. +//! +//! ## Background +//! +//! Module constants (`math.pi`, `math.e`, `float.epsilon`, +//! `float.infinity`, …) live hand-encoded across FOUR parallel surfaces: +//! +//! 1. typechecker schemes — `src/typechecker/builtins/float.rs`, +//! `.../math.rs` (`intern("math.pi")`, …); +//! 2. VM globals — `src/vm/dispatch.rs::register_builtins` +//! (nine hand-written `self.globals.insert("math.pi", Value::Float(…))` +//! lines); +//! 3. registry — `module::builtin_module_constants` +//! (`src/module.rs`); +//! 4. docs table — `src/typechecker/builtins/docs.rs`. +//! +//! The *function* path is registry-driven: `register_builtins` loops +//! `module::builtin_module_functions(m)` for every `BUILTIN_MODULES` +//! entry, so a typechecker function name with no matching VM global is +//! impossible to introduce without tripping +//! `builtin_module_function_value_access_runtime_tests` / +//! `comprehensive_module_function_parity_tests`. +//! +//! The *constant* path is NOT registry-driven — the inserts at +//! `dispatch.rs:248-295` are nine literal lines decoupled from +//! `builtin_module_constants`. The comprehensive function-parity test +//! only closes the typechecker→registry direction (it asserts every +//! typechecker name appears in `functions ∪ constants`). NOTHING closed +//! the registry→runtime direction for constants: a 10th constant added +//! to `builtin_module_constants("math")` plus a typechecker scheme but +//! NOT to `register_builtins` would pass `silt check`, offer in +//! LSP/REPL completion, keep the comprehensive parity test GREEN — yet +//! `fn main() { math.tau }` would crash at runtime with +//! `undefined global: math.tau`, the exact round-72 value-access class. +//! +//! ## Lock +//! +//! `every_registry_constant_is_value_accessible` enumerates +//! `module::builtin_module_constants(m)` for EVERY `m` in +//! `module::BUILTIN_MODULES` and runs a real `silt run` program that +//! references `.` as a bare value. The assertion is that +//! the program neither fails nor surfaces the canonical +//! `undefined global: .` wording. Because the source of +//! truth for the enumeration is the registry itself, adding a constant +//! to `builtin_module_constants` without seeding it as a VM global makes +//! THIS test fail (`undefined global`) — closing the +//! registry→runtime direction that the function-parity tests close for +//! functions. + +use std::process::Command; + +/// Run a silt source program via the `silt run` subcommand and return +/// `(stdout, stderr, success)`. The temp file path includes both the +/// caller label AND the test process id / thread id so parallel tests +/// in this same binary cannot clobber each other's source files. +fn run_silt_raw(label: &str, src: &str) -> (String, String, bool) { + let pid = std::process::id(); + let tid = format!("{:?}", std::thread::current().id()); + let tid = tid + .trim_start_matches("ThreadId(") + .trim_end_matches(')') + .to_string(); + let tmp = std::env::temp_dir().join(format!("silt_const_access_rt_{label}_p{pid}_t{tid}.silt")); + std::fs::write(&tmp, src).expect("write temp file"); + let bin = env!("CARGO_BIN_EXE_silt"); + let out = Command::new(bin) + .arg("run") + .arg(&tmp) + .output() + .expect("spawn silt run"); + let stdout = String::from_utf8_lossy(&out.stdout).into_owned(); + let stderr = String::from_utf8_lossy(&out.stderr).into_owned(); + let _ = std::fs::remove_file(&tmp); + (stdout, stderr, out.status.success()) +} + +#[test] +fn every_registry_constant_is_value_accessible() { + let mut checked = 0usize; + for &module in silt::module::BUILTIN_MODULES { + for konst in silt::module::builtin_module_constants(module) { + checked += 1; + let label = format!("{module}_{konst}"); + // Bind the constant as a first-class value (forces a + // `GetGlobal(".")` lookup) and print a derived + // marker so the program both compiles AND runs to completion. + // We intentionally don't print the raw float (NaN/inf format + // differences are irrelevant here) — the marker proves the + // global resolved and the VM reached the end of `main`. + let src = format!( + r#" +import {module} +fn main() {{ + let c = {module}.{konst} + let _ = c + println("OK:{module}.{konst}") +}} +"# + ); + let (stdout, stderr, ok) = run_silt_raw(&label, &src); + let undef = format!("undefined global: {module}.{konst}"); + assert!( + !stderr.contains(&undef), + "registry constant `{module}.{konst}` is NOT seeded as a VM \ + value global — `Vm::register_builtins` in src/vm/dispatch.rs \ + must `self.globals.insert(\"{module}.{konst}\", …)`. This is \ + the round-72 undefined-global class re-opening through the \ + hand-coded constant path.\nstderr={stderr:?}" + ); + assert!( + ok, + "first-class value access of registry constant \ + `{module}.{konst}` failed at runtime.\n\ + stdout={stdout:?}\nstderr={stderr:?}" + ); + assert!( + stdout.contains(&format!("OK:{module}.{konst}")), + "expected `OK:{module}.{konst}` in stdout; got {stdout:?}" + ); + } + } + // Guard against the enumeration silently becoming empty (e.g. a + // refactor that makes `builtin_module_constants` return `vec![]` for + // everything would otherwise make this test vacuously pass). + assert!( + checked >= 9, + "expected at least the 9 known module constants (math.pi/e + 7 \ + float.*); only enumerated {checked} — registry regressed?" + ); +} diff --git a/tests/duplicate_module_not_imported_tests.rs b/tests/duplicate_module_not_imported_tests.rs index 472b9f0c..c919b305 100644 --- a/tests/duplicate_module_not_imported_tests.rs +++ b/tests/duplicate_module_not_imported_tests.rs @@ -1,9 +1,9 @@ //! Round 60 B9 regression lock. //! //! The typechecker emits `"module 'X' is not imported; add \`import X\`..."` -//! at `src/typechecker/inference.rs:1897`. The compiler independently -//! re-emits the identical sentence at `src/compiler/mod.rs:1923, -//! :2029, :2782`. Before this fix, the CLI pipeline fed both phases' +//! at `src/typechecker/inference.rs:2909`. The compiler independently +//! re-emits the identical sentence at `src/compiler/mod.rs:2561, +//! :2657, :3476`. Before this fix, the CLI pipeline fed both phases' //! errors into the combined diagnostic vec with no deduplication, so //! `silt check main.silt` rendered the same message twice — once as //! `error[type]`, once as `error[compile]`. diff --git a/tests/editor_grammar_builtins_tests.rs b/tests/editor_grammar_builtins_tests.rs new file mode 100644 index 00000000..4637da86 --- /dev/null +++ b/tests/editor_grammar_builtins_tests.rs @@ -0,0 +1,265 @@ +//! Regression lock: every builtin free-function name listed by +//! `src/module.rs::builtin_free_function_names` (the authoritative source +//! of truth for the `print`/`println`/`panic` trio) must appear in both +//! editor syntax-highlighting grammars, and neither grammar may list a +//! stray builtin not present in that helper. +//! +//! GAP lock (round 95 follow-up): `tests/builtin_free_function_parity_tests.rs` +//! already locks `builtin_free_function_names()` against the typechecker +//! registry, REPL completion, LSP completion, and the GLOBALS_MD docs +//! surface — but NOT the two editor grammars. The trio is hand-encoded in: +//! - editors/vim/syntax/silt.vim +//! (`syntax keyword siltBuiltin print println panic`) +//! - editors/vscode/syntaxes/silt.tmLanguage.json +//! (`"builtins"` entry: `"match": "\\b(print|println|panic)\\b"`) +//! Adding or removing a builtin free function (e.g. a future `assert` or +//! `debug`) updates the helper + its 4 locked consumers but would silently +//! leave both grammars stale — exactly the drift the round-59/60/72/79 +//! constructor & primitive locks were created to prevent. +//! +//! Mirrors the pattern of: +//! - tests/editor_grammar_constructors_tests.rs +//! - tests/editor_grammar_primitives_tests.rs +//! +//! If this test fails after changing `builtin_free_function_names()`, add +//! or remove the name in both grammar files above. + +use silt::module::builtin_free_function_names; + +use std::collections::BTreeSet; +use std::fs; +use std::path::PathBuf; + +fn repo_root() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")) +} + +fn read_grammar(rel: &str) -> String { + let path = repo_root().join(rel); + fs::read_to_string(&path).unwrap_or_else(|e| panic!("failed to read {}: {}", path.display(), e)) +} + +/// Returns every line from the vim grammar that declares a `siltBuiltin` +/// keyword group, concatenated. Panics if no such line exists — that is +/// itself a regression. +fn vim_builtin_scope(vim: &str) -> String { + let lines: Vec<&str> = vim + .lines() + .filter(|l| l.contains("siltBuiltin") && l.contains("syntax keyword")) + .collect(); + assert!( + !lines.is_empty(), + "editors/vim/syntax/silt.vim must contain at least one \ + `syntax keyword siltBuiltin ...` line listing the builtin \ + free-function names — this regression-lock test needs it." + ); + lines.join("\n") +} + +/// Returns the portion of the VS Code grammar JSON that defines the +/// `builtins` repository entry (from the `"builtins"` key through the +/// closing brace of that object). A naive forward scan to the first `}` +/// is sufficient because the `"match"` value is a single-line regex +/// without nested braces. +fn vscode_builtins_block(vscode: &str) -> String { + let marker = "\"builtins\""; + let start = vscode.find(marker).expect( + "editors/vscode/syntaxes/silt.tmLanguage.json must contain a \"builtins\" \ + repository entry listing builtin free-function names — this regression-lock test needs it.", + ); + let tail = &vscode[start..]; + let end_rel = tail + .find('}') + .expect("`\"builtins\"` entry is malformed: no closing brace found"); + tail[..=end_rel].to_string() +} + +/// Does `grammar` mention `name` as a whole identifier (flanked by +/// non-word characters on both sides)? This avoids false positives like +/// `print` matching `println`, while remaining agnostic to the specific +/// regex-alternation punctuation used by each grammar. +fn grammar_mentions_name(grammar: &str, name: &str) -> bool { + let bytes = grammar.as_bytes(); + let nlen = name.len(); + let nbytes = name.as_bytes(); + if bytes.len() < nlen { + return false; + } + let is_word = |c: u8| c.is_ascii_alphanumeric() || c == b'_'; + let mut i = 0; + while i + nlen <= bytes.len() { + if &bytes[i..i + nlen] == nbytes { + let left_ok = i == 0 || !is_word(bytes[i - 1]); + let right_ok = i + nlen == bytes.len() || !is_word(bytes[i + nlen]); + if left_ok && right_ok { + return true; + } + } + i += 1; + } + false +} + +#[test] +fn editor_grammars_include_all_builtin_free_functions() { + let vim_raw = read_grammar("editors/vim/syntax/silt.vim"); + let vscode_raw = read_grammar("editors/vscode/syntaxes/silt.tmLanguage.json"); + + // Narrow to the regions that actually define the builtin + // alternation/keyword list, so unrelated mentions of e.g. `print` + // elsewhere in the grammar (in a comment) cannot mask a removal. + let vim_scope = vim_builtin_scope(&vim_raw); + let vscode_scope = vscode_builtins_block(&vscode_raw); + + let mut missing: Vec = Vec::new(); + + for &name in builtin_free_function_names() { + if !grammar_mentions_name(&vim_scope, name) { + missing.push(format!( + "editors/vim/syntax/silt.vim (siltBuiltin keyword list) is missing \ + builtin free function `{}`", + name + )); + } + if !grammar_mentions_name(&vscode_scope, name) { + missing.push(format!( + "editors/vscode/syntaxes/silt.tmLanguage.json (\"builtins\" entry) is \ + missing builtin free function `{}`", + name + )); + } + } + + assert!( + missing.is_empty(), + "Editor syntax grammars are out of sync with \ + src/module.rs::builtin_free_function_names.\n\ + Add the following builtin name(s) to the grammar file(s) listed:\n - {}\n\ + Authoritative source: src/module.rs (builtin_free_function_names).", + missing.join("\n - ") + ); +} + +/// Extracts whitespace-delimited tokens from each +/// `syntax keyword siltBuiltin ...` line in the vim scope, stripping the +/// leading `syntax keyword siltBuiltin` prefix on each line. Mirrors +/// `vim_constructor_tokens` in tests/editor_grammar_constructors_tests.rs. +fn vim_builtin_tokens(vim_scope: &str) -> BTreeSet { + let mut tokens = BTreeSet::new(); + for line in vim_scope.lines() { + // Strip vim line-comment tail (vim uses `"`); the keyword + // declaration lines themselves never contain a `"`. + let code = line.split('"').next().unwrap_or("").trim(); + for tok in code + .split_whitespace() + .skip_while(|t| *t != "siltBuiltin") + .skip(1) + { + tokens.insert(tok.to_string()); + } + } + tokens +} + +/// Extracts the alternation tokens from the VS Code `"builtins"` block. +/// In the JSON source the regex anchors appear as `\\b(` … `)\\b` (the +/// backslashes are JSON-escaped), so on the Rust side we match the +/// literal 4-byte sequence `\\b(` written as `"\\\\b("`. Mirrors +/// `vscode_constructor_tokens` in +/// tests/editor_grammar_constructors_tests.rs. +fn vscode_builtin_tokens(block: &str) -> BTreeSet { + let open_anchor = "\\\\b("; + let close_anchor = ")\\\\b"; + let open = block + .find(open_anchor) + .expect("VS Code \"builtins\" block missing `\\\\b(` opening anchor in JSON source"); + let rest = &block[open + open_anchor.len()..]; + let close = rest + .find(close_anchor) + .expect("VS Code \"builtins\" block missing `)\\\\b` closing anchor in JSON source"); + rest[..close] + .split('|') + .map(|t| t.trim().to_string()) + .filter(|t| !t.is_empty()) + .collect() +} + +/// Authoritative set of builtin free-function names per +/// `src/module.rs::builtin_free_function_names`. +fn authoritative_builtin_names() -> BTreeSet { + builtin_free_function_names() + .iter() + .map(|s| s.to_string()) + .collect() +} + +#[test] +fn every_vim_builtin_entry_is_authoritative() { + // Reverse direction: catch stale entries left in the vim grammar + // after a builtin is removed from + // `src/module.rs::builtin_free_function_names`. The forward test + // above only catches additions; without this test, removals silently + // leave ghost highlighting in editors. Mirrors + // `every_vim_constructor_entry_is_authoritative` in + // tests/editor_grammar_constructors_tests.rs. + let vim_raw = read_grammar("editors/vim/syntax/silt.vim"); + let vim_scope = vim_builtin_scope(&vim_raw); + let tokens = vim_builtin_tokens(&vim_scope); + + let authoritative = authoritative_builtin_names(); + + let mut stray: Vec = Vec::new(); + for tok in &tokens { + if !authoritative.contains(tok) { + stray.push(format!( + "editors/vim/syntax/silt.vim siltBuiltin keyword list contains stray \ + entry `{}` not in src/module.rs::builtin_free_function_names", + tok + )); + } + } + + assert!( + stray.is_empty(), + "vim grammar lists builtin names not present in \ + builtin_free_function_names. Either remove the stray entries from \ + editors/vim/syntax/silt.vim, or (if the builtin is genuinely new) \ + add it to src/module.rs::builtin_free_function_names first:\n - {}", + stray.join("\n - ") + ); +} + +#[test] +fn every_vscode_builtin_entry_is_authoritative() { + // Reverse direction: catch stale entries left in the VS Code grammar + // after a builtin is removed from + // `src/module.rs::builtin_free_function_names`. Mirrors + // `every_vscode_constructor_entry_is_authoritative` in + // tests/editor_grammar_constructors_tests.rs. + let vscode_raw = read_grammar("editors/vscode/syntaxes/silt.tmLanguage.json"); + let vscode_scope = vscode_builtins_block(&vscode_raw); + let tokens = vscode_builtin_tokens(&vscode_scope); + + let authoritative = authoritative_builtin_names(); + + let mut stray: Vec = Vec::new(); + for tok in &tokens { + if !authoritative.contains(tok) { + stray.push(format!( + "editors/vscode/syntaxes/silt.tmLanguage.json \"builtins\" alternation \ + contains stray entry `{}` not in src/module.rs::builtin_free_function_names", + tok + )); + } + } + + assert!( + stray.is_empty(), + "VS Code grammar lists builtin names not present in \ + builtin_free_function_names. Either remove the stray entries from \ + editors/vscode/syntaxes/silt.tmLanguage.json, or (if the builtin \ + is genuinely new) add it to \ + src/module.rs::builtin_free_function_names first:\n - {}", + stray.join("\n - ") + ); +} diff --git a/tests/io_pool_size_env_knob_tests.rs b/tests/io_pool_size_env_knob_tests.rs index f6ada8bc..44260771 100644 --- a/tests/io_pool_size_env_knob_tests.rs +++ b/tests/io_pool_size_env_knob_tests.rs @@ -41,7 +41,7 @@ impl EnvGuard { fn acquire() -> Self { let lock = ENV_LOCK.lock().unwrap_or_else(|p| p.into_inner()); // SAFETY: matches the convention in - // src/scheduler.rs:1614 — Rust 1.80+ uses thread-local env + // src/scheduler.rs:1830 — Rust 1.80+ uses thread-local env // caches and these tests are serialized by ENV_LOCK so no // concurrent reader observes the transient remove. unsafe { std::env::remove_var("SILT_IO_POOL_SIZE") }; diff --git a/tests/lsp_rename_unicode_ident_rejection_tests.rs b/tests/lsp_rename_unicode_ident_rejection_tests.rs new file mode 100644 index 00000000..24d2c136 --- /dev/null +++ b/tests/lsp_rename_unicode_ident_rejection_tests.rs @@ -0,0 +1,303 @@ +//! Regression: LSP rename must reject new names that begin with a +//! non-ASCII Unicode letter, because the lexer's identifier-start set +//! is ASCII-only (`'a'..='z' | 'A'..='Z' | '_'` at `src/lexer.rs`). +//! +//! Before the fix, `is_valid_silt_ident` used `char::is_alphabetic` for +//! the first character, which returns `true` for Unicode letters like +//! `é` / `名` / `équipe`. rename then happily produced a `WorkspaceEdit` +//! rewriting every reference to a name that fails to lex on the next +//! `silt run` / `check`, silently corrupting the user's source. +//! +//! The fix switches the first-char check to `is_ascii_alphabetic`, so +//! the rename handler returns an `InvalidParams` error +//! (`is not a valid silt identifier`) instead of an edit. +//! +//! This locks the behaviour end-to-end through the LSP transport. +//! Harness mirrors `tests/lsp_rename_gated_constructor_rejection_tests.rs`. + +use std::io::{BufRead, BufReader, Read, Write}; +use std::process::{Child, ChildStdin, Command, Stdio}; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::mpsc::{Receiver, RecvTimeoutError, channel}; +use std::thread; +use std::time::{Duration, Instant}; + +use serde_json::{Value, json}; + +static REQ_COUNTER: AtomicU64 = AtomicU64::new(1); +const READ_TIMEOUT: Duration = Duration::from_secs(15); + +fn next_id() -> u64 { + REQ_COUNTER.fetch_add(1, Ordering::SeqCst) +} + +fn reader_loop(stdout: std::process::ChildStdout, tx: std::sync::mpsc::Sender) { + let mut reader = BufReader::new(stdout); + loop { + let mut header = String::new(); + let mut content_length: Option = None; + loop { + header.clear(); + match reader.read_line(&mut header) { + Ok(0) => return, + Ok(_) => {} + Err(_) => return, + } + if header == "\r\n" || header == "\n" { + break; + } + if let Some(rest) = header.trim_end().strip_prefix("Content-Length:") { + content_length = rest.trim().parse().ok(); + } + } + let Some(len) = content_length else { return }; + let mut buf = vec![0u8; len]; + if reader.read_exact(&mut buf).is_err() { + return; + } + let Ok(value) = serde_json::from_slice::(&buf) else { + return; + }; + if tx.send(value).is_err() { + return; + } + } +} + +struct LspClient { + child: Child, + stdin: ChildStdin, + rx: Receiver, +} + +impl LspClient { + fn spawn() -> Self { + let mut child = Command::new(env!("CARGO_BIN_EXE_silt")) + .arg("lsp") + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn silt lsp"); + let stdin = child.stdin.take().expect("stdin"); + let stdout = child.stdout.take().expect("stdout"); + let (tx, rx) = channel::(); + thread::spawn(move || reader_loop(stdout, tx)); + let mut client = LspClient { child, stdin, rx }; + client.initialize(); + client + } + + fn send_raw(&mut self, msg: &Value) { + let body = serde_json::to_string(msg).unwrap(); + let framed = format!("Content-Length: {}\r\n\r\n{}", body.len(), body); + self.stdin.write_all(framed.as_bytes()).unwrap(); + self.stdin.flush().unwrap(); + } + + fn recv_response_for(&self, id: u64) -> Value { + let deadline = Instant::now() + READ_TIMEOUT; + loop { + let remaining = deadline + .checked_duration_since(Instant::now()) + .unwrap_or(Duration::from_millis(0)); + if remaining.is_zero() { + panic!("timed out waiting for response id={id}"); + } + match self.rx.recv_timeout(remaining) { + Ok(msg) => { + if msg.get("id").and_then(|v| v.as_u64()) == Some(id) { + return msg; + } + } + Err(RecvTimeoutError::Timeout) => { + panic!("timed out waiting for response id={id}"); + } + Err(RecvTimeoutError::Disconnected) => { + panic!("server disconnected waiting for id={id}"); + } + } + } + } + + fn initialize(&mut self) { + let id = next_id(); + self.send_raw(&json!({ + "jsonrpc": "2.0", + "id": id, + "method": "initialize", + "params": { "capabilities": {} } + })); + let _ = self.recv_response_for(id); + self.send_raw(&json!({ + "jsonrpc": "2.0", + "method": "initialized", + "params": {} + })); + } + + fn did_open_and_wait(&mut self, uri: &str, text: &str) { + self.send_raw(&json!({ + "jsonrpc": "2.0", + "method": "textDocument/didOpen", + "params": { + "textDocument": { + "uri": uri, + "languageId": "silt", + "version": 1, + "text": text + } + } + })); + let deadline = Instant::now() + READ_TIMEOUT; + loop { + let remaining = deadline + .checked_duration_since(Instant::now()) + .unwrap_or(Duration::from_millis(0)); + if remaining.is_zero() { + panic!("timed out waiting for publishDiagnostics for {uri}"); + } + match self.rx.recv_timeout(remaining) { + Ok(msg) => { + if msg.get("id").is_none() + && msg.get("method").and_then(|v| v.as_str()) + == Some("textDocument/publishDiagnostics") + && msg.pointer("/params/uri").and_then(|v| v.as_str()) == Some(uri) + { + return; + } + } + Err(RecvTimeoutError::Timeout) => { + panic!("diagnostic timeout for {uri}"); + } + Err(RecvTimeoutError::Disconnected) => { + panic!("server disconnected"); + } + } + } + } + + fn request(&mut self, method: &str, params: Value) -> Value { + let id = next_id(); + self.send_raw(&json!({ + "jsonrpc": "2.0", + "id": id, + "method": method, + "params": params + })); + self.recv_response_for(id) + } + + fn shutdown(mut self) { + let id = next_id(); + self.send_raw(&json!({ + "jsonrpc": "2.0", + "id": id, + "method": "shutdown" + })); + let _ = self.rx.recv_timeout(READ_TIMEOUT); + self.send_raw(&json!({"jsonrpc": "2.0", "method": "exit"})); + let deadline = Instant::now() + Duration::from_secs(3); + loop { + match self.child.try_wait() { + Ok(Some(_)) => return, + Ok(None) if Instant::now() >= deadline => { + let _ = self.child.kill(); + let _ = self.child.wait(); + return; + } + Ok(None) => thread::sleep(Duration::from_millis(20)), + Err(_) => return, + } + } + } +} + +/// Drive `textDocument/rename` on a user-defined `fn foo` with a +/// Unicode-leading new name and assert the server rejects it with an +/// `InvalidParams` error mentioning "not a valid silt identifier", +/// rather than returning a `WorkspaceEdit`. +fn assert_unicode_rename_rejected(new_name: &str) { + let mut client = LspClient::spawn(); + let uri = "file:///tmp/silt_rn_unicode_ident.silt"; + // `foo` starts at line=0, char=3 (`fn foo`). + let source = "fn foo() { 0 }\nfn main() { foo() }\n"; + client.did_open_and_wait(uri, source); + + let resp = client.request( + "textDocument/rename", + json!({ + "textDocument": { "uri": uri }, + "position": { "line": 0, "character": 3 }, + "newName": new_name + }), + ); + + // The fix must reject the rename: an `error` response with the + // identifier-validation message. A `WorkspaceEdit` (a `result` with + // non-empty `changes`) would mean the server is about to corrupt the + // user's source with a name the lexer cannot tokenize. + let error_msg = resp + .pointer("/error/message") + .and_then(|v| v.as_str()) + .unwrap_or(""); + let has_changes = resp + .pointer("/result/changes") + .and_then(|c| c.as_object()) + .map(|o| !o.is_empty()) + .unwrap_or(false); + + assert!( + !has_changes, + "rename to Unicode-leading name `{new_name}` must NOT return a \ + WorkspaceEdit — the resulting source fails to lex; got {resp}" + ); + assert!( + error_msg.contains("is not a valid silt identifier"), + "rename to Unicode-leading name `{new_name}` must be rejected \ + with an InvalidParams `is not a valid silt identifier` error; \ + got {resp}" + ); + client.shutdown(); +} + +#[test] +fn rename_to_latin_accented_name_is_rejected() { + assert_unicode_rename_rejected("é"); +} + +#[test] +fn rename_to_cjk_name_is_rejected() { + assert_unicode_rename_rejected("名"); +} + +#[test] +fn rename_to_accented_word_is_rejected() { + assert_unicode_rename_rejected("équipe"); +} + +/// Positive control: a plain ASCII new name still succeeds, so the +/// fix only narrows the first-char set and does not block legitimate +/// renames. +#[test] +fn rename_to_ascii_name_still_succeeds() { + let mut client = LspClient::spawn(); + let uri = "file:///tmp/silt_rn_ascii_ident.silt"; + let source = "fn foo() { 0 }\nfn main() { foo() }\n"; + client.did_open_and_wait(uri, source); + + let resp = client.request( + "textDocument/rename", + json!({ + "textDocument": { "uri": uri }, + "position": { "line": 0, "character": 3 }, + "newName": "bar" + }), + ); + let result = resp.get("result").expect("rename result present"); + assert!( + !result.is_null(), + "rename to plain ASCII name `bar` must succeed; got {resp}" + ); + client.shutdown(); +} diff --git a/tests/main_thread_select_deadlock_tests.rs b/tests/main_thread_select_deadlock_tests.rs new file mode 100644 index 00000000..5f86513e --- /dev/null +++ b/tests/main_thread_select_deadlock_tests.rs @@ -0,0 +1,98 @@ +//! Regression lock: a main-thread `channel.select` over an op set with +//! no possible counterparty and no other live tasks must report a +//! "deadlock on main thread" error within the confirm window — NOT spin +//! forever. +//! +//! Before the fix, the `channel.select` builtin's main-thread branch was +//! left on the legacy polling loop (`cvar.wait_for(1s)` forever). It +//! never called `install_main_signal` / `park_main` / `main_thread_is_starved`, +//! so a select with no sender just looped on a benign 1s poll and never +//! returned an error — whereas the equivalent `channel.receive(ch)` / +//! `channel.send(ch, v)` programs DO report a deadlock. +//! +//! The fix routes the main-thread select branch through the same +//! wake-graph-driven `main_thread_wait_for_select` protocol the +//! receive/send/join paths use. These tests mirror +//! `scheduler_deadlock_detector_tests::test_real_deadlock_still_detected` +//! but exercise the select path. +//! +//! NOTE: with the pre-fix polling loop, `run_trial()` returns +//! `timed_out == true` (the budget elapses) and `saw_deadlock()` is +//! false — so the assertions below FAIL before the fix and PASS after. + +use std::time::Duration; + +use silt::scheduler::test_support::InProcessRunner; + +/// A bare receive-arm select on an unbuffered channel with no sender and +/// no other live task: the wake graph proves no counterparty can ever +/// drive the arm ready, so the main thread must report a deadlock. +#[test] +fn main_thread_select_recv_no_counterparty_reports_deadlock() { + let src = r#" +import channel + +fn main() { + let ch = channel.new(0) + match channel.select([Recv(ch)]) { + (_, Message(_)) -> 1 + _ -> 2 + } +} +"#; + let runner = InProcessRunner::new(src).with_budget(Duration::from_secs(3)); + let outcome = runner.run_trial(); + assert!( + !outcome.timed_out, + "main-thread select with no counterparty must report a deadlock, \ + not spin until the budget elapses; outcome={outcome:?}", + ); + assert!( + outcome.saw_deadlock(), + "expected the 'deadlock on main thread' diagnostic; outcome={outcome:?}", + ); + assert!( + outcome + .error_message + .as_deref() + .map(|m| m.contains("no counterparty")) + .unwrap_or(false), + "expected the 'no counterparty' phrase; outcome={outcome:?}", + ); + assert!( + outcome.result.is_none(), + "main must not have produced a value; outcome={outcome:?}", + ); +} + +/// A send-arm select on a full (capacity-0, no receiver) channel with no +/// other live task: same deadlock proof from the send side. +#[test] +fn main_thread_select_send_no_counterparty_reports_deadlock() { + let src = r#" +import channel + +fn main() { + let ch = channel.new(0) + match channel.select([Send(ch, 42)]) { + (_, Sent) -> 1 + _ -> 2 + } +} +"#; + let runner = InProcessRunner::new(src).with_budget(Duration::from_secs(3)); + let outcome = runner.run_trial(); + assert!( + !outcome.timed_out, + "main-thread select send with no counterparty must report a deadlock, \ + not spin until the budget elapses; outcome={outcome:?}", + ); + assert!( + outcome.saw_deadlock(), + "expected the 'deadlock on main thread' diagnostic; outcome={outcome:?}", + ); + assert!( + outcome.result.is_none(), + "main must not have produced a value; outcome={outcome:?}", + ); +} diff --git a/tests/orpattern_bind_position_tests.rs b/tests/orpattern_bind_position_tests.rs index cdc6e9a7..d66907f5 100644 --- a/tests/orpattern_bind_position_tests.rs +++ b/tests/orpattern_bind_position_tests.rs @@ -61,7 +61,11 @@ fn pick(e: E) -> Int { } fn main() -> Int { pick(B(99, 20)) } "#); - assert_eq!(b, Value::Int(20), "pick(B(99, 20)) should bind field 1, not 99"); + assert_eq!( + b, + Value::Int(20), + "pick(B(99, 20)) should bind field 1, not 99" + ); } /// Control case from the report: same position across alternatives must @@ -150,7 +154,11 @@ fn classify(e: E) -> Int { } fn main() -> Int { classify(B(99, 20)) } "#); - assert_eq!(r, Value::Int(40), "x must be 20 (field 1): 20>50 false, so 20*2=40"); + assert_eq!( + r, + Value::Int(40), + "x must be 20 (field 1): 20>50 false, so 20*2=40" + ); // And when the correctly-bound value DOES pass the guard: let hi = run(r#" @@ -190,7 +198,11 @@ fn pick(p: (Int, E)) -> Int { } fn main() -> Int { pick((0, B(99, 22))) } "#); - assert_eq!(right, Value::Int(22), "nested B binds inner field 1, not 99"); + assert_eq!( + right, + Value::Int(22), + "nested B binds inner field 1, not 99" + ); } /// Multiple shared bindings at swapped positions across alternatives. diff --git a/tests/round69_range_pattern_collapse_lock_tests.rs b/tests/round69_range_pattern_collapse_lock_tests.rs index 27b09f2b..db58379a 100644 --- a/tests/round69_range_pattern_collapse_lock_tests.rs +++ b/tests/round69_range_pattern_collapse_lock_tests.rs @@ -369,7 +369,7 @@ fn symmetry_negated_head_applies_sign() { // ── 4. i64 boundary cases ──────────────────────────────────────────── /// silt's lexer rejects `9223372036854775808` (one past i64::MAX) at -/// lex time — see src/lexer.rs:599-603 ("number literal too large"). +/// lex time — see src/lexer.rs:626-627 ("number literal too large"). /// This is the invariant that lets the parser's helper safely write /// `-n` for the head and `-m` for the tail without overflow checks: /// `Token::Int(n)` always satisfies `0 <= n <= i64::MAX`, and diff --git a/tests/round71_lsp_rename_let_and_field_tests.rs b/tests/round71_lsp_rename_let_and_field_tests.rs index 9fe41565..e6cafb29 100644 --- a/tests/round71_lsp_rename_let_and_field_tests.rs +++ b/tests/round71_lsp_rename_let_and_field_tests.rs @@ -15,7 +15,7 @@ //! //! - **DX-2** — `collect_references_in_expr` matched on //! `ExprKind::FieldAccess(_, field) if *field == name`, but -//! `FieldAccess.span = receiver.span` (parser.rs:2369-2370). Symbols +//! `FieldAccess.span = receiver.span` (parser.rs:2598/2674). Symbols //! are interned, so a top-level `let name` and a record field `name` //! share the same `Symbol`. Renaming the let pushed the receiver's //! span as a "reference", silently corrupting `r.name` into diff --git a/tests/round75_op_and_or_unreachable_tests.rs b/tests/round75_op_and_or_unreachable_tests.rs index 1bf9b53f..c42cc9f4 100644 --- a/tests/round75_op_and_or_unreachable_tests.rs +++ b/tests/round75_op_and_or_unreachable_tests.rs @@ -6,10 +6,10 @@ //! `src/vm/execute.rs:1282-1313` previously contained eager-eval //! dispatch arms for `Op::And` and `Op::Or` that popped two booleans //! and pushed `a && b` / `a || b`. The compiler does not emit those -//! opcodes — `compiler/mod.rs:2010, 2021` lower `BinOp::And` to +//! opcodes — `compiler/mod.rs:2326, 2337` lower `BinOp::And` to //! `JumpIfFalse` short-circuit and `BinOp::Or` to `JumpIfTrue` //! short-circuit; the `BinOp::And | BinOp::Or => unreachable!()` at -//! `compiler/mod.rs:2047` confirms no other emission path exists. +//! `compiler/mod.rs:2358` confirms no other emission path exists. //! //! Round-75 VM-2 replaces the two arms with `unreachable!()` to match //! the `Op::LoopSetup` precedent at `execute.rs:2070-2076` (deliberate @@ -72,7 +72,7 @@ fn op_or_discriminant_is_pinned() { /// Source-grep lock: the compiler must never emit Op::And or Op::Or. /// The `BinOp::And | BinOp::Or => unreachable!()` at -/// `compiler/mod.rs:2047` already enforces this at the AST → bytecode +/// `compiler/mod.rs:2358` already enforces this at the AST → bytecode /// boundary, but a regression that introduces a new emission path /// (e.g. a future "fold both operands eagerly" optimization) would /// be silently wrong. This test scans the compiler source for @@ -101,8 +101,8 @@ fn compiler_source_does_not_emit_op_and_or() { "compiler/mod.rs contains `{forbidden}` — round-75 VM-2 \ requires that Op::And/Op::Or never be emitted. The \ compiler must lower BinOp::And/Or to JumpIfFalse / \ - JumpIfTrue short-circuit form (see compiler/mod.rs:2010, \ - 2021); the dispatch arms in execute.rs are now \ + JumpIfTrue short-circuit form (see compiler/mod.rs:2326, \ + 2337); the dispatch arms in execute.rs are now \ `unreachable!()`." ); } diff --git a/tests/round75_silt_init_builtin_collision_tests.rs b/tests/round75_silt_init_builtin_collision_tests.rs index 988903bb..67607852 100644 --- a/tests/round75_silt_init_builtin_collision_tests.rs +++ b/tests/round75_silt_init_builtin_collision_tests.rs @@ -257,7 +257,30 @@ fn round75_validate_package_name_rejects_builtins_accepts_others() { ); } - // A handful of non-builtin identifiers are accepted. + // Every reserved keyword must be rejected too: a package named + // after a keyword lexes as that keyword, so `import loop` can never + // parse. Same import-shadowing footgun as the builtin collision. + for name in silt::lexer::KEYWORDS + .iter() + .chain(silt::lexer::KEYWORD_LITERALS.iter()) + { + let result = silt::manifest::validate_package_name(name); + assert!( + result.is_err(), + "validate_package_name should reject reserved keyword `{name}`, got Ok" + ); + let msg = result.unwrap_err(); + assert!( + msg.contains("reserved silt keyword"), + "rejection for keyword `{name}` should describe a keyword collision: {msg}" + ); + assert!( + msg.contains("pick a different name"), + "rejection for keyword `{name}` should suggest picking a different name: {msg}" + ); + } + + // A handful of non-builtin, non-keyword identifiers are accepted. for name in ["mypkg", "my_app", "calc", "_priv", "foo123"] { let result = silt::manifest::validate_package_name(name); assert!( @@ -277,3 +300,83 @@ fn round75_validate_package_name_rejects_builtins_accepts_others() { ); } } + +// ─── 8. reserved-keyword GAP: `silt init` in a keyword-named dir ─────── +// +// A directory named after a silt keyword (e.g. `loop`) would previously +// produce a `silt.toml` whose `name = "loop"`. The package runs as a +// single file, but `import loop` can never parse: `loop` lexes as a +// keyword token, not an `Ident`. Same import-shadowing footgun the +// builtin-collision check above guards against, now extended to keywords. + +#[test] +fn keyword_init_in_dir_named_loop_is_rejected() { + let dir = temp_dir_named("loop"); + let out = silt_cmd() + .arg("init") + .current_dir(&dir) + .output() + .expect("failed to invoke silt init"); + assert!( + !out.status.success(), + "silt init should refuse a package whose name is the reserved keyword `loop`; \ + stdout={} stderr={}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr) + ); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + stderr.contains("loop"), + "stderr should name the offending package name `loop`: {stderr}" + ); + assert!( + stderr.contains("reserved silt keyword"), + "stderr should describe the collision as a reserved-keyword clash: {stderr}" + ); + assert!( + stderr.contains("pick a different name"), + "stderr should suggest picking a different name (canonical shape): {stderr}" + ); + // No partial state left behind. + assert!( + !dir.join("silt.toml").exists(), + "silt.toml must not be written when init refuses a keyword name" + ); + assert!( + !dir.join("src").exists(), + "src/ must not be created when init refuses a keyword name" + ); +} + +// ─── 9. reserved-keyword GAP: every keyword name is rejected ─────────── + +#[test] +fn keyword_init_rejects_every_reserved_keyword() { + for name in silt::lexer::KEYWORDS + .iter() + .chain(silt::lexer::KEYWORD_LITERALS.iter()) + { + let dir = temp_dir_named(name); + let out = silt_cmd() + .arg("init") + .current_dir(&dir) + .output() + .expect("failed to invoke silt init"); + assert!( + !out.status.success(), + "silt init must refuse reserved-keyword name `{name}` \ + (stdout={}, stderr={})", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr) + ); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + stderr.contains("reserved silt keyword"), + "stderr for `{name}` should describe a reserved-keyword collision: {stderr}" + ); + assert!( + !dir.join("silt.toml").exists(), + "silt.toml must not be written when init refuses (`{name}`)" + ); + } +} diff --git a/tests/round76_value_type_name_parity_tests.rs b/tests/round76_value_type_name_parity_tests.rs index c2953016..5d563e41 100644 --- a/tests/round76_value_type_name_parity_tests.rs +++ b/tests/round76_value_type_name_parity_tests.rs @@ -28,6 +28,17 @@ //! `Vm::type_name(v)`). It also asserts the four specific pre-fix //! strings ("got Fn" for BuiltinFn, "got Constructor", "got Type" for //! both descriptor variants) NEVER appear for those variants. +//! +//! **Round 76 follow-up — compile-locked enumeration.** A later audit +//! flagged that the enumeration walked a hand-maintained `AllVariants` +//! struct with no compile-time link to `Value`: a new 25th variant could +//! be added with `value_kind` and `Vm::type_name` silently disagreeing on +//! the chosen string while all three source matches compiled and this +//! test stayed green. The enumeration is now driven by `expected_kind`, +//! an EXHAUSTIVE `match v { ... }` over `&Value` with NO wildcard arm — +//! adding a variant makes that match non-exhaustive and reds the test at +//! COMPILE time, forcing the new variant into the three-way parity check +//! (`three_way_kind_parity_holds_for_every_value_variant`). use std::collections::{BTreeMap, BTreeSet}; use std::sync::Arc; @@ -128,30 +139,101 @@ fn build_all_variants() -> AllVariants { } } +/// Map a `Value` to the kind string this parity lock expects, via an +/// **exhaustive** `match` with NO wildcard arm. This is the compile-time +/// link between the test and the `Value` enum that the round 76 GAP +/// follow-up demanded: adding a 25th `Value` variant makes this `match` +/// non-exhaustive and the test FAILS TO COMPILE — forcing the new +/// variant to be enrolled in the three-way parity check rather than +/// silently escaping it. +/// +/// The previous incarnation iterated a hand-maintained `AllVariants` +/// struct (24 fields) with no compile link to `Value`, so a new variant +/// would compile green and drift undetected. We keep `AllVariants` only +/// as a builder of concrete sample values; the *enumeration* and the +/// expected names are now driven by this exhaustive match. +fn expected_kind(v: &Value) -> &'static str { + match v { + Value::Int(_) => "Int", + Value::Float(_) => "Float", + Value::ExtFloat(_) => "ExtFloat", + Value::Bool(_) => "Bool", + Value::String(_) => "String", + Value::List(_) => "List", + Value::Range(..) => "Range", + Value::Map(_) => "Map", + Value::Set(_) => "Set", + Value::Tuple(_) => "Tuple", + Value::Record(..) => "Record", + Value::Variant(..) => "Variant", + Value::VmClosure(_) => "Fn", + Value::BuiltinFn(_) => "BuiltinFn", + Value::VariantConstructor(..) => "VariantConstructor", + Value::TypeDescriptor(_) => "TypeDescriptor", + Value::PrimitiveDescriptor(_) => "PrimitiveDescriptor", + Value::Channel(_) => "Channel", + Value::Handle(_) => "Handle", + Value::Bytes(_) => "Bytes", + Value::TcpListener(_) => "TcpListener", + Value::TcpStream(_) => "TcpStream", + Value::Unit => "Unit", + // NO wildcard arm: a new `Value` variant breaks this match's + // exhaustiveness at COMPILE time and reds the parity lock. + } +} + +/// Drive every sample value built by `AllVariants` through `f`, pairing +/// each with its expected kind via the exhaustive `expected_kind` match. +/// +/// `AllVariants` supplies the *concrete instances* (some variants — e.g. +/// `TcpStream` — cannot be `Default`-constructed, so they are built once +/// in `build_all_variants`). The compile-time exhaustiveness guarantee +/// lives in `expected_kind`, which is invoked for every sample here: a new +/// variant added to `Value` makes `expected_kind` fail to compile, so this +/// lock cannot go green while a variant is uncovered. fn for_each_variant(av: &AllVariants, mut f: F) { - f(&av.int, "Int"); - f(&av.float, "Float"); - f(&av.ext_float, "ExtFloat"); - f(&av.bool_, "Bool"); - f(&av.string, "String"); - f(&av.list, "List"); - f(&av.range, "Range"); - f(&av.map, "Map"); - f(&av.set, "Set"); - f(&av.tuple, "Tuple"); - f(&av.record, "Record"); - f(&av.variant, "Variant"); - f(&av.vm_closure, "Fn"); - f(&av.builtin_fn, "BuiltinFn"); - f(&av.variant_constructor, "VariantConstructor"); - f(&av.type_descriptor, "TypeDescriptor"); - f(&av.primitive_descriptor, "PrimitiveDescriptor"); - f(&av.channel, "Channel"); - f(&av.handle, "Handle"); - f(&av.bytes, "Bytes"); - f(&av.tcp_listener, "TcpListener"); - f(&av.tcp_stream, "TcpStream"); - f(&av.unit, "Unit"); + let samples: [&Value; 23] = [ + &av.int, + &av.float, + &av.ext_float, + &av.bool_, + &av.string, + &av.list, + &av.range, + &av.map, + &av.set, + &av.tuple, + &av.record, + &av.variant, + &av.vm_closure, + &av.builtin_fn, + &av.variant_constructor, + &av.type_descriptor, + &av.primitive_descriptor, + &av.channel, + &av.handle, + &av.bytes, + &av.tcp_listener, + &av.tcp_stream, + &av.unit, + ]; + // Coverage cross-check: the sample list must contain one instance of + // every distinct kind named by the exhaustive `expected_kind` match. + // (`expected_kind` is the compile-time exhaustiveness anchor; this + // asserts the *runtime* sample set is just as wide.) + let mut seen: BTreeSet<&'static str> = BTreeSet::new(); + for v in samples { + seen.insert(expected_kind(v)); + } + assert_eq!( + seen.len(), + samples.len(), + "duplicate or missing kinds among the variant samples — every \ + Value variant must have exactly one distinct sample (samples: {samples:?})" + ); + for v in samples { + f(v, expected_kind(v)); + } } // ── Test 1: parametric — every FromValue impl uses canonical kind ──── @@ -359,3 +441,90 @@ fn from_value_error_messages_no_longer_use_pre_fix_drift_strings() { } } } + +// ── Test 3: compile-locked three-way parity over EVERY variant ────── + +/// Extract `value_type_name`'s output for `v` from the observable +/// `i64::from_value` error message (`"expected Int, got "`). +/// `value_type_name` is private in `src/value.rs` and delegates to +/// `crate::builtins::value_kind`; the `FromValue` impl is the public +/// path that surfaces its string, so this is the canonical observation +/// point. (Int values are excluded by the caller, since they succeed.) +fn value_type_name_via_ffi(v: &Value) -> String { + let err = i64::from_value(v).expect_err("non-Int should fail"); + err.strip_prefix("expected Int, got ") + .unwrap_or(&err) + .to_string() +} + +/// THE GAP LOCK. The round 76 ERR-1 follow-up audit flagged that the +/// parity enumeration (`for_each_variant`) walked a hand-maintained +/// struct with no compile-time link to `Value`, so a new 25th variant +/// could be added with `value_kind` and `Vm::type_name` drifting on the +/// chosen string while all three source matches compiled and this test +/// stayed green. +/// +/// This test asserts, for EVERY variant enumerated through the now +/// compile-locked `expected_kind` exhaustive match, that all three kind +/// oracles return the SAME string: +/// - `builtins::common::value_kind` (src/builtins/common.rs) +/// - `vm::Vm::type_name` (src/vm/mod.rs) +/// - `value::value_type_name` (via FFI obs) (src/value.rs) +/// +/// Compile-lock proof: adding a `Value` variant makes `expected_kind`'s +/// match non-exhaustive, so this test fails to COMPILE until the new +/// variant is enrolled and given a sample + expected name. It cannot go +/// green with an uncovered variant. +#[test] +fn three_way_kind_parity_holds_for_every_value_variant() { + let av = build_all_variants(); + let vm = Vm::new(); + + let mut covered = 0usize; + for_each_variant(&av, |v, expected_kind| { + covered += 1; + + // Oracle 1: builtins::common::value_kind + let k_value_kind = value_kind(v); + // Oracle 2: vm::Vm::type_name + let k_type_name = vm.type_name(v); + + assert_eq!( + k_value_kind, expected_kind, + "value_kind drift on {v:?}: got {k_value_kind}, expected {expected_kind}" + ); + assert_eq!( + k_type_name, expected_kind, + "Vm::type_name drift on {v:?}: got {k_type_name}, expected {expected_kind}" + ); + + // Oracle 3: value::value_type_name (observed via the i64 FromValue + // impl). Int succeeds, so observe it directly there. + let k_value_type_name = if matches!(v, Value::Int(_)) { + // Int: value_type_name(Int) == value_kind(Int) by delegation; + // assert that identity holds. + value_kind(v).to_string() + } else { + value_type_name_via_ffi(v) + }; + assert_eq!( + k_value_type_name, expected_kind, + "value_type_name drift on {v:?}: got {k_value_type_name}, \ + expected {expected_kind}" + ); + + // The three must agree pairwise — the whole point of the lock. + assert_eq!(k_value_kind, k_type_name); + assert_eq!(k_value_kind, k_value_type_name.as_str()); + }); + + // Sanity: every variant sample was visited. This count tracks the + // exhaustive `expected_kind` match; if a variant is added it must be + // enrolled there (compile error) and here, keeping the lock honest. + assert_eq!( + covered, 23, + "expected to cover all 23 Value variants; covered {covered}. \ + If Value grew, enroll the new variant in expected_kind, \ + build_all_variants, and the samples array." + ); +} diff --git a/tests/round81_lsp_completion_parity_tests.rs b/tests/round81_lsp_completion_parity_tests.rs index eef2f2e8..d5fed526 100644 --- a/tests/round81_lsp_completion_parity_tests.rs +++ b/tests/round81_lsp_completion_parity_tests.rs @@ -208,7 +208,7 @@ fn lsp_auto_derive_non_ordering_arm_matches_typechecker() { /// 3. Convert each trait name in `non_ordering_traits` to its method /// name via the canonical lowercase mapping (`Equal -> equal`, /// `Hash -> hash`, `Display -> display` — see -/// `builtin_trait_decls` at `src/typechecker/mod.rs:6926`: every +/// `builtin_trait_decls` at `src/typechecker/mod.rs:7560`: every /// auto-derive built-in trait has a single method whose name is /// the lowercase form of the trait name). Assert the resulting /// set equals the LSP method set. @@ -262,7 +262,7 @@ fn lsp_auto_derive_method_names_match_typechecker_trait_names() { // LSP method set out of the arm, and assert they match. // // Mapping rationale: `builtin_trait_decls()` at - // src/typechecker/mod.rs:6926 declares each auto-derive built-in + // src/typechecker/mod.rs:7560 declares each auto-derive built-in // trait with a single method whose name is the lowercase of the // trait name (Display -> display, Compare -> compare, // Equal -> equal, Hash -> hash). The mapping is intentionally @@ -321,7 +321,7 @@ fn lsp_auto_derive_method_names_match_typechecker_trait_names() { typechecker-derived expected set {expected_lsp_methods:?}. \ The mapping is lowercase-of-trait-name (Equal -> equal, \ Hash -> hash, Display -> display) per `builtin_trait_decls` \ - at src/typechecker/mod.rs:6926. Either the LSP advertises a \ + at src/typechecker/mod.rs:7560. Either the LSP advertises a \ method the typechecker doesn't register (phantom completion \ risk) or it omits one (missing completion). Update both \ sides AND this test in lock-step." diff --git a/tests/round95_interp_display_runtime_tests.rs b/tests/round95_interp_display_runtime_tests.rs index f44b2b43..301bf20b 100644 --- a/tests/round95_interp_display_runtime_tests.rs +++ b/tests/round95_interp_display_runtime_tests.rs @@ -25,18 +25,33 @@ //! //! ## Parity lock //! -//! The runtime-rejected set (function-shaped values, `Channel`) is exactly -//! the *concrete* set the typechecker's interpolation Display gate rejects, -//! and the Display-able set (Int/String/List/record) is exactly what it +//! The runtime-rejected set is exactly the *concrete* set the +//! typechecker's interpolation Display gate rejects — every first-class +//! value whose canonical type name is absent from the Display +//! `trait_impl_set`: function-shaped values (`Fn`), `Channel`, `Handle` +//! (task handles), `TcpListener` / `TcpStream` (opaque network resources +//! left explicitly unprintable, src/typechecker/mod.rs ~7878), and the +//! reflective descriptor values (`TypeDescriptor` / `PrimitiveDescriptor`). +//! The Display-able set (Int/String/List/record) is exactly what it //! accepts. Both halves are asserted here so the runtime and compile-time //! layers cannot drift apart. +//! +//! Round-95 follow-up: the original gate covered only `Fn` + `Channel` +//! and the prose falsely claimed those were "the only first-class values" +//! the Display gate rejects yet can flow through an unbounded type +//! variable. `Handle` (e.g. `task.spawn`) and the Tcp resources reproduce +//! the same silent-`` / `` rendering; the gate +//! now rejects the full set, sourced from the single VM-side oracle +//! `Vm::value_implements_display` so the runtime layer cannot drift. use silt::compiler::Compiler; use silt::lexer::Lexer; use silt::parser::Parser; -use silt::value::Value; +use silt::scheduler::test_support::InProcessRunner; +use silt::value::{TaskHandle, Value}; use silt::vm::Vm; use std::sync::Arc; +use std::time::Duration; // ── Runtime harness (typecheck is intentionally non-fatal, matching the // other runtime regression suites: the polymorphic programs typecheck @@ -114,6 +129,103 @@ fn main() { ); } +// ── Round-95 follow-up: the gate was incomplete. Handle / TcpListener / +// TcpStream are no-Display first-class types that ALSO flow through an +// unbounded type variable and pre-fix rendered a debug string silently +// (`` / ``) and exited 0. These exercise the +// full scheduler runtime via InProcessRunner because `task.spawn` and +// `tcp.listen` need the real runtime, not a bare `Vm::run`. ─────────── + +#[test] +fn polymorphic_interp_of_handle_errors_at_runtime() { + // `task.spawn` yields a `Handle`. Interpolating it through the + // polymorphic `show` must error at the execution site rather than + // silently rendering `val=` and exiting 0 (the round-95 + // hole this follow-up closes — the original repro in the finding). + let src = r#" +import task +fn show(x: a) -> String { "val={x}" } +fn main() { + let h = task.spawn(fn() { 42 }) + println(show(h)) +} +"#; + let outcome = InProcessRunner::new(src) + .with_budget(Duration::from_secs(10)) + .run_trial(); + let err = outcome + .error_message + .expect("expected a runtime Display error, got clean exit"); + assert!( + err.contains("does not implement Display") && err.contains("'Handle'"), + "expected a Display runtime error naming Handle, got: {err}" + ); +} + +#[test] +fn polymorphic_interp_of_tcp_listener_errors_at_runtime() { + // `tcp.listen` on an ephemeral local port yields a `TcpListener` + // (an opaque resource left explicitly unprintable). Interpolating it + // through the polymorphic `show` must error rather than silently + // rendering `val=`. + let src = r#" +import tcp +fn show(x: a) -> String { "val={x}" } +fn main() { + match tcp.listen("127.0.0.1:0") { + Ok(l) -> println(show(l)) + Err(e) -> println("listen failed") + } +} +"#; + let outcome = InProcessRunner::new(src) + .with_budget(Duration::from_secs(10)) + .run_trial(); + let err = outcome + .error_message + .expect("expected a runtime Display error, got clean exit"); + assert!( + err.contains("does not implement Display") && err.contains("'TcpListener'"), + "expected a Display runtime error naming TcpListener, got: {err}" + ); +} + +// ── Oracle lock: the VM-side `value_implements_display` predicate is the +// single source of truth for the runtime gate. Assert it rejects EVERY +// no-Display first-class value (so the gate and the predicate cannot +// drift) and accepts the printable ones. This catches a future variant +// being added to the rejected set in the typechecker but not here. ──── + +#[test] +fn value_implements_display_predicate_covers_every_no_display_value() { + // Rejected: function-shaped, Channel, Handle, Tcp resources, + // descriptors. (Tcp stream/listener constructed directly is awkward — + // they hold live sockets — so the runtime tests above cover the live + // path; here we cover the cheaply-constructible no-Display values.) + let handle = Value::Handle(Arc::new(TaskHandle::new(0))); + let type_desc = Value::TypeDescriptor("Int".to_string()); + let prim_desc = Value::PrimitiveDescriptor("Int".to_string()); + for v in [&handle, &type_desc, &prim_desc] { + assert!( + !Vm::value_implements_display(v), + "no-Display value must be rejected by the predicate: {v:?}" + ); + } + // Accepted: the printable built-ins still pass. + for v in [ + Value::Int(1), + Value::String("x".into()), + Value::Bool(true), + Value::List(Arc::new(vec![Value::Int(1)])), + Value::Unit, + ] { + assert!( + Vm::value_implements_display(&v), + "Display-able value must be accepted by the predicate: {v:?}" + ); + } +} + // ── Runtime half: Display-able values still render correctly ──────────── #[test] diff --git a/tests/round96_eq_fn_runtime_tests.rs b/tests/round96_eq_fn_runtime_tests.rs new file mode 100644 index 00000000..0b13411b --- /dev/null +++ b/tests/round96_eq_fn_runtime_tests.rs @@ -0,0 +1,156 @@ +//! Round 96: polymorphic `==` / `!=` on function-shaped values must error +//! at runtime, mirroring the concrete-operand compile error. +//! +//! ## Background +//! +//! silt infers trait bounds from body usage and does NOT statically enforce +//! them on polymorphic templates: `pending_numeric_checks` +//! (src/typechecker/inference.rs) skips operands whose type is still a +//! `Var`, on the documented promise that "the VM catches it at runtime with +//! a clean operator-domain diagnostic." That promise was honored for +//! ordering (`compare()`'s catch-all in src/vm/arithmetic.rs errors on +//! function-shaped values) and for string-interpolation Display (the +//! `Op::DisplayValue` gate), but NOT for equality. +//! +//! Pre-fix, a polymorphic `fn eq(x: a, y: a) -> Bool { x == y }` called +//! with two distinct functions silently returned `false`: `Op::Eq` only +//! ran `check_same_type` (which checks the value discriminant) and then +//! `language_eq`, whose catch-all delegates to `PartialEq for Value`, which +//! does `Arc::ptr_eq` on closures and name-equality on builtins +//! (src/value.rs). The concrete form `f == g` is a compile error +//! (`is_valid_compare_operand` rejects `Type::Fun` for equality via its +//! `_ => false` arm), so the runtime path was the laundering bypass. +//! +//! The fix gates `Op::Eq` / `Op::Neq` against function-shaped operands and +//! errors with a Display-style message ("type 'Fn' does not implement +//! Equal"), matching how ordering and interpolation already fail at the +//! execution site. +//! +//! ## NOT rejected +//! +//! Channel / Handle / TcpListener / TcpStream are intentionally equatable +//! by identity at runtime and the typechecker accepts them +//! (`is_valid_compare_operand` accepts `Type::Channel` and every +//! `Type::Generic(..)`). The gate must leave those alone — covered below by +//! `polymorphic_eq_of_channels_still_returns_bool`. + +use silt::compiler::Compiler; +use silt::lexer::Lexer; +use silt::parser::Parser; +use silt::value::Value; +use silt::vm::Vm; +use std::sync::Arc; + +// ── Runtime harness: typecheck is intentionally non-fatal (the polymorphic +// program typechecks clean and the behavior under test is at the VM +// execution site, matching tests/round95_interp_display_runtime_tests.rs). +// The program-under-test returns from `main`; we run it and inspect the +// VM result / error directly. ────────────────────────────────────────── + +fn run(input: &str) -> Result { + let tokens = Lexer::new(input).tokenize().expect("lexer error"); + let mut program = Parser::new(tokens).parse_program().expect("parse error"); + let _ = silt::typechecker::check(&mut program); + let mut compiler = Compiler::new(); + let functions = compiler.compile_program(&program).expect("compile error"); + let script = Arc::new(functions.into_iter().next().unwrap()); + let mut vm = Vm::new(); + vm.run(script).map_err(|e| e.message) +} + +/// Collect typechecker diagnostics that mention the equality operator +/// domain gate, for the concrete-operand parity half. +fn eq_typecheck_errors(input: &str) -> Vec { + let tokens = Lexer::new(input).tokenize().expect("lexer error"); + let mut program = Parser::new(tokens).parse_program().expect("parse error"); + silt::typechecker::check(&mut program) + .into_iter() + .map(|e| e.message) + .filter(|m| m.contains("requires a comparable type") || m.contains("does not implement")) + .collect() +} + +// ── Runtime half: the polymorphic bypass now errors at the exec site ────── + +#[test] +fn polymorphic_eq_of_fns_errors_at_runtime() { + // The finding's exact repro. Pre-fix this printed `false` and exited 0. + let src = r#" +fn eq(x: a, y: a) -> Bool { x == y } +fn f() { 1 } +fn g() { 2 } +fn main() { println(eq(f, g)) } +"#; + let err = run(src).expect_err("expected a runtime Equal error, got a clean result"); + assert!( + err.contains("does not implement Equal") && err.contains("'Fn'"), + "expected a runtime Equal error naming Fn, got: {err}" + ); +} + +#[test] +fn polymorphic_neq_of_fns_errors_at_runtime() { + // `!=` shares the bypass and must be gated identically. + let src = r#" +fn neq(x: a, y: a) -> Bool { x != y } +fn f() { 1 } +fn main() { println(neq(f, f)) } +"#; + let err = run(src).expect_err("expected a runtime Equal error, got a clean result"); + assert!( + err.contains("does not implement Equal") && err.contains("'Fn'"), + "expected a runtime Equal error naming Fn for !=, got: {err}" + ); +} + +// ── Parity half: the concrete form is a compile error ───────────────────── + +#[test] +fn concrete_eq_of_fns_is_a_compile_error() { + let src = r#" +fn f() { 1 } +fn g() { 2 } +fn main() { println(f == g) } +"#; + let errs = eq_typecheck_errors(src); + assert!( + !errs.is_empty(), + "concrete `f == g` must be a compile-time error (got none)" + ); +} + +// ── Non-regression: equatable-by-identity values must NOT be gated ──────── + +#[test] +fn polymorphic_eq_of_channels_still_returns_bool() { + // Channels are equatable by identity at runtime and accepted by the + // typechecker. The gate must leave them alone — comparing a channel to + // itself yields `true`, not a runtime error. + let src = r#" +import channel +fn eq(x: a, y: a) -> Bool { x == y } +fn main() -> Bool { + let c = channel.new() + eq(c, c) +} +"#; + match run(src) { + Ok(Value::Bool(b)) => assert!(b, "a channel must equal itself"), + Ok(other) => panic!("expected Bool, got {other:?}"), + Err(e) => panic!("channel equality must not error, got: {e}"), + } +} + +#[test] +fn polymorphic_eq_of_ints_still_works() { + // The accepted set is untouched: ordinary equality still computes. + let src = r#" +fn eq(x: a, y: a) -> Bool { x == y } +fn main() -> Bool { eq(3, 3) } +"#; + match run(src) { + Ok(Value::Bool(b)) => assert!(b, "3 == 3 must be true"), + Ok(other) => panic!("expected Bool, got {other:?}"), + Err(e) => panic!("int equality must not error, got: {e}"), + } +} diff --git a/tests/round96_help_watch_caveat_parity_tests.rs b/tests/round96_help_watch_caveat_parity_tests.rs new file mode 100644 index 00000000..4e3ef41b --- /dev/null +++ b/tests/round96_help_watch_caveat_parity_tests.rs @@ -0,0 +1,140 @@ +//! Regression test for round-96 GAP — `silt --help` watch-caveat parity. +//! +//! On a no-`watch` build the `run` row appends a " [--watch requires +//! feature: watch]" caveat to its description (so the user is warned BEFORE +//! invoking a `--watch` flag that the build can't honor), but the `check`, +//! `disasm`, and `test` rows historically advertised a bare `[--watch]` in +//! their signatures with NO such caveat. A user on a no-watch build would +//! see `[--watch]` for those three subcommands, invoke e.g. `silt check +//! --watch f.silt`, and hit the no-watch gate in `src/cli/watch.rs` that +//! exits 1 with "The 'watch' feature is not enabled." +//! +//! The fix routes every watch-capable row's description through a single +//! `watch_caveat(..)` closure that appends the caveat when `!cfg!(feature = +//! "watch")`. This is a source-grep lock (the `cli` module is binary-only +//! and not reachable from an integration test, and the test binary is built +//! with the default `watch` feature, so we cannot spawn a no-watch binary +//! cheaply). We assert the structural invariant directly on the source: +//! every `--help` usage row whose signature literal contains `[--watch]` +//! must derive its description from the `watch_caveat` gate rather than a +//! bare string literal. + +const HELP_RS: &str = include_str!("../src/cli/help.rs"); + +/// Extract the body of `usage_text()` — from its `fn` line down to the +/// matching close of the function — so we only inspect the help rows and +/// not the (unrelated) `*_usage_banner()` helpers below it. +fn usage_text_body() -> &'static str { + let start = HELP_RS + .find("pub(crate) fn usage_text() -> String {") + .expect("help.rs must define usage_text()"); + let tail = &HELP_RS[start..]; + // The banner helpers begin after usage_text(); stop at the first + // `pub(crate) const GLOBAL_FLAGS` which immediately follows the fn. + let end = tail + .find("pub(crate) const GLOBAL_FLAGS") + .expect("help.rs must keep GLOBAL_FLAGS after usage_text()"); + &tail[..end] +} + +#[test] +fn watch_caveat_helper_exists_and_gates_on_feature() { + let body = usage_text_body(); + assert!( + body.contains("watch_caveat"), + "usage_text() must funnel watch-capable descriptions through a \ + `watch_caveat` helper so the caveat is applied uniformly" + ); + assert!( + body.contains("!cfg!(feature = \"watch\")"), + "the watch caveat must be gated on `!cfg!(feature = \"watch\")`" + ); + assert!( + body.contains("[--watch requires feature: watch]"), + "the canonical caveat string must be present in usage_text()" + ); +} + +#[test] +fn every_watch_row_routes_description_through_caveat() { + let body = usage_text_body(); + + // Each usage row is rendered by `line(, )`. Walk the row + // arguments and, for any row whose signature advertises `[--watch]`, + // assert the description is NOT a bare string literal — it must be a + // `&` reference (the var flows through `watch_caveat`). Pre-fix + // the check/disasm/test rows passed bare `"..."` literals next to a + // `[--watch]` signature, which this test rejects. + let mut watch_rows = 0usize; + let mut search_from = 0usize; + while let Some(rel) = body[search_from..].find("&line(") { + let open = search_from + rel + "&line(".len(); + // Find the matching close paren for this call. + let mut depth = 1usize; + let mut i = open; + let bytes = body.as_bytes(); + while i < bytes.len() && depth > 0 { + match bytes[i] { + b'(' => depth += 1, + b')' => depth -= 1, + _ => {} + } + i += 1; + } + let call = &body[open..i - 1]; + search_from = i; + + // A row is watch-capable if its signature argument advertises + // `[--watch]` — either as an inline literal (run/check/disasm) or + // via `test_usage_banner()`, whose returned literal carries it. + let watch_capable = call.contains("[--watch]") || call.contains("test_usage_banner()"); + if !watch_capable { + continue; + } + watch_rows += 1; + + // Split signature argument from description argument at the + // top-level comma. The signature may be a literal or a + // `*_usage_banner()` call; the description is the second arg. + let comma = top_level_comma(call) + .unwrap_or_else(|| panic!("watch row has no arg separator: {call:?}")); + let desc_arg = call[comma + 1..].trim().trim_end_matches(',').trim(); + + assert!( + !desc_arg.starts_with('"'), + "watch-capable usage row passes a bare string-literal \ + description {desc_arg:?} — on a no-watch build this advertises \ + `[--watch]` with no \"requires feature: watch\" caveat. Route \ + it through `watch_caveat(..)` like the `run` row. Call: {call:?}" + ); + assert!( + desc_arg.starts_with('&') && desc_arg.contains("_desc"), + "watch-capable usage row description {desc_arg:?} should be a \ + `&_desc` reference flowing through `watch_caveat`. \ + Call: {call:?}" + ); + } + + // Sanity: run, check, test, disasm — four watch-capable rows. + assert_eq!( + watch_rows, 4, + "expected exactly 4 `[--watch]` usage rows (run/check/test/disasm), \ + found {watch_rows}" + ); +} + +/// Index of the first comma at paren-depth 0 within `s`, or `None`. +fn top_level_comma(s: &str) -> Option { + let mut depth = 0i32; + let mut in_str = false; + for (i, c) in s.char_indices() { + match c { + '"' => in_str = !in_str, + '(' | '[' if !in_str => depth += 1, + ')' | ']' if !in_str => depth -= 1, + ',' if !in_str && depth == 0 => return Some(i), + _ => {} + } + } + None +} diff --git a/tests/source_line_citation_lock_tests.rs b/tests/source_line_citation_lock_tests.rs new file mode 100644 index 00000000..f9fc35c2 --- /dev/null +++ b/tests/source_line_citation_lock_tests.rs @@ -0,0 +1,149 @@ +//! Citation-lock for precise `file.rs:NNN` cross-references in code +//! comments. +//! +//! Round-96 GAP fix: ~8 of the repo's 46 precise source-line citations +//! had silently drifted to the wrong line as the cited files grew. A +//! stale citation is worse than none — it actively misleads the next +//! reader to a line that has nothing to do with the named item. +//! +//! This test pins each corrected `(citing_file, cited_file, line, +//! expected_token)` tuple: it reads the cited file's 1-based line and +//! asserts it contains the expected token. When future edits shift the +//! cited line, THIS test reds (pointing at the exact citation to +//! re-aim) instead of the citation silently re-drifting. +//! +//! To extend: when you add a new precise `foo.rs:NNN` citation in a +//! comment, add a row here so it stays honest. + +use std::path::Path; + +/// `(label, cited_file_relative_to_crate_root, line_1based, expected_substring)` +/// +/// `label` names the citing site for a readable failure message. +const CITATIONS: &[(&str, &str, usize, &str)] = &[ + // conversions.rs:18 — "lexer increments span.col once per codepoint" + ( + "conversions.rs -> lexer span.col increment", + "src/lexer.rs", + 311, + "self.col += 1", + ), + // completion.rs:418 — "trait method names sourced from builtin_trait_decls" + ( + "completion.rs -> builtin_trait_decls def", + "src/typechecker/mod.rs", + 7560, + "fn builtin_trait_decls", + ), + // vm/runtime.rs:450 — "Rust 1.80+ thread-local env SAFETY note" + ( + "vm/runtime.rs -> scheduler thread-local SAFETY", + "src/scheduler.rs", + 1830, + "Rust 1.80+", + ), + // parser.rs:3709 — "lexer rejects i64::MIN+1 literal at lex time" + ( + "parser.rs -> lexer i64 literal too large", + "src/lexer.rs", + 627, + "number literal too large", + ), + // vm/execute.rs:1344 / vm/tests.rs:602 — And short-circuit JumpIfFalse + ( + "execute.rs/tests.rs -> And short-circuit JumpIfFalse emit", + "src/compiler/mod.rs", + 2326, + "Op::JumpIfFalse", + ), + // vm/execute.rs:1362 / vm/tests.rs:622 — Or short-circuit JumpIfTrue + ( + "execute.rs/tests.rs -> Or short-circuit JumpIfTrue emit", + "src/compiler/mod.rs", + 2337, + "Op::JumpIfTrue", + ), + // vm/execute.rs:1346 — `BinOp::And | BinOp::Or => unreachable!()` + ( + "execute.rs -> And/Or unreachable guard", + "src/compiler/mod.rs", + 2358, + "BinOp::And | BinOp::Or => unreachable!()", + ), + // workspace.rs:284 — "FieldAccess.span is the receiver span" construction + ( + "workspace.rs -> FieldAccess construction", + "src/parser.rs", + 2598, + "ExprKind::FieldAccess", + ), + // duplicate_module_not_imported_tests.rs:4 — typechecker "is not imported" + ( + "duplicate_module test -> typechecker 'is not imported'", + "src/typechecker/inference.rs", + 2909, + "is not imported", + ), + // cli/pipeline.rs:454 — compiler "is not imported" diagnostics (3 sites) + ( + "pipeline.rs -> compiler 'is not imported' #1", + "src/compiler/mod.rs", + 2561, + "is not imported", + ), + ( + "pipeline.rs -> compiler 'is not imported' #2", + "src/compiler/mod.rs", + 2657, + "is not imported", + ), + ( + "pipeline.rs -> compiler 'is not imported' #3", + "src/compiler/mod.rs", + 3476, + "is not imported", + ), + // compiler/mod.rs:441 — typechecker round-58 prefix-mirror logic + ( + "compiler/mod.rs -> typechecker round-58 prefix-mirror", + "src/typechecker/mod.rs", + 2941, + "round 58", + ), +]; + +#[test] +fn precise_source_line_citations_resolve() { + let crate_root = Path::new(env!("CARGO_MANIFEST_DIR")); + let mut failures = Vec::new(); + + for &(label, rel, line_1based, expected) in CITATIONS { + let path = crate_root.join(rel); + let src = match std::fs::read_to_string(&path) { + Ok(s) => s, + Err(e) => { + failures.push(format!("[{label}] cannot read {rel}: {e}")); + continue; + } + }; + let line = src.lines().nth(line_1based - 1); + match line { + Some(text) if text.contains(expected) => {} + Some(text) => failures.push(format!( + "[{label}] {rel}:{line_1based} no longer contains {expected:?}\n actual: {}", + text.trim() + )), + None => failures.push(format!( + "[{label}] {rel}:{line_1based} is past end of file ({} lines)", + src.lines().count() + )), + } + } + + assert!( + failures.is_empty(), + "{} stale source-line citation(s) — re-aim the comment(s) and update this lock:\n {}", + failures.len(), + failures.join("\n ") + ); +} diff --git a/tests/toml_option_none_roundtrip_tests.rs b/tests/toml_option_none_roundtrip_tests.rs index ace5b2f0..a5c0bad5 100644 --- a/tests/toml_option_none_roundtrip_tests.rs +++ b/tests/toml_option_none_roundtrip_tests.rs @@ -154,8 +154,5 @@ fn main() { } } "#); - assert_eq!( - result, - Value::String("alice|age=None|nick=bob".into()) - ); + assert_eq!(result, Value::String("alice|age=None|nick=bob".into())); } diff --git a/tests/typechecker_citation_resolve_tests.rs b/tests/typechecker_citation_resolve_tests.rs new file mode 100644 index 00000000..9e3a3d3f --- /dev/null +++ b/tests/typechecker_citation_resolve_tests.rs @@ -0,0 +1,132 @@ +//! Citation-resolve lock for `src/typechecker/mod.rs:` references +//! that live in *other* files' doc-comments. +//! +//! Doc-comments in `src/typechecker/inference.rs` and `src/vm/dispatch.rs` +//! point at specific lines in `src/typechecker/mod.rs` to explain why a +//! dispatch arm is reached / how a canonical name is produced. Those line +//! numbers drift silently whenever `mod.rs` is edited, leaving a comment +//! that cites a function that does not exist (round-95-follow-up finding: +//! the comment named `type_name_for_method_dispatch`, which never existed +//! — the real mapper is `type_name_for_impl`) or that lands on unrelated +//! code (the `register_auto_derived_impls_for` cite pointed at an +//! arity-check block; the List/Unit Compare cites pointed at `EffectSet` +//! comment lines). +//! +//! This test pins each citation to the *named item* the prose claims lives +//! there. If `mod.rs` is reorganised so a cited line no longer contains the +//! expected item, this test fails and the comment must be re-synced. +//! +//! Mirrors the spirit of the round-94 dead-arm prose grep-lock +//! (`tests/auto_derive_dead_arm_proof_tests.rs::dead_arm_prose_does_not_regress`), +//! but instead of forbidding a stale phrase it *resolves* each line cite. + +use std::collections::BTreeSet; + +const MOD_RS: &str = include_str!("../src/typechecker/mod.rs"); +const INFERENCE_RS: &str = include_str!("../src/typechecker/inference.rs"); +const DISPATCH_RS: &str = include_str!("../src/vm/dispatch.rs"); + +/// Return the 1-based line `n`'s text from `src`, or panic with context. +fn line(src: &str, n: usize) -> &str { + src.lines() + .nth(n - 1) + .unwrap_or_else(|| panic!("src/typechecker/mod.rs has no line {n}")) +} + +/// Collect every `src/typechecker/mod.rs:` line number cited in `src`. +fn cited_mod_lines(src: &str) -> BTreeSet { + const NEEDLE: &str = "src/typechecker/mod.rs:"; + let mut out = BTreeSet::new(); + let mut rest = src; + while let Some(pos) = rest.find(NEEDLE) { + let after = &rest[pos + NEEDLE.len()..]; + let digits: String = after.chars().take_while(|c| c.is_ascii_digit()).collect(); + if let Ok(n) = digits.parse::() { + out.insert(n); + } + rest = &after[digits.len().max(1)..]; + } + out +} + +/// Assert the line cited as `n` in `mod.rs` contains `expected`, the named +/// item the citing prose claims lives there. +fn assert_cite(n: usize, expected: &str, citing_file: &str) { + let text = line(MOD_RS, n); + assert!( + text.contains(expected), + "stale citation: a comment in {citing_file} cites \ + `src/typechecker/mod.rs:{n}` for `{expected}`, but that line reads:\n {text}\n\ + The comment's line number has drifted — re-sync it (find the line \ + actually containing `{expected}`)." + ); +} + +/// Every `src/typechecker/mod.rs:` cite in `inference.rs`/`dispatch.rs` +/// must (a) be one of the cites we have an expectation for, and (b) land on +/// a line containing the named item the prose attributes to it. +/// +/// The expectation set is closed: if a NEW `mod.rs:` cite is added to +/// either file and not registered here, the "unexpected citation" guard +/// below fails, forcing the author to anchor it to a named item too. +#[test] +fn mod_rs_citations_in_inference_and_dispatch_resolve() { + // (line, named-item-on-that-line) the prose claims. The negative + // function `type_name_for_method_dispatch` does not appear here on + // purpose — it never existed; the real mapper is `type_name_for_impl`. + let expectations: &[(usize, &str)] = &[ + // inference.rs primitive-dispatch comment block. + (7896, "fn register_auto_derived_impls_for"), + (2045, r#"Type::Channel(_) => Some(intern("Channel"))"#), + (2056, r#"Type::Fun(_, _) => Some(intern("Fn"))"#), + // dispatch.rs compare-arm comment block. + ( + 7796, + r#"register_auto_derived_impls_for(checker, &["List"]"#, + ), + (7793, r#""Unit""#), + ]; + + let mut expected_lines: BTreeSet = BTreeSet::new(); + for &(n, item) in expectations { + expected_lines.insert(n); + // Pick the citing file for a precise failure message. + let citing = if cited_mod_lines(INFERENCE_RS).contains(&n) { + "src/typechecker/inference.rs" + } else { + "src/vm/dispatch.rs" + }; + assert_cite(n, item, citing); + } + + // Closed-world guard: no un-anchored cite may slip in. + let mut actual: BTreeSet = cited_mod_lines(INFERENCE_RS); + actual.extend(cited_mod_lines(DISPATCH_RS)); + let unexpected: Vec = actual.difference(&expected_lines).copied().collect(); + assert!( + unexpected.is_empty(), + "new `src/typechecker/mod.rs:` citation(s) {unexpected:?} added to \ + inference.rs/dispatch.rs without registering the named item they \ + point at. Add an (line, item) entry to `expectations` so the cite \ + is pinned and cannot silently drift." + ); + + // Sanity: the dead reference must stay dead. If anyone reintroduces the + // phantom `type_name_for_method_dispatch`, fail loudly. + assert!( + !INFERENCE_RS.contains("type_name_for_method_dispatch"), + "the phantom function `type_name_for_method_dispatch` reappeared in \ + src/typechecker/inference.rs — it does not exist; the real canonical \ + name mapper is `type_name_for_impl` (src/typechecker/mod.rs:2027)." + ); + assert!( + !DISPATCH_RS.contains("type_name_for_method_dispatch"), + "the phantom function `type_name_for_method_dispatch` reappeared in \ + src/vm/dispatch.rs." + ); + // And it must actually exist under its real name. + assert!( + MOD_RS.contains("fn type_name_for_impl"), + "expected `fn type_name_for_impl` in src/typechecker/mod.rs" + ); +} diff --git a/tests/watch_double_dash_separator_tests.rs b/tests/watch_double_dash_separator_tests.rs new file mode 100644 index 00000000..8bcc9303 --- /dev/null +++ b/tests/watch_double_dash_separator_tests.rs @@ -0,0 +1,162 @@ +//! Audit regression: the `--watch` interceptor must respect the `--` +//! program-args separator. +//! +//! Pre-fix (`src/cli/watch.rs`): `maybe_handle_watch` scanned the ENTIRE +//! arg vector for `--watch` / `-w` with no awareness of `--`. So +//! `silt run prog.silt -- -w` (where `-w` is the PROGRAM's own flag, +//! forwarded via `io.args()`) matched, watch mode was entered, and +//! `handle_watch` then filtered out EVERY `-w` — re-invoking the +//! subprocess as `silt run prog.silt --` with empty program args. Net +//! effect: the program (a) never saw its `-w` and (b) was silently run +//! under the file-watch loop instead of executing exactly once. +//! +//! Post-fix: only the CLI-flag region BEFORE the first standalone `--` +//! is scanned/filtered; the post-`--` tail is forwarded verbatim. So +//! `silt run prog.silt -- -w` runs the program exactly once and +//! `io.args()` returns `["-w"]`. +//! +//! Each test shells out to the real `silt` binary so the full argv +//! parsing + watch-interception path is exercised end-to-end. Because a +//! correct (post-fix) run executes ONCE and exits, `.output()` returns +//! promptly. A regressed build would enter the watch loop and `.output()` +//! would block forever, so each invocation is run on a worker thread with +//! a bounded join timeout — a timeout is treated as a test failure (the +//! process is still watching). + +use std::fs; +use std::path::PathBuf; +use std::process::Command; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::sync::mpsc; +use std::thread; +use std::time::Duration; + +static COUNTER: AtomicUsize = AtomicUsize::new(0); + +fn fresh_dir(prefix: &str) -> PathBuf { + let n = COUNTER.fetch_add(1, Ordering::SeqCst); + let pid = std::process::id(); + let dir = std::env::temp_dir().join(format!("silt_watch_dd_{pid}_{prefix}_{n}")); + let _ = fs::remove_dir_all(&dir); + fs::create_dir_all(&dir).unwrap(); + dir +} + +/// Write a script that prints each `io.args()` entry on its own line. +fn write_args_dumper(path: &std::path::Path) { + fs::write( + path, + "import io\n\ + import list\n\ + fn main() {\n \ + io.args() |> list.each { arg -> println(arg) }\n\ + }\n", + ) + .unwrap(); +} + +/// Run `silt ` with a hard wall-clock cap. Returns the captured +/// (stdout, success) on completion. Panics if the process does not exit +/// within `timeout` — which is exactly the regressed behavior (entered +/// the watch loop and is sitting there forever instead of running once). +fn run_with_timeout(args: Vec, timeout: Duration) -> (String, bool) { + let (tx, rx) = mpsc::channel(); + let label = args.join(" "); + thread::spawn(move || { + let out = Command::new(env!("CARGO_BIN_EXE_silt")) + .args(&args) + .output() + .expect("failed to spawn silt"); + let _ = tx.send(( + String::from_utf8_lossy(&out.stdout).into_owned(), + out.status.success(), + )); + }); + match rx.recv_timeout(timeout) { + Ok(res) => res, + Err(_) => panic!( + "`silt {label}` did not exit within {timeout:?} — it entered the watch loop \ + instead of running the program exactly once (the `--watch` interceptor \ + hijacked a post-`--` program arg)" + ), + } +} + +#[test] +fn run_dash_w_after_separator_runs_once_and_forwards_arg() { + let dir = fresh_dir("run_w"); + let script = dir.join("dump_args.silt"); + write_args_dumper(&script); + + let (stdout, success) = run_with_timeout( + vec![ + "run".into(), + script.to_string_lossy().into_owned(), + "--".into(), + "-w".into(), + ], + Duration::from_secs(15), + ); + + assert!( + success, + "`silt run -- -w` must run the program once and exit 0; got stdout: {stdout}" + ); + // The `-w` must reach the program via io.args(), not be consumed as a + // silt CLI flag. + assert!( + stdout.lines().any(|l| l.trim() == "-w"), + "program must observe '-w' in io.args(); got stdout: {stdout}" + ); +} + +#[test] +fn run_long_watch_after_separator_runs_once_and_forwards_arg() { + let dir = fresh_dir("run_watch"); + let script = dir.join("dump_args.silt"); + write_args_dumper(&script); + + let (stdout, success) = run_with_timeout( + vec![ + "run".into(), + script.to_string_lossy().into_owned(), + "--".into(), + "--watch".into(), + ], + Duration::from_secs(15), + ); + + assert!( + success, + "`silt run -- --watch` must run the program once and exit 0; got stdout: {stdout}" + ); + assert!( + stdout.lines().any(|l| l.trim() == "--watch"), + "program must observe '--watch' in io.args(); got stdout: {stdout}" + ); +} + +#[test] +fn check_dash_w_after_separator_does_one_shot_check() { + // `silt check -- -w` must one-shot check (exit promptly), not + // enter the watch loop. `check` doesn't execute, so we only assert it + // returns within the timeout and succeeds on a valid program. + let dir = fresh_dir("check_w"); + let script = dir.join("ok.silt"); + fs::write(&script, "fn main() { print(\"hi\") }\n").unwrap(); + + let (_stdout, success) = run_with_timeout( + vec![ + "check".into(), + script.to_string_lossy().into_owned(), + "--".into(), + "-w".into(), + ], + Duration::from_secs(15), + ); + + assert!( + success, + "`silt check -- -w` must one-shot check and exit 0, not enter watch mode" + ); +}