From 8339ee413e115a6e583809c8aaa7bcc2dc55c5c5 Mon Sep 17 00:00:00 2001 From: Michael Schimmel Date: Mon, 9 Mar 2026 11:49:17 +0100 Subject: [PATCH] Doc: Use exhaustive matching in LambdaCollector --- docs/Analysis_AST_Swallowing_Bugs.md | 73 ++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 docs/Analysis_AST_Swallowing_Bugs.md diff --git a/docs/Analysis_AST_Swallowing_Bugs.md b/docs/Analysis_AST_Swallowing_Bugs.md new file mode 100644 index 0000000..095ad24 --- /dev/null +++ b/docs/Analysis_AST_Swallowing_Bugs.md @@ -0,0 +1,73 @@ +# Analysis: AST Traversal & Symbol Resolution "Swallow" Bugs + +## Overview + +The Myc compiler utilizes several passes that traverse the Abstract Syntax Tree (AST) at different stages (Untyped, Bound/Typed). A recurring pattern in the codebase was the use of non-exhaustive `match` arms, specifically `_ => Ok(node)` or `_ => {}`. + +While convenient, this pattern created "Swallow Traps": when new node types were added to the language (like `Again` for tail-calls or `Pipe` for streaming), these passes would silently ignore them. This led to incomplete symbol registration and binder errors. + +## Identified Issues + +### 1. LambdaCollector (Fixed) +**Location:** `src/ast/compiler/lambda_collector.rs` + +- **Problem:** This pass collects all global function definitions (lambdas) for the Specializer (monomorphization). It used `_ => {}`, missing nodes with children like `Again`, `Pipe`, and `Destructure`. +- **Impact:** Functions defined inside an `again` block or a `pipe` lambda were not registered. The Specializer could not find their AST, leading to "Undefined variable" or specialization failures. +- **Fix:** Refactored `visit` to use exhaustive matching. Every `BoundKind` variant is now explicitly handled. Nodes with children are traversed recursively; leaf nodes are explicitly ignored. + +### 2. Global Discovery Pass (Current Bottleneck) +**Location:** `src/ast/environment.rs` -> `discover_globals` + +- **Problem:** This pass runs on the **Untyped AST** before binding. Its job is to find all `def` and `macro` declarations to populate the global symbol table. +- **Critical Failure:** It currently only matches `UntypedKind::Def`, `UntypedKind::MacroDecl`, and `UntypedKind::Block`. It **misses** declarations hidden inside `Again`, `Pipe`, `If`, and `Lambda` bodies. +- **Impact:** Even if the `LambdaCollector` is fixed, the `Binder` won't know a symbol is global if `discover_globals` didn't find it first. This explains why tests for `Again` and `Pipe` still fail with "Undefined variable". + +### 3. Destructuring Def Bug (Discovered) +**Location:** `src/ast/environment.rs` -> `discover_globals` + +- **Observation:** `discover_globals` only registers a global if the `Def` target is a single `Identifier`. +- **Bug:** `(def [a b] [1 2])` is currently **ignored** for global registration. These variables are treated as local or remain undefined in the global scope. + +## 4. VM: Evaluation & Pattern Matching +**Location:** `src/ast/vm.rs` + +- **`eval_core` (L309):** The main execution loop. It covers most variants but lacks an exhaustive check. New `BoundKind` variants will not be caught at compile-time. +- **`unpack` (L994):** Used for parameter destructuring. + ```rust + match &pattern.kind { + UntypedKind::Identifier(sym) => { ... } + UntypedKind::Tuple { elements } => { ... } + _ => Err("Invalid node in parameter pattern".to_string()), + } + ``` + Any new `UntypedKind` (like Record destructuring) will trigger a runtime error instead of a compile-time exhaustiveness check. +- **`after_eval` (Observer):** Uses `_ => {}`, meaning new control-flow nodes might lack proper debug/trace logging. + +## 5. Optimizer: Engine & Inlining +**Location:** `src/ast/compiler/optimizer/engine.rs`, `inliner.rs` + +- **`flatten_tuple` (L693 in engine.rs):** + ```rust + match &node.kind { + BoundKind::Tuple { elements } => { ... } + BoundKind::Nop => {} + _ => into.push(node), + } + ``` + Incorrectly assumes that any unknown node is atomic. If a new sequence-like node is added, it won't be flattened, potentially breaking assumptions in the Optimizer's call-site logic. +- **Inliner Analysis:** Uses `_ => false` for inlinability checks. This prevents new node types from being inlined, leading to sub-optimal code without any warning to the developer. + +## Regression Testing + +A suite of integration tests has been added to `src/integration_test.rs` to reveal these gaps: + +- `test_lambda_collector_misses_again_arguments`: (Currently FAILED) +- `test_lambda_collector_misses_pipe_lambda`: (Currently FAILED) +- `test_lambda_collector_misses_destructure_value`: (Now PASSED after LambdaCollector fix) +- `test_lambda_collector_misses_get_field_rec`: (PASSED, but requires verification against aggressive optimization) + +## Next Steps + +1. **Refactor `discover_globals`**: Apply the same exhaustive matching strategy used in the `LambdaCollector` to `src/ast/environment.rs`. +2. **Implement Pattern Traversal**: Update `discover_globals` to extract all identifiers from destructuring patterns (e.g., in `Def [a b] ...`). +3. **Audit Other Passes**: Systematically check `type_checker.rs`, `optimizer/engine.rs`, and `vm.rs` for remaining `_ =>` swallow traps on node kinds.