design(joinir): Phase 73 - ScopeManager BindingId Migration Design + PoC
Phase 73 plans migration from name-based to BindingId-based scope management in JoinIR lowering, aligning with MIR's lexical scope model. Design decision: Option A (Parallel BindingId Layer) with gradual migration. Migration roadmap: Phases 74-77, ~8-12 hours total, zero production impact. Changes: - phase73-scope-manager-design.md: SSOT design (~700 lines) - phase73-completion-summary.md: Deliverables summary - phase73-index.md: Navigation index - scope_manager_bindingid_poc/: Working PoC (437 lines, dev-only) Tests: 6/6 PoC tests PASS, lib 950/950 PASS Implementation: Parallel layer (no changes to existing code paths) 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
This commit is contained in:
nyash-codex
310
docs/development/current/main/phase73-completion-summary.md
Normal file
310
docs/development/current/main/phase73-completion-summary.md
Normal file
@ -0,0 +1,310 @@
|
||||
# Phase 73: Completion Summary
|
||||
|
||||
**Date**: 2025-12-13
|
||||
**Status**: ✅ Complete (Design Phase)
|
||||
**Scope**: JoinIR ScopeManager → BindingId-Based Design
|
||||
|
||||
---
|
||||
|
||||
## Deliverables
|
||||
|
||||
### 1. Design Document (SSOT)
|
||||
**File**: `docs/development/current/main/phase73-scope-manager-design.md`
|
||||
|
||||
**Contents**:
|
||||
- ✅ Current state analysis (MIR + JoinIR scope systems)
|
||||
- ✅ Problem identification (name-based vs BindingId mismatch)
|
||||
- ✅ Proposed architecture (Option A: Parallel BindingId Layer)
|
||||
- ✅ Integration with MIR Builder (binding_map additions)
|
||||
- ✅ Migration path (Phases 74-77 roadmap)
|
||||
- ✅ Example scenarios (shadowing, promoted variables)
|
||||
|
||||
**Key Insights**:
|
||||
- MIR builder uses **BindingId** for lexical scope tracking (Phase 68-69)
|
||||
- JoinIR lowering uses **name-based** lookup (fragile for shadowing)
|
||||
- Naming convention hacks (`is_digit_pos`, `is_ch_match`) can be replaced with BindingId maps
|
||||
- Gradual migration strategy minimizes risk
|
||||
|
||||
---
|
||||
|
||||
### 2. Proof-of-Concept Implementation
|
||||
**File**: `src/mir/join_ir/lowering/scope_manager_bindingid_poc/mod.rs`
|
||||
|
||||
**Status**: ✅ All tests passing (6/6)
|
||||
**Feature Gate**: `#[cfg(feature = "normalized_dev")]` (dev-only)
|
||||
|
||||
**Implemented Structures**:
|
||||
- `BindingId` type wrapper
|
||||
- `ConditionEnvV2` (parallel name + BindingId lookup)
|
||||
- `CarrierInfoV2` (BindingId-based promotion tracking)
|
||||
- `ScopeManagerV2` trait + `Pattern2ScopeManagerV2` implementation
|
||||
|
||||
**Test Coverage**:
|
||||
```
|
||||
test mir::join_ir::lowering::scope_manager_bindingid_poc::tests::test_condition_env_v2_basic ... ok
|
||||
test mir::join_ir::lowering::scope_manager_bindingid_poc::tests::test_shadowing_simulation ... ok
|
||||
test mir::join_ir::lowering::scope_manager_bindingid_poc::tests::test_promoted_binding_resolution ... ok
|
||||
test mir::join_ir::lowering::scope_manager_bindingid_poc::tests::test_scope_manager_v2_binding_lookup ... ok
|
||||
test mir::join_ir::lowering::scope_manager_bindingid_poc::tests::test_scope_manager_v2_promoted_lookup ... ok
|
||||
test mir::join_ir::lowering::scope_manager_bindingid_poc::tests::test_unified_lookup_fallback ... ok
|
||||
```
|
||||
|
||||
**Key Validations**:
|
||||
- ✅ Parallel lookup (BindingId + name fallback) works
|
||||
- ✅ Shadowing simulation (multiple bindings for same name)
|
||||
- ✅ Promoted variable resolution (BindingId → BindingId mapping)
|
||||
- ✅ Unified lookup with graceful fallback
|
||||
|
||||
---
|
||||
|
||||
## Design Highlights
|
||||
|
||||
### Problem Statement
|
||||
**Before (Current)**:
|
||||
```rust
|
||||
// JoinIR lowering (name-based)
|
||||
env.get("digit_pos") → searches for "is_digit_pos" via naming convention
|
||||
→ fragile, breaks if naming convention changes
|
||||
```
|
||||
|
||||
**After (Phase 76+)**:
|
||||
```rust
|
||||
// JoinIR lowering (BindingId-based)
|
||||
env.get_by_binding(BindingId(5)) → resolves promoted_bindings[BindingId(5)] = BindingId(10)
|
||||
→ type-safe, no string matching
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Proposed Architecture (Option A)
|
||||
|
||||
**Gradual Migration Strategy**:
|
||||
1. **Phase 74**: Add BindingId infrastructure (binding_map, binding_to_join)
|
||||
2. **Phase 75**: Migrate Pattern 1 (simple, no carriers)
|
||||
3. **Phase 76**: Migrate Pattern 2 (carrier promotion)
|
||||
4. **Phase 77**: Migrate Pattern 3-4, remove legacy code
|
||||
|
||||
**Backward Compatibility**:
|
||||
- New fields added alongside existing name-based maps
|
||||
- Legacy code continues to work during transition
|
||||
- Fallback mechanism ensures no breakage
|
||||
|
||||
---
|
||||
|
||||
### Integration Points
|
||||
|
||||
#### MirBuilder Changes (Phase 74)
|
||||
```rust
|
||||
pub struct MirBuilder {
|
||||
pub variable_map: HashMap<String, ValueId>, // Existing (SSA conversion)
|
||||
pub binding_map: HashMap<String, BindingId>, // NEW (lexical scope)
|
||||
next_binding_id: u32, // NEW (allocator)
|
||||
}
|
||||
```
|
||||
|
||||
#### ConditionEnv Changes (Phase 74)
|
||||
```rust
|
||||
pub struct ConditionEnv {
|
||||
name_to_join: BTreeMap<String, ValueId>, // Legacy (keep)
|
||||
binding_to_join: BTreeMap<BindingId, ValueId>, // NEW (Phase 74+)
|
||||
name_to_binding: BTreeMap<String, BindingId>, // NEW (shadowing)
|
||||
}
|
||||
```
|
||||
|
||||
#### CarrierInfo Changes (Phase 76)
|
||||
```rust
|
||||
pub struct CarrierInfo {
|
||||
promoted_loopbodylocals: Vec<String>, // Legacy (Phase 224)
|
||||
promoted_bindings: BTreeMap<BindingId, BindingId>, // NEW (Phase 76+)
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## No Production Code Changes
|
||||
|
||||
**Confirmation**:
|
||||
- ✅ No changes to `src/mir/builder.rs`
|
||||
- ✅ No changes to `src/mir/join_ir/lowering/*.rs` (except mod.rs for PoC)
|
||||
- ✅ PoC is feature-gated (`normalized_dev` only)
|
||||
- ✅ All existing tests still pass
|
||||
|
||||
**Modified Files**:
|
||||
1. `docs/development/current/main/phase73-scope-manager-design.md` (new)
|
||||
2. `docs/development/current/main/phase73-completion-summary.md` (new)
|
||||
3. `src/mir/join_ir/lowering/scope_manager_bindingid_poc/mod.rs` (new, dev-only)
|
||||
4. `src/mir/join_ir/lowering/mod.rs` (1 line added for PoC module)
|
||||
|
||||
---
|
||||
|
||||
## Migration Roadmap
|
||||
|
||||
### Phase 74: Infrastructure (Estimated 2-3 hours)
|
||||
**Goal**: Add BindingId tracking without breaking existing code
|
||||
|
||||
**Tasks**:
|
||||
- [ ] Add `binding_map` to `MirBuilder`
|
||||
- [ ] Add `binding_to_join` to `ConditionEnv`
|
||||
- [ ] Update `declare_local_in_current_scope` to return `BindingId`
|
||||
- [ ] Add BindingId allocator tests
|
||||
|
||||
**Acceptance**: All existing tests pass, BindingId populated
|
||||
|
||||
---
|
||||
|
||||
### Phase 75: Pattern 1 Pilot (Estimated 1-2 hours)
|
||||
**Goal**: Prove BindingId integration with simplest pattern
|
||||
|
||||
**Tasks**:
|
||||
- [ ] Update `CarrierInfo::from_variable_map` to accept `binding_map`
|
||||
- [ ] Migrate Pattern 1 lowering to use BindingId
|
||||
- [ ] Add E2E test with BindingId
|
||||
|
||||
**Acceptance**: Pattern 1 uses BindingId, legacy fallback works
|
||||
|
||||
---
|
||||
|
||||
### Phase 76: Pattern 2 Carrier Promotion (Estimated 2-3 hours)
|
||||
**Goal**: Eliminate naming convention hacks
|
||||
|
||||
**Tasks**:
|
||||
- [ ] Add `promoted_bindings: BTreeMap<BindingId, BindingId>` to `CarrierInfo`
|
||||
- [ ] Update `resolve_promoted_join_id` to use BindingId
|
||||
- [ ] Migrate Pattern 2 lowering
|
||||
|
||||
**Acceptance**: DigitPos pattern works without string matching
|
||||
|
||||
---
|
||||
|
||||
### Phase 77: Pattern 3-4 + Cleanup (Estimated 2-3 hours)
|
||||
**Goal**: Complete migration, remove legacy code
|
||||
|
||||
**Tasks**:
|
||||
- [ ] Migrate Pattern 3 (multi-carrier)
|
||||
- [ ] Migrate Pattern 4 (generic case A)
|
||||
- [ ] Remove `name_to_join`, `promoted_loopbodylocals` (legacy fields)
|
||||
|
||||
**Acceptance**: All patterns BindingId-only, full test suite passes
|
||||
|
||||
---
|
||||
|
||||
## Open Questions (for Future Phases)
|
||||
|
||||
### Q1: BindingId Scope (Per-Function vs Global)
|
||||
**Current Assumption**: Per-function (like ValueId)
|
||||
|
||||
**Reasoning**:
|
||||
- Each function has independent binding scope
|
||||
- No cross-function binding references
|
||||
- Simpler allocation (no global state)
|
||||
|
||||
**Alternative**: Global BindingId pool (for Phase 63 ownership analysis integration)
|
||||
|
||||
---
|
||||
|
||||
### Q2: Captured Variable Handling
|
||||
**Proposed**: Add `binding_id` to `CapturedVar`
|
||||
|
||||
```rust
|
||||
pub struct CapturedVar {
|
||||
name: String,
|
||||
host_id: ValueId,
|
||||
host_binding: BindingId, // Phase 73+ (NEW)
|
||||
is_immutable: bool,
|
||||
}
|
||||
```
|
||||
|
||||
**Impact**: Requires updating `function_scope_capture` module
|
||||
|
||||
---
|
||||
|
||||
### Q3: Performance Impact
|
||||
**Concern**: Dual maps (`binding_to_join` + `name_to_join`) double memory
|
||||
|
||||
**Mitigation**:
|
||||
- Phase 74-76: Both maps active (transition period)
|
||||
- Phase 77: Remove `name_to_join` after migration
|
||||
- BTreeMap overhead minimal (<10 variables per loop typically)
|
||||
|
||||
**Measurement**: Profile after Phase 74 implementation
|
||||
|
||||
---
|
||||
|
||||
## Success Criteria (Phase 73)
|
||||
|
||||
### Design Document ✅
|
||||
- [x] Current state analysis (MIR + JoinIR)
|
||||
- [x] Proposed architecture (Option A)
|
||||
- [x] Integration points (MirBuilder changes)
|
||||
- [x] Migration path (Phases 74-77)
|
||||
- [x] Example scenarios
|
||||
|
||||
### Proof-of-Concept ✅
|
||||
- [x] BindingId type + structures
|
||||
- [x] Parallel lookup (BindingId + name)
|
||||
- [x] Shadowing simulation test
|
||||
- [x] Promoted variable resolution test
|
||||
- [x] All tests passing (6/6)
|
||||
|
||||
### No Production Impact ✅
|
||||
- [x] Feature-gated (`normalized_dev`)
|
||||
- [x] No production code changes
|
||||
- [x] Existing tests unaffected
|
||||
|
||||
### Documentation ✅
|
||||
- [x] SSOT design document
|
||||
- [x] Completion summary (this document)
|
||||
- [x] PoC code comments
|
||||
|
||||
---
|
||||
|
||||
## References
|
||||
|
||||
### Related Phases
|
||||
- **Phase 68-69**: MIR lexical scope + shadowing (existing)
|
||||
- **Phase 63**: Ownership analysis (dev-only, uses BindingId)
|
||||
- **Phase 231**: ScopeManager trait (current implementation)
|
||||
- **Phase 224**: Promoted LoopBodyLocal (naming convention hacks)
|
||||
|
||||
### Key Files
|
||||
- `docs/development/current/main/phase73-scope-manager-design.md` (SSOT)
|
||||
- `src/mir/join_ir/lowering/scope_manager_bindingid_poc/mod.rs` (PoC)
|
||||
- `src/mir/builder/vars/lexical_scope.rs` (MIR lexical scope)
|
||||
- `src/mir/join_ir/lowering/scope_manager.rs` (current ScopeManager)
|
||||
- `src/mir/join_ir/lowering/carrier_info.rs` (current CarrierInfo)
|
||||
|
||||
---
|
||||
|
||||
## Estimated Total Effort (Phases 74-77)
|
||||
|
||||
| Phase | Task | Hours |
|
||||
|-------|------|-------|
|
||||
| 74 | Infrastructure | 2-3 |
|
||||
| 75 | Pattern 1 Pilot | 1-2 |
|
||||
| 76 | Pattern 2 Promotion | 2-3 |
|
||||
| 77 | Pattern 3-4 + Cleanup | 2-3 |
|
||||
| **Total** | **Full Migration** | **8-12** |
|
||||
|
||||
**Risk Level**: Low (gradual migration, backward compatible)
|
||||
|
||||
---
|
||||
|
||||
## Next Steps
|
||||
|
||||
1. **User Review**: Confirm design makes sense
|
||||
2. **Phase 74 Start**: Implement BindingId infrastructure
|
||||
3. **Iterative Migration**: Phases 75-77 (one pattern at a time)
|
||||
|
||||
---
|
||||
|
||||
## Conclusion
|
||||
|
||||
**Phase 73 Success**: ✅ Design + PoC Complete
|
||||
|
||||
**Key Achievements**:
|
||||
- Comprehensive design document (SSOT for BindingId migration)
|
||||
- Working proof-of-concept (6 tests passing)
|
||||
- Clear migration path (Phases 74-77 roadmap)
|
||||
- No production code impact (feature-gated)
|
||||
|
||||
**Ready for Phase 74**: Infrastructure implementation can begin immediately.
|
||||
Reference in New Issue
Block a user