diff --git a/core/engine/src/bytecompiler/mod.rs b/core/engine/src/bytecompiler/mod.rs index db4607d1abd..ac48df7a52f 100644 --- a/core/engine/src/bytecompiler/mod.rs +++ b/core/engine/src/bytecompiler/mod.rs @@ -2259,9 +2259,14 @@ impl<'ctx> ByteCompiler<'ctx> { self.bytecode.emit_store_undefined(value.variable()); } - // TODO(@abhinavs1920): Add resource to disposal stack - // For now, we just bind the variable like a let declaration - // Full implementation will add: AddDisposableResource opcode + // Add resource to disposal stack + #[cfg(feature = "experimental")] + self.bytecode.emit_add_disposable_resource(value.variable()); + + #[cfg(not(feature = "experimental"))] + self.emit_type_error( + "using declarations require the 'experimental' feature", + ); self.emit_binding(BindingOpcode::InitLexical, ident, &value); self.register_allocator.dealloc(value); @@ -2275,7 +2280,14 @@ impl<'ctx> ByteCompiler<'ctx> { self.bytecode.emit_store_undefined(value.variable()); } - // TODO: Same as above + // Add resource to disposal stack + #[cfg(feature = "experimental")] + self.bytecode.emit_add_disposable_resource(value.variable()); + + #[cfg(not(feature = "experimental"))] + self.emit_type_error( + "using declarations require the 'experimental' feature", + ); self.compile_declaration_pattern( pattern, @@ -2300,9 +2312,11 @@ impl<'ctx> ByteCompiler<'ctx> { self.bytecode.emit_store_undefined(value.variable()); } - // TODO: Add resource to async disposal stack - // For now, we just bind the variable like a let declaration - // Full implementation will add: AddAsyncDisposableResource opcode + // await using is not yet implemented even under experimental + #[cfg(not(feature = "experimental"))] + self.emit_type_error( + "await using declarations require the 'experimental' feature", + ); self.emit_binding(BindingOpcode::InitLexical, ident, &value); self.register_allocator.dealloc(value); @@ -2316,7 +2330,12 @@ impl<'ctx> ByteCompiler<'ctx> { self.bytecode.emit_store_undefined(value.variable()); } - // TODO: SAME + // await using is not yet implemented even under experimental + #[cfg(not(feature = "experimental"))] + self.emit_type_error( + "await using declarations require the 'experimental' feature", + ); + self.compile_declaration_pattern( pattern, BindingOpcode::InitLexical, diff --git a/core/engine/src/bytecompiler/statement/block.rs b/core/engine/src/bytecompiler/statement/block.rs index 99da2023627..16690a2e843 100644 --- a/core/engine/src/bytecompiler/statement/block.rs +++ b/core/engine/src/bytecompiler/statement/block.rs @@ -1,12 +1,43 @@ use crate::bytecompiler::ByteCompiler; +#[cfg(not(feature = "experimental"))] use boa_ast::statement::Block; +#[cfg(feature = "experimental")] +use boa_ast::{ + declaration::LexicalDeclaration, + operations::{LexicallyScopedDeclaration, lexically_scoped_declarations}, + statement::Block, +}; impl ByteCompiler<'_> { /// Compile a [`Block`] `boa_ast` node pub(crate) fn compile_block(&mut self, block: &Block, use_expr: bool) { let scope = self.push_declarative_scope(block.scope()); self.block_declaration_instantiation(block); + + // Count how many `using` bindings are in this block (statically known at compile time) + #[cfg(feature = "experimental")] + let using_count: u32 = lexically_scoped_declarations(block) + .iter() + .filter_map(|decl| { + if let LexicallyScopedDeclaration::LexicalDeclaration( + LexicalDeclaration::Using(u) | LexicalDeclaration::AwaitUsing(u), + ) = decl + { + Some(u.as_ref().len() as u32) + } else { + None + } + }) + .sum(); + self.compile_statement_list(block.statement_list(), use_expr, true); + + // Emit DisposeResources with the static count if there are any using declarations + #[cfg(feature = "experimental")] + if using_count > 0 { + self.bytecode.emit_dispose_resources(using_count.into()); + } + self.pop_declarative_scope(scope); } } diff --git a/core/engine/src/vm/code_block.rs b/core/engine/src/vm/code_block.rs index d9eec2fc949..587ee193c1e 100644 --- a/core/engine/src/vm/code_block.rs +++ b/core/engine/src/vm/code_block.rs @@ -875,6 +875,8 @@ impl CodeBlock { | Instruction::PopPrivateEnvironment | Instruction::Generator | Instruction::AsyncGenerator => String::new(), + Instruction::AddDisposableResource { value } => format!("value: {value}"), + Instruction::DisposeResources { count } => format!("count: {count}"), Instruction::Reserved1 | Instruction::Reserved2 | Instruction::Reserved3 @@ -932,9 +934,7 @@ impl CodeBlock { | Instruction::Reserved55 | Instruction::Reserved56 | Instruction::Reserved57 - | Instruction::Reserved58 - | Instruction::Reserved59 - | Instruction::Reserved60 => unreachable!("Reserved opcodes are unreachable"), + | Instruction::Reserved58 => unreachable!("Reserved opcodes are unreachable"), } } } diff --git a/core/engine/src/vm/flowgraph/mod.rs b/core/engine/src/vm/flowgraph/mod.rs index 2f96e6edcbd..ab6087a89e4 100644 --- a/core/engine/src/vm/flowgraph/mod.rs +++ b/core/engine/src/vm/flowgraph/mod.rs @@ -374,6 +374,14 @@ impl CodeBlock { Instruction::Return => { graph.add_node(previous_pc, NodeShape::Diamond, label.into(), Color::Red); } + Instruction::AddDisposableResource { value } => { + let label = format!("AddDisposableResource value: {value}"); + graph.add_node(previous_pc, NodeShape::None, label.into(), Color::None); + } + Instruction::DisposeResources { count } => { + let label = format!("DisposeResources count: {count}"); + graph.add_node(previous_pc, NodeShape::None, label.into(), Color::None); + } Instruction::Reserved1 | Instruction::Reserved2 | Instruction::Reserved3 @@ -431,9 +439,7 @@ impl CodeBlock { | Instruction::Reserved55 | Instruction::Reserved56 | Instruction::Reserved57 - | Instruction::Reserved58 - | Instruction::Reserved59 - | Instruction::Reserved60 => unreachable!("Reserved opcodes are unreachable"), + | Instruction::Reserved58 => unreachable!("Reserved opcodes are unreachable"), } } diff --git a/core/engine/src/vm/mod.rs b/core/engine/src/vm/mod.rs index f01c9bb47dd..033b9d9295d 100644 --- a/core/engine/src/vm/mod.rs +++ b/core/engine/src/vm/mod.rs @@ -98,6 +98,12 @@ pub struct Vm { pub(crate) shadow_stack: ShadowStack, + /// Stack of disposable resources for explicit resource management. + /// + /// Resources are added via `using` declarations and disposed in reverse order (LIFO) + /// when the scope exits. + pub(crate) disposal_stack: Vec<(JsValue, JsValue)>, + #[cfg(feature = "trace")] pub(crate) trace: bool, #[cfg(feature = "trace")] @@ -349,6 +355,7 @@ impl Vm { native_active_function: None, host_call_depth: 0, shadow_stack: ShadowStack::default(), + disposal_stack: Vec::new(), #[cfg(feature = "trace")] trace: false, #[cfg(feature = "trace")] @@ -598,6 +605,16 @@ impl Vm { pub(crate) fn take_return_value(&mut self) -> JsValue { std::mem::take(&mut self.return_value) } + + /// Push a disposable resource onto the disposal stack. + pub(crate) fn push_disposable_resource(&mut self, value: JsValue, method: JsValue) { + self.disposal_stack.push((value, method)); + } + + /// Pop a disposable resource from the disposal stack. + pub(crate) fn pop_disposable_resource(&mut self) -> Option<(JsValue, JsValue)> { + self.disposal_stack.pop() + } } #[allow(clippy::print_stdout)] diff --git a/core/engine/src/vm/opcode/disposal/add_disposable.rs b/core/engine/src/vm/opcode/disposal/add_disposable.rs new file mode 100644 index 00000000000..a0402baca07 --- /dev/null +++ b/core/engine/src/vm/opcode/disposal/add_disposable.rs @@ -0,0 +1,47 @@ +use crate::{ + Context, JsResult, + vm::opcode::{Operation, RegisterOperand}, +}; + +/// `AddDisposableResource` implements the AddDisposableResource operation. +/// +/// This opcode adds a resource to the disposal stack for later cleanup. +/// +/// Operation: +/// - Stack: **=>** +/// - Registers: +/// - Input: value +pub(crate) struct AddDisposableResource; + +impl AddDisposableResource { + pub(crate) fn operation(value: RegisterOperand, context: &mut Context) -> JsResult<()> { + let value = context.vm.get_register(value.into()).clone(); + + // Per spec: If value is null or undefined, return + if value.is_null_or_undefined() { + return Ok(()); + } + + // Get the dispose method (value[Symbol.dispose]) + let key = crate::JsSymbol::dispose(); + let dispose_method = value.get_method(key, context)?; + + // If dispose method is None, return + let Some(dispose_method) = dispose_method else { + return Ok(()); + }; + + // Add to disposal stack + context + .vm + .push_disposable_resource(value, dispose_method.into()); + + Ok(()) + } +} + +impl Operation for AddDisposableResource { + const NAME: &'static str = "AddDisposableResource"; + const INSTRUCTION: &'static str = "INST - AddDisposableResource"; + const COST: u8 = 3; +} diff --git a/core/engine/src/vm/opcode/disposal/dispose_resources.rs b/core/engine/src/vm/opcode/disposal/dispose_resources.rs new file mode 100644 index 00000000000..1da9b7cfb80 --- /dev/null +++ b/core/engine/src/vm/opcode/disposal/dispose_resources.rs @@ -0,0 +1,60 @@ +use crate::{ + Context, JsError, JsNativeError, JsResult, + vm::opcode::{IndexOperand, Operation}, +}; + +/// `DisposeResources` implements the DisposeResources operation. +/// +/// This opcode disposes the last `count` resources from the disposal stack. +/// The count is statically determined by the bytecompiler. +/// +/// Operation: +/// - Stack: **=>** +pub(crate) struct DisposeResources; + +impl DisposeResources { + pub(crate) fn operation(count: IndexOperand, context: &mut Context) -> JsResult<()> { + let count = u32::from(count) as usize; + let mut suppressed_error: Option = None; + + // Dispose exactly `count` resources in reverse order (LIFO) + for _ in 0..count { + if let Some((value, method)) = context.vm.pop_disposable_resource() { + let result = method.call(&value, &[], context); + + if let Err(err) = result { + suppressed_error = Some(match suppressed_error { + None => err, + Some(previous) => create_suppressed_error(err, &previous, context), + }); + } + } + } + + if let Some(err) = suppressed_error { + return Err(err); + } + + Ok(()) + } +} + +impl Operation for DisposeResources { + const NAME: &'static str = "DisposeResources"; + const INSTRUCTION: &'static str = "INST - DisposeResources"; + const COST: u8 = 5; +} + +/// Helper function to create a SuppressedError +fn create_suppressed_error( + _error: JsError, + suppressed: &JsError, + _context: &mut Context, +) -> JsError { + // For now, we'll create a simple error that contains both errors + // TODO: Implement proper SuppressedError builtin in Phase 2 + let message = format!("An error was suppressed during disposal: {suppressed}"); + + // This is a temporary solution until SuppressedError is implemented + JsNativeError::error().with_message(message).into() +} diff --git a/core/engine/src/vm/opcode/disposal/mod.rs b/core/engine/src/vm/opcode/disposal/mod.rs new file mode 100644 index 00000000000..28518342a7e --- /dev/null +++ b/core/engine/src/vm/opcode/disposal/mod.rs @@ -0,0 +1,5 @@ +mod add_disposable; +mod dispose_resources; + +pub(crate) use add_disposable::*; +pub(crate) use dispose_resources::*; diff --git a/core/engine/src/vm/opcode/mod.rs b/core/engine/src/vm/opcode/mod.rs index e304514a8fc..e096d81394d 100644 --- a/core/engine/src/vm/opcode/mod.rs +++ b/core/engine/src/vm/opcode/mod.rs @@ -20,6 +20,7 @@ mod control_flow; mod copy; mod define; mod delete; +mod disposal; mod environment; mod function; mod generator; @@ -59,6 +60,8 @@ pub(crate) use define::*; #[doc(inline)] pub(crate) use delete::*; #[doc(inline)] +pub(crate) use disposal::*; +#[doc(inline)] pub(crate) use environment::*; #[doc(inline)] pub(crate) use function::*; @@ -2139,6 +2142,23 @@ generate_opcodes! { /// - Output: dst CreateUnmappedArgumentsObject { dst: RegisterOperand }, + /// Add a disposable resource to the disposal stack. + /// + /// This opcode implements the AddDisposableResource abstract operation. + /// It gets the dispose method from the value and adds it to the disposal stack. + /// + /// - Registers: + /// - Input: value + AddDisposableResource { #[allow(dead_code)] value: RegisterOperand }, + + /// Dispose all resources in the current disposal stack. + /// + /// This opcode implements the DisposeResources abstract operation. + /// It calls the last `count` dispose methods in reverse order (LIFO). + /// The count is statically determined by the bytecompiler. + /// + /// - Stack: **=>** + DisposeResources { count: IndexOperand }, /// Reserved [`Opcode`]. Reserved1 => Reserved, /// Reserved [`Opcode`]. @@ -2255,8 +2275,4 @@ generate_opcodes! { Reserved57 => Reserved, /// Reserved [`Opcode`]. Reserved58 => Reserved, - /// Reserved [`Opcode`]. - Reserved59 => Reserved, - /// Reserved [`Opcode`]. - Reserved60 => Reserved, } diff --git a/core/engine/tests/disposal.rs b/core/engine/tests/disposal.rs new file mode 100644 index 00000000000..f042770eebe --- /dev/null +++ b/core/engine/tests/disposal.rs @@ -0,0 +1,188 @@ +//! Tests for explicit resource management (using declarations). +//! +//! This module tests the core disposal mechanism for `using` declarations, +//! verifying that resources are properly disposed when scopes exit. + +#![allow(unused_crate_dependencies)] + +use boa_engine::{Context, JsValue, Source}; + +#[test] +fn basic_disposal() { + let mut context = Context::default(); + + let result = context.eval(Source::from_bytes( + r" + let disposed = false; + { + using x = { + [Symbol.dispose]() { + disposed = true; + } + }; + } + disposed; + ", + )); + + assert!(result.is_ok()); + let value = result.unwrap(); + assert_eq!(value, JsValue::from(true)); +} + +#[test] +fn disposal_order() { + let mut context = Context::default(); + + let result = context.eval(Source::from_bytes( + r" + let order = []; + { + using a = { + [Symbol.dispose]() { + order.push('a'); + } + }; + using b = { + [Symbol.dispose]() { + order.push('b'); + } + }; + } + order.join(','); + ", + )); + + assert!(result.is_ok()); + let value = result.unwrap(); + // Should dispose in reverse order: b, then a + assert_eq!(value.to_string(&mut context).unwrap(), "b,a"); +} + +#[test] +fn null_undefined_disposal() { + let mut context = Context::default(); + + let result = context.eval(Source::from_bytes( + r" + { + using x = null; + using y = undefined; + } + 'ok'; + ", + )); + + assert!(result.is_ok()); + let value = result.unwrap(); + assert_eq!(value.to_string(&mut context).unwrap(), "ok"); +} + +#[test] +fn disposal_with_no_method() { + let mut context = Context::default(); + + let result = context.eval(Source::from_bytes( + r" + { + using x = { + // No Symbol.dispose method + }; + } + 'ok'; + ", + )); + + assert!(result.is_ok()); + let value = result.unwrap(); + assert_eq!(value.to_string(&mut context).unwrap(), "ok"); +} + +#[test] +#[ignore = "Disposal on exception requires try-finally integration - will be implemented in next phase"] +fn disposal_on_exception() { + let mut context = Context::default(); + + let result = context.eval(Source::from_bytes( + r" + let disposed = false; + try { + using x = { + [Symbol.dispose]() { + disposed = true; + } + }; + throw new Error('test error'); + } catch (e) { + // Disposal should happen before catch + } + disposed; + ", + )); + + assert!(result.is_ok()); + let value = result.unwrap(); + assert_eq!(value, JsValue::from(true)); +} + +#[test] +fn nested_scopes() { + let mut context = Context::default(); + + let result = context.eval(Source::from_bytes( + r" + let order = []; + { + using a = { + [Symbol.dispose]() { + order.push('a'); + } + }; + { + using b = { + [Symbol.dispose]() { + order.push('b'); + } + }; + } + // b should be disposed here + order.push('middle'); + } + // a should be disposed here + order.join(','); + ", + )); + + assert!(result.is_ok()); + let value = result.unwrap(); + // Should dispose b first, then a + assert_eq!(value.to_string(&mut context).unwrap(), "b,middle,a"); +} + +#[test] +fn multiple_resources_in_one_declaration() { + let mut context = Context::default(); + + let result = context.eval(Source::from_bytes( + r" + let order = []; + { + using a = { + [Symbol.dispose]() { + order.push('a'); + } + }, b = { + [Symbol.dispose]() { + order.push('b'); + } + }; + } + order.join(','); + ", + )); + + assert!(result.is_ok()); + let value = result.unwrap(); + // Should dispose in reverse order: b, then a + assert_eq!(value.to_string(&mut context).unwrap(), "b,a"); +}