diff --git a/crates/oxide_builtins/src/iterator.rs b/crates/oxide_builtins/src/iterator.rs index 19e78620..ee8457aa 100644 --- a/crates/oxide_builtins/src/iterator.rs +++ b/crates/oxide_builtins/src/iterator.rs @@ -36,9 +36,40 @@ pub fn make_iterator_for_value(vm: &mut H, value: JsValue) -> Result< let next_fn = make_native_function(vm, "next", iterator_wrapper_next:: as *const (), 0); vm.set_or_create_prop_value(wrapper_obj, next_si, next_fn); + // Forward IteratorClose to the inner iterator so for-of abrupt completion can clean up. + let return_si = vm.kernel_core().perm_interner().intern("return").0; + let return_fn = make_native_function(vm, "return", iterator_wrapper_return:: as *const (), 0); + vm.set_or_create_prop_value(wrapper_obj, return_si, return_fn); + Ok(JsValue::from_js_object(wrapper)) } +fn iterator_wrapper_return(vm: &mut H, args: &[u8]) -> NativeResult { + let this_val = vm.reg(if args.is_empty() { 0 } else { args[0] }); + if !this_val.is_object() { + return NativeResult::Ok(JsValue::undefined()); + } + let wrapper = unsafe { &*this_val.as_js_object_ptr() }; + let inner_si = vm.kernel_core().perm_interner().intern(INNER_PROP).0; + let inner = match vm.ordinary_get(wrapper, inner_si, this_val) { + Ok(inner) if inner.is_object() => inner, + _ => return NativeResult::Ok(JsValue::undefined()), + }; + let inner_obj = unsafe { &*inner.as_js_object_ptr() }; + let return_si = vm.kernel_core().perm_interner().intern("return").0; + let return_fn = match vm.ordinary_get(inner_obj, return_si, inner) { + Ok(f) if is_callable(f) => f, + _ => return NativeResult::Ok(JsValue::undefined()), + }; + match vm.call_function_sync(return_fn, inner, &[]) { + Ok(result) => NativeResult::Ok(result), + Err(err) => match vm.take_uncaught_value() { + Some(original) => NativeResult::Err(original), + None => NativeResult::Err(crate::error::create_type_error(vm, &err)), + }, + } +} + pub fn iterator_wrapper_next(vm: &mut H, args: &[u8]) -> NativeResult { let this_val = vm.reg(if args.is_empty() { 0 } else { args[0] }); if !this_val.is_object() { @@ -66,7 +97,12 @@ pub fn iterator_wrapper_next(vm: &mut H, args: &[u8]) -> NativeResult }; return match vm.call_function_sync(next, inner, &[]) { Ok(result) => NativeResult::Ok(result), - Err(err) => NativeResult::Err(crate::error::create_type_error(vm, &err)), + // Forward the ORIGINAL thrown value (any type) instead of re-wrapping it as a + // TypeError, so a surrounding try/catch sees the real error. + Err(err) => match vm.take_uncaught_value() { + Some(original) => NativeResult::Err(original), + None => NativeResult::Err(crate::error::create_type_error(vm, &err)), + }, }; } diff --git a/crates/oxide_runtime_api/src/lib.rs b/crates/oxide_runtime_api/src/lib.rs index 69127277..f8043e37 100644 --- a/crates/oxide_runtime_api/src/lib.rs +++ b/crates/oxide_runtime_api/src/lib.rs @@ -101,6 +101,9 @@ pub trait VmHost { // Call infrastructure fn call_function_sync(&mut self, callee: JsValue, receiver: JsValue, args: &[JsValue]) -> Result; + /// Take the original thrown JsValue preserved across a String-flattening call boundary, + /// so an iterator wrapper can re-throw the original error instead of re-wrapping it. + fn take_uncaught_value(&mut self) -> Option; // Error handling fn checked_object_ptr(&mut self, val: JsValue, error_msg: &str) -> Result, String>; diff --git a/crates/oxide_vm/src/vm.rs b/crates/oxide_vm/src/vm.rs index 9a2b7fbf..1d14254c 100644 --- a/crates/oxide_vm/src/vm.rs +++ b/crates/oxide_vm/src/vm.rs @@ -113,6 +113,8 @@ pub struct TryHandler { pub catch_pc: Option, pub finally_pc: Option, pub frame_depth: usize, + /// for_of_iters length at try entry — bounds which iterators IteratorClose on unwind. + pub for_of_depth: usize, } /// Heap-allocated snapshot used by `call_bytecode_function_inline`. @@ -164,6 +166,10 @@ pub struct Vm { pub(crate) save_stack: Vec, pub(crate) try_stack: Vec, pub(crate) exception_value: Option, + /// Side-channel carrying the JsValue thrown by a sync call whose error was flattened + /// to a String by `call_function_sync`/`unwind`, so for-of can re-throw the original + /// value. Plain field (NOT in InlineSyncState) so it survives the inline-call restore. + pub(crate) last_uncaught_value: Option, pub(crate) pending_exception: Option, pub(crate) pending_error_kind: Option<&'static str>, pub(crate) root_reg_limit: u8, @@ -635,7 +641,10 @@ impl Vm { self.regs = saved_regs; return match result { NativeResult::Ok(val) => Ok(val), - NativeResult::Err(err) => Err(self.error_text(err)), + NativeResult::Err(err) => { + self.last_uncaught_value = Some(err); + Err(self.error_text(err)) + } NativeResult::TailCall { callee, this, args } => self.call_function_sync(callee, this, &args), }; } @@ -1267,6 +1276,7 @@ mod tests { catch_pc: Some(1), finally_pc: None, frame_depth: 0, + for_of_depth: 0, }); vm.exception_value = Some(JsValue::int(2)); vm.pending_exception = Some(JsValue::int(3)); @@ -1520,6 +1530,9 @@ impl oxide_runtime_api::VmHost for Vm { fn epoch(&self) -> &Epoch { self.epoch() } + fn take_uncaught_value(&mut self) -> Option { + self.last_uncaught_value.take() + } fn property_key_si(&mut self, val: JsValue) -> u32 { self.property_key_si(val) } diff --git a/crates/oxide_vm/src/vm_dispatch_ctrl.rs b/crates/oxide_vm/src/vm_dispatch_ctrl.rs index 430dc991..5410395f 100644 --- a/crates/oxide_vm/src/vm_dispatch_ctrl.rs +++ b/crates/oxide_vm/src/vm_dispatch_ctrl.rs @@ -177,6 +177,7 @@ impl Vm { catch_pc, finally_pc: None, frame_depth: self.frames.len(), + for_of_depth: self.iters.for_of_iters.len(), }); } @@ -193,6 +194,7 @@ impl Vm { catch_pc: None, finally_pc: Some(finally_pc), frame_depth: self.frames.len(), + for_of_depth: self.iters.for_of_iters.len(), }); } diff --git a/crates/oxide_vm/src/vm_dispatch_misc.rs b/crates/oxide_vm/src/vm_dispatch_misc.rs index 3816277e..b7b7ddfd 100644 --- a/crates/oxide_vm/src/vm_dispatch_misc.rs +++ b/crates/oxide_vm/src/vm_dispatch_misc.rs @@ -402,17 +402,26 @@ impl Vm { return Err("FOR_OF_DONE iterator is not an object".into()); } + self.last_uncaught_value = None; let iter_obj = unsafe { &*iterator.as_js_object_ptr() }; let next_si = self.kernel_core.perm_interner().intern("next").0; - let next_fn = self.ordinary_get(iter_obj, next_si, iterator)?; - let result = self.call_function_sync(next_fn, iterator, &[])?; + let next_fn = match self.ordinary_get(iter_obj, next_si, iterator) { + Ok(v) => v, + Err(e) => return self.throw_for_of_error(e), + }; + let result = match self.call_function_sync(next_fn, iterator, &[]) { + Ok(v) => v, + Err(e) => return self.throw_for_of_error(e), + }; if !result.is_object() { return self.raise_type_error("iterator result is not an object"); } - let result_obj = unsafe { &*result.as_js_object_ptr() }; let done_si = self.kernel_core.perm_interner().intern("done").0; - let done_val = self.ordinary_get(result_obj, done_si, result)?; + let done_val = match self.ordinary_get(result_obj, done_si, result) { + Ok(v) => v, + Err(e) => return self.throw_for_of_error(e), + }; let done = to_boolean(done_val); self.iters.set_last_for_of_result(result); self.regs[rd] = JsValue::bool(!done); @@ -421,6 +430,7 @@ impl Vm { pub(crate) fn dispatch_for_of_next(&mut self, rd: usize) -> Result<(), String> { vm_trace!("FOR_OF_NEXT rd={}", rd); + self.last_uncaught_value = None; let result = self.iters.last_for_of_result(); if !result.is_object() { self.regs[rd] = JsValue::undefined(); @@ -428,30 +438,86 @@ impl Vm { } let result_obj = unsafe { &*result.as_js_object_ptr() }; let value_si = self.kernel_core.perm_interner().intern("value").0; - self.regs[rd] = self.ordinary_get(result_obj, value_si, result)?; + self.regs[rd] = match self.ordinary_get(result_obj, value_si, result) { + Ok(v) => v, + Err(e) => return self.throw_for_of_error(e), + }; Ok(()) } + /// next()/value access threw: route through unwind so a surrounding try/catch can catch + /// it, re-throwing the ORIGINAL value when available. Per ECMA-262 the iterator is NOT + /// closed via return() on a next()-throw — pop it so the unwinding IteratorClose pass skips it. + fn throw_for_of_error(&mut self, msg: String) -> Result<(), String> { + self.iters.pop_for_of(); + let exc = match self.last_uncaught_value.take() { + Some(v) => v, + None => oxide_builtins::error::create_error(self, &msg), + }; + self.exception_value = Some(exc); + self.pending_error_kind = Some(self.thrown_error_kind(exc)); + self.unwind() + } + pub(crate) fn dispatch_for_of_close(&mut self) -> Result<(), String> { vm_trace!("FOR_OF_CLOSE"); let Some(iterator) = self.iters.pop_for_of() else { return Ok(()); }; + // Normal / break / return exit: no prior abrupt completion, so return()'s own throw propagates. + self.close_for_of_iterator(iterator, false) + } + + /// Call `iterator.return()` (IteratorClose). When `suppress_return_error` is true the call + /// is being made because an enclosing abrupt completion is unwinding: return()'s own result + /// is discarded and the in-flight exception is preserved across the call. When false (normal + /// for-of exit) return()'s error propagates. + pub(crate) fn close_for_of_iterator( + &mut self, iterator: JsValue, suppress_return_error: bool, + ) -> Result<(), String> { if !iterator.is_object() { return Ok(()); } let iter_obj = unsafe { &*iterator.as_js_object_ptr() }; let return_si = self.kernel_core.perm_interner().intern("return").0; - let return_fn = self.ordinary_get(iter_obj, return_si, iterator)?; + let return_fn = match self.ordinary_get(iter_obj, return_si, iterator) { + Ok(v) => v, + Err(e) => { + if suppress_return_error { + return Ok(()); + } + return Err(e); + } + }; if return_fn.is_object() { let return_obj = unsafe { &*return_fn.as_js_object_ptr() }; if return_obj.is_function() { - let _ = self.call_function_sync(return_fn, iterator, &[])?; + if suppress_return_error { + let saved_exc = self.exception_value; + let saved_kind = self.pending_error_kind; + let _ = self.call_function_sync(return_fn, iterator, &[]); + self.exception_value = saved_exc; + self.pending_error_kind = saved_kind; + } else { + let _ = self.call_function_sync(return_fn, iterator, &[])?; + } } } Ok(()) } + /// IteratorClose every active for-of iterator above `depth`, used by `unwind()` to close + /// loops abandoned by a throw. Pops before closing so a re-entrant call can't re-close, + /// and so the next()-throw path (which already popped its own iterator) is skipped. + pub(crate) fn close_for_of_above(&mut self, depth: usize) { + while self.iters.for_of_iters.len() > depth { + let Some(iterator) = self.iters.pop_for_of() else { + break; + }; + let _ = self.close_for_of_iterator(iterator, true); + } + } + pub(crate) fn dispatch_rest_object(&mut self, rd: usize, a: usize) -> Result<(), String> { vm_trace!("REST_OBJECT rd={}", rd); let src = self.regs[a]; diff --git a/crates/oxide_vm/src/vm_runtime.rs b/crates/oxide_vm/src/vm_runtime.rs index 770c5f12..82bbd67b 100644 --- a/crates/oxide_vm/src/vm_runtime.rs +++ b/crates/oxide_vm/src/vm_runtime.rs @@ -176,6 +176,8 @@ impl Vm { self.restore_frame(frame); } } + // IteratorClose any for-of loops abandoned inside this handler's scope. + self.close_for_of_above(handler.for_of_depth); if let Some(finally_pc) = handler.finally_pc { vm_trace!("unwind: entering finally at pc={}", finally_pc); if self.pending_exception.is_none() { @@ -193,10 +195,14 @@ impl Vm { return Ok(()); } } + self.close_for_of_above(0); while let Some(frame) = self.frames.pop() { self.restore_frame(frame); } let exc = self.exception_value.take().unwrap_or(JsValue::undefined()); + // Preserve the escaping JsValue so a for-of next()-throw can re-throw the original + // value instead of the flattened string (consumed by dispatch_for_of_done/next). + self.last_uncaught_value = Some(exc); let kind_str = self.pending_error_kind.take().unwrap_or("Error"); let exc_text = self.error_text(exc); let msg = if exc_text.starts_with(kind_str) { diff --git a/crates/oxide_vm/src/vm_support.rs b/crates/oxide_vm/src/vm_support.rs index bbaac710..39f9529e 100644 --- a/crates/oxide_vm/src/vm_support.rs +++ b/crates/oxide_vm/src/vm_support.rs @@ -38,6 +38,7 @@ impl Vm { save_stack: Vec::new(), try_stack: Vec::new(), exception_value: None, + last_uncaught_value: None, pending_exception: None, pending_error_kind: None, root_reg_limit: 0, @@ -95,6 +96,7 @@ impl Vm { save_stack: Vec::new(), try_stack: Vec::new(), exception_value: None, + last_uncaught_value: None, pending_exception: None, pending_error_kind: None, root_reg_limit: 0, diff --git a/crates/oxide_vm/tests/for_of_abrupt_completion_tests.rs b/crates/oxide_vm/tests/for_of_abrupt_completion_tests.rs new file mode 100644 index 00000000..97702aa7 --- /dev/null +++ b/crates/oxide_vm/tests/for_of_abrupt_completion_tests.rs @@ -0,0 +1,126 @@ +use oxide_compiler::compiler::Compiler; +use oxide_parser::Allocator; +use oxide_vm::vm::Vm; + +fn eval(source: &str) -> String { + let allocator = Allocator::default(); + let program = match oxide_parser::parse(&allocator, source) { + Ok(p) => p, + Err(e) => return format!("parse error: {}", e[0].message), + }; + let module = match Compiler::new().compile(&program) { + Ok(m) => m, + Err(e) => return format!("compile error: {e}"), + }; + let mut vm = Vm::new(); + match vm.run(&module) { + Ok(result) => format!("{result}"), + Err(e) => format!("vm error: {e}"), + } +} + +// Spec (ECMA-262 §13.7.5): next()-throw propagates the ORIGINAL value WITHOUT calling +// return(); IteratorClose (return()) runs only on BODY abrupt completion (throw/break/return). +// return() is observed via globalThis (a function CAN read/write globalThis properties; +// it cannot write a plain outer `var` in this engine). + +#[test] +fn for_of_next_throw_is_catchable_with_original_type() { + assert_eq!( + eval( + "try{for(var v of {next:function(){throw new TypeError('boom');}}){}}\ + catch(e){e.name==='TypeError' && e.message==='boom'}" + ), + "true", + "next() throw catchable with original error type+message" + ); +} + +#[test] +fn for_of_next_throw_preserves_bare_thrown_value() { + assert_eq!( + eval("try{for(var v of {next:function(){throw 7;}}){}}catch(e){e===7}"), + "true", + "a bare thrown value is preserved, not re-wrapped" + ); +} + +#[test] +fn for_of_next_throw_does_not_call_return() { + assert_eq!( + eval( + "globalThis.r=false;\ + var it={next:function(){throw new Error('x');},return:function(){globalThis.r=true;return {};}};\ + try{for(var v of it){}}catch(e){}globalThis.r===false" + ), + "true", + "return() must NOT be called when next() throws" + ); +} + +#[test] +fn for_of_next_throw_without_return_still_propagates() { + assert_eq!( + eval("try{for(var v of {next:function(){throw new TypeError('e');}}){}}catch(e){e instanceof TypeError}"), + "true" + ); +} + +#[test] +fn for_of_body_throw_calls_return() { + assert_eq!( + eval( + "globalThis.r=false;\ + var it={next:function(){return {value:1,done:false};},return:function(){globalThis.r=true;return {};}};\ + try{for(var v of it){throw new Error('boom');}}catch(e){}globalThis.r===true" + ), + "true", + "body throw calls return() (IteratorClose)" + ); +} + +#[test] +fn for_of_body_throw_propagates_body_error() { + assert_eq!( + eval( + "var it={next:function(){return {value:1,done:false};},return:function(){return {};}};\ + try{for(var v of it){throw new Error('boom');}}catch(e){e.message==='boom'}" + ), + "true" + ); +} + +#[test] +fn for_of_body_throw_return_also_throws_body_error_wins() { + assert_eq!( + eval( + "var it={next:function(){return {value:1,done:false};},return:function(){throw new Error('B');}};\ + try{for(var v of it){throw new Error('A');}}catch(e){e.message==='A'}" + ), + "true", + "when return() also throws, the body's original error wins" + ); +} + +#[test] +fn for_of_break_calls_return() { + assert_eq!( + eval( + "globalThis.r=false;\ + var it={next:function(){return {value:1,done:false};},return:function(){globalThis.r=true;return {};}};\ + for(var v of it){break;}globalThis.r===true" + ), + "true", + "break calls return() (via FOR_OF_CLOSE)" + ); +} + +#[test] +fn for_of_array_regression_still_iterates() { + assert_eq!(eval("var r=0;for(var v of [1,2,3]){r=r+v;}r===6"), "true"); +} + +#[test] +fn for_of_string_regression_still_iterates() { + assert_eq!(eval("var r=0;for(var c of 'abc'){r=r+1;}r===3"), "true"); +}