Skip to content

Commit 845e94a

Browse files
timfennisclaude
andcommitted
Change NativeFunction error type from String to VmError
NativeFunction.func now returns Result<Value, VmError> instead of Result<Value, String>. VmError carries an optional span so errors from HOF callbacks (e.g. a failing lambda inside map) preserve the inner span rather than collapsing to the call site. The VM fills in the call-site span only when the error arrives without one. Also extracts VmError into ndc_vm/src/error.rs to break the circular dependency between value.rs and vm.rs, and removes the now-unused attach_vm_native_hof helper. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
1 parent af967da commit 845e94a

14 files changed

Lines changed: 667 additions & 297 deletions

File tree

ndc_bin/src/diagnostic.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ impl From<InterpreterError> for NdcReport {
7777
},
7878
InterpreterError::Vm(err) => Self {
7979
message: err.message.clone(),
80-
span: Some(span_to_source_span(err.span)),
80+
span: err.span.map(span_to_source_span),
8181
label: "related to this",
8282
help: None,
8383
},

ndc_interpreter/src/environment.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ impl Environment {
132132
root.global_functions.push(new_function.clone());
133133
}
134134

135+
135136
#[must_use]
136137
pub fn get(&self, var: ResolvedVar) -> Value {
137138
match var {

ndc_interpreter/src/function.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,11 @@ impl Function {
9191
}
9292
}
9393

94+
/// Attaches or replaces the VmNative body on this function.
95+
pub fn set_vm_native(&mut self, native: Rc<VmNativeFunction>) {
96+
self.vm_native = Some(native);
97+
}
98+
9499
pub fn vm_native(&self) -> Option<Rc<VmNativeFunction>> {
95100
self.vm_native.clone()
96101
}
@@ -349,7 +354,7 @@ impl FunctionBody {
349354
.iter()
350355
.map(|a| crate::vm_bridge::interp_to_vm(a.clone()))
351356
.collect();
352-
let result = (native.func)(&vm_args).map_err(|e| {
357+
let result = (native.func)(&vm_args, &[]).map_err(|e| {
353358
FunctionCarrier::IntoEvaluationError(Box::new(anyhow::anyhow!(e)))
354359
})?;
355360
return Ok(crate::vm_bridge::vm_to_interp(&result));

ndc_interpreter/src/vm_bridge.rs

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use std::rc::Rc;
66

77
use ndc_core::int::Int;
88
use ndc_core::num::Number;
9+
use ndc_vm::VmError;
910
use ndc_vm::VmIterator;
1011
use ndc_vm::value::{
1112
Function as VmFunction, NativeFunction, Object as VmObject, OrdValue, Value as VmValue,
@@ -82,7 +83,7 @@ fn wrap_function(
8283
) -> VmValue {
8384
let name = func.name().to_string();
8485
let static_type = func.static_type();
85-
let native = move |args: &[VmValue]| -> Result<VmValue, String> {
86+
let native = move |args: &[VmValue], _globals: &[VmValue]| -> Result<VmValue, VmError> {
8687
let globals = Rc::new(globals_cell.borrow().clone());
8788
// Convert VM args to interpreter args, preserving Rc identity for heap/deque
8889
// values that appear more than once (so pointer-equality comparisons like `h == h` work).
@@ -112,8 +113,10 @@ fn wrap_function(
112113
}
113114
Ok(interp_to_vm(result))
114115
}
115-
Err(FunctionCarrier::IntoEvaluationError(e)) => return Err(e.to_string()),
116-
Err(e) => return Err(e.to_string()),
116+
Err(FunctionCarrier::IntoEvaluationError(e)) => {
117+
return Err(VmError::native(e.to_string()));
118+
}
119+
Err(e) => return Err(VmError::native(e.to_string())),
117120
}
118121
};
119122
VmValue::Object(Rc::new(VmObject::Function(VmFunction::Native(Rc::new(
@@ -363,12 +366,12 @@ fn interp_to_vm_for_inverted_bridge(value: &InterpValue) -> VmValue {
363366
let callback = Rc::new(NativeFunction {
364367
name,
365368
static_type,
366-
func: Box::new(move |vm_args: &[VmValue]| {
369+
func: Box::new(move |vm_args: &[VmValue], _globals: &[VmValue]| {
367370
let mut interp_args: Vec<InterpValue> = vm_args.iter().map(vm_to_interp).collect();
368371
let dummy_env = Rc::new(RefCell::new(Environment::new(Box::new(Vec::<u8>::new()))));
369372
f.call(&mut interp_args, &dummy_env)
370373
.map(|v| interp_to_vm(v))
371-
.map_err(|e| e.to_string())
374+
.map_err(|e| VmError::native(e.to_string()))
372375
}),
373376
});
374377
return VmValue::Object(Rc::new(VmObject::Function(VmFunction::Native(callback))));
@@ -408,8 +411,8 @@ pub(crate) fn call_vm_native(
408411
let vm_args: Vec<VmValue> = args.iter().map(interp_to_vm_for_inverted_bridge).collect();
409412

410413
// 2. Call the vm_native closure
411-
let vm_result = (native.func)(&vm_args)
412-
.map_err(|e| FunctionCarrier::IntoEvaluationError(Box::new(anyhow::anyhow!(e))))?;
414+
let vm_result = (native.func)(&vm_args, &[])
415+
.map_err(|e| FunctionCarrier::IntoEvaluationError(Box::new(anyhow::anyhow!(e.message))))?;
413416

414417
// 3. Sync mutations back (vm → interp direction).
415418
// Strings do NOT need syncing — interp_to_vm_for_inverted_bridge shares the
@@ -440,7 +443,7 @@ fn vm_to_interp_callable(value: &VmValue, globals: Rc<Vec<VmValue>>) -> InterpVa
440443
let vm_args: Vec<VmValue> =
441444
args.iter().map(|a| interp_to_vm(a.clone())).collect();
442445
let result = Vm::call_function(f.clone(), vm_args, (*globals).clone())
443-
.map_err(|e| anyhow::anyhow!(e))?;
446+
.map_err(|e| anyhow::anyhow!(e.message))?;
444447
Ok(vm_to_interp(&result))
445448
});
446449
let data: Rc<dyn std::any::Any> = Rc::new(VmFunctionWrapper {

ndc_macros/src/function.rs

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -512,13 +512,13 @@ fn try_generate_vm_native(
512512
parameters: Some(vec![#(#param_types.clone()),*]),
513513
return_type: Box::new(#return_static_type),
514514
},
515-
func: Box::new(|args| match args {
515+
func: Box::new(|args, _globals| match args {
516516
[#(#raw_args),*] => {
517517
#(#extracts)*
518518
let result = #inner_ident(#(#passes),*);
519519
#return_code
520520
}
521-
_ => Err(format!("expected {} arguments, got {}", #n, args.len())),
521+
_ => Err(ndc_vm::error::VmError::native(format!("expected {} arguments, got {}", #n, args.len()))),
522522
}),
523523
});
524524
};
@@ -806,6 +806,43 @@ fn create_temp_variable(
806806
}];
807807
}
808808

809+
// &VmCallable — only used in VmNative functions (HOF path).
810+
// Dead-code stub for the interpreter wrapper: extract function from InterpValue,
811+
// convert to VmValue, then construct a VmCallable with empty globals.
812+
// This code path is unreachable at runtime since VmNative bodies bypass the wrapper.
813+
if path_ends_with(ty, "VmCallable") {
814+
let tmp_ident = syn::Ident::new(
815+
&format!("tmp_{argument_var_name}"),
816+
argument_var_name.span(),
817+
);
818+
return vec![Argument {
819+
param_type: quote! {
820+
ndc_interpreter::function::StaticType::Function {
821+
parameters: None,
822+
return_type: Box::new(ndc_interpreter::function::StaticType::Any),
823+
}
824+
},
825+
param_name: quote! { #original_name },
826+
argument: quote! { #argument_var_name },
827+
initialize_code: quote! {
828+
// Dead-code stub: VmCallable params only appear in functions with
829+
// FunctionBody::VmNative, so this interpreter wrapper is never called.
830+
let #tmp_ident =
831+
ndc_interpreter::vm_bridge::interp_to_vm(#argument_var_name.clone());
832+
let #argument_var_name = if let ndc_vm::value::Value::Object(obj) = &#tmp_ident {
833+
if let ndc_vm::value::Object::Function(f) = obj.as_ref() {
834+
ndc_vm::vm::VmCallable { function: f.clone(), globals: &[] }
835+
} else {
836+
panic!("VmCallable stub: expected Function variant");
837+
}
838+
} else {
839+
panic!("VmCallable stub: expected function object");
840+
};
841+
let #argument_var_name = &#argument_var_name;
842+
},
843+
}];
844+
}
845+
809846
// The pattern is Callable
810847
if path_ends_with(ty, "Callable") {
811848
let temp_var = syn::Ident::new(&format!("temp_{argument_var_name}"), identifier.span());

0 commit comments

Comments
 (0)