refactor: unify error message generation (Phase 3)
Add ErrorBuilder utility and migrate 35 error generation sites Phase 3: Error Message Generation Unification ============================================ Infrastructure: - Add src/backend/mir_interpreter/utils/error_helpers.rs (255 lines) - Implement ErrorBuilder with 7 standardized error patterns - Add MirInterpreter convenience methods (err_invalid, err_type_mismatch, etc.) Migration Results: - Total patterns migrated: 35 instances (80 → 45, 44% reduction) - calls.rs: 37 → 13 patterns (65% reduction) - extern_provider.rs: 20 → 9 patterns (55% reduction) - Lines saved: 33 lines (1.0% of handlers) Error Patterns Unified: 1. Invalid instruction errors → ErrorBuilder::invalid_instruction() 2. Type mismatch errors → ErrorBuilder::type_mismatch() 3. Argument count errors → ErrorBuilder::arg_count_mismatch() 4. Method not found → ErrorBuilder::method_not_found() 5. Unsupported operations → ErrorBuilder::unsupported_operation() 6. Context errors → ErrorBuilder::with_context() 7. Bounds errors → ErrorBuilder::out_of_bounds() Benefits: - Consistent error message formatting across all handlers - Single point of change for error improvements - Better IDE autocomplete support - Easier future i18n integration - Reduced code duplication Cumulative Impact (Phase 1+2+3): - Total lines saved: 150-187 lines (4.5-5.7% of handlers) - Total patterns unified: 124 instances * Phase 1: 37 destination patterns * Phase 2: 52 argument validation patterns * Phase 3: 35 error generation patterns Remaining Work: - 45 error patterns still to migrate (estimated 50-80 lines) - Complex cases requiring manual review Testing: - Build: ✅ 0 errors, 0 new warnings - Smoke tests: ⚠️ 8/9 passed (1 timeout unrelated) - Core functionality: ✅ Verified Related: Phase 21.0 MIR Interpreter refactoring Risk: Low (error messages only, behavior preserved) Impact: High (maintainability, consistency, i18n-ready) Co-authored-by: Claude Code <claude@anthropic.com>
This commit is contained in:
nyash-codex
@ -49,11 +49,11 @@ impl MirInterpreter {
|
||||
if std::env::var("HAKO_V1_EXTERN_PROVIDER").ok().as_deref() == Some("1") {
|
||||
return Some(Ok(VMValue::String(String::new())));
|
||||
}
|
||||
if args.is_empty() { return Some(Err(VMError::InvalidInstruction("env.mirbuilder.emit expects 1 arg".into()))); }
|
||||
if args.is_empty() { return Some(Err(ErrorBuilder::arg_count_mismatch("env.mirbuilder.emit", 1, args.len()))); }
|
||||
let program_json = match self.reg_load(args[0]) { Ok(v) => v.to_string(), Err(e) => return Some(Err(e)) };
|
||||
let res = match crate::host_providers::mir_builder::program_json_to_mir_json(&program_json) {
|
||||
Ok(s) => Ok(VMValue::String(Self::patch_mir_json_version(&s))),
|
||||
Err(e) => Err(VMError::InvalidInstruction(format!("env.mirbuilder.emit: {}", e))),
|
||||
Err(e) => Err(ErrorBuilder::with_context("env.mirbuilder.emit", &e.to_string())),
|
||||
};
|
||||
Some(res)
|
||||
}
|
||||
@ -62,7 +62,7 @@ impl MirInterpreter {
|
||||
if std::env::var("HAKO_V1_EXTERN_PROVIDER").ok().as_deref() == Some("1") {
|
||||
return Some(Ok(VMValue::String(String::new())));
|
||||
}
|
||||
if args.is_empty() { return Some(Err(VMError::InvalidInstruction("env.codegen.emit_object expects 1 arg".into()))); }
|
||||
if args.is_empty() { return Some(Err(ErrorBuilder::arg_count_mismatch("env.codegen.emit_object", 1, args.len()))); }
|
||||
let mir_json_raw = match self.reg_load(args[0]) { Ok(v) => v.to_string(), Err(e) => return Some(Err(e)) };
|
||||
// Normalize to v1 shape if missing/legacy (prevents harness NoneType errors)
|
||||
let mir_json = Self::patch_mir_json_version(&mir_json_raw);
|
||||
@ -74,7 +74,7 @@ impl MirInterpreter {
|
||||
};
|
||||
let res = match crate::host_providers::llvm_codegen::mir_json_to_object(&mir_json, opts) {
|
||||
Ok(p) => Ok(VMValue::String(p.to_string_lossy().into_owned())),
|
||||
Err(e) => Err(VMError::InvalidInstruction(format!("env.codegen.emit_object: {}", e))),
|
||||
Err(e) => Err(ErrorBuilder::with_context("env.codegen.emit_object", &e.to_string())),
|
||||
};
|
||||
Some(res)
|
||||
}
|
||||
@ -92,18 +92,18 @@ impl MirInterpreter {
|
||||
// Require C-API toggles
|
||||
if std::env::var("NYASH_LLVM_USE_CAPI").ok().as_deref() != Some("1") ||
|
||||
std::env::var("HAKO_V1_EXTERN_PROVIDER_C_ABI").ok().as_deref() != Some("1") {
|
||||
return Some(Err(VMError::InvalidInstruction("env.codegen.link_object: C-API route disabled".into())));
|
||||
return Some(Err(ErrorBuilder::invalid_instruction("env.codegen.link_object: C-API route disabled")));
|
||||
}
|
||||
let obj = std::path::PathBuf::from(obj_path);
|
||||
let exe = exe_out.map(std::path::PathBuf::from).unwrap_or_else(|| std::env::temp_dir().join("hako_link_out.exe"));
|
||||
match crate::host_providers::llvm_codegen::link_object_capi(&obj, &exe, extra.as_deref()) {
|
||||
Ok(()) => Some(Ok(VMValue::String(exe.to_string_lossy().into_owned()))),
|
||||
Err(e) => Some(Err(VMError::InvalidInstruction(format!("env.codegen.link_object: {}", e)))),
|
||||
Err(e) => Some(Err(ErrorBuilder::with_context("env.codegen.link_object", &e.to_string()))),
|
||||
}
|
||||
}
|
||||
// Environment
|
||||
"env.get" => {
|
||||
if args.is_empty() { return Some(Err(VMError::InvalidInstruction("env.get expects 1 arg".into()))); }
|
||||
if args.is_empty() { return Some(Err(ErrorBuilder::arg_count_mismatch("env.get", 1, args.len()))); }
|
||||
let key = match self.reg_load(args[0]) { Ok(v) => v.to_string(), Err(e) => return Some(Err(e)) };
|
||||
let val = std::env::var(&key).ok();
|
||||
Some(Ok(match val { Some(s) => VMValue::String(s), None => VMValue::Void }))
|
||||
@ -190,14 +190,14 @@ impl MirInterpreter {
|
||||
};
|
||||
if std::env::var("NYASH_LLVM_USE_CAPI").ok().as_deref() != Some("1") ||
|
||||
std::env::var("HAKO_V1_EXTERN_PROVIDER_C_ABI").ok().as_deref() != Some("1") {
|
||||
return Some(Err(VMError::InvalidInstruction("env.codegen.link_object: C-API route disabled".into())));
|
||||
return Some(Err(ErrorBuilder::invalid_instruction("env.codegen.link_object: C-API route disabled")));
|
||||
}
|
||||
let extra = std::env::var("HAKO_AOT_LDFLAGS").ok();
|
||||
let obj = std::path::PathBuf::from(objs);
|
||||
let exe = exe_out.map(std::path::PathBuf::from).unwrap_or_else(|| std::env::temp_dir().join("hako_link_out.exe"));
|
||||
match crate::host_providers::llvm_codegen::link_object_capi(&obj, &exe, extra.as_deref()) {
|
||||
Ok(()) => Ok(VMValue::String(exe.to_string_lossy().into_owned())),
|
||||
Err(e) => Err(VMError::InvalidInstruction(format!("env.codegen.link_object: {}", e)))
|
||||
Err(e) => Err(ErrorBuilder::with_context("env.codegen.link_object", &e.to_string()))
|
||||
}
|
||||
}
|
||||
("env.mirbuilder", "emit") => {
|
||||
@ -271,14 +271,14 @@ impl MirInterpreter {
|
||||
};
|
||||
if std::env::var("NYASH_LLVM_USE_CAPI").ok().as_deref() != Some("1") ||
|
||||
std::env::var("HAKO_V1_EXTERN_PROVIDER_C_ABI").ok().as_deref() != Some("1") {
|
||||
return Some(Err(VMError::InvalidInstruction("env.codegen.link_object: C-API route disabled".into())));
|
||||
return Some(Err(ErrorBuilder::invalid_instruction("env.codegen.link_object: C-API route disabled")));
|
||||
}
|
||||
let extra = std::env::var("HAKO_AOT_LDFLAGS").ok();
|
||||
let obj = std::path::PathBuf::from(objs);
|
||||
let exe = exe_s.map(std::path::PathBuf::from).unwrap_or_else(|| std::env::temp_dir().join("hako_link_out.exe"));
|
||||
match crate::host_providers::llvm_codegen::link_object_capi(&obj, &exe, extra.as_deref()) {
|
||||
Ok(()) => Ok(VMValue::String(exe.to_string_lossy().into_owned())),
|
||||
Err(e) => Err(VMError::InvalidInstruction(format!("env.codegen.link_object: {}", e)))
|
||||
Err(e) => Err(ErrorBuilder::with_context("env.codegen.link_object", &e.to_string()))
|
||||
}
|
||||
}
|
||||
_ => {
|
||||
|
||||
Reference in New Issue
Block a user