Michael Schimmel
74 lines
4.7 KiB
Markdown
74 lines
4.7 KiB
Markdown
# 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.
|