Fix audit findings: round 97

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:2561`, `:2657`, `:3476`)
+/// identical wording, see `src/compiler/mod.rs:2562`, `:2658`, `:3477`)
 /// 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.

src/formatter.rs

@@ -750,6 +750,26 @@ enum ScanMode {
 /// rather than a per-line walk; behavior is intentionally divergent
 /// for that reason and the two scaffolds are not unified.
 fn compute_bracket_end_line(start_line: usize, open: char, close: char) -> usize {
+    compute_bracket_end_line_opts(start_line, open, close, false)
+}
+
+/// Variant of [`compute_bracket_end_line`] that, when
+/// `require_open_on_start_line` is set, returns `start_line` unchanged if
+/// the `open` delimiter is not found (in code context) on `start_line`
+/// itself. `expr_max_line` runs over arbitrary expressions — including
+/// paren-less trailing-block calls (`list.map { ... }`) that have no `(`
+/// of their own. The plain scanner would skip past such a construct and
+/// latch onto a *later* statement's bracket, dragging the intervening
+/// standalone comments into the wrong statement on re-layout. Requiring
+/// the opener on the first scanned line confines the match to this
+/// construct. All pre-existing callers pass `false` (they already pass the
+/// exact line the opener lives on), preserving their behavior unchanged.
+fn compute_bracket_end_line_opts(
+    start_line: usize,
+    open: char,
+    close: char,
+    require_open_on_start_line: bool,
+) -> usize {
     FMT_STATE.with(|cell| {
         let borrowed = cell.borrow();
         let Some(state) = borrowed.as_ref() else {
@@ -900,6 +920,13 @@ fn compute_bracket_end_line(start_line: usize, open: char, close: char) -> usize
                     }
                 }
             }
+            // Strict mode: if the opener never appeared on the first
+            // scanned line, this construct has no delimiter of its own
+            // (e.g. a paren-less trailing-block call). Don't scan into
+            // later statements looking for an unrelated bracket.
+            if require_open_on_start_line && line_idx == start_idx && !found_open {
+                return start_line;
+            }
             // End-of-line fallback: a `}` that pops back into a regular
             // string is allowed (we're in InRegular at end of line) as
             // long as the bracket pair has been closed at the
@@ -2475,17 +2502,45 @@ fn expr_max_line(expr: &Expr) -> usize {
                     ListElem::Single(e) | ListElem::Spread(e) => visit(e),
                 }
             }
+            // Include the construct's own closing-bracket line so a
+            // trailing comment on the close-delimiter line (e.g.
+            // `[\n  1\n] -- note`) — which no element occupies — is
+            // reachable to the outer statement drain and not orphaned in
+            // `trailing_map`. Mirrors the Lambda/Block handling below.
+            let close = compute_bracket_end_line_opts(expr.span.line, '[', ']', true);
+            if close > max {
+                max = close;
+            }
         }
         ExprKind::Map(pairs) => {
             for (k, v) in pairs {
                 visit(k);
                 visit(v);
             }
+            let close = compute_bracket_end_line_opts(expr.span.line, '{', '}', true);
+            if close > max {
+                max = close;
+            }
         }
-        ExprKind::SetLit(elems) | ExprKind::Tuple(elems) => {
+        ExprKind::SetLit(elems) => {
             for e in elems {
                 visit(e);
             }
+            // `#[ ... ]` — the `#` is a scalar the scanner skips; the
+            // bracket pair is `[`/`]`.
+            let close = compute_bracket_end_line_opts(expr.span.line, '[', ']', true);
+            if close > max {
+                max = close;
+            }
+        }
+        ExprKind::Tuple(elems) => {
+            for e in elems {
+                visit(e);
+            }
+            let close = compute_bracket_end_line_opts(expr.span.line, '(', ')', true);
+            if close > max {
+                max = close;
+            }
         }
         ExprKind::FieldAccess(e, _) => visit(e),
         ExprKind::Binary(l, _, r) => {
@@ -2502,6 +2557,15 @@ fn expr_max_line(expr: &Expr) -> usize {
             for a in args {
                 visit(a);
             }
+            // Include the call's own `)` close line so a trailing comment
+            // on the close-paren line (e.g. `bar(\n  1\n) -- note`) is
+            // reachable to the outer statement drain rather than orphaned
+            // in `trailing_map`. The call's span is at the callee and the
+            // opening `(` lives on the same line. Mirrors Lambda/Block.
+            let close = compute_bracket_end_line_opts(expr.span.line, '(', ')', true);
+            if close > max {
+                max = close;
+            }
         }
         ExprKind::Lambda { body, .. } => {
             visit(body);
@@ -2523,12 +2587,25 @@ fn expr_max_line(expr: &Expr) -> usize {
             for (_, v) in fields {
                 visit(v);
             }
+            // Include the record's `}` close line so a trailing comment on
+            // the close-brace line is reachable to the outer statement
+            // drain. Mirrors Lambda/Block.
+            let close = compute_bracket_end_line_opts(expr.span.line, '{', '}', true);
+            if close > max {
+                max = close;
+            }
         }
-        ExprKind::RecordUpdate { expr, fields } => {
-            visit(expr);
+        ExprKind::RecordUpdate { expr: recv, fields } => {
+            visit(recv);
             for (_, v) in fields {
                 visit(v);
             }
+            // `RecordUpdate.span` is the receiver's span; the update brace
+            // is the first `{` at or after that line. Reach its close `}`.
+            let close = compute_bracket_end_line_opts(expr.span.line, '{', '}', true);
+            if close > max {
+                max = close;
+            }
         }
         ExprKind::AnonRecord { spread, fields } => {
             if let Some(s) = spread {
@@ -2537,6 +2614,10 @@ fn expr_max_line(expr: &Expr) -> usize {
             for (_, v) in fields {
                 visit(v);
             }
+            let close = compute_bracket_end_line_opts(expr.span.line, '{', '}', true);
+            if close > max {
+                max = close;
+            }
         }
         ExprKind::Match { expr, arms } => {
             if let Some(e) = expr.as_deref() {

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:7560):
+    // (src/typechecker/mod.rs:7589):
     //   Display → display, Compare → compare, Equal → equal, Hash → hash.
     //
     // `Bytes` is a stdlib alias for `List(Int)` and thus collapses onto

src/typechecker/effects_infer.rs

@@ -326,8 +326,34 @@ fn stmt_effects(stmt: &Stmt, env: &TypeEnv, aliases: &mut AliasMap) -> EffectSet
             val_effects
         }
         Stmt::When {
-            expr, else_body, ..
-        } => walk_expr(expr, env, aliases).union(walk_expr(else_body, env, aliases)),
+            pattern,
+            expr,
+            else_body,
+        } => {
+            // Round 97 BROKEN (soundness): `when let pat = expr else { .. }`
+            // binds `pat`'s names into the enclosing block scope, exactly
+            // like `Stmt::Let`. The arm previously ignored the pattern, so a
+            // binder that shadows a same-named imported module was still
+            // treated as a live module qualifier — a later `name.fn(...)`
+            // resolved through the MODULE's qualified scheme and charged the
+            // module fn's declared effects instead of conservative TOP,
+            // letting a real effectful field-fn slip past --strict-effects.
+            // Mirror the `Stmt::Let` arm: resolve any aliasing source first
+            // (the scrutinee sits outside the new binding's scope), then mark
+            // the binders rebound and invalidate stale alias entries.
+            let scrutinee_effects = walk_expr(expr, env, aliases);
+            let src = resolvable_callee_name(expr, env, aliases);
+            aliases.mark_pattern_rebound(pattern);
+            if let PatternKind::Ident(name) = &pattern.kind {
+                if let Some(src) = src {
+                    let src_eff = scheme_effects_of(src, env, aliases);
+                    aliases.insert(*name, src_eff);
+                } else {
+                    aliases.remove(name);
+                }
+            }
+            scrutinee_effects.union(walk_expr(else_body, env, aliases))
+        }
         Stmt::WhenBool {
             condition,
             else_body,

src/typechecker/inference.rs

@@ -3189,7 +3189,7 @@ 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:7896`); Channel and Fn are not
+                    // in `src/typechecker/mod.rs:7925`); Channel and Fn are not
                     // auto-derived but user-defined trait impls register entries
                     // under the canonical names "Channel" / "Fn" via
                     // `type_name_for_impl` (see `src/typechecker/mod.rs:2045`,
@@ -5755,6 +5755,13 @@ pub(super) fn is_valid_arith_operand(ty: &Type, allow_string: bool) -> bool {
 /// nondeterministic) is rejected there with a field-precise message.
 /// This free function stays stateless because every other arm is
 /// purely structural.
+///
+/// Round 97: the same applies to CONTAINER heads. `List(_)` / `Range(_)`
+/// (ordering + equality) and `Tuple`/`Map`/`Set` (equality) pass this
+/// shape gate, but a container whose element / component / value type is
+/// `Fn`-shaped would launder into the same Arc-pointer-address ordering
+/// at runtime. `operand_builtin_trait_violation` recurses into the
+/// element types (via `gate_field_supports_trait`) and rejects those.
 pub(super) fn is_valid_compare_operand(ty: &Type, is_equality: bool) -> bool {
     match ty {
         Type::Int

src/typechecker/mod.rs

@@ -6099,6 +6099,36 @@ impl TypeChecker {
                     })
                 });
             }
+            // Round 97: container HEADS (List/Range/Tuple/Map/Set) pass the
+            // structural shape gate in `is_valid_compare_operand`, but the
+            // Value-level operation recurses into element types — and the VM
+            // fallback for a `Fn`-shaped element is Arc-pointer-address
+            // ordering (ASLR-nondeterministic), exactly the bug round 3 fixed
+            // for bare `Fn` operands and round 93 fixed for nominal fields.
+            // Mirror the round-93 field walk: a container is compare/equality
+            // -valid IFF every element / component / value type is itself
+            // valid. `gate_field_supports_trait` already does this recursion
+            // honestly (and bottoms out at `Type::Fun(..) => false`), so we
+            // reuse it and surface the whole container type as the reason.
+            Type::List(_) | Type::Range(_) | Type::Tuple(_) | Type::Map(..) | Type::Set(_) => {
+                let no_rescue: std::collections::HashSet<(Symbol, Symbol)> =
+                    std::collections::HashSet::new();
+                let no_negatives = HashMap::new();
+                return (!self.gate_field_supports_trait(
+                    trait_sym,
+                    &resolved,
+                    &no_rescue,
+                    &no_negatives,
+                    0,
+                ))
+                .then(|| {
+                    format!(
+                        "type '{resolved}' cannot derive '{}': element type is not {}",
+                        resolve(trait_sym),
+                        builtin_trait_adjective(trait_sym),
+                    )
+                });
+            }
             _ => return None,
         };
         let canon = canonicalize_type_name(&self.resolver, name);

src/vm/dispatch.rs

@@ -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:7796), so a value of
+                    // List (see src/typechecker/mod.rs:7825), 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
@@ -394,7 +394,7 @@ impl Vm {
                     | (Value::Record(..), Value::Record(..)) => receiver.cmp(other),
                     //
                     // Unit vs Unit: typechecker auto-derives Compare for `()`
-                    // (src/typechecker/mod.rs:7793). All units are equal.
+                    // (src/typechecker/mod.rs:7822). All units are equal.
                     (Value::Unit, Value::Unit) => std::cmp::Ordering::Equal,
                     _ => {
                         return Some(Err(VmError::new(format!(

src/vm/execute.rs

@@ -1393,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:2326`); the
+                // `src/compiler/mod.rs:2327`); the
                 // `BinOp::And | BinOp::Or => unreachable!()` at
-                // `compiler/mod.rs:2358` confirms no other emission
+                // `compiler/mod.rs:2359` confirms no other emission
                 // path exists. Match the LoopSetup precedent
                 // (execute.rs:Op::LoopSetup) and crash loudly on
                 // accidental re-emission rather than silently
@@ -1411,7 +1411,7 @@ impl Vm {
             }
             Op::Or => {
                 // See Op::And above — same rationale. Compiler emits
-                // `JumpIfTrue` short-circuit at compiler/mod.rs:2337;
+                // `JumpIfTrue` short-circuit at compiler/mod.rs:2338;
                 // `Op::Or` is reserved-but-never-emitted.
                 unreachable!(
                     "compiler always lowers BinOp::And/Or to JumpIfFalse/JumpIfTrue \

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:2326`). Hand-emitting Op::And here
+/// circuit (see `compiler/mod.rs:2327`). 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:2337`, so direct emission of Op::Or
+/// circuit at `compiler/mod.rs:2338`, so direct emission of Op::Or
 /// must crash with the unreachable! panic.
 #[test]
 #[should_panic(

tests/round73_row_narrowing_tests.rs

@@ -22,6 +22,25 @@
 
 use silt::typechecker;
 use silt::types::Severity;
+use std::process::Command;
+
+/// Drive the full public pipeline (lexer → parser → typechecker →
+/// compiler → VM) the same way `silt run` does, by spawning the built
+/// `silt` binary. Returns `(stdout, stderr, success)` so callers can
+/// assert that a type-rejected program never reaches runtime.
+fn run_silt_raw(label: &str, src: &str) -> (String, String, bool) {
+    let tmp = std::env::temp_dir().join(format!("silt_round73_row_{label}.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();
+    (stdout, stderr, out.status.success())
+}
 
 fn type_errors(input: &str) -> Vec<String> {
     let tokens = silt::lexer::Lexer::new(input)
@@ -148,4 +167,30 @@ fn main() {
             .any(|e| e.message.contains("zzznosuchfield")),
         "typecheck error must mention the bogus field 'zzznosuchfield'; got: {hard_errors:?}"
     );
+
+    // Compile-stage guard (the claim this lock makes): drive the SAME
+    // source through the full public pipeline `silt run` uses. The CLI
+    // gates on hard type errors (src/cli/pipeline.rs `compile_file_with_options`:
+    // it `process::exit(1)`s before handing functions to the VM), so the
+    // rejected program must EXIT NON-ZERO, never reach runtime, and print
+    // the field error to stderr. If a future regression let the compiler
+    // hand out bytecode despite the typecheck error, the program would run
+    // and crash — this assertion catches that.
+    let (stdout, stderr, ok) = run_silt_raw("bogus_field_blocks_compile", src);
+    assert!(
+        !ok,
+        "silt run must reject the bogus-field program before runtime; \
+         instead it succeeded — the compiler handed out runnable bytecode. \
+         stdout={stdout:?}, stderr={stderr:?}"
+    );
+    assert!(
+        stderr.contains("zzznosuchfield"),
+        "the pipeline must surface the field error on stderr; \
+         stdout={stdout:?}, stderr={stderr:?}"
+    );
+    assert!(
+        stdout.is_empty(),
+        "a type-rejected program must never produce runtime output; \
+         got stdout={stdout:?}"
+    );
 }