Files
hakorune/docs/development/current/main/PHASE_225_SUMMARY.md

274 lines
9.7 KiB
Markdown
Raw Normal View History

# Phase 225: LoopBodyLocalInit MethodCall Meta-Driven Lowering - Complete Summary
## Overview
Phase 225 successfully eliminated ALL hardcoding from `loop_body_local_init.rs` by delegating MethodCall lowering to `MethodCallLowerer` and using `CoreMethodId` metadata exclusively.
## Problem Statement
Phase 193 introduced MethodCall support in body-local init expressions (e.g., `local digit_pos = digits.indexOf(ch)`), but the implementation contained hardcoded method names and box name mappings:
```rust
// Line 387: Hardcoded whitelist
const SUPPORTED_INIT_METHODS: &[&str] = &["indexOf", "get", "toString"];
// Lines 433-438: Hardcoded box name mapping
let box_name = match method {
"indexOf" => "StringBox".to_string(),
"get" => "ArrayBox".to_string(),
"toString" => "IntegerBox".to_string(),
_ => unreachable!("Whitelist check should have caught this"),
};
```
This caused errors like:
```
Method 'substring' not supported in body-local init (Phase 193 limitation - only indexOf, get, toString supported)
```
## Solution: Meta-Driven Architecture
### Architecture Change
**Before (Phase 193 - Hardcoded)**:
```
LoopBodyLocalInitLowerer
└─ emit_method_call_init (static method)
├─ SUPPORTED_INIT_METHODS whitelist ❌
├─ match method { "indexOf" => "StringBox" } ❌
└─ Emit BoxCall instruction
```
**After (Phase 225 - Meta-Driven)**:
```
LoopBodyLocalInitLowerer
└─ emit_method_call_init (static method)
└─ Delegates to MethodCallLowerer::lower_for_init
├─ Resolve method_name → CoreMethodId ✅
├─ Check allowed_in_init() ✅
├─ Get box_name from CoreMethodId ✅
├─ Check arity ✅
└─ Emit BoxCall instruction
```
### Key Changes
1. **Deleted hardcoded whitelist** (`SUPPORTED_INIT_METHODS` constant)
2. **Deleted hardcoded box name match** (`indexOf → StringBox` mapping)
3. **Delegated to MethodCallLowerer** (single responsibility principle)
4. **All decisions driven by `CoreMethodId` metadata** (SSOT)
## Implementation Details
### Files Modified
1. **`src/mir/join_ir/lowering/loop_body_local_init.rs`** (main refactoring)
- Import `MethodCallLowerer`
- Refactor `emit_method_call_init` to delegate
- Delete `lower_init_arg` helper (no longer needed)
- Update module documentation
- **Net change**: -82 lines (158 deleted - 76 added)
2. **`src/mir/builder/control_flow/joinir/patterns/pattern_pipeline.rs`** (test fixes)
- Add `condition_aliases: Vec::new()` to CarrierInfo test structs (2 occurrences)
3. **`src/mir/builder/control_flow/joinir/patterns/pattern4_carrier_analyzer.rs`** (test fixes)
- Add `condition_aliases: Vec::new()` to CarrierInfo test struct (1 occurrence)
4. **`docs/development/current/main/joinir-architecture-overview.md`** (documentation)
- Added Phase 225 section to LoopBodyLocal init history
5. **`CURRENT_TASK.md`** (status update)
- Added Phase 225 completion summary
### CoreMethodId Metadata
The `allowed_in_init()` method already included the methods we needed:
```rust
pub fn allowed_in_init(&self) -> bool {
use CoreMethodId::*;
match self {
// String operations - allowed
StringLength | StringSubstring | StringIndexOf => true, // ✅
// String transformations - allowed for init
StringUpper | StringLower | StringTrim => true,
// Array operations - allowed
ArrayLength | ArrayGet => true,
// Map operations - allowed
MapGet | MapHas | MapKeys => true,
// ...
}
}
```
No changes were needed to metadata - it was already correct!
## Results
### Code Quality
- **-82 net lines** (158 deletions - 76 additions)
- **Zero hardcoded method names** (all resolved via `CoreMethodId::iter()`)
- **Zero hardcoded box names** (all from `method_id.box_id().name()`)
- **Single Responsibility**: MethodCallLowerer is the only place that handles MethodCall → JoinIR
### Test Results
#### Unit Tests
- **MethodCallLowerer**: 8/8 tests PASS
- **loop_body_local_init**: 3/3 tests PASS
#### Integration Tests
- **877/884 tests PASS** (99.2% pass rate)
- **7 failures**: Pre-existing Pattern 3 accumulator variable issues (not related to Phase 225)
#### E2E Verification
- **substring now works in body-local init** ✅
- Simple test case: `local ch = s.substring(p, p+1)` compiles without error
- Complex test case: `phase2235_p2_digit_pos_min.hako` progresses past substring error to cascading dependency issue (which is a pre-existing limitation from Phase 193)
### New Capabilities
- **substring** method now usable in loop body-local init
- **Any method with `allowed_in_init() == true`** automatically supported
- **Easy to extend**: Add method to `CoreMethodId`, set `allowed_in_init()`, done!
## Known Limitations
### Cascading LoopBodyLocal Dependencies
The test `apps/tests/phase2235_p2_digit_pos_min.hako` reveals a **pre-existing limitation** (not introduced by Phase 225):
```nyash
loop(p < s.length()) {
local ch = s.substring(p, p+1) // ✅ Now works (Phase 225)
local digit_pos = digits.indexOf(ch) // ❌ Error: 'ch' not found in ConditionEnv
...
}
```
**Root cause**: When lowering `digit_pos = digits.indexOf(ch)`, the argument `ch` is looked up in `ConditionEnv` only. However, `ch` is a LoopBodyLocal variable, so it should be looked up in `LoopBodyLocalEnv`.
**Status**: This limitation existed in Phase 193 - the original `lower_init_arg` also only checked `ConditionEnv`.
**Future work**: Phase 226+ could extend argument lowering to check `LoopBodyLocalEnv` for cascading dependencies.
## Architecture Benefits
### 1. Metadata-Driven
- **Single Source of Truth**: `CoreMethodId` defines all method metadata
- **No duplication**: Method name, box name, arity, whitelist all in one place
- **Easy to extend**: Add new methods by updating `CoreMethodId` only
### 2. Single Responsibility
- **MethodCallLowerer**: "MethodCall → JoinIR" conversion (Phase 224-B)
- **LoopBodyLocalInitLowerer**: Loop body-local init coordination (Phase 186)
- **Clear boundary**: Init lowerer delegates, doesn't duplicate logic
### 3. Fail-Fast
- **Unknown methods** → immediate error (not silent fallback)
- **Arity mismatch** → immediate error
- **Not whitelisted** → immediate error with clear message
### 4. Type Safety
- **No string matching** → use enum (`CoreMethodId`)
- **Compile-time checks** → catch errors early
- **Refactoring-safe** → rename detection
### 5. Maintainability
- **Add new method**: Update `CoreMethodId` only (one place)
- **Change whitelist**: Update `allowed_in_init()` only
- **No scattered hardcoding** across files
## Comparison: Phase 193 vs Phase 225
| Aspect | Phase 193 | Phase 225 |
|--------|-----------|-----------|
| Method whitelist | Hardcoded constant | CoreMethodId metadata |
| Box name mapping | Match statement | `method_id.box_id().name()` |
| Supported methods | 3 (indexOf, get, toString) | 15+ (all with `allowed_in_init() == true`) |
| Code lines | 158 | 76 (-82 lines) |
| Extensibility | Add to 2 places | Add to `CoreMethodId` only |
| Type safety | String matching | Enum-based |
| Single responsibility | Violated | Achieved |
## Future Work (Not in Phase 225)
### Phase 226+: Potential Improvements
1. **Cascading LoopBodyLocal support**
- Extend argument lowering to check `LoopBodyLocalEnv`
- Enable `ch``digit_pos` → condition chains
2. **Type inference**
- Use actual receiver type instead of heuristics
- More accurate box name resolution
3. **Custom method support**
- User-defined box methods in init expressions
4. **Optimization**
- Dead code elimination for unused method calls
- Common subexpression elimination
5. **Error messages**
- Better diagnostics with suggestions
- "Did you mean...?" for typos
## Commit Message
```
refactor(joinir): Phase 225 - LoopBodyLocalInit MethodCall meta-driven
Eliminate ALL hardcoding from loop_body_local_init.rs by delegating
to MethodCallLowerer and using CoreMethodId metadata exclusively.
Changes:
- Delete SUPPORTED_INIT_METHODS whitelist constant
- Delete hardcoded box name match (indexOf→StringBox, etc.)
- Delegate emit_method_call_init to MethodCallLowerer::lower_for_init
- Use CoreMethodId metadata for allowed_in_init() whitelist
- Delete lower_init_arg helper (no longer needed)
- Fix test structs to include condition_aliases field
Results:
- substring method now works in body-local init
- Net change: -82 lines (158 deleted - 76 added)
- 877/884 tests PASS (7 pre-existing P3 failures)
- Zero hardcoded method/box names remaining
Architecture:
- Single Source of Truth: CoreMethodId metadata
- Single Responsibility: MethodCallLowerer handles all MethodCall lowering
- Fail-Fast: Unknown methods → immediate error
- Type Safety: Enum-based instead of string matching
Phase 225 complete - meta-driven architecture achieved ✅
```
## Success Criteria ✅
All success criteria met:
1.`cargo build --release` succeeds
2. ✅ All unit tests in `method_call_lowerer.rs` pass (8/8)
3. ✅ All unit tests in `loop_body_local_init.rs` pass (3/3)
4.`substring` method now works in body-local init
5.**Zero hardcoded method names or box names** in `emit_method_call_init`
6. ✅ No regressions in existing tests (877/884 pass, 7 pre-existing failures)
## References
- **Design Document**: [phase225-bodylocal-init-methodcall-design.md](phase225-bodylocal-init-methodcall-design.md)
- **Phase 186**: Loop Body-Local Variable Initialization (initial implementation)
- **Phase 193**: MethodCall support in body-local init (hardcoded version)
- **Phase 224-B**: MethodCallLowerer Box creation (metadata-driven)
- **Phase 224-C**: MethodCallLowerer argument support
- **Phase 224-D**: ConditionAlias variable resolution
- **Architecture Overview**: [joinir-architecture-overview.md](joinir-architecture-overview.md)