|
| 1 | +# Sprint 50.1: MIR Copy Propagation Soundness Hotfix |
| 2 | + |
| 3 | +## Goal |
| 4 | + |
| 5 | +Fix the semantic unsoundness in MIR copy propagation introduced in Sprint 50 by enforcing reassignment and control-flow safety. Preserve optimization benefits while guaranteeing behavior equivalence with `--no-optimize`. |
| 6 | + |
| 7 | +--- |
| 8 | + |
| 9 | +## Why This Sprint Exists |
| 10 | + |
| 11 | +Current copy propagation in `src/mir/optimize.rs` builds a global substitution map and rewrites uses without proving safety across: |
| 12 | + |
| 13 | +1. source local reassignments, and |
| 14 | +2. control-flow joins/branches. |
| 15 | + |
| 16 | +This can rewrite a use to a newer value than the original copied value, producing incorrect runtime behavior. |
| 17 | + |
| 18 | +--- |
| 19 | + |
| 20 | +## Scope |
| 21 | + |
| 22 | +### 50.1.1 Make Copy Propagation Sound by Construction (P0) |
| 23 | + |
| 24 | +**Files:** `src/mir/optimize.rs` |
| 25 | + |
| 26 | +Replace current global substitution strategy with a conservative, semantics-safe approach. |
| 27 | + |
| 28 | +### Required implementation constraints |
| 29 | + |
| 30 | +Implement **block-local forward propagation only** for Phase 1: |
| 31 | + |
| 32 | +1. Only propagate from `Assign(dest, Use(Copy(src)|Local(src)|Move(src)))`. |
| 33 | +2. `dest` must be a compiler temp (`LocalDecl.name == None`). |
| 34 | +3. Propagation candidates are valid **only within the same basic block**. |
| 35 | +4. Stop propagation when either `src` or `dest` is assigned again. |
| 36 | +5. Do not propagate through `Ref`, `Discriminant`, `EnumField`, or contract-sensitive contexts. |
| 37 | +6. Do not propagate into/through terminators if safety cannot be proven for block-local ordering. |
| 38 | +7. Never create/keep cyclic substitutions. |
| 39 | + |
| 40 | +This is intentionally conservative. Correctness is mandatory; aggressiveness can return in Sprint 51 with data-flow analysis. |
| 41 | + |
| 42 | +### Acceptance criteria |
| 43 | + |
| 44 | +1. No behavior divergence between optimized and non-optimized execution on reassignment/control-flow edge cases. |
| 45 | +2. Copy propagation still optimizes trivial safe chains inside a block. |
| 46 | +3. Existing optimizer tests pass. |
| 47 | + |
| 48 | +--- |
| 49 | + |
| 50 | +### 50.1.2 Add Regression Tests for the Bug Class (P0) |
| 51 | + |
| 52 | +**Files:** `src/mir/optimize.rs` (test module), `tests/cli_tests.rs`, optionally `tests/forma/test_optimization.forma` |
| 53 | + |
| 54 | +Add targeted tests that failed conceptually under old logic: |
| 55 | + |
| 56 | +1. **Reassignment barrier test (unit):** |
| 57 | + - copy temp from `x`, |
| 58 | + - reassign `x`, |
| 59 | + - ensure return/use still points to copied temp semantics. |
| 60 | +2. **Branch/join test (unit):** |
| 61 | + - copy in one block, |
| 62 | + - reassign source in successor, |
| 63 | + - verify no illegal substitution through join. |
| 64 | +3. **CLI equivalence regression (integration):** |
| 65 | + - run fixture with and without `--no-optimize`, |
| 66 | + - assert identical stdout and exit code. |
| 67 | + |
| 68 | +### Acceptance criteria |
| 69 | + |
| 70 | +1. At least 3 new tests directly covering the unsoundness class. |
| 71 | +2. New tests fail on old algorithm and pass with fix. |
| 72 | + |
| 73 | +--- |
| 74 | + |
| 75 | +### 50.1.3 Validate Optimizer Invariants Post-Pass (P1) |
| 76 | + |
| 77 | +**Files:** `src/mir/optimize.rs` |
| 78 | + |
| 79 | +After each optimization round (or in debug/test builds), run `validate_mir()` and fail fast in tests if invariants break. |
| 80 | + |
| 81 | +### Acceptance criteria |
| 82 | + |
| 83 | +1. Optimization tests assert `validate_mir(program).is_empty()`. |
| 84 | +2. Any malformed MIR from pass bugs is caught immediately. |
| 85 | + |
| 86 | +--- |
| 87 | + |
| 88 | +### 50.1.4 Sprint-Report Accuracy Cleanup (P2) |
| 89 | + |
| 90 | +**Files:** sprint notes/docs only as needed |
| 91 | + |
| 92 | +Clarify .forma suite status from this environment: |
| 93 | +- `test_contract_errors.forma` is intentional negative. |
| 94 | +- `test_tcp.forma` may be environment-dependent and not introduced by Sprint 50. |
| 95 | + |
| 96 | +Do not block merge on infra-dependent networking tests unless regression is proven against `--no-optimize`. |
| 97 | + |
| 98 | +### Acceptance criteria |
| 99 | + |
| 100 | +1. Sprint summary language is precise and non-misleading. |
| 101 | + |
| 102 | +--- |
| 103 | + |
| 104 | +## Implementation Notes |
| 105 | + |
| 106 | +Recommended minimal algorithm (safe): |
| 107 | + |
| 108 | +1. For each basic block, scan statements top-to-bottom. |
| 109 | +2. Maintain a local map `dest -> src` valid for current scan window. |
| 110 | +3. On assignment to any local `l`, remove mappings where `dest == l` or `src == l`. |
| 111 | +4. Rewrite operands only using currently valid mappings. |
| 112 | +5. Do not carry mapping across block boundaries. |
| 113 | + |
| 114 | +This gives deterministic safety without requiring dominance/use-def infrastructure. |
| 115 | + |
| 116 | +--- |
| 117 | + |
| 118 | +## Verification Plan |
| 119 | + |
| 120 | +```bash |
| 121 | +# Core |
| 122 | +cargo fmt --all -- --check |
| 123 | +cargo clippy --all-targets -- -D warnings |
| 124 | +cargo test --all |
| 125 | + |
| 126 | +# Targeted optimizer tests |
| 127 | +cargo test mir::optimize::tests -- --nocapture |
| 128 | + |
| 129 | +# Optimization equivalence check |
| 130 | +./target/release/forma run --allow-all tests/forma/test_optimization.forma > /tmp/opt_on.out |
| 131 | +./target/release/forma run --allow-all --no-optimize tests/forma/test_optimization.forma > /tmp/opt_off.out |
| 132 | +diff -u /tmp/opt_off.out /tmp/opt_on.out |
| 133 | +``` |
| 134 | + |
| 135 | +Optional reproduction fixture (for manual validation): |
| 136 | + |
| 137 | +```forma |
| 138 | +f main() -> Int |
| 139 | + x := 1 |
| 140 | + z := if true then x else 0 |
| 141 | + x := 2 |
| 142 | + if z == 1 then 0 else 99 |
| 143 | +``` |
| 144 | + |
| 145 | +Expected exit code is identical with and without optimization. |
| 146 | + |
| 147 | +--- |
| 148 | + |
| 149 | +## Out of Scope |
| 150 | + |
| 151 | +1. Global data-flow copy propagation across blocks. |
| 152 | +2. Dominator tree construction. |
| 153 | +3. Dead store elimination. |
| 154 | +4. CSE/inlining/loop opts. |
| 155 | + |
| 156 | +--- |
| 157 | + |
| 158 | +## Definition of Done |
| 159 | + |
| 160 | +1. Copy propagation no longer performs unsound cross-assignment or cross-flow rewrites. |
| 161 | +2. New regression tests for reassignment + branch/join cases are present and passing. |
| 162 | +3. `cargo test --all` passes. |
| 163 | +4. CLI output/exit equivalence for optimization fixture remains identical (`opt` vs `--no-optimize`). |
| 164 | +5. MIR invariants are validated after optimization in tests. |
| 165 | + |
0 commit comments