diff --git a/ISSUES.md b/ISSUES.md index 86f0bd3..caf3228 100644 --- a/ISSUES.md +++ b/ISSUES.md @@ -211,6 +211,51 @@ Resolved entries are removed (not kept around as a changelog). Look at `git log` **Summary:** The runtime staticlib only uses `std::alloc`, `std::process::abort()`, and `eprintln!`, yet linking against precompiled `std` bundles objects with `_Unwind_*` symbol references. This forces the linker to pass `-lunwind` on Linux (workaround in `src/linker.rs`). Migrating to `#![no_std]` with `extern crate alloc` eliminates the dependency entirely. **Resolution:** Replace `std::alloc` with `alloc::alloc` (identical API). Replace `eprintln!` + `process::abort()` with `extern "C" { fn abort() -> !; }`. Add `#[panic_handler]` that aborts. Keep the `rlib` crate-type for `cargo test` via a `#[cfg(test)]` std gate. `ryu` already supports `no_std`. Benefits: smaller archive, faster link times, no hidden unwind dependency, simpler cross-compilation. +### I-045 — Loop fixed-point uses a scratch sink and re-walks the body to suppress duplicates +**Files:** `src/ownership.rs` (`analyze_while_loop`, `analyze_for_range`) +**Summary:** Both loop helpers walk the body once into a throwaway `DiagSink`, compare entry vs. post-body Moved-ness via `states_differ`, and then either replay the scratch diagnostics (converged) or re-walk the body from the merged state against the real sink (didn't converge). Diagnostic output is implicitly a function of *which iteration emitted it*, not of the converged lattice. Today this happens to work because the M8.1 patterns converge in ≤2 iterations, but the body-twice-with-discard shape couples diagnostic emission to control-flow analysis instead of running checks against a fixed-point. +**Resolution:** Refactor to a propagate-only first pass (state mutations only, no diagnostics), iterate to fixed-point, then a single check pass at the converged lattice that emits diagnostics. Removes the scratch sink entirely and makes the loop helpers symmetric with `analyze_if_stmt`. + +### I-047 — UIR `is_move` field is a pass-through +**Files:** `src/uir.rs` (`UirParam`), `src/astgen.rs`, `src/sema.rs` +**Summary:** `is_move` is threaded lexer → parser → AST → UIR → TIR. The UIR copy is never read: astgen propagates the AST flag in, sema reads it back out into `TirParam`, and no UIR pass inspects it. UIR is structural lowering with no semantic meaning, so `UirParam::is_move` is dead weight that exists only to bridge two layers it shouldn't. +**Resolution:** Drop `UirParam::is_move`. Sema can read the flag straight from the AST `FuncBody` (or via a side-channel keyed by FuncBody) when it constructs `TirParam`. Wait until any other UIR-level pass needs the flag before re-introducing it. + +### I-048 — Ownership pass looks up call conventions by name at every Call site +**Files:** `src/ownership.rs` (`visit_expr` Call arm), `src/sema.rs` +**Summary:** For every `Call` instruction, the ownership walker rebuilds a `by_name: HashMap` over all functions and indexes `params[i].is_move` to decide whether each arg should consume or borrow. The map is threaded through nine functions, and builtins need a special "no entry → all borrow" branch. Sema already knows the callee signature when it lowers the call; encoding the per-arg convention into TIR there would let ownership read it directly. +**Resolution:** Add an `arg_modes: ExtraRange` (or a per-arg `ParamMode` enum) alongside `args` in the TIR `Call` view, populated by sema. Ownership then reads `tir.call_view(r).arg_modes[i]` and the `by_name` plumbing disappears. Builtins become uniform (sema stamps `Borrow` for them). Also gives a place to put future indirect-call / fn-pointer conventions. + +### I-049 — `synthetic_param_ref` encoding is informal; model `Owner` as an enum +**Files:** `src/ownership.rs` (`synthetic_param_ref`, `Ownership::states`/`origin`) +**Summary:** Parameters live in the same `states: HashMap` as instruction refs by encoding their key as `u32::MAX - name.raw()`. Correctness depends on real per-function `TirRef`s never approaching `u32::MAX/2` and on `name.raw()` never being zero — neither asserted, both relying on convention. A future change to either `TirRef` numbering or `StringId` interning silently breaks the assumption. Cleaner shape: model owners as an explicit `enum Owner { Param(StringId), Inst(TirRef) }` and key the lattice maps on `Owner`. +**Resolution:** Introduce `Owner` and migrate `Ownership::states`/`origin`/`pending_dead_store` and `current_owner` value types over to it. Drops the `synthetic_param_ref` helper, the implicit numbering coupling, and gives a clean place to add `Owner::Borrow(...)` once real borrow expressions land. + +### I-050 — Var-arm holds UAM detection; consume sites are silent by policy +**Files:** `src/ownership.rs` (`visit_expr` Var arm, `consume_for_assignment`, `analyze_return`, Call arm) +**Summary:** Use-after-move detection only fires in the `Var` arm of `visit_expr`. The four consume sites (VarDecl, Assign, Return, move-Call) all carry comments explaining why they *don't* re-emit, and the policy works only because every consumable operand currently flows through `Var`. Three reviewers (and one bug fix during M8.1b) flagged this as the wrong altitude: any future producer pattern that bypasses `Var` (e.g., a directly-passed `Call` result that was already moved upstream) silently sidesteps the check. +**Resolution:** Invert responsibility. The consume helper becomes the single authority on UAM (it already inspects `underlying_owner` + state); the `Var` arm restricts itself to bookkeeping (origin link + dead-store clear). Pair the change with regression coverage that exercises a non-Var operand path so the new authority is observably tested. + +### I-051 — `analyze_while_loop` / `analyze_for_range` are 90% identical +**Files:** `src/ownership.rs` +**Summary:** After their distinct preludes (visit `cond` vs visit `start` + `end`), the bodies are byte-for-byte the same — entry snapshot, scratch-sink walk, `states_differ` check, optional re-walk, final `merge_two`. Two near-clones of a non-trivial fixed-point loop is exactly where divergence creeps in (only one gets a fix when behavior needs to change). +**Resolution:** Extract `analyze_loop_body(tir, pool, own, sink, by_name, body: &[TirRef])` after the caller has visited the loop's prelude. Folds with I-045 (propagate-only + check pass) — both refactors touch the same bodies. + +### I-053 — `OwnerState::Borrowed` is currently parameter-only +**Files:** `src/ownership.rs` (`OwnerState`) +**Summary:** `OwnerState::Borrowed` is set only at parameter init; no expression produces it. The two sites that read it (E0021, E0022) could equivalently look up `tir.params` for the underlying owner's source param and check `is_move`. The state is anticipating real borrow expressions (`&x`) which the spec migrated to in commit 2ccf6b6 but the compiler doesn't lower yet. +**Resolution:** Document the invariant inline ("only ever set at param init in M8.1b; transitions arrive when `&x` borrow expressions land in a future milestone"). No code change today. + +### I-054 — `parse_source` and lex error paths bypass `finalize_diags` +**Files:** `src/pipeline.rs` (`parse_source`, `display_tokens`) +**Summary:** `finalize_diags` consolidates the drain + render + `Err(CompilerError::Diagnostics(_))` shape for the sink-using stages (`lower_and_analyze`, `ir_command`). The lex error paths and `parse_source`'s parse-error branch still hand-roll the same pattern over a `Vec` they build directly (no `DiagSink`). Drift risk is real: parse/lex paths skip the `Severity::Error` filter (they assume every diag they emit is an error, which holds today but isn't enforced), and any future change to the rendering convention has to be applied in three places. +**Resolution:** Generalize `finalize_diags` to take `Vec` (or `impl IntoIterator`); have `DiagSink::into_diags()` feed the new entry point. Then the three lex/parse error paths become `finalize_diags(vec![diag], input, &name)` and the render+wrap pattern lives in exactly one place. Folds naturally with I-014's lexer-DiagSink migration. + +### I-055 — `pending_dead_store.insert` duplicated between VarDecl and Assign +**Files:** `src/ownership.rs` (`analyze_var_decl`, `analyze_assign`) +**Summary:** Both helpers register a Move-typed binding into `own.pending_dead_store` with `(name, span)` after `rebind_to_init`. The pattern was a single site before commit 20cdd02 added W0001 reassignment coverage; it is now a 2-site duplication. If W0001 ever needs different policy at one site (e.g., distinguishing fresh declaration from reassignment), the duplication will silently allow it. +**Resolution:** Extract `register_pending_dead_store(own, owner, name, span)`, or fold the registration into `rebind_to_init` (which already takes the binding) and have it accept the span/name pair. Either is a 3-line change. + --- ## Cross-References diff --git a/src/ast.rs b/src/ast.rs index f9f2c62..e45bfe7 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -77,9 +77,11 @@ impl Statement { println!("{}FunctionDef: {}", prefix, pool.str(func.name.name)); let inner = format!("{} ", prefix); for param in &func.params { + let move_prefix = if param.is_move { "move " } else { "" }; println!( - "{}├── param: {}: {}", + "{}├── param: {}{}: {}", inner, + move_prefix, pool.str(param.name.name), pool.str(param.type_annotation.name), ); @@ -255,6 +257,7 @@ pub struct FunctionDef { pub struct Param { pub name: Ident, pub type_annotation: TypeExpr, + pub is_move: bool, pub span: SimpleSpan, } diff --git a/src/astgen.rs b/src/astgen.rs index 81d2c48..cf81441 100644 --- a/src/astgen.rs +++ b/src/astgen.rs @@ -190,7 +190,8 @@ fn gen_function_def( pool, sink, ), - span: p.name.span, + is_move: p.is_move, + span: p.span, }) .collect(); diff --git a/src/diag.rs b/src/diag.rs index 47232c5..04a40f9 100644 --- a/src/diag.rs +++ b/src/diag.rs @@ -43,7 +43,6 @@ pub struct Diag { #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Severity { Error, - #[allow(dead_code)] Warning, #[allow(dead_code)] Note, @@ -97,6 +96,10 @@ pub enum DiagCode { RangeArgType, /// User attempted to declare a function or variable with a reserved builtin name. ReservedBuiltinName, + /// `move` annotation on a Copy-typed parameter (int, float, bool). + /// Accepted, but redundant — Copy types are duplicated on + /// assignment regardless. W-prefixed because it's a warning. + RedundantMove, /// A declaration's resolution requires its own resolution to be /// already complete — e.g. a chain of decls whose types depend @@ -107,6 +110,16 @@ pub enum DiagCode { /// comptime / inferred-return-type work doesn't stack-overflow. CycleInResolution, + // --- ownership (M8.1b) --- + /// Use of a value after it has been moved. + UseAfterMove, + /// Attempted to move out of a borrowed parameter. + MoveOutOfBorrowedParam, + /// Attempted to return a borrowed value (Rule 5). + ReturnBorrowedValue, + /// A Move-typed value is declared (or assigned) but never used. + DeadStore, + // --- parser --- ParseError, @@ -140,6 +153,16 @@ impl Diag { } } + pub fn warning(span: Span, code: DiagCode, message: impl Into) -> Self { + Diag { + span, + severity: Severity::Warning, + code, + message: message.into(), + notes: Vec::new(), + } + } + #[allow(dead_code)] pub fn with_note(mut self, span: Option, message: impl Into) -> Self { self.notes.push(DiagNote { @@ -148,6 +171,12 @@ impl Diag { }); self } + + /// Top-level help note (no span). Renders with a "help: " prefix + /// so callers don't need to encode the convention. + pub fn with_help(self, message: impl Into) -> Self { + self.with_note(None, format!("help: {}", message.into())) + } } /// Accumulator for diagnostics emitted by a single compilation pass. diff --git a/src/lexer.rs b/src/lexer.rs index c2ea72f..f82608f 100644 --- a/src/lexer.rs +++ b/src/lexer.rs @@ -46,6 +46,7 @@ pub enum Token { Else, Return, Mut, + Move, Struct, Enum, Match, @@ -116,6 +117,7 @@ impl fmt::Display for Token { Self::Else => write!(f, "else"), Self::Return => write!(f, "return"), Self::Mut => write!(f, "mut"), + Self::Move => write!(f, "move"), Self::Struct => write!(f, "struct"), Self::Enum => write!(f, "enum"), Self::Match => write!(f, "match"), @@ -191,6 +193,8 @@ pub(crate) enum RawToken<'a> { Return, #[token("mut")] Mut, + #[token("move")] + Move, #[token("struct")] Struct, #[token("enum")] @@ -423,6 +427,7 @@ fn intern_token(raw: RawToken<'_>, span: Span, pool: &mut InternPool) -> Result< RawToken::Else => Token::Else, RawToken::Return => Token::Return, RawToken::Mut => Token::Mut, + RawToken::Move => Token::Move, RawToken::Struct => Token::Struct, RawToken::Enum => Token::Enum, RawToken::Match => Token::Match, @@ -514,6 +519,13 @@ mod tests { assert_eq!(toks[7], Token::Match); } + #[test] + fn lex_move_keyword() { + let (toks, _) = lex_strings("move"); + assert_eq!(toks.len(), 1); + assert!(matches!(toks[0], Token::Move)); + } + #[test] fn lex_simple_identifier() { let (toks, pool) = lex_strings("foo"); diff --git a/src/main.rs b/src/main.rs index 5e1a413..cea180a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -10,6 +10,7 @@ mod errors; mod indent; mod lexer; mod linker; +mod ownership; mod parser; mod pipeline; #[allow(dead_code)] diff --git a/src/ownership.rs b/src/ownership.rs new file mode 100644 index 0000000..1fab524 --- /dev/null +++ b/src/ownership.rs @@ -0,0 +1,932 @@ +//! Ownership pass — validates move safety on per-`TirRef` lattice. +//! +//! Runs between sema and codegen. Walks each `Tir` forward, tracking +//! ownership state for every Move-typed value. Catches use-after-move, +//! moves out of borrowed parameters, and returns of borrowed values. +//! Emits diagnostics into the shared `DiagSink` — does not mutate TIR +//! and does not insert Free instructions (that lands in M8.1c). +//! +//! ## State lattice +//! +//! Per-TirRef state, not per-binding. A binding name resolves through +//! a shadow `current_owner: HashMap` to whichever +//! SSA value currently owns the underlying allocation. Anonymous owned +//! temporaries (concat results, formatter outputs) live in the same +//! `states` map with no shadow entry. +//! +//! See `docs/superpowers/specs/2026-05-20-milestone-8.1-heap-str-and-move-semantics-design.md` +//! sub-milestone 8.1b for the full algorithm. +//! +//! ## Mojo reference +//! +//! See `docs/dev/mojo_reference.md`. + +use crate::diag::{Diag, DiagCode, DiagSink}; +use crate::tir::{Span, Tir, TirData, TirRef, TirTag}; +use crate::types::{InternPool, StringId, TypeId, TypeKind}; +use std::collections::{HashMap, HashSet}; + +// ---------- Classification ---------- + +/// True for types whose values transfer ownership on `=` and must be +/// tracked through the function body. Today: `str` only. Future heap +/// types (`List[T]`, `Dict[K, V]`) will join this set. +pub(crate) fn is_move_type(ty: TypeId, pool: &InternPool) -> bool { + matches!(pool.kind(ty), TypeKind::Str) +} + +/// Predicate the ownership walk uses to decide whether a `TirRef` +/// needs a lattice slot. Currently identical to `is_move_type`, but +/// kept as its own name so the walk reads correctly when borrows +/// land and the answer becomes "move OR borrowed-of-move". +pub(crate) fn needs_tracking(ty: TypeId, pool: &InternPool) -> bool { + is_move_type(ty, pool) +} + +// ---------- Lattice ---------- + +/// Per-`TirRef` ownership state. Anything Copy-typed lives in +/// `NotTracked` for its whole lifetime (the walk skips it). Move- +/// typed values start at `Valid` on definition, transition to +/// `Borrowed` while a borrow is live, and to `Moved` once consumed. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) enum OwnerState { + NotTracked, + Valid, + Borrowed, + Moved { moved_at: Span }, +} + +/// Per-function ownership state. `states` is the lattice itself, +/// keyed by the `TirRef` that produced the value. `current_owner` +/// is a shadow map from binding name to whichever SSA value +/// currently owns the underlying allocation (so reassignment +/// reseats ownership without disturbing the producing SSA value). +/// `origin` records, for each tracked `TirRef`, the upstream value +/// it derives from (or `None` for fresh allocations) — used to walk +/// back to the root owner when diagnosing a use-after-move. +#[derive(Default, Clone)] +pub(crate) struct Ownership { + pub states: HashMap, + pub current_owner: HashMap, + pub origin: HashMap>, + /// VarDecls of Move-typed values, keyed by the underlying owner + /// `TirRef`. Cleared when the binding is read (`Var`) or consumed + /// (move/return). Whatever remains at function end is a dead + /// store, surfaced as W0001. + pub pending_dead_store: HashMap, +} + +impl Ownership { + /// Conservatively merge per-branch lattices into `self`. For each + /// tracked `TirRef`, if any branch left it `Moved` the merged + /// state is `Moved` — that way a value consumed on only one + /// branch is still treated as moved at the join point and a + /// post-`if` use trips E0020. Non-`Moved` states pick the first + /// observed branch state. `current_owner` entries from any branch + /// are preserved so reseats inside a branch survive the join. + fn merge_branches(&mut self, branches: &[&Ownership]) { + // Snapshot pre-branch (name, owner) pairs before we start + // touching `self.states`. After the per-TirRef merge below + // we revisit each pre-branch binding and recompute its state + // through whichever owner each branch ended on, so a branch + // that reseats `current_owner[name]` to a fresh TirRef still + // contributes its end-of-branch state to the merged binding. + // Without this, post-`if` reads of `name` resolve through the + // pre-branch owner whose state reflects only what happened to + // *that* TirRef, missing reseats inside branches. + let pre_branch_owners: Vec<(StringId, TirRef)> = + self.current_owner.iter().map(|(n, t)| (*n, *t)).collect(); + + // Rule: any branch Moved → Moved; otherwise first observed + // (across branches) wins. Walk each branch once and merge + // directly into `self.states` — no intermediate set of keys. + for b in branches { + for (r, s) in &b.states { + self.states + .entry(*r) + .and_modify(|cur| { + if !matches!(cur, OwnerState::Moved { .. }) + && matches!(s, OwnerState::Moved { .. }) + { + *cur = s.clone(); + } + }) + .or_insert_with(|| s.clone()); + } + } + for b in branches { + for (name, owner) in &b.current_owner { + self.current_owner.entry(*name).or_insert(*owner); + } + for (k, v) in &b.origin { + self.origin.entry(*k).or_insert(*v); + } + } + + // Binding-aware override: for each name that existed before + // the branches walked, look up where each branch left that + // binding (b.current_owner[name]), read that owner's state in + // the branch, and merge across branches with the same any- + // Moved-wins rule. Write the merged state back onto the + // pre-branch owner in `self.states` so post-merge reads of + // `name` (which still resolve via `self.current_owner[name]` + // = owner_pre) see the union of what each branch did to its + // respective end-of-branch owner. + for (name, owner_pre) in &pre_branch_owners { + let mut merged: Option = None; + for b in branches { + let owner_b = b.current_owner.get(name).copied().unwrap_or(*owner_pre); + let state_b = b + .states + .get(&owner_b) + .cloned() + .unwrap_or(OwnerState::NotTracked); + merged = Some(match (&merged, &state_b) { + (None, _) => state_b, + (Some(OwnerState::Moved { .. }), _) => merged.clone().unwrap(), + (_, OwnerState::Moved { .. }) => state_b, + (Some(_), _) => merged.clone().unwrap(), + }); + } + if let Some(state) = merged { + self.states.insert(*owner_pre, state); + } + } + + // Pending dead-store entries: a key falls into one of two + // buckets relative to the pre-branch snapshot in `self`. + // + // (1) Pre-branch entries (already in `self.pending_dead_store` + // before the branches walked): every branch started with + // the entry. If any branch cleared it, the value was + // used somewhere — drop it. Rule: intersect across + // branches. + // + // (2) Branch-local entries (introduced inside a branch by a + // VarDecl): only the introducing branch has the key. If + // that branch ended with the entry still pending, W0001 + // should still fire after the join. Rule: union across + // branches (skipping keys that were pre-branch — those + // are governed by rule (1)). + // + // Snapshot the pre-branch key set so the union step can + // distinguish branch-local keys from pre-branch keys that + // (1) may have just dropped. + let pre_branch_keys: HashSet = self.pending_dead_store.keys().copied().collect(); + self.pending_dead_store.retain(|k, _| { + branches + .iter() + .all(|b| b.pending_dead_store.contains_key(k)) + }); + for b in branches { + for (k, v) in &b.pending_dead_store { + if !pre_branch_keys.contains(k) { + self.pending_dead_store.insert(*k, *v); + } + } + } + } +} + +/// Validate move safety for every function body. Emits diagnostics +/// into `sink`. Returns nothing — codegen continues to consume the +/// unchanged `&[Tir]`. +pub fn check(tirs: &[Tir], pool: &InternPool, sink: &mut DiagSink) { + // Name-keyed lookup so call sites can read the callee's per- + // parameter `is_move` flags. Builtins (`print`, `assert`, + // `int_to_str`) are not in this map and default to borrowing. + let by_name: HashMap = tirs.iter().map(|t| (t.name, t)).collect(); + for tir in tirs { + analyze_function(tir, pool, sink, &by_name); + } +} + +fn analyze_function( + tir: &Tir, + pool: &InternPool, + sink: &mut DiagSink, + by_name: &HashMap, +) { + let mut own = Ownership::default(); + + // Initialise per-parameter state. Move-typed params start at + // `Valid` (the callee owns them); borrowed params start at + // `Borrowed`. Copy-typed params skip the lattice entirely. + for param in &tir.params { + if !needs_tracking(param.ty, pool) { + continue; + } + let synthetic = synthetic_param_ref(param.name); + let state = if param.is_move { + OwnerState::Valid + } else { + OwnerState::Borrowed + }; + own.states.insert(synthetic, state); + own.current_owner.insert(param.name, synthetic); + } + + for stmt in tir.body_stmts() { + analyze_stmt(tir, pool, &mut own, sink, by_name, stmt); + } + + // Anything left pending was declared but never read or + // consumed — emit W0001 once per surviving binding. + for (_owner, (name, span)) in own.pending_dead_store.drain() { + sink.emit(Diag::warning( + span, + DiagCode::DeadStore, + format!("value `{}` is declared but never used", pool.str(name)), + )); + } +} + +/// Build a stable synthetic [`TirRef`] for a parameter so it can +/// share the `states` map with real instruction refs. Encoded near +/// `u32::MAX` to keep it well clear of real per-function indices +/// (which start at 1 and grow with body size). `TirRef` wraps +/// `NonZeroU32`, so we clamp the lower bound to 1 to defend against +/// the edge case `name.raw() == u32::MAX` (which would otherwise +/// panic at `from_raw`). The collision-free property relies on real +/// instruction refs never reaching `u32::MAX - max_string_id`, which +/// is structurally impossible for any realistic program. +fn synthetic_param_ref(name: StringId) -> TirRef { + let raw = u32::MAX.saturating_sub(name.raw()).max(1); + TirRef::from_raw(raw) +} + +fn analyze_stmt( + tir: &Tir, + pool: &InternPool, + own: &mut Ownership, + sink: &mut DiagSink, + by_name: &HashMap, + stmt: TirRef, +) { + let inst = *tir.inst(stmt); + match inst.tag { + TirTag::VarDecl => analyze_var_decl(tir, pool, own, sink, by_name, stmt), + TirTag::Assign => analyze_assign(tir, pool, own, sink, by_name, stmt), + TirTag::Return => analyze_return(tir, pool, own, sink, by_name, stmt), + TirTag::ReturnVoid => {} + TirTag::IfStmt => analyze_if_stmt(tir, pool, own, sink, by_name, stmt), + TirTag::WhileLoop => analyze_while_loop(tir, pool, own, sink, by_name, stmt), + TirTag::ForRange => analyze_for_range(tir, pool, own, sink, by_name, stmt), + TirTag::Break | TirTag::Continue => { + // 8.1c attaches Free metadata here; 8.1b is a no-op. + } + TirTag::CompoundAssign => { + // Sema rejects compound-assign on Move-typed values today + // (str doesn't support `+=`/`-=`/etc.). Enforce the invariant + // here so a future sema relaxation that reaches the ownership + // pass without ownership-aware handling trips a debug build + // instead of silently falling through to no analysis. + // See ISSUES.md I-046. + let view = tir.compound_assign_view(stmt); + debug_assert!( + !needs_tracking(tir.inst(view.value).ty, pool), + "compound-assign on Move-typed value reached ownership pass; \ + sema should have rejected — see ISSUES.md I-046", + ); + } + TirTag::ExprStmt => { + if let TirData::UnOp(o) = inst.data { + visit_expr(tir, pool, own, sink, by_name, o); + } + } + _ => { + visit_expr(tir, pool, own, sink, by_name, stmt); + } + } +} + +/// Move-typed `VarDecl` is a consumer: the new binding takes +/// ownership of the initializer's underlying value. If the +/// initializer aliases a borrowed parameter, this is the E0021 +/// "move out of borrowed parameter" site; if the underlying owner is +/// already `Moved`, this is the E0020 "use after move" site. +fn analyze_var_decl( + tir: &Tir, + pool: &InternPool, + own: &mut Ownership, + sink: &mut DiagSink, + by_name: &HashMap, + r: TirRef, +) { + let view = tir.var_decl_view(r); + let init = view.initializer; + let init_ty = tir.inst(init).ty; + visit_expr(tir, pool, own, sink, by_name, init); + if needs_tracking(init_ty, pool) { + let span = tir.span(r); + let consumed_name = consumed_binding_name(tir, init); + consume_for_assignment(pool, own, sink, init, span, consumed_name); + rebind_to_init(own, view.name, init); + // Register the new binding as pending dead-store. The walk + // clears this entry on any later read or consumption; a + // surviving entry at function end fires W0001. Keyed by + // `init`, the same TirRef `rebind_to_init` stamped Valid. + own.pending_dead_store.insert(init, (view.name, span)); + } else { + own.current_owner.insert(view.name, init); + } +} + +/// Reassignment to a Move-typed binding. Same consumption rules as +/// `VarDecl`; the existing binding name is reseated to whichever +/// SSA value owns the new underlying allocation. +fn analyze_assign( + tir: &Tir, + pool: &InternPool, + own: &mut Ownership, + sink: &mut DiagSink, + by_name: &HashMap, + r: TirRef, +) { + let view = tir.assign_view(r); + let value_ty = tir.inst(view.value).ty; + visit_expr(tir, pool, own, sink, by_name, view.value); + if needs_tracking(value_ty, pool) { + let span = tir.span(r); + let consumed_name = consumed_binding_name(tir, view.value); + consume_for_assignment(pool, own, sink, view.value, span, consumed_name); + rebind_to_init(own, view.name, view.value); + own.pending_dead_store.insert(view.value, (view.name, span)); + } +} + +/// Move-typed `Return` is a consumer: the returned value flows out of +/// the function and the caller takes ownership. Borrowed parameters +/// cannot be returned (E0022). If the underlying owner is already +/// `Moved`, this is the E0020 "use after move" site. +fn analyze_return( + tir: &Tir, + pool: &InternPool, + own: &mut Ownership, + sink: &mut DiagSink, + by_name: &HashMap, + r: TirRef, +) { + let inst = *tir.inst(r); + let operand = match inst.data { + TirData::UnOp(o) => o, + _ => unreachable!("Return must carry TirData::UnOp"), + }; + let ty = tir.inst(operand).ty; + visit_expr(tir, pool, own, sink, by_name, operand); + if !needs_tracking(ty, pool) { + return; + } + let span = tir.span(r); + let consumed_name = consumed_binding_name(tir, operand); + consume_underlying( + pool, + own, + sink, + operand, + span, + consumed_name, + BorrowedAction::ReturnBorrowed, + ); +} + +/// Reseat `name` to point at `init` as its current owner. After a +/// consume, the source binding's underlying value is `Moved`; the +/// new binding takes a fresh, independent slot in the lattice so +/// subsequent reads of `name` resolve to a `Valid` owner instead of +/// tripping over the just-moved underlying. We do this by severing +/// `init`'s `origin` link (if any) and stamping it `Valid`. +fn rebind_to_init(own: &mut Ownership, name: StringId, init: TirRef) { + own.origin.insert(init, None); + own.states.insert(init, OwnerState::Valid); + own.current_owner.insert(name, init); +} + +/// Walk back from `init` to whichever SSA value currently owns the +/// underlying allocation. `visit_expr` is responsible for populating +/// `origin` for `Var` reads; for fresh producers (`StrConst`, +/// `StrConcat`, `Call`) `init` is itself the owner. +fn underlying_owner(own: &Ownership, init: TirRef) -> TirRef { + match own.origin.get(&init).copied() { + Some(Some(owner)) => owner, + _ => init, + } +} + +/// Which diagnostic the shared `consume_underlying` helper should +/// emit on the `Borrowed` arm. `consume_for_assignment` and +/// `analyze_return` run identical state transitions otherwise — the +/// only thing that diverges is the error code / wording / help when +/// the underlying owner is borrowed. +enum BorrowedAction { + /// E0021 — `consume_for_assignment` site (VarDecl / Assign / + /// move-typed Call argument). + MoveOutOfParam, + /// E0022 — `analyze_return` site. + ReturnBorrowed, +} + +/// Apply the consumption transition for a Move-typed initializer. +/// Caller must have already populated origin/state for `init` via +/// `visit_expr`. +fn consume_for_assignment( + pool: &InternPool, + own: &mut Ownership, + sink: &mut DiagSink, + init: TirRef, + span: Span, + name: Option, +) { + consume_underlying( + pool, + own, + sink, + init, + span, + name, + BorrowedAction::MoveOutOfParam, + ); +} + +/// Shared transition for every consume site (assignment, return, +/// move-typed Call argument). Walks back to the underlying owner, +/// reads its state, and either: +/// +/// * `Valid` → stamp `Moved { moved_at: span }` and clear any pending +/// dead-store entry, +/// * `Borrowed` → emit E0021 or E0022 per `on_borrowed`, +/// * `Moved` → silent (the `Var` arm of `visit_expr` already emitted +/// E0020 for the aliasing read that brought us here; emitting again +/// would double-report a single fault — see commit 2c5985d), +/// * `NotTracked` → no-op. +fn consume_underlying( + pool: &InternPool, + own: &mut Ownership, + sink: &mut DiagSink, + operand: TirRef, + span: Span, + name: Option, + on_borrowed: BorrowedAction, +) { + let underlying = underlying_owner(own, operand); + let state = own + .states + .get(&underlying) + .cloned() + .unwrap_or(OwnerState::NotTracked); + match state { + OwnerState::Valid => { + own.pending_dead_store.remove(&underlying); + own.states + .insert(underlying, OwnerState::Moved { moved_at: span }); + } + OwnerState::Borrowed => { + let (code, msg, help) = match on_borrowed { + BorrowedAction::MoveOutOfParam => ( + DiagCode::MoveOutOfBorrowedParam, + format!( + "cannot move out of borrowed parameter {}", + format_binding(name, pool), + ), + "add `move` to the parameter declaration if you need ownership", + ), + BorrowedAction::ReturnBorrowed => ( + DiagCode::ReturnBorrowedValue, + format!( + "cannot return borrowed value {} (Rule 5)", + format_binding(name, pool), + ), + "return a locally-allocated value, or accept the parameter as `move`", + ), + }; + sink.emit(Diag::error(span, code, msg).with_help(help)); + } + OwnerState::Moved { .. } => { + // No diagnostic here — the `Var` arm in `visit_expr` + // already emits E0020 for any aliasing read of a moved + // owner, which covers every path that lands here today + // (the only operands that reach a consume site with + // `Moved` underlying state arrive via `Var`). Direct + // producers (StrConst/StrConcat/Call) are stamped + // `Valid` in `visit_expr` on a freshly-walked branch and + // cannot be `Moved` here. Emitting again would + // double-report a single fault. + // + // If a future producer pattern bypasses `Var` and lands + // here in `Moved` state, prefer no diagnostic over a + // duplicate; revisit then. + } + OwnerState::NotTracked => {} + } +} + +/// Render a binding name for inclusion in a diagnostic message. +/// Returns `` `name` `` for known bindings and `value` for anonymous +/// temporaries (concat results, fresh allocations, etc.). +fn format_binding(name: Option, pool: &InternPool) -> String { + match name { + Some(n) => format!("`{}`", pool.str(n)), + None => "value".to_string(), + } +} + +/// If `r` is a direct `Var` read, return the binding name it aliases. +/// Used at consume sites to thread the source binding name into +/// E0020/E0021/E0022 messages. Returns `None` for fresh producers +/// (StrConst, StrConcat, Call), where there's no source binding. +fn consumed_binding_name(tir: &Tir, r: TirRef) -> Option { + match tir.inst(r).data { + TirData::Var(n) => Some(n), + _ => None, + } +} + +/// CFG join for `if` / `elif` / `else`. The naïve forward walk +/// would let a move inside a then-branch persist past the merge +/// regardless of whether else also moved — wrong for the spec's +/// guarantee that conditionally-moved values are not safe to use +/// after the join. Snapshot the lattice before each branch, walk +/// each branch from the snapshot independently, then merge. If any +/// branch left a value `Moved`, the post-`if` state is `Moved`; +/// when no `else` is present, the implicit fall-through branch is +/// the pre-`if` snapshot itself, so an unconsumed pre-`if` value +/// stays usable after the join. +fn analyze_if_stmt( + tir: &Tir, + pool: &InternPool, + own: &mut Ownership, + sink: &mut DiagSink, + by_name: &HashMap, + r: TirRef, +) { + let view = tir.if_stmt_view(r); + visit_expr(tir, pool, own, sink, by_name, view.cond); + + let snapshot = own.clone(); + + for stmt in &view.then_stmts { + analyze_stmt(tir, pool, own, sink, by_name, *stmt); + } + let then_state = own.clone(); + + let mut branch_results = vec![then_state]; + for elif in &view.elif_branches { + *own = snapshot.clone(); + visit_expr(tir, pool, own, sink, by_name, elif.cond); + for stmt in &elif.body { + analyze_stmt(tir, pool, own, sink, by_name, *stmt); + } + branch_results.push(own.clone()); + } + + if let Some(else_stmts) = &view.else_stmts { + *own = snapshot.clone(); + for stmt in else_stmts { + analyze_stmt(tir, pool, own, sink, by_name, *stmt); + } + branch_results.push(own.clone()); + } else { + branch_results.push(snapshot.clone()); + } + + // Final restore: `snapshot` has no further uses, so move instead + // of clone. + *own = snapshot; + let refs: Vec<&Ownership> = branch_results.iter().collect(); + own.merge_branches(&refs); +} + +/// 2-pass approximation of a fixed-point ownership analysis for +/// `while`. Walks the body once from the entry state into a scratch +/// sink; if no tracked TirRef changed Moved-ness across the back-edge, +/// the body is loop-invariant for ownership purposes and the scratch +/// diagnostics are flushed. Otherwise we re-walk from the merged +/// (entry ⊔ post-body) state and emit diagnostics on the second pass +/// — a binding moved inside the body without rebinding before the +/// back-edge surfaces as E0020 on iteration two. +/// +/// Why two passes suffice for the M8.1 pattern set (instead of a +/// general fixed-point loop): the merge is monotonic over the +/// Moved-ness sub-lattice (`Valid → Moved` is the only transition, +/// and merge takes "any branch Moved → Moved"), and there are no +/// iteration-count-dependent conditionals — so a TirRef that flips +/// from `Valid` to `Moved` in pass one stays `Moved` after the +/// (entry ⊔ post-body) merge and a third walk would observe nothing +/// new. Original phrasing for traceability: +/// "Converges in at most 2 iterations for the M8.1 pattern set". +/// +/// **Maintainer note.** If a future change introduces non-monotonic +/// merges (e.g., `Moved → Valid` re-entry as a real lattice transition +/// rather than via rebinding) or iteration-count-dependent control +/// flow inside the body, revert this to a true fixed-point loop +/// (iterate until `states_differ` returns false). +fn analyze_while_loop( + tir: &Tir, + pool: &InternPool, + own: &mut Ownership, + sink: &mut DiagSink, + by_name: &HashMap, + r: TirRef, +) { + let view = tir.while_loop_view(r); + let entry = own.clone(); + + // First pass into a scratch sink — diagnostics are kept only if + // a single iteration suffices. If the lattice changes we re-walk + // and the scratch diags are dropped (the second walk re-emits + // anything that's still wrong). + let mut scratch_sink = DiagSink::new(); + visit_expr(tir, pool, own, &mut scratch_sink, by_name, view.cond); + for stmt in &view.body { + analyze_stmt(tir, pool, own, &mut scratch_sink, by_name, *stmt); + } + let after_first = own.clone(); + + if states_differ(&entry, &after_first) { + *own = merge_two(&entry, &after_first); + visit_expr(tir, pool, own, sink, by_name, view.cond); + for stmt in &view.body { + analyze_stmt(tir, pool, own, sink, by_name, *stmt); + } + let after_second = own.clone(); + *own = merge_two(&entry, &after_second); + } else { + for d in scratch_sink.into_diags() { + sink.emit(d); + } + *own = merge_two(&entry, &after_first); + } +} + +/// `for i in range(start, end)` loop var is `int` (Copy), so the +/// induction variable never enters the lattice. The body runs the +/// same fixed-point as `while`. +fn analyze_for_range( + tir: &Tir, + pool: &InternPool, + own: &mut Ownership, + sink: &mut DiagSink, + by_name: &HashMap, + r: TirRef, +) { + let view = tir.for_range_view(r); + // Start/end are visited unconditionally — they're plain `int` + // exprs, so they don't move anything, but they may contain nested + // reads we want to record. + visit_expr(tir, pool, own, sink, by_name, view.start); + visit_expr(tir, pool, own, sink, by_name, view.end); + + let entry = own.clone(); + + let mut scratch_sink = DiagSink::new(); + for stmt in &view.body { + analyze_stmt(tir, pool, own, &mut scratch_sink, by_name, *stmt); + } + let after_first = own.clone(); + + if states_differ(&entry, &after_first) { + *own = merge_two(&entry, &after_first); + for stmt in &view.body { + analyze_stmt(tir, pool, own, sink, by_name, *stmt); + } + let after_second = own.clone(); + *own = merge_two(&entry, &after_second); + } else { + for d in scratch_sink.into_diags() { + sink.emit(d); + } + *own = merge_two(&entry, &after_first); + } +} + +/// Compare two lattices on the Moved-ness of every tracked `TirRef`. +/// Per the M8.1b plan, the only transition that forces a re-walk is a +/// `Valid`/`Borrowed` → `Moved` flip across the back-edge — non-Moved +/// state changes (e.g. fresh definitions added inside the body) are +/// loop-invariant for the use-after-move check. +fn states_differ(a: &Ownership, b: &Ownership) -> bool { + // Diverge iff some TirRef is Moved in one and not the other. + // Walk each side once and check the other's matching entry — + // missing entries default to NotTracked, so a Moved with no + // counterpart counts as a divergence on its own. + for (k, av) in &a.states { + let a_moved = matches!(av, OwnerState::Moved { .. }); + let b_moved = matches!(b.states.get(k), Some(OwnerState::Moved { .. })); + if a_moved != b_moved { + return true; + } + } + for (k, bv) in &b.states { + if !matches!(bv, OwnerState::Moved { .. }) { + continue; + } + // Only need to catch keys exclusive to b that are Moved — + // keys present in a were handled in the first loop. + if !a.states.contains_key(k) { + return true; + } + } + false +} + +/// Merge two lattices by reusing the existing branch-merge join: if +/// either input has a TirRef `Moved`, the result is `Moved`. Used to +/// seed the second pass and to compute the post-loop state. +fn merge_two(a: &Ownership, b: &Ownership) -> Ownership { + let mut merged = a.clone(); + merged.merge_branches(&[a, b]); + merged +} + +fn visit_expr( + tir: &Tir, + pool: &InternPool, + own: &mut Ownership, + sink: &mut DiagSink, + by_name: &HashMap, + r: TirRef, +) { + let inst = *tir.inst(r); + match inst.tag { + // ---- Allocating instructions ---- + // `StrConst` materializes a fresh heap string at runtime; + // `StrConcat` produces a brand-new allocation from its two + // operands. Both enter the lattice as `Valid` with no + // upstream origin. + TirTag::StrConst => { + if needs_tracking(inst.ty, pool) { + own.states.insert(r, OwnerState::Valid); + own.origin.insert(r, None); + } + } + TirTag::StrConcat => { + if needs_tracking(inst.ty, pool) { + own.states.insert(r, OwnerState::Valid); + own.origin.insert(r, None); + } + if let TirData::BinOp { lhs, rhs } = inst.data { + visit_expr(tir, pool, own, sink, by_name, lhs); + visit_expr(tir, pool, own, sink, by_name, rhs); + } + } + TirTag::Call => { + // A str-returning call (e.g. `int_to_str`) is a producer. + if needs_tracking(inst.ty, pool) { + own.states.insert(r, OwnerState::Valid); + own.origin.insert(r, None); + } + let view = tir.call_view(r); + // Look up callee parameter conventions. Builtins + // (`print`, `assert`, `int_to_str`) have no entry and + // default to borrowing all args. + let callee_params = by_name.get(&view.name).map(|t| t.params.as_slice()); + for (i, arg) in view.args.iter().enumerate() { + visit_expr(tir, pool, own, sink, by_name, *arg); + let arg_ty = tir.inst(*arg).ty; + if !needs_tracking(arg_ty, pool) { + continue; + } + let is_move_param = callee_params + .and_then(|ps| ps.get(i)) + .map(|p| p.is_move) + .unwrap_or(false); + if is_move_param { + let consumed_name = consumed_binding_name(tir, *arg); + consume_for_assignment(pool, own, sink, *arg, tir.span(r), consumed_name); + } + // Borrow path: no extra check here. With the current + // set of producers (StrConst/StrConcat/Call/Var), every + // tracked str argument flows through `Var`, and the + // `Var` arm already fires E0020 for use-after-move. + // Adding a call-site check would double-report. If a + // future producer pattern bypasses `Var` (e.g. passing + // a moved Call result directly), revisit this. + } + } + // ---- Aliasing read ---- + // `Var` is a non-consuming read. Record which SSA value it + // currently aliases so a later use-after-move diagnostic can + // walk back to the root owner. Per the design spec, an + // aliasing read of an already-`Moved` owner is itself an + // E0020 use-after-move. Reads of `Borrowed` owners are fine + // (Rule 2 — borrowed parameters can be freely read). + TirTag::Var => { + let name = match inst.data { + TirData::Var(n) => n, + _ => unreachable!("Var must carry TirData::Var"), + }; + if let Some(&owner) = own.current_owner.get(&name) + && needs_tracking(inst.ty, pool) + { + // Any read counts as "used" for dead-store purposes, + // even if it ultimately fires E0020 — once the + // programmer's code looked at the value, they + // didn't ignore it. + own.pending_dead_store.remove(&owner); + own.origin.insert(r, Some(owner)); + if let Some(OwnerState::Moved { moved_at, .. }) = own.states.get(&owner).cloned() { + sink.emit( + Diag::error( + tir.span(r), + DiagCode::UseAfterMove, + format!("use of moved value `{}`", pool.str(name)), + ) + .with_note(Some(moved_at), "value moved here") + .with_help( + "consider using the value before the move, or pass by default (borrow) instead of `move`", + ), + ); + } + } + } + // ---- Everything else: recurse on operands so nested + // ---- producers/aliases are still observed. + _ => { + recurse_operands(tir, pool, own, sink, by_name, r); + } + } +} + +fn recurse_operands( + tir: &Tir, + pool: &InternPool, + own: &mut Ownership, + sink: &mut DiagSink, + by_name: &HashMap, + r: TirRef, +) { + let inst = *tir.inst(r); + match inst.data { + TirData::UnOp(o) => visit_expr(tir, pool, own, sink, by_name, o), + TirData::BinOp { lhs, rhs } => { + visit_expr(tir, pool, own, sink, by_name, lhs); + visit_expr(tir, pool, own, sink, by_name, rhs); + } + // `Extra`-shaped instructions (VarDecl, Assign, Call, + // IfStmt, WhileLoop, ForRange, CompoundAssign) have + // bespoke decoders. Consumption logic lands in subsequent + // tasks; until then their operands are deliberately not + // descended into here so we avoid double-visits when those + // tasks introduce per-tag handling. + TirData::Extra(_) => {} + TirData::None + | TirData::Int(_) + | TirData::Float(_) + | TirData::Str(_) + | TirData::Bool(_) + | TirData::Var(_) => {} + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn copy_types_classified() { + let pool = InternPool::new(); + assert!(pool.is_copy(pool.int())); + assert!(pool.is_copy(pool.float())); + assert!(pool.is_copy(pool.bool_())); + assert!(!pool.is_copy(pool.str_())); + } + + #[test] + fn move_types_classified() { + let pool = InternPool::new(); + assert!(is_move_type(pool.str_(), &pool)); + assert!(!is_move_type(pool.int(), &pool)); + assert!(!is_move_type(pool.bool_(), &pool)); + } + + #[test] + fn needs_tracking_matches_move() { + let pool = InternPool::new(); + assert!(needs_tracking(pool.str_(), &pool)); + assert!(!needs_tracking(pool.int(), &pool)); + } + + #[test] + fn str_const_walk_no_panic() { + use crate::tir::TirBuilder; + use chumsky::span::{SimpleSpan, Span as _}; + + let mut pool = InternPool::new(); + let str_ty = pool.str_(); + let void = pool.void(); + let main = pool.intern_str("main"); + let hello = pool.intern_str("hello"); + let span = SimpleSpan::new((), 0..0); + + // fn main() -> void: as expr_stmt + let mut b = TirBuilder::new(main, vec![], void, span); + let s = b.str_const(hello, str_ty, span); + let stmt = b.unary(TirTag::ExprStmt, void, s, span); + let tir = b.finish(&[stmt]); + + let mut sink = DiagSink::new(); + check(std::slice::from_ref(&tir), &pool, &mut sink); + assert!(sink.is_empty()); + } +} diff --git a/src/parser.rs b/src/parser.rs index 08bb6ec..3426afd 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -312,13 +312,16 @@ where let type_expr = select! { Token::Ident(name) => name }.map_with(|name, e| TypeExpr::new(name, e.span())); - let param = select! { Token::Ident(name) => name } - .map_with(|name, e| Ident::new(name, e.span())) + let param = just(Token::Move) + .or_not() + .map(|m| m.is_some()) + .then(select! { Token::Ident(name) => name }.map_with(|name, e| Ident::new(name, e.span()))) .then_ignore(just(Token::Colon)) .then(type_expr) - .map_with(|(name, type_annotation), e| Param { + .map_with(|((is_move, name), type_annotation), e| Param { name, type_annotation, + is_move, span: e.span(), }); @@ -1417,4 +1420,33 @@ mod tests { let result = lex_and_parse(code); assert!(result.is_err(), "range() should fail with arity error"); } + + #[test] + fn parse_move_parameter() { + let (program, pool) = lex_and_parse("fn consume(move s: str):\n\tprint(s)\n").unwrap(); + let func = match &program.statements[0].kind { + StmtKind::FunctionDef(f) => f, + other => panic!("expected FunctionDef, got {:?}", other), + }; + assert_eq!(func.params.len(), 1); + assert!(func.params[0].is_move, "param `s` should be marked move"); + assert_eq!(pool.str(func.params[0].name.name), "s"); + assert_eq!(pool.str(func.params[0].type_annotation.name), "str"); + } + + #[test] + fn parse_default_parameter_is_not_move() { + let (program, pool) = lex_and_parse("fn read(s: str):\n\tprint(s)\n").unwrap(); + let func = match &program.statements[0].kind { + StmtKind::FunctionDef(f) => f, + other => panic!("expected FunctionDef, got {:?}", other), + }; + assert_eq!(func.params.len(), 1); + assert!( + !func.params[0].is_move, + "bare param `s` should default to is_move=false" + ); + assert_eq!(pool.str(func.params[0].name.name), "s"); + assert_eq!(pool.str(func.params[0].type_annotation.name), "str"); + } } diff --git a/src/pipeline.rs b/src/pipeline.rs index 110f692..b937e28 100644 --- a/src/pipeline.rs +++ b/src/pipeline.rs @@ -220,14 +220,19 @@ fn diag_code_str(code: DiagCode) -> &'static str { DiagCode::UnsupportedOperator => "E0015", DiagCode::VoidValueInExpression => "E0017", DiagCode::ConditionNotBool => "E0018", - DiagCode::ImmutableAssign => "E0020", - DiagCode::DuplicateDeclaration => "E0021", - DiagCode::UndefinedAssignTarget => "E0022", + DiagCode::ImmutableAssign => "E0028", + DiagCode::DuplicateDeclaration => "E0029", + DiagCode::UndefinedAssignTarget => "E0030", DiagCode::FloatModulo => "E0023", DiagCode::BreakOutsideLoop => "E0024", DiagCode::ContinueOutsideLoop => "E0025", DiagCode::RangeArgType => "E0026", DiagCode::ReservedBuiltinName => "E0027", + DiagCode::RedundantMove => "W0002", + DiagCode::UseAfterMove => "E0020", + DiagCode::MoveOutOfBorrowedParam => "E0021", + DiagCode::ReturnBorrowedValue => "E0022", + DiagCode::DeadStore => "W0001", DiagCode::CycleInResolution => "E0016", DiagCode::ParseError => "E0100", DiagCode::TooManyDiagnostics => "E0101", @@ -285,7 +290,7 @@ pub(crate) fn ir_command(file: &Path, emit: &[EmitKind]) -> Result<(), CompilerE if !(want.tir || want.clif) { // UIR-only run. Surface astgen diagnostics now, with a // non-zero exit if anything fired. - return finish_with_diags(sink, &input, &name); + return finalize_diags(sink, &input, &name); } // For TIR / CLIF we also run sema. Per the §4.5 design, sema @@ -293,6 +298,7 @@ pub(crate) fn ir_command(file: &Path, emit: &[EmitKind]) -> Result<(), CompilerE // slots), and `--emit=tir` deliberately prints that partial // TIR — the whole point of the flag is debugging sema. let tirs = sema::analyze(&uir, &mut pool, &mut sink, &input, file); + crate::ownership::check(&tirs, &pool, &mut sink); if want.tir { display_tir(&tirs, &pool); @@ -304,12 +310,16 @@ pub(crate) fn ir_command(file: &Path, emit: &[EmitKind]) -> Result<(), CompilerE // failed, surface the diagnostics and abort — we cannot // produce a meaningful CLIF dump from a broken TIR. if sink.has_errors() { - return finish_with_diags(sink, &input, &name); + return finalize_diags(sink, &input, &name); } generate_and_display_ir(&tirs, &pool)?; } - finish_with_diags(sink, &input, &name) + // Tail block: drains the sink whether sema/ownership were + // clean or only emitted warnings. Without this `ryo ir` would + // silently swallow W0001/W0002 on success — the bug I-044 + // tracked. + finalize_diags(sink, &input, &name) } /// Resolve `--emit` flag values into a normalized set. Membership @@ -348,14 +358,28 @@ impl EmitSet { } } -/// Render any pending diagnostics and translate them into a -/// `CompilerError::Diagnostics` (or `Ok(())` if the sink is -/// clean). Centralized so every `ryo ir` exit path uses the same -/// rendering plumbing. -fn finish_with_diags(sink: DiagSink, input: &str, source_name: &str) -> Result<(), CompilerError> { - if sink.has_errors() { - let diags = sink.into_diags(); +/// Drain `sink`, render whatever is queued, and translate into a +/// terminal pipeline result. +/// +/// Single tail-block used by every front-end driver (`ryo run`, +/// `ryo build`, `ryo ir`) so that: +/// +/// * warnings (`W0001` DeadStore, `W0002` RedundantMove, …) reach +/// the user on otherwise-successful runs, and +/// * the success and error paths render the *same* diagnostics +/// exactly once — never via two separate `render_diags` calls +/// that could drift out of sync. +/// +/// Returns `Err(CompilerError::Diagnostics(_))` iff at least one +/// diagnostic has `Severity::Error`; warnings/notes alone do not +/// fail the build. +fn finalize_diags(sink: DiagSink, input: &str, source_name: &str) -> Result<(), CompilerError> { + let has_errors = sink.has_errors(); + let diags = sink.into_diags(); + if !diags.is_empty() { render_diags(&diags, input, source_name); + } + if has_errors { Err(CompilerError::Diagnostics(diags)) } else { Ok(()) @@ -389,11 +413,13 @@ fn lower_and_analyze( // keeps cascades in check, and surfacing every problem in one // run is the whole point of the structured-diagnostics phase. let tirs = sema::analyze(&uir, pool, &mut sink, input, file_path); - if sink.has_errors() { - let diags = sink.into_diags(); - render_diags(&diags, input, source_name); - return Err(CompilerError::Diagnostics(diags)); - } + crate::ownership::check(&tirs, pool, &mut sink); + // Single tail block: render-if-non-empty, Err iff any errors. + // Same shape as `ir_command` so warnings (`W0001` DeadStore, + // `W0002` RedundantMove, …) surface on the success path + // without a separate render block that could drift from the + // error path. + finalize_diags(sink, input, source_name)?; Ok(tirs) } diff --git a/src/sema.rs b/src/sema.rs index 6ad4edc..6e743b7 100644 --- a/src/sema.rs +++ b/src/sema.rs @@ -357,12 +357,33 @@ fn analyze_function(sema: &mut Sema<'_>, body: &FuncBody) -> Tir { scope.insert_binding(param.name, param.ty, false); } + // W0002: warn on `move` annotations applied to Copy-typed + // parameters. Copy types (int, float, bool) are duplicated on + // every read regardless of the annotation, so `move` is + // redundant noise. `move` on `str` (and other heap types) stays + // silent — that's the whole reason the keyword exists. + for param in &body.params { + if param.is_move && sema.pool.is_copy(param.ty) { + let name = sema.pool.str(param.name).to_string(); + let ty_str = sema.pool.display(param.ty).to_string(); + sema.sink.emit(Diag::warning( + param.span, + DiagCode::RedundantMove, + format!( + "redundant 'move' on Copy-typed parameter '{}': {} values are copied on every read", + name, ty_str, + ), + )); + } + } + let params: Vec = body .params .iter() .map(|p| TirParam { name: p.name, ty: p.ty, + is_move: p.is_move, span: p.span, }) .collect(); diff --git a/src/tir.rs b/src/tir.rs index e7e4696..c6089ff 100644 --- a/src/tir.rs +++ b/src/tir.rs @@ -246,6 +246,7 @@ pub struct TypedInst { pub struct TirParam { pub name: StringId, pub ty: TypeId, + pub is_move: bool, pub span: Span, } @@ -963,6 +964,9 @@ impl<'a> fmt::Display for TirDump<'a> { if i > 0 { write!(f, ", ")?; } + if p.is_move { + write!(f, "move ")?; + } write!(f, "{}: {}", self.pool.str(p.name), self.pool.display(p.ty))?; } writeln!(f, ") -> {}", self.pool.display(tir.return_type))?; diff --git a/src/types.rs b/src/types.rs index 868ed20..7840296 100644 --- a/src/types.rs +++ b/src/types.rs @@ -347,6 +347,19 @@ impl InternPool { id.0 == ID_NEVER } + /// True for types whose values are duplicated on `=` without + /// invalidating the source. Mirrors Mojo's `Copyable` trait for + /// the scalar primitives Ryo currently has: `int`, `float`, + /// `bool`. Used by sema (to flag redundant `move` annotations) + /// and by the ownership pass (to short-circuit liveness on + /// these values — they never enter the lattice). + pub fn is_copy(&self, ty: TypeId) -> bool { + matches!( + self.kind(ty), + TypeKind::Int | TypeKind::Float | TypeKind::Bool + ) + } + /// Compatibility predicate that absorbs the `Error` sentinel. /// Used anywhere sema would otherwise emit a type-mismatch. pub fn compatible(&self, a: TypeId, b: TypeId) -> bool { diff --git a/src/uir.rs b/src/uir.rs index 5664eec..67a8f15 100644 --- a/src/uir.rs +++ b/src/uir.rs @@ -249,6 +249,7 @@ pub struct Inst { pub struct UirParam { pub name: StringId, pub ty: TypeId, + pub is_move: bool, pub span: Span, } diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs index 07cf35f..2feb505 100644 --- a/tests/integration_tests.rs +++ b/tests/integration_tests.rs @@ -957,6 +957,32 @@ fn ir_emit_tir_prints_partial_tir_with_unreachable_on_sema_error() { ); } +#[test] +fn test_ryo_ir_surfaces_warnings_on_success() { + // I-044 regression: `ryo ir` used to call sema + ownership + // against a sink and only render on the error path, so any + // warnings (W0001 DeadStore, W0002 RedundantMove) emitted on + // a successful run were silently dropped. After the + // single-tail-block refactor, `ryo ir` should surface the same + // diagnostics on stderr that `ryo run` / `ryo build` already + // do (cf. test_redundant_move_on_int_warns). + let temp_dir = TempDir::new().expect("temp"); + let code = "fn f(move x: int):\n\tprint(int_to_str(x))\n\nf(42)"; + let test_file = create_test_file(temp_dir.path(), "ir_w0002.ryo", code); + let output = run_ryo_command(&["ir", "--emit=tir", "ir_w0002.ryo"], &test_file).expect("run"); + assert!( + output.status.success(), + "STDERR: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("W0002"), + "expected W0002 from ryo ir on stderr: {}", + stderr + ); +} + // ============================================================================ // Milestone 8b: Conditionals & Logical Operators // ============================================================================ @@ -2279,3 +2305,504 @@ fn test_str_shadowed_by_int_assignment_does_not_panic() { stdout ); } + +#[test] +fn test_redundant_move_on_int_warns() { + let temp_dir = TempDir::new().expect("temp dir"); + let code = "fn f(move x: int):\n\tprint(int_to_str(x))\n\nf(42)"; + let test_file = create_test_file(temp_dir.path(), "redundant_move.ryo", code); + let output = run_ryo_command(&["run", "redundant_move.ryo"], &test_file).expect("run"); + assert!( + output.status.success(), + "STDERR: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("W0002"), + "expected W0002 in stderr: {}", + stderr + ); +} + +#[test] +fn test_move_on_str_no_warning() { + let temp_dir = TempDir::new().expect("temp dir"); + let code = "fn f(move s: str):\n\tprint(s)\n\nf(\"hi\")"; + let test_file = create_test_file(temp_dir.path(), "move_on_str.ryo", code); + let output = run_ryo_command(&["run", "move_on_str.ryo"], &test_file).expect("run"); + assert!( + output.status.success(), + "STDERR: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(!stderr.contains("W0002"), "unexpected W0002: {}", stderr); +} + +#[test] +fn test_use_after_move_assignment() { + let temp_dir = TempDir::new().expect("temp"); + let code = "name: str = \"Alice\"\nother: str = name\nprint(name)"; + let test_file = create_test_file(temp_dir.path(), "uam_assign.ryo", code); + let output = run_ryo_command(&["run", "uam_assign.ryo"], &test_file).expect("run"); + assert!(!output.status.success(), "expected compile error"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("E0020"), + "expected E0020 in stderr: {}", + stderr + ); +} + +#[test] +fn test_move_out_of_borrowed_parameter() { + let temp_dir = TempDir::new().expect("temp"); + let code = "fn process(data: str):\n\tother: str = data\n\nprocess(\"hi\")"; + let test_file = create_test_file(temp_dir.path(), "move_borrowed.ryo", code); + let output = run_ryo_command(&["run", "move_borrowed.ryo"], &test_file).expect("run"); + assert!(!output.status.success()); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("E0021"), "expected E0021: {}", stderr); +} + +#[test] +fn test_move_param_consumed_ok() { + let temp_dir = TempDir::new().expect("temp"); + let code = "fn consume(move s: str):\n\tother: str = s\n\nconsume(\"hi\")"; + let test_file = create_test_file(temp_dir.path(), "move_ok.ryo", code); + let output = run_ryo_command(&["run", "move_ok.ryo"], &test_file).expect("run"); + assert!( + output.status.success(), + "STDERR: {}", + String::from_utf8_lossy(&output.stderr) + ); +} + +#[test] +fn test_str_concat_then_use_ok() { + let temp_dir = TempDir::new().expect("temp"); + let code = "a: str = \"hi\"\nb: str = \"there\"\nc: str = a + b\nprint(c)"; + let test_file = create_test_file(temp_dir.path(), "concat_ok.ryo", code); + let output = run_ryo_command(&["run", "concat_ok.ryo"], &test_file).expect("run"); + assert!(output.status.success()); +} + +#[test] +fn test_str_concat_then_use_after_move_in_concat() { + let temp_dir = TempDir::new().expect("temp"); + // a + b reads a and b as borrows (not moves); a is still valid. + let code = "a: str = \"hi\"\nb: str = \"!\"\nc: str = a + b\nprint(a)"; + let test_file = create_test_file(temp_dir.path(), "concat_borrows.ryo", code); + let output = run_ryo_command(&["run", "concat_borrows.ryo"], &test_file).expect("run"); + assert!( + output.status.success(), + "STDERR: {}", + String::from_utf8_lossy(&output.stderr) + ); +} + +#[test] +fn test_use_after_move_return() { + let temp_dir = TempDir::new().expect("temp"); + let code = "fn make() -> str:\n\ts: str = \"hi\"\n\treturn s\n\nx: str = make()\nprint(x)"; + let test_file = create_test_file(temp_dir.path(), "ret_ok.ryo", code); + let output = run_ryo_command(&["run", "ret_ok.ryo"], &test_file).expect("run"); + assert!( + output.status.success(), + "STDERR: {}", + String::from_utf8_lossy(&output.stderr) + ); +} + +#[test] +fn test_return_borrowed_param() { + let temp_dir = TempDir::new().expect("temp"); + let code = + "fn passthrough(s: str) -> str:\n\treturn s\n\nx: str = passthrough(\"hi\")\nprint(x)"; + let test_file = create_test_file(temp_dir.path(), "ret_borrowed.ryo", code); + let output = run_ryo_command(&["run", "ret_borrowed.ryo"], &test_file).expect("run"); + assert!(!output.status.success(), "expected E0022"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("E0022"), "expected E0022: {}", stderr); +} + +#[test] +fn test_use_after_move_param() { + let temp_dir = TempDir::new().expect("temp"); + let code = + "fn consume(move s: str):\n\tprint(s)\n\nname: str = \"Alice\"\nconsume(name)\nprint(name)"; + let test_file = create_test_file(temp_dir.path(), "uam_param.ryo", code); + let output = run_ryo_command(&["run", "uam_param.ryo"], &test_file).expect("run"); + assert!(!output.status.success()); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("E0020"), "expected E0020: {}", stderr); + let count = stderr.matches("E0020").count(); + assert_eq!( + count, 1, + "expected exactly one E0020, got {}: {}", + count, stderr + ); +} + +#[test] +fn test_use_after_move_in_vardecl_one_diag() { + let temp_dir = TempDir::new().expect("temp"); + let code = r#" +fn consume(move s: str): + print(s) + +fn main(): + name: str = "Alice" + consume(name) + other: str = name +"#; + let test_file = create_test_file(temp_dir.path(), "uam_vardecl_one.ryo", code); + let output = run_ryo_command(&["run", "uam_vardecl_one.ryo"], &test_file).expect("run"); + assert!( + !output.status.success(), + "expected compile error, STDERR: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stderr = String::from_utf8_lossy(&output.stderr); + let count = stderr.matches("E0020").count(); + assert_eq!( + count, 1, + "expected exactly one E0020, got {}: {}", + count, stderr + ); +} + +#[test] +fn test_use_after_move_in_return_one_diag() { + let temp_dir = TempDir::new().expect("temp"); + let code = r#" +fn consume(move s: str): + print(s) + +fn forward(move s: str) -> str: + consume(s) + return s + +fn main(): + x: str = forward("hi") + print(x) +"#; + let test_file = create_test_file(temp_dir.path(), "uam_ret_one.ryo", code); + let output = run_ryo_command(&["run", "uam_ret_one.ryo"], &test_file).expect("run"); + assert!( + !output.status.success(), + "expected compile error, STDERR: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stderr = String::from_utf8_lossy(&output.stderr); + let count = stderr.matches("E0020").count(); + assert_eq!( + count, 1, + "expected exactly one E0020, got {}: {}", + count, stderr + ); +} + +#[test] +fn test_e0020_message_includes_binding_name() { + let temp_dir = TempDir::new().expect("temp"); + let code = r#" +fn consume(move s: str): + print(s) + +fn main(): + name: str = "Alice" + consume(name) + print(name) +"#; + let test_file = create_test_file(temp_dir.path(), "e0020_label.ryo", code); + let output = run_ryo_command(&["run", "e0020_label.ryo"], &test_file).expect("run"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("E0020"), "stderr: {}", stderr); + assert!( + stderr.contains("name"), + "expected binding name in message: {}", + stderr + ); + assert!( + stderr.contains("moved here") || stderr.contains("moved into"), + "expected move-site note: {}", + stderr + ); +} + +#[test] +fn test_borrow_param_then_use_ok() { + let temp_dir = TempDir::new().expect("temp"); + let code = "fn print_twice(s: str):\n\tprint(s)\n\tprint(s)\n\nprint_twice(\"hi\")"; + let test_file = create_test_file(temp_dir.path(), "borrow_ok.ryo", code); + let output = run_ryo_command(&["run", "borrow_ok.ryo"], &test_file).expect("run"); + assert!( + output.status.success(), + "STDERR: {}", + String::from_utf8_lossy(&output.stderr) + ); +} + +#[test] +fn test_conditional_move_then_use_fails() { + let temp_dir = TempDir::new().expect("temp"); + let code = r#" +fn consume(move s: str): + print(s) + +fn main(): + flag: bool = true + name: str = "Alice" + if flag: + consume(name) + print(name) +"#; + let test_file = create_test_file(temp_dir.path(), "cond_move.ryo", code); + let output = run_ryo_command(&["run", "cond_move.ryo"], &test_file).expect("run"); + assert!( + !output.status.success(), + "expected E0020 — name moved on then-branch" + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("E0020"), "expected E0020: {}", stderr); +} + +#[test] +fn test_conditional_move_both_branches_fails() { + let temp_dir = TempDir::new().expect("temp"); + let code = r#" +fn consume(move s: str): + print(s) + +fn main(): + flag: bool = true + name: str = "Alice" + if flag: + consume(name) + else: + consume(name) + print(name) +"#; + let test_file = create_test_file(temp_dir.path(), "cond_both.ryo", code); + let output = run_ryo_command(&["run", "cond_both.ryo"], &test_file).expect("run"); + assert!(!output.status.success()); +} + +#[test] +fn test_conditional_use_inside_branch_ok() { + let temp_dir = TempDir::new().expect("temp"); + let code = r#" +fn main(): + flag: bool = true + name: str = "Alice" + if flag: + print(name) + else: + print(name) +"#; + let test_file = create_test_file(temp_dir.path(), "cond_ok.ryo", code); + let output = run_ryo_command(&["run", "cond_ok.ryo"], &test_file).expect("run"); + assert!( + output.status.success(), + "STDERR: {}", + String::from_utf8_lossy(&output.stderr) + ); +} + +#[test] +fn test_move_in_while_without_rebind_fails() { + let temp_dir = TempDir::new().expect("temp"); + let code = r#" +fn consume(move s: str): + print(s) + +fn main(): + mut name: str = "Alice" + mut i: int = 0 + while i < 3: + consume(name) + i = i + 1 +"#; + let test_file = create_test_file(temp_dir.path(), "loop_move.ryo", code); + let output = run_ryo_command(&["run", "loop_move.ryo"], &test_file).expect("run"); + assert!( + !output.status.success(), + "expected E0020 — name moved without rebind" + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("E0020"), "expected E0020: {}", stderr); +} + +#[test] +fn test_borrow_in_while_ok() { + let temp_dir = TempDir::new().expect("temp"); + let code = r#" +fn main(): + name: str = "Alice" + mut i: int = 0 + while i < 3: + print(name) + i = i + 1 +"#; + let test_file = create_test_file(temp_dir.path(), "loop_borrow.ryo", code); + let output = run_ryo_command(&["run", "loop_borrow.ryo"], &test_file).expect("run"); + assert!( + output.status.success(), + "STDERR: {}", + String::from_utf8_lossy(&output.stderr) + ); +} + +#[test] +fn test_move_then_rebind_in_loop_ok() { + let temp_dir = TempDir::new().expect("temp"); + let code = r#" +fn consume(move s: str): + print(s) + +fn main(): + mut i: int = 0 + while i < 3: + mut name: str = "Alice" + consume(name) + i = i + 1 +"#; + let test_file = create_test_file(temp_dir.path(), "loop_rebind.ryo", code); + let output = run_ryo_command(&["run", "loop_rebind.ryo"], &test_file).expect("run"); + assert!( + output.status.success(), + "STDERR: {}", + String::from_utf8_lossy(&output.stderr) + ); +} + +#[test] +fn test_conditional_partial_rebind_then_use_fails() { + let temp_dir = TempDir::new().expect("temp"); + let code = r#" +fn consume(move s: str): + print(s) + +fn main(): + mut s: str = "hello" + flag: bool = true + if flag: + s = "world" + consume(s) + else: + print("else") + print(s) +"#; + let test_file = create_test_file(temp_dir.path(), "cond_partial_rebind.ryo", code); + let output = run_ryo_command(&["run", "cond_partial_rebind.ryo"], &test_file).expect("run"); + assert!( + !output.status.success(), + "expected E0020: STDERR: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("E0020"), "expected E0020: {}", stderr); +} + +#[test] +fn test_loop_consume_then_rebind_ok() { + let temp_dir = TempDir::new().expect("temp"); + let code = r#" +fn consume(move s: str): + print(s) + +fn main(): + mut name: str = "Alice" + mut i: int = 0 + while i < 3: + consume(name) + name = "Bob" + i = i + 1 +"#; + let test_file = create_test_file(temp_dir.path(), "loop_consume_rebind.ryo", code); + let output = run_ryo_command(&["run", "loop_consume_rebind.ryo"], &test_file).expect("run"); + assert!( + output.status.success(), + "STDERR: {}", + String::from_utf8_lossy(&output.stderr) + ); +} + +#[test] +fn test_both_branches_consume_and_rebind_ok() { + let temp_dir = TempDir::new().expect("temp"); + let code = r#" +fn consume(move s: str): + print(s) + +fn main(): + mut name: str = "Alice" + flag: bool = true + if flag: + consume(name) + name = "Bob" + else: + consume(name) + name = "Charlie" + print(name) +"#; + let test_file = create_test_file(temp_dir.path(), "both_branches_rebind.ryo", code); + let output = run_ryo_command(&["run", "both_branches_rebind.ryo"], &test_file).expect("run"); + assert!( + output.status.success(), + "STDERR: {}", + String::from_utf8_lossy(&output.stderr) + ); +} + +#[test] +fn test_dead_store_warning() { + let temp_dir = TempDir::new().expect("temp"); + let code = "name: str = \"Alice\"\nprint(\"hello\")"; + let test_file = create_test_file(temp_dir.path(), "dead_store.ryo", code); + let output = run_ryo_command(&["run", "dead_store.ryo"], &test_file).expect("run"); + // Warning, not error — exit success. + assert!( + output.status.success(), + "STDERR: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("W0001"), + "expected W0001 in stderr: {}", + stderr + ); +} + +#[test] +fn test_no_dead_store_when_used() { + let temp_dir = TempDir::new().expect("temp"); + let code = "name: str = \"Alice\"\nprint(name)"; + let test_file = create_test_file(temp_dir.path(), "live_store.ryo", code); + let output = run_ryo_command(&["run", "live_store.ryo"], &test_file).expect("run"); + assert!(output.status.success()); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(!stderr.contains("W0001"), "unexpected W0001: {}", stderr); +} + +#[test] +fn test_dead_store_warning_reassignment() { + let temp_dir = TempDir::new().expect("temp"); + let code = "fn main():\n\tmut name: str = \"Alice\"\n\tprint(name)\n\tname = \"Bob\"\n\tprint(\"done\")\n"; + let test_file = create_test_file(temp_dir.path(), "reassign_dead.ryo", code); + let output = run_ryo_command(&["run", "reassign_dead.ryo"], &test_file).expect("run"); + assert!( + output.status.success(), + "STDERR: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("W0001"), + "expected W0001 in stderr: {}", + stderr + ); +}