Unify condition lowering logic across Pattern 2/4 with trait-based API. New infrastructure: - condition_lowering_box.rs: ConditionLoweringBox trait + ConditionContext (293 lines) - ExprLowerer implements ConditionLoweringBox trait (+51 lines) Pattern migrations: - Pattern 2 (loop_with_break_minimal.rs): Use trait API - Pattern 4 (loop_with_continue_minimal.rs): Use trait API Benefits: - Unified condition lowering interface - Extensible for future lowering strategies - Clean API boundary between patterns and lowering logic - Zero code duplication Test results: 911/911 PASS (+2 new tests) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
6.5 KiB
Phase 243-EX: Investigation Summary (Quick Reference)
Status: ✅ Investigation Complete Test Status: 909 tests PASS (maintained) Date: 2025-12-11
TL;DR
Found: 5 major refactoring opportunities in JoinIR lowering (74 files, 23K lines)
Top Priority: ConditionLoweringBox (⭐⭐⭐) - Unify 19 files touching condition logic
Next Steps: Phase 244-248 implementation (4-6 days estimated)
Quick Stats
| Metric | Value |
|---|---|
| Total Files | 74 |
| Total Lines | 23,183 |
| Root Files | 52 (too flat!) |
| Large Files (>500 lines) | 15 (41% of code) |
| TODO Items | 9 |
| Subdirectories | 5 |
Top 3 Opportunities
1. ConditionLoweringBox (⭐⭐⭐)
- Impact: 19 files affected
- Effort: Medium (1,639 lines to reorganize)
- Value: High (single API for all condition lowering)
- Files: condition_lowerer.rs, condition_to_joinir.rs, condition_env.rs, condition_pattern.rs, condition_var_extractor.rs
- Next: Phase 244
2. CarrierManagerBox (⭐⭐)
- Impact: 7 files affected
- Effort: Low (extends Phase 228)
- Value: Medium (consolidates lifecycle)
- Files: carrier_info.rs, carrier_update_emitter.rs, inline_boundary.rs
- Next: Phase 245
3. Module Reorganization (⭐⭐)
- Impact: All 74 files
- Effort: Large (but low risk)
- Value: Medium (navigation + clarity)
- Result: 52 root files → 7 directories
- Next: Phase 246
Recommended Roadmap
| Phase | Goal | Effort | Risk | Priority |
|---|---|---|---|---|
| 244 | ConditionLoweringBox | 1-2 days | Medium | ⭐⭐⭐ |
| 245 | CarrierManagerBox | 0.5-1 day | Low | ⭐⭐ |
| 246 | Module Reorganization | 1-2 days | Low | ⭐⭐ |
| 247 | PatternDetectorBox | 1 day | Medium | ⭐⭐ |
| 248 | Legacy Cleanup | 0.5 day | Low | ⭐ |
Total: 4-6 days
Key Findings
Fragmentation Issues
- Condition Logic: Scattered across 19 files (1,639 lines)
- Carrier Logic: Spread across 7 files (2,359 lines)
- Pattern Detection: Split across 4 files (1,300 lines)
Existing Infrastructure (Ready to Build On)
- ✅ Phase 227: CarrierRole (LoopState vs ConditionOnly)
- ✅ Phase 228: CarrierInit (FromHost vs BoolConst)
- ✅ Phase 231: ExprLowerer + ScopeManager trait
- ✅ Phase 33-10: ExitLineReconnector (already boxified!)
No Blocking Issues
- ✅ No circular dependencies
- ✅ Clean dependency flow (top → bottom)
- ✅ All 909 tests passing
Proposed Module Structure (Phase 246)
Before (Flat)
src/mir/join_ir/lowering/
├── *.rs (52 files in root) ← Hard to navigate
└── (5 subdirectories)
After (Hierarchical)
src/mir/join_ir/lowering/
├── core/ # Core lowering boxes
│ ├── condition_lowering/
│ ├── carrier_management/
│ └── exit_line/
├── infrastructure/ # Shared utilities
├── patterns/ # Pattern-specific lowering
│ ├── detection/
│ ├── loop_patterns/
│ └── if_lowering/
├── specialized/ # Function-specific lowering
└── generic_case_a/ # Generic Case A
Improvement: 86% reduction in root-level files (52 → 7)
Risk Assessment
| Candidate | Risk Level | Mitigation |
|---|---|---|
| ConditionLoweringBox | Medium | Backward compat shims, gradual migration |
| CarrierManagerBox | Low | Extends existing API (no breaking changes) |
| Module Reorganization | Low | Pure reorganization (no logic changes) |
| PatternDetectorBox | Medium | API unification requires coordination |
Overall Risk: Low-Medium (mitigated by test-driven approach)
Dependencies to Watch
High-Dependency Modules (10+ users)
condition_lowerer.rs→ 10 files depend on itcarrier_info.rs→ 7 files depend on itjoin_value_space.rs→ 20+ files depend on it
Critical Path (6 levels deep)
Pattern 3 → ExprLowerer → ScopeManager → ConditionEnv → CarrierInfo → InlineBoundary
Quick Wins (Low-Hanging Fruit)
-
Remove bool_expr_lowerer.rs (446 lines, unused)
- TODO comment: "Consider removal"
- All tests commented out
- Superseded by ExprLowerer (Phase 231)
- Action: Remove in Phase 248
-
Consolidate TODO items (9 scattered)
- Collect in single tracking issue
- Prioritize by impact
-
Update outdated comments
- Phase 242-EX-A removed loop_with_if_phi_minimal.rs
- Update references in mod.rs
Success Metrics
Phase 244 (ConditionLoweringBox)
- 19 files migrated to new API
- 5 new modules created
- 909 tests still passing
- Backward compat shims in place
Phase 245 (CarrierManagerBox)
- 7 files using unified manager
- 3 methods consolidated (init, update, exit)
- All carrier tests passing
Phase 246 (Module Reorganization)
- 52 root files → 7 directories
- All imports updated
- Documentation updated
- 909 tests still passing
Phase 247 (PatternDetectorBox)
- 3 detectors unified
- Single API for all pattern detection
- Pattern routing simplified
Phase 248 (Legacy Cleanup)
- Backward compat shims removed
- bool_expr_lowerer.rs removed
- All TODOs addressed or tracked
Documents Generated
-
phase243-ex-refactoring-opportunities.md (12K words)
- Full analysis with implementation sketches
- Priority scoring
- Risk assessment
- Recommended roadmap
-
phase243-ex-dependency-graph.md (8K words)
- Visual dependency graphs
- Module relationships
- Impact analysis by phase
-
phase243-ex-summary.md (this document)
- Quick reference
- TL;DR for decision-makers
Next Steps
- Review documents with stakeholders
- Approve Phase 244 implementation plan
- Start Phase 244: ConditionLoweringBox
- Track progress in CURRENT_TASK.md
Questions?
- Full report: phase243-ex-refactoring-opportunities.md
- Dependency analysis: phase243-ex-dependency-graph.md
- JoinIR architecture: joinir-architecture-overview.md
Status: Ready for Phase 244 implementation! 🚀
Confidence: High (clean dependency graph, solid foundation, test coverage)
Risk: Low-Medium (mitigated by gradual migration + backward compat)
End of Phase 243-EX Summary