tomoaki/hakorune
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor: complete MIR interpreter utility migration (Phase 2)

Browse Source 
Migrate remaining argument validation and destination write patterns
to utility helpers introduced in Phase 1.

Changes:
- boxes_map.rs: 135 → 123 lines (-12)
  * 6 arg validations → validate_args_exact()
  * 7 destination writes → write_result()

- externals.rs: 219 → 206 lines (-13)
  * 12 destination patterns unified
  * Simplified env/future/modules handling

- boxes_string.rs: 209 → 197 lines (-12)
  * 4 arg validations + 4 destinations unified
  * Methods: replace, contains, lastIndexOf, concat, etc.

- boxes_array.rs: 64 lines (3 validations migrated)
  * Methods: push, get, set

- boxes_object_fields.rs: 400 → 394 lines (-6)
  * 2 arg validations (getField, setField)

Pattern Elimination:
- Argument validation: 15 → 0 (100% eliminated)
- Destination writes: 51 → 28 (45% eliminated)

Code Reduction:
- Phase 2: 43 lines removed (1.3%)
- Cumulative: 117-154 lines removed (3.5-4.6% of handlers/)

Test Results:
- Build: ✅ SUCCESS (no new warnings)
- MapBox: ✅ 37/37 passed
- StringBox: ✅ 30/30 passed
- ArrayBox: ⚠️ 13/14 (1 pre-existing failure)

Benefits:
- Unified error messages across all handlers
- Single source of truth for validation logic
- Easier maintenance and future refactoring
- Consistent behavior throughout codebase

Related: Phase 21.0 refactoring (DUPLICATION_ANALYSIS_REPORT.md)
Risk: Low (pure refactoring, behavior preserved)
This commit is contained in:
nyash-codex
2025-11-06 22:59:47 +09:00
parent 8d179e9499
commit edf4513b5a
5 changed files with 44 additions and 87 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
src/backend/mir_interpreter/handlers/boxes_array.rs
View File

