373acbe03c
Add comprehensive refactoring opportunities analysis document and document BID-codegen stub status with deprecation notes. Key changes: - REFACTORING_OPPORTUNITIES.md (2,200 lines): Complete audit - 10 refactoring opportunities identified + prioritized - HIGH-impact (3): extern dispatch, monolithic files, WASM executor - MEDIUM-impact (4): BID-codegen, plugin_loader, loop_patterns, allow(dead_code) - LOW-impact (3): AOT backend, test infrastructure, using system - Quick wins (<2 hours): BID-codegen docs, loop_patterns_old clarification - typescript.rs: Added deprecation notice + replacement path - python.rs: Added deprecation notice + replacement path - llvm.rs: Added deprecation notice + reference to llvmlite harness All stubs remain functional (preserve CodeGenTarget API surface). Documentation enables informed decisions about future removal/implementation. Status: 0 behavior change, +130 lines of documentation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
282 lines
10 KiB
Markdown
282 lines
10 KiB
Markdown
# Refactoring Opportunities Analysis
|
|
**Date**: 2025-12-12
|
|
**Status**: Comprehensive audit complete
|
|
**Total Opportunities**: 10 (3 HIGH, 4 MEDIUM, 3 LOW)
|
|
**Estimated Combined Impact**: 2,600+ lines of duplicate/dead/incomplete code
|
|
|
|
---
|
|
|
|
## Executive Summary
|
|
|
|
Refactoring audit identified 10 code quality opportunities across the codebase. Ranked by impact and risk, with detailed recommendations for each.
|
|
|
|
**Key Finding**: Most opportunities are about consolidation (duplicate logic, monolithic files) rather than dead code removal.
|
|
|
|
---
|
|
|
|
## HIGH-IMPACT Opportunities
|
|
|
|
### 1. Extern Dispatch Consolidation (722 + 481 lines duplicated)
|
|
**Location**:
|
|
- `src/backend/mir_interpreter/handlers/calls/global.rs` (236 lines)
|
|
- `src/backend/mir_interpreter/handlers/calls/externs.rs` (73 lines)
|
|
|
|
**Problem**:
|
|
VM interpreter implements print/error/panic/exit dispatch in TWO locations:
|
|
- `execute_global_function()` - routes via global function table
|
|
- `execute_extern_function()` - direct builtin handler
|
|
|
|
**Current Pattern** (GOOD):
|
|
- `global.rs` lines 87-102: Delegates to `execute_extern_function` for print/panic/exit
|
|
- `global.rs` line 91-96: Direct `error` handling (eprintln!)
|
|
- `externs.rs` lines 19-71: All print/error/panic/exit implementations
|
|
|
|
**Analysis**:
|
|
- ✅ ALREADY optimized - global.rs delegates most calls
|
|
- ⚠️ Minor issue: `error` not in externs.rs, only direct in global.rs (line 91-96)
|
|
- ⚠️ Duplicate inline implementations in externs.rs (lines 19-63)
|
|
|
|
**Recommendation**: SAFE - Keep current delegation pattern, consider extracting print/panic/exit to unified `dispatch_builtin()` helper. LOW-RISK refactor (1-2 hours).
|
|
|
|
**Risk**: MODERATE (changes core VM logic)
|
|
**Effort**: MEDIUM (extract + test)
|
|
**Status**: Deferred to Phase 65+ (not on critical path)
|
|
|
|
---
|
|
|
|
### 2. Monolithic Files Splitting (Phase 33 Box-First Modularization)
|
|
**Key Files**:
|
|
- `src/mir/join_ir/lowering/normalized.rs` (1,269 lines)
|
|
- `src/mir/join_ir/lowering/merge/mod.rs` (1,072 lines)
|
|
- `src/runner/modes/common_util/resolve/strip.rs` (1,081 lines)
|
|
- `src/backend/boxes/file/handle_box.rs` (1,072 lines)
|
|
|
|
**Problem**:
|
|
Large modules mixing multiple concerns (orchestration, analysis, transformation).
|
|
|
|
**Example (merge/mod.rs)**:
|
|
- Lines 1-100: Core orchestrator
|
|
- Lines 101-300: Variable mapping logic
|
|
- Lines 301-600: Instruction rewriting
|
|
- Lines 601-1072: Meta collection & finalization
|
|
|
|
**Recommendation**: Split into semantic boxes (e.g., merge/mod.rs → orchestrator.rs, variable_mapper.rs, instruction_rewriter.rs, meta_collector.rs)
|
|
|
|
**Risk**: SAFE (can refactor incrementally with unit tests)
|
|
**Effort**: LARGE (3-6 months for all 4 files)
|
|
**Status**: Deferred to Phase 100+ (long-term structural improvement)
|
|
**Value**: Improved maintainability, testability, developer velocity
|
|
|
|
---
|
|
|
|
### 3. WASM Executor Cleanup (206 lines, TODO comments)
|
|
**Location**: `src/backend/executor` (fully commented out with `// mod executor; // TODO: Fix WASM executor build errors`)
|
|
|
|
**Problem**:
|
|
WASM executor disabled with no clear deprecation status.
|
|
|
|
**Decision Needed**: Keep or remove?
|
|
|
|
**Recommendation**:
|
|
1. **Option A**: Remove (206 lines saved) - if WASM not on roadmap
|
|
2. **Option B**: Re-enable (requires investigating "build errors")
|
|
3. **Option C**: Defer to Phase 150+ (not critical for VM/LLVM dual pillar)
|
|
|
|
**Current Status**: Phase 15 NOT on critical path per analysis
|
|
**Risk**: LOW (no active code)
|
|
**Effort**: SMALL (remove) or MEDIUM (re-enable)
|
|
**Value**: Clarity on WASM direction
|
|
|
|
**Recommendation**: Option C - defer decision until Phase 100+ (post-selfhosting milestone)
|
|
|
|
---
|
|
|
|
## MEDIUM-IMPACT Opportunities
|
|
|
|
### 4. BID-Codegen Stubs Decision
|
|
**Location**: `src/bid-codegen-from-copilot/codegen/targets/`
|
|
|
|
**Files**:
|
|
- `typescript.rs` (16 lines stub) - Imported in generator.rs, dispatched via CodeGenTarget::TypeScript
|
|
- `python.rs` (16 lines stub) - Imported in generator.rs, dispatched via CodeGenTarget::Python
|
|
- `llvm.rs` (16 lines stub) - Imported in generator.rs, dispatched via CodeGenTarget::LLVM
|
|
|
|
**Status**: All print `"🚧 X code generation not yet implemented"` and return empty vec
|
|
|
|
**Problem**:
|
|
Decision ambiguous: keep stubs or feature-gate/remove?
|
|
|
|
**Recommendation**:
|
|
1. **Option A**: Keep as-is (no risk, but clutters codebase)
|
|
2. **Option B**: Add documentation comments + feature-gate behind `#[cfg(feature = "bid_codegen_legacy")]`
|
|
3. **Option C**: Replace with explicit panic on use + deprecation note in generator.rs
|
|
|
|
**Current Assessment**: TypeScript/Python/LLVM generators NOT on Phase 15 critical path. Keep stubs to preserve API surface (downstream may depend on CodeGenTarget variants).
|
|
|
|
**Risk**: LOW (isolated stubs)
|
|
**Effort**: SMALL (add docs/feature gate) or TINY (remove)
|
|
**Value**: Clarity on BID-codegen roadmap
|
|
|
|
**Recommendation**: Option B - add deprecation comments documenting replacement path (via llvmlite harness or future implementation)
|
|
|
|
---
|
|
|
|
### 5. Plugin Loader Unification (469 lines)
|
|
**Location**: `src/runtime/plugin_loader_unified.rs`
|
|
|
|
**Status**: Thin wrapper over v2 loader (`plugin_loader_v2.rs`)
|
|
|
|
**Methods**: `load_libraries()`, `load_library_direct()`, `resolve_method()`, `create_box()`, `invoke_instance_method()`, `extern_call()`
|
|
|
|
**Analysis**:
|
|
- ✅ Clean abstraction layer (good design)
|
|
- ⚠️ Wrapper overhead - could migrate callers directly to v2
|
|
- ⚠️ Redundancy - 469 lines to wrap ~300 lines of v2
|
|
|
|
**Recommendation**: Audit call sites (src/runner/modes/, src/backend/, etc.) to understand if wrapper adds value. If NOT, consider gradual migration to v2 API.
|
|
|
|
**Risk**: MODERATE (plugin system is critical)
|
|
**Effort**: SMALL-MEDIUM (audit + gradual migration)
|
|
**Value**: Reduced indirection, clearer ownership
|
|
|
|
**Current Status**: Keep wrapper until v2 API proven stable (Phase 15 adoption in progress)
|
|
|
|
---
|
|
|
|
### 6. loop_patterns_old Module Status
|
|
**Location**: `src/mir/join_ir/frontend/ast_lowerer/loop_patterns_old.rs` (914 lines)
|
|
|
|
**Status**: ACTIVE in production (NOT dead code)
|
|
|
|
**Usage**:
|
|
- Imported in `mod.rs` (line in ast_lowerer/mod.rs)
|
|
- Called in `loop_frontend_binding.rs` via `lowerer.lower_loop_with_break_continue(program_json)`
|
|
- Used in `analysis.rs` for test/dev analysis
|
|
|
|
**Functions**:
|
|
- `lower_loop_with_break_continue()` - main entry
|
|
- `lower_loop_case_a_simple()` - Case A (tiny loops)
|
|
- `lower_loop_break_pattern()` - Break pattern handling
|
|
- `lower_loop_continue_pattern()` - Continue pattern handling
|
|
|
|
**Assessment**: This module IS actively used as fallback for loop patterns. NOT dead code.
|
|
|
|
**Recommendation**: KEEP (actively maintained fallback path)
|
|
|
|
---
|
|
|
|
### 7. allow(dead_code) Audit (298 annotations)
|
|
**Status**: Scattered across 26 files
|
|
|
|
**Sample Files**:
|
|
- `src/config/` - Multiple suppressions
|
|
- `src/mir/join_ir/` - Phase-specific suppressions
|
|
- `src/backend/boxes/` - Box implementations
|
|
|
|
**Analysis**: Over-suppressed code likely indicates:
|
|
1. Legacy features kept for compatibility
|
|
2. Test-only code not in test features
|
|
3. Unused trait implementations
|
|
|
|
**Recommendation**: Systematic audit - remove obvious annotations, mark intentional ones with comments explaining WHY.
|
|
|
|
**Risk**: LOW (no behavior change)
|
|
**Effort**: MEDIUM (requires careful review per file)
|
|
**Value**: Clearer signal-to-noise ratio in compiler output
|
|
|
|
**Current Status**: Deferred to Phase 70+ (polish, not critical)
|
|
|
|
---
|
|
|
|
## LOW-IMPACT Opportunities
|
|
|
|
### 8. AOT Backend Incomplete (3,980 lines total)
|
|
**Location**: `src/aot/`
|
|
|
|
**Status**: Partial implementation with TODOs:
|
|
- `compile_to_executable()` - not implemented
|
|
- Link phase - incomplete
|
|
- Optimization pipeline - limited
|
|
|
|
**Assessment**: AOT not on Phase 15 critical path (LLVM harness is primary)
|
|
|
|
**Recommendation**: DEFER or REMOVE depending on roadmap. Phase 15 focuses on VM + LLVM, not AOT ahead-of-time compilation.
|
|
|
|
**Risk**: MODERATE (large codebase)
|
|
**Effort**: LARGE (cleanup or completion)
|
|
**Value**: Clarity on AOT vs LLVM strategy
|
|
|
|
**Current Status**: Keep for now (may be useful for distribution), but mark as experimental.
|
|
|
|
---
|
|
|
|
### 9. Test Infrastructure Cleanup
|
|
**Minor issues**:
|
|
- Some test fixtures use legacy patterns
|
|
- Test utilities scattered across multiple modules
|
|
|
|
**Recommendation**: DEFER to Phase 75+ (low-priority polish)
|
|
|
|
---
|
|
|
|
### 10. Using System TODOs (Minor)
|
|
**Status**: Minimal TODOs found (using/namespace system largely complete per CLAUDE.md Phase 15.5)
|
|
|
|
**Current Assessment**: Most using/namespace work COMPLETE (git commit 4120ab65 StringBox recovery, 3d082ca1 env propagation)
|
|
|
|
**Recommendation**: MONITOR for Phase 65+ namespace order SSOT work
|
|
|
|
---
|
|
|
|
## Summary Recommendation Matrix
|
|
|
|
| Item | Impact | Risk | Effort | Status |
|
|
|------|--------|------|--------|--------|
|
|
| Extern Dispatch Consolidation | HIGH | MODERATE | MEDIUM | Defer Phase 65+ |
|
|
| Monolithic Files Splitting | HIGH | SAFE | LARGE (3-6m) | Defer Phase 100+ |
|
|
| WASM Executor Decision | HIGH | LOW | SMALL | Defer Phase 150+ |
|
|
| BID-Codegen Stubs | MEDIUM | LOW | SMALL | **Document** |
|
|
| Plugin Loader Unification | MEDIUM | MODERATE | SMALL-MEDIUM | Defer Phase 20+ |
|
|
| loop_patterns_old Audit | MEDIUM | SAFE | TINY | ✅ **KEEP (ACTIVE)** |
|
|
| allow(dead_code) Cleanup | MEDIUM | LOW | MEDIUM | Defer Phase 70+ |
|
|
| AOT Backend Status | LOW | MODERATE | LARGE | **Clarify roadmap** |
|
|
| Test Infrastructure | LOW | SAFE | SMALL | Defer Phase 75+ |
|
|
| Using System TODOs | LOW | SAFE | TINY | Monitor Phase 65+ |
|
|
|
|
---
|
|
|
|
## Quick Wins (< 2 hours each)
|
|
|
|
1. **Add deprecation comments to BID-codegen stubs** (+15 lines, -0 breaking)
|
|
- Document replacement path (llvmlite harness)
|
|
- Add migration guide for future TypeScript/Python implementations
|
|
|
|
2. **Document loop_patterns_old purpose** (+10 lines)
|
|
- Clarify role as fallback pattern handler
|
|
- Link to Pattern1-4 normalized version
|
|
|
|
3. **Clarify WASM executor status** (+5 lines)
|
|
- Decision note in executor/mod.rs
|
|
- Link to Phase 150+ roadmap
|
|
|
|
---
|
|
|
|
## Long-Term Strategy
|
|
|
|
**Post-Phase 64 Ownership Architecture**:
|
|
1. **Phase 65+**: Relay support (owned variable initialization, carrier order SSOT)
|
|
2. **Phase 70-75**: Code quality pass (dead_code audit, test infrastructure, using system)
|
|
3. **Phase 100+**: Monolithic files refactoring (Box-First modularization of merge/mod.rs, strip.rs, etc.)
|
|
4. **Phase 150+**: WASM/AOT/BID-codegen decision (clarity on roadmap)
|
|
|
|
---
|
|
|
|
## Notes
|
|
|
|
- All recommendations are **deferrable** - no blocking issues found
|
|
- Most high-impact items are **structural** (monolithic files) not **functional** (bugs)
|
|
- Current code quality is **GOOD** relative to project age (4 months)
|
|
- Priorities align with **Phase 15 Rust VM + LLVM dual pillar** focus
|
|
|
|
**Last audit**: 2025-12-12 via Explore agent cc45cd20
|