@ -25,7 +25,7 @@ pub(super) fn try_handle_array_box(
return Ok(true); return Ok(true);
} }
"push" => { "push" => {
if args.len() != 1 { return Err(VMError::InvalidInstruction("push expects 1 arg".into())); } this.validate_args_exact("push", args, 1)?;
let val = this.reg_load(args[0])?.to_nyash_box(); let val = this.reg_load(args[0])?.to_nyash_box();
let _ = ab.push(val); let _ = ab.push(val);
this.write_void(dst); this.write_void(dst);
@ -43,14 +43,14 @@ pub(super) fn try_handle_array_box(
return Ok(true); return Ok(true);
} }
"get" => { "get" => {
if args.len() != 1 { return Err(VMError::InvalidInstruction("get expects 1 arg".into())); } this.validate_args_exact("get", args, 1)?;
let idx = this.reg_load(args[0])?.to_nyash_box(); let idx = this.reg_load(args[0])?.to_nyash_box();
let ret = ab.get(idx); let ret = ab.get(idx);
this.write_result(dst, VMValue::from_nyash_box(ret)); this.write_result(dst, VMValue::from_nyash_box(ret));
return Ok(true); return Ok(true);
} }
"set" => { "set" => {
if args.len() != 2 { return Err(VMError::InvalidInstruction("set expects 2 args".into())); } this.validate_args_exact("set", args, 2)?;
let idx = this.reg_load(args[0])?.to_nyash_box(); let idx = this.reg_load(args[0])?.to_nyash_box();
let val = this.reg_load(args[1])?.to_nyash_box(); let val = this.reg_load(args[1])?.to_nyash_box();
let _ = ab.set(idx, val); let _ = ab.set(idx, val);

38
src/backend/mir_interpreter/handlers/boxes_map.rs
View File

@ -26,48 +26,36 @@ pub(super) fn try_handle_map_box(
} }
// Field bridge: treat getField/setField as get/set with string key // Field bridge: treat getField/setField as get/set with string key
"getField" => { "getField" => {
if args.len() != 1 { this.validate_args_exact("MapBox.getField", args, 1)?;
return Err(VMError::InvalidInstruction(
"MapBox.getField expects 1 arg".into(),
));
}
let k_vm = this.reg_load(args[0])?; let k_vm = this.reg_load(args[0])?;
// Field access expects a String key; otherwise return a stable tag. // Field access expects a String key; otherwise return a stable tag.
if !matches!(k_vm, VMValue::String(_)) { if !matches!(k_vm, VMValue::String(_)) {
if let Some(d) = dst { this.regs.insert(d, VMValue::String("[map/bad-key] field name must be string".to_string())); } this.write_result(dst, VMValue::String("[map/bad-key] field name must be string".to_string()));
return Ok(true); return Ok(true);
} }
let k = k_vm.to_nyash_box(); let k = k_vm.to_nyash_box();
let ret = mb.get(k); let ret = mb.get(k);
if let Some(d) = dst { this.write_result(dst, VMValue::from_nyash_box(ret));
this.regs.insert(d, VMValue::from_nyash_box(ret));
}
return Ok(true); return Ok(true);
} }
"setField" => { "setField" => {
if args.len() != 2 { this.validate_args_exact("MapBox.setField", args, 2)?;
return Err(VMError::InvalidInstruction(
"MapBox.setField expects 2 args".into(),
));
}
let k_vm = this.reg_load(args[0])?; let k_vm = this.reg_load(args[0])?;
if !matches!(k_vm, VMValue::String(_)) { if !matches!(k_vm, VMValue::String(_)) {
if let Some(d) = dst { this.regs.insert(d, VMValue::String("[map/bad-key] field name must be string".to_string())); } this.write_result(dst, VMValue::String("[map/bad-key] field name must be string".to_string()));
return Ok(true); return Ok(true);
} }
let k = k_vm.to_nyash_box(); let k = k_vm.to_nyash_box();
let v = this.reg_load(args[1])?.to_nyash_box(); let v = this.reg_load(args[1])?.to_nyash_box();
let ret = mb.set(k, v); let ret = mb.set(k, v);
if let Some(d) = dst { this.write_result(dst, VMValue::from_nyash_box(ret));
this.regs.insert(d, VMValue::from_nyash_box(ret));
}
return Ok(true); return Ok(true);
} }
"set" => { "set" => {
if args.len() != 2 { return Err(VMError::InvalidInstruction("MapBox.set expects 2 args".into())); } this.validate_args_exact("MapBox.set", args, 2)?;
let k_vm = this.reg_load(args[0])?; let k_vm = this.reg_load(args[0])?;
if !matches!(k_vm, VMValue::String(_)) { if !matches!(k_vm, VMValue::String(_)) {
if let Some(d) = dst { this.regs.insert(d, VMValue::String("[map/bad-key] key must be string".to_string())); } this.write_result(dst, VMValue::String("[map/bad-key] key must be string".to_string()));
return Ok(true); return Ok(true);
} }
let k = k_vm.to_nyash_box(); let k = k_vm.to_nyash_box();
@ -77,10 +65,10 @@ pub(super) fn try_handle_map_box(
return Ok(true); return Ok(true);
} }
"get" => { "get" => {
if args.len() != 1 { return Err(VMError::InvalidInstruction("MapBox.get expects 1 arg".into())); } this.validate_args_exact("MapBox.get", args, 1)?;
let k_vm = this.reg_load(args[0])?; let k_vm = this.reg_load(args[0])?;
if !matches!(k_vm, VMValue::String(_)) { if !matches!(k_vm, VMValue::String(_)) {
if let Some(d) = dst { this.regs.insert(d, VMValue::String("[map/bad-key] key must be string".to_string())); } this.write_result(dst, VMValue::String("[map/bad-key] key must be string".to_string()));
return Ok(true); return Ok(true);
} }
let k = k_vm.to_nyash_box(); let k = k_vm.to_nyash_box();
@ -89,17 +77,17 @@ pub(super) fn try_handle_map_box(
return Ok(true); return Ok(true);
} }
"has" => { "has" => {
if args.len() != 1 { return Err(VMError::InvalidInstruction("MapBox.has expects 1 arg".into())); } this.validate_args_exact("MapBox.has", args, 1)?;
let k = this.reg_load(args[0])?.to_nyash_box(); let k = this.reg_load(args[0])?.to_nyash_box();
let ret = mb.has(k); let ret = mb.has(k);
this.write_result(dst, VMValue::from_nyash_box(ret)); this.write_result(dst, VMValue::from_nyash_box(ret));
return Ok(true); return Ok(true);
} }
"delete" => { "delete" => {
if args.len() != 1 { return Err(VMError::InvalidInstruction("MapBox.delete expects 1 arg".into())); } this.validate_args_exact("MapBox.delete", args, 1)?;
let k_vm = this.reg_load(args[0])?; let k_vm = this.reg_load(args[0])?;
if !matches!(k_vm, VMValue::String(_)) { if !matches!(k_vm, VMValue::String(_)) {
if let Some(d) = dst { this.regs.insert(d, VMValue::String("[map/bad-key] key must be string".to_string())); } this.write_result(dst, VMValue::String("[map/bad-key] key must be string".to_string()));
return Ok(true); return Ok(true);
} }
let k = k_vm.to_nyash_box(); let k = k_vm.to_nyash_box();

10
src/backend/mir_interpreter/handlers/boxes_object_fields.rs
View File

@ -38,9 +38,7 @@ pub(super) fn try_handle_object_fields(
match method { match method {
"getField" => { "getField" => {
if args.len() != 1 { this.validate_args_exact("getField", args, 1)?;
return Err(VMError::InvalidInstruction("getField expects 1 arg".into()));
}
// MapBox special-case: bridge to MapBox.get, with string-only key // MapBox special-case: bridge to MapBox.get, with string-only key
if let Ok(VMValue::BoxRef(bref)) = this.reg_load(box_val) { if let Ok(VMValue::BoxRef(bref)) = this.reg_load(box_val) {
if bref.as_any().downcast_ref::<crate::boxes::map_box::MapBox>().is_some() { if bref.as_any().downcast_ref::<crate::boxes::map_box::MapBox>().is_some() {
@ -296,11 +294,7 @@ pub(super) fn try_handle_object_fields(
Ok(true) Ok(true)
} }
"setField" => { "setField" => {
if args.len() != 2 { this.validate_args_exact("setField", args, 2)?;
return Err(VMError::InvalidInstruction(
"setField expects 2 args".into(),
));
}
// MapBox special-case: bridge to MapBox.set, with string-only key // MapBox special-case: bridge to MapBox.set, with string-only key
if let Ok(VMValue::BoxRef(bref)) = this.reg_load(box_val) { if let Ok(VMValue::BoxRef(bref)) = this.reg_load(box_val) {
if bref.as_any().downcast_ref::<crate::boxes::map_box::MapBox>().is_some() { if bref.as_any().downcast_ref::<crate::boxes::map_box::MapBox>().is_some() {

28
src/backend/mir_interpreter/handlers/boxes_string.rs
View File

@ -36,9 +36,7 @@ pub(super) fn try_handle_string_box(
} }
"replace" => { "replace" => {
// replace(old, new) -> string with first occurrence replaced (Rust replace = all; match Core minimal // replace(old, new) -> string with first occurrence replaced (Rust replace = all; match Core minimal
if args.len() != 2 { this.validate_args_exact("replace", args, 2)?;
return Err(VMError::InvalidInstruction("replace expects 2 args".into()));
}
let old_s = this.reg_load(args[0])?.to_string(); let old_s = this.reg_load(args[0])?.to_string();
let new_s = this.reg_load(args[1])?.to_string(); let new_s = this.reg_load(args[1])?.to_string();
// Core policy: replace only the first occurrence // Core policy: replace only the first occurrence
@ -51,9 +49,7 @@ pub(super) fn try_handle_string_box(
} else { } else {
sb_norm.value.clone() sb_norm.value.clone()
}; };
if let Some(d) = dst { this.write_result(dst, VMValue::from_nyash_box(Box::new(crate::box_trait::StringBox::new(out))));
this.regs.insert(d, VMValue::from_nyash_box(Box::new(crate::box_trait::StringBox::new(out))));
}
return Ok(true); return Ok(true);
} }
"trim" => { "trim" => {
@ -99,9 +95,7 @@ pub(super) fn try_handle_string_box(
"contains" => { "contains" => {
// contains(search) -> boolean (true if found, false otherwise) // contains(search) -> boolean (true if found, false otherwise)
// Implemented as indexOf(search) >= 0 // Implemented as indexOf(search) >= 0
if args.len() != 1 { this.validate_args_exact("contains", args, 1)?;
return Err(VMError::InvalidInstruction("contains expects 1 arg".into()));
}
let needle = this.reg_load(args[0])?.to_string(); let needle = this.reg_load(args[0])?.to_string();
let found = sb_norm.value.contains(&needle); let found = sb_norm.value.contains(&needle);
this.write_result(dst, VMValue::Bool(found)); this.write_result(dst, VMValue::Bool(found));
@ -109,9 +103,7 @@ pub(super) fn try_handle_string_box(
} }
"lastIndexOf" => { "lastIndexOf" => {
// lastIndexOf(substr) -> last index or -1 // lastIndexOf(substr) -> last index or -1
if args.len() != 1 { this.validate_args_exact("lastIndexOf", args, 1)?;
return Err(VMError::InvalidInstruction("lastIndexOf expects 1 arg".into()));
}
let needle = this.reg_load(args[0])?.to_string(); let needle = this.reg_load(args[0])?.to_string();
let idx = sb_norm.value.rfind(&needle).map(|i| i as i64).unwrap_or(-1); let idx = sb_norm.value.rfind(&needle).map(|i| i as i64).unwrap_or(-1);
this.write_result(dst, VMValue::Integer(idx)); this.write_result(dst, VMValue::Integer(idx));
@ -133,9 +125,7 @@ pub(super) fn try_handle_string_box(
} }
} }
quoted.push('"'); quoted.push('"');
if let Some(d) = dst { this.write_result(dst, VMValue::from_nyash_box(Box::new(crate::box_trait::StringBox::new(quoted))));
this.regs.insert(d, VMValue::from_nyash_box(Box::new(crate::box_trait::StringBox::new(quoted))));
}
return Ok(true); return Ok(true);
} }
"substring" => { "substring" => {
@ -164,16 +154,14 @@ pub(super) fn try_handle_string_box(
let end = e_idx.max(start as i64).min(len) as usize; let end = e_idx.max(start as i64).min(len) as usize;
let chars: Vec<char> = sb_norm.value.chars().collect(); let chars: Vec<char> = sb_norm.value.chars().collect();
let sub: String = chars[start..end].iter().collect(); let sub: String = chars[start..end].iter().collect();
if let Some(d) = dst { this.regs.insert(d, VMValue::from_nyash_box(Box::new(crate::box_trait::StringBox::new(sub)))) ; } this.write_result(dst, VMValue::from_nyash_box(Box::new(crate::box_trait::StringBox::new(sub))));
return Ok(true); return Ok(true);
} }
"concat" => { "concat" => {
if args.len() != 1 { this.validate_args_exact("concat", args, 1)?;
return Err(VMError::InvalidInstruction("concat expects 1 arg".into()));
}
let rhs = this.reg_load(args[0])?; let rhs = this.reg_load(args[0])?;
let new_s = format!("{}{}", sb_norm.value, rhs.to_string()); let new_s = format!("{}{}", sb_norm.value, rhs.to_string());
if let Some(d) = dst { this.regs.insert(d, VMValue::from_nyash_box(Box::new(crate::box_trait::StringBox::new(new_s)))) ; } this.write_result(dst, VMValue::from_nyash_box(Box::new(crate::box_trait::StringBox::new(new_s))));
return Ok(true); return Ok(true);
} }
"is_digit_char" => { "is_digit_char" => {

45
src/backend/mir_interpreter/handlers/externals.rs
View File

@ -30,14 +30,13 @@ impl MirInterpreter {
if let Some(a0) = args.get(0) { if let Some(a0) = args.get(0) {
let key = self.reg_load(*a0)?.to_string(); let key = self.reg_load(*a0)?.to_string();
let val = std::env::var(&key).ok(); let val = std::env::var(&key).ok();
if let Some(d) = dst { let result = if let Some(s) = val {
if let Some(s) = val { VMValue::String(s)
self.regs.insert(d, VMValue::String(s));
} else { } else {
// Represent missing env as null-equivalent (Void) // Represent missing env as null-equivalent (Void)
self.regs.insert(d, VMValue::Void); VMValue::Void
} };
} self.write_result(dst, result);
} }
Ok(()) Ok(())
} }
@ -81,9 +80,7 @@ impl MirInterpreter {
let v = self.reg_load(*a0)?; let v = self.reg_load(*a0)?;
fut.set_result(v.to_nyash_box()); fut.set_result(v.to_nyash_box());
} }
if let Some(d) = dst { self.write_result(dst, VMValue::Future(fut));
self.regs.insert(d, VMValue::Future(fut));
}
Ok(()) Ok(())
} }
("env.future", "set") => { ("env.future", "set") => {
@ -96,9 +93,7 @@ impl MirInterpreter {
return Err(VMError::TypeError("env.future.set expects Future".into())); return Err(VMError::TypeError("env.future.set expects Future".into()));
} }
} }
if let Some(d) = dst { self.write_void(dst);
self.regs.insert(d, VMValue::Void);
}
Ok(()) Ok(())
} }
("env.future", "await") => { ("env.future", "await") => {
@ -107,9 +102,7 @@ impl MirInterpreter {
match f { match f {
VMValue::Future(fut) => { VMValue::Future(fut) => {
let v = fut.get(); let v = fut.get();
if let Some(d) = dst { self.write_result(dst, VMValue::from_nyash_box(v));
self.regs.insert(d, VMValue::from_nyash_box(v));
}
} }
_ => { _ => {
return Err(VMError::TypeError("await expects Future".into())); return Err(VMError::TypeError("await expects Future".into()));
@ -120,9 +113,7 @@ impl MirInterpreter {
} }
("env.runtime", "checkpoint") => { ("env.runtime", "checkpoint") => {
crate::runtime::global_hooks::safepoint_and_poll(); crate::runtime::global_hooks::safepoint_and_poll();
if let Some(d) = dst { self.write_void(dst);
self.regs.insert(d, VMValue::Void);
}
Ok(()) Ok(())
} }
("env.modules", "set") => { ("env.modules", "set") => {
@ -131,9 +122,7 @@ impl MirInterpreter {
let v = self.reg_load(args[1])?.to_nyash_box(); let v = self.reg_load(args[1])?.to_nyash_box();
crate::runtime::modules_registry::set(k, v); crate::runtime::modules_registry::set(k, v);
} }
if let Some(d) = dst { self.write_void(dst);
self.regs.insert(d, VMValue::Void);
}
Ok(()) Ok(())
} }
("env.modules", "get") => { ("env.modules", "get") => {
@ -141,26 +130,24 @@ impl MirInterpreter {
let k = self.reg_load(*a0)?.to_string(); let k = self.reg_load(*a0)?.to_string();
let vb = crate::runtime::modules_registry::get(&k) let vb = crate::runtime::modules_registry::get(&k)
.unwrap_or_else(|| Box::new(crate::box_trait::VoidBox::new())); .unwrap_or_else(|| Box::new(crate::box_trait::VoidBox::new()));
if let Some(d) = dst { self.write_result(dst, VMValue::from_nyash_box(vb));
self.regs.insert(d, VMValue::from_nyash_box(vb));
}
} }
Ok(()) Ok(())
} }
("env", "get") => { ("env", "get") => {
// Delegate to provider // Delegate to provider
let ret = self.extern_provider_dispatch("env.get", args).unwrap_or(Ok(VMValue::Void))?; let ret = self.extern_provider_dispatch("env.get", args).unwrap_or(Ok(VMValue::Void))?;
if let Some(d) = dst { self.regs.insert(d, ret); } self.write_result(dst, ret);
Ok(()) Ok(())
} }
("env.mirbuilder", "emit") => { ("env.mirbuilder", "emit") => {
let ret = self.extern_provider_dispatch("env.mirbuilder.emit", args).unwrap_or(Ok(VMValue::Void))?; let ret = self.extern_provider_dispatch("env.mirbuilder.emit", args).unwrap_or(Ok(VMValue::Void))?;
if let Some(d) = dst { self.regs.insert(d, ret); } self.write_result(dst, ret);
Ok(()) Ok(())
} }
("env.codegen", "emit_object") => { ("env.codegen", "emit_object") => {
let ret = self.extern_provider_dispatch("env.codegen.emit_object", args).unwrap_or(Ok(VMValue::Void))?; let ret = self.extern_provider_dispatch("env.codegen.emit_object", args).unwrap_or(Ok(VMValue::Void))?;
if let Some(d) = dst { self.regs.insert(d, ret); } self.write_result(dst, ret);
Ok(()) Ok(())
} }
("env.codegen", "link_object") => { ("env.codegen", "link_object") => {
@ -197,13 +184,13 @@ impl MirInterpreter {
let exe = exe_out.map(std::path::PathBuf::from).unwrap_or_else(|| std::env::temp_dir().join("hako_link_out.exe")); let exe = exe_out.map(std::path::PathBuf::from).unwrap_or_else(|| std::env::temp_dir().join("hako_link_out.exe"));
crate::host_providers::llvm_codegen::link_object_capi(&obj, &exe, extra.as_deref()) crate::host_providers::llvm_codegen::link_object_capi(&obj, &exe, extra.as_deref())
.map_err(|e| VMError::InvalidInstruction(format!("env.codegen.link_object: {}", e)))?; .map_err(|e| VMError::InvalidInstruction(format!("env.codegen.link_object: {}", e)))?;
if let Some(d) = dst { self.regs.insert(d, VMValue::String(exe.to_string_lossy().into_owned())); } self.write_result(dst, VMValue::String(exe.to_string_lossy().into_owned()));
Ok(()) Ok(())
} }
("hostbridge", "extern_invoke") => { ("hostbridge", "extern_invoke") => {
if let Some(res) = self.extern_provider_dispatch("hostbridge.extern_invoke", args) { if let Some(res) = self.extern_provider_dispatch("hostbridge.extern_invoke", args) {
match res { match res {
Ok(v) => { if let Some(d) = dst { self.regs.insert(d, v); } } Ok(v) => { self.write_result(dst, v); }
Err(e) => { return Err(e); } Err(e) => { return Err(e); }
} }
return Ok(()); return Ok(());