Skip to content

Commit 7023cb2

Browse files
committed
refactor: encode using declaration count statically in DisposeResources opcode, removing runtime scope depth tracking
Signed-off-by: Abhinav Sharma <abhinavs1920bpl@gmail.com>
1 parent d0517fe commit 7023cb2

File tree

8 files changed

+42
-110
lines changed

8 files changed

+42
-110
lines changed

core/engine/src/bytecompiler/statement/block.rs

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -11,26 +11,26 @@ impl ByteCompiler<'_> {
1111
let scope = self.push_declarative_scope(block.scope());
1212
self.block_declaration_instantiation(block);
1313

14-
// Check if this block has any using declarations
15-
let has_using = lexically_scoped_declarations(block).iter().any(|decl| {
16-
matches!(
17-
decl,
18-
LexicallyScopedDeclaration::LexicalDeclaration(
19-
LexicalDeclaration::Using(_) | LexicalDeclaration::AwaitUsing(_)
20-
)
21-
)
22-
});
23-
24-
// Push disposal scope if this block has using declarations
25-
if has_using {
26-
self.bytecode.emit_push_disposal_scope();
27-
}
14+
// Count how many `using` bindings are in this block (statically known at compile time)
15+
let using_count: u32 = lexically_scoped_declarations(block)
16+
.iter()
17+
.filter_map(|decl| {
18+
if let LexicallyScopedDeclaration::LexicalDeclaration(
19+
LexicalDeclaration::Using(u) | LexicalDeclaration::AwaitUsing(u),
20+
) = decl
21+
{
22+
Some(u.as_ref().len() as u32)
23+
} else {
24+
None
25+
}
26+
})
27+
.sum();
2828

2929
self.compile_statement_list(block.statement_list(), use_expr, true);
3030

31-
// Dispose resources if this block has using declarations
32-
if has_using {
33-
self.bytecode.emit_dispose_resources();
31+
// Emit DisposeResources with the static count if there are any using declarations
32+
if using_count > 0 {
33+
self.bytecode.emit_dispose_resources(using_count.into());
3434
}
3535

3636
self.pop_declarative_scope(scope);

core/engine/src/vm/code_block.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -892,10 +892,9 @@ impl CodeBlock {
892892
| Instruction::SuperCallSpread
893893
| Instruction::PopPrivateEnvironment
894894
| Instruction::Generator
895-
| Instruction::AsyncGenerator
896-
| Instruction::AddDisposableResource { .. }
897-
| Instruction::DisposeResources
898-
| Instruction::PushDisposalScope => String::new(),
895+
| Instruction::AsyncGenerator => String::new(),
896+
Instruction::AddDisposableResource { value } => format!("value: {value}"),
897+
Instruction::DisposeResources { count } => format!("count: {count}"),
899898
Instruction::Reserved4
900899
| Instruction::Reserved5
901900
| Instruction::Reserved6
@@ -947,7 +946,8 @@ impl CodeBlock {
947946
| Instruction::Reserved52
948947
| Instruction::Reserved53
949948
| Instruction::Reserved54
950-
| Instruction::Reserved55 => unreachable!("Reserved opcodes are unreachable"),
949+
| Instruction::Reserved55
950+
| Instruction::Reserved56 => unreachable!("Reserved opcodes are unreachable"),
951951
}
952952
}
953953
}

core/engine/src/vm/flowgraph/mod.rs

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -383,21 +383,9 @@ impl CodeBlock {
383383
let label = format!("AddDisposableResource value: {value}");
384384
graph.add_node(previous_pc, NodeShape::None, label.into(), Color::None);
385385
}
386-
Instruction::DisposeResources => {
387-
graph.add_node(
388-
previous_pc,
389-
NodeShape::None,
390-
"DisposeResources".into(),
391-
Color::None,
392-
);
393-
}
394-
Instruction::PushDisposalScope => {
395-
graph.add_node(
396-
previous_pc,
397-
NodeShape::None,
398-
"PushDisposalScope".into(),
399-
Color::None,
400-
);
386+
Instruction::DisposeResources { count } => {
387+
let label = format!("DisposeResources count: {count}");
388+
graph.add_node(previous_pc, NodeShape::None, label.into(), Color::None);
401389
}
402390
Instruction::Reserved4
403391
| Instruction::Reserved5

core/engine/src/vm/mod.rs

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -101,13 +101,9 @@ pub struct Vm {
101101
/// Stack of disposable resources for explicit resource management.
102102
///
103103
/// Resources are added via `using` declarations and disposed in reverse order (LIFO)
104-
/// when the scope exits. Each entry contains (`value`, `dispose_method`, `scope_depth`).
104+
/// when the scope exits.
105105
pub(crate) disposal_stack: Vec<(JsValue, JsValue)>,
106106

107-
/// Tracks the disposal stack depth for each scope level.
108-
/// When a scope exits, we dispose resources back to this depth.
109-
pub(crate) disposal_scope_depths: Vec<usize>,
110-
111107
#[cfg(feature = "trace")]
112108
pub(crate) trace: bool,
113109
#[cfg(feature = "trace")]
@@ -360,7 +356,6 @@ impl Vm {
360356
host_call_depth: 0,
361357
shadow_stack: ShadowStack::default(),
362358
disposal_stack: Vec::new(),
363-
disposal_scope_depths: Vec::new(),
364359
#[cfg(feature = "trace")]
365360
trace: false,
366361
#[cfg(feature = "trace")]
@@ -620,21 +615,6 @@ impl Vm {
620615
pub(crate) fn pop_disposable_resource(&mut self) -> Option<(JsValue, JsValue)> {
621616
self.disposal_stack.pop()
622617
}
623-
624-
/// Mark the current disposal stack depth for a new scope.
625-
pub(crate) fn push_disposal_scope(&mut self) {
626-
self.disposal_scope_depths.push(self.disposal_stack.len());
627-
}
628-
629-
/// Get the disposal stack depth for the current scope.
630-
pub(crate) fn current_disposal_scope_depth(&self) -> usize {
631-
self.disposal_scope_depths.last().copied().unwrap_or(0)
632-
}
633-
634-
/// Pop the disposal scope depth marker.
635-
pub(crate) fn pop_disposal_scope(&mut self) {
636-
self.disposal_scope_depths.pop();
637-
}
638618
}
639619

640620
#[allow(clippy::print_stdout)]
Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,43 +1,36 @@
1-
use crate::{Context, JsError, JsNativeError, JsResult, vm::opcode::Operation};
1+
use crate::{
2+
Context, JsError, JsNativeError, JsResult,
3+
vm::opcode::{IndexOperand, Operation},
4+
};
25

36
/// `DisposeResources` implements the DisposeResources operation.
47
///
5-
/// This opcode disposes all resources in the current disposal stack.
8+
/// This opcode disposes the last `count` resources from the disposal stack.
9+
/// The count is statically determined by the bytecompiler.
610
///
711
/// Operation:
812
/// - Stack: **=>**
913
pub(crate) struct DisposeResources;
1014

1115
impl DisposeResources {
12-
pub(crate) fn operation((): (), context: &mut Context) -> JsResult<()> {
16+
pub(crate) fn operation(count: IndexOperand, context: &mut Context) -> JsResult<()> {
17+
let count = u32::from(count) as usize;
1318
let mut suppressed_error: Option<JsError> = None;
1419

15-
// Get the scope depth to know how many resources to dispose
16-
let scope_depth = context.vm.current_disposal_scope_depth();
17-
18-
// Dispose resources in reverse order (LIFO) until we reach the scope depth
19-
while context.vm.disposal_stack.len() > scope_depth {
20+
// Dispose exactly `count` resources in reverse order (LIFO)
21+
for _ in 0..count {
2022
if let Some((value, method)) = context.vm.pop_disposable_resource() {
21-
// Call the dispose method
2223
let result = method.call(&value, &[], context);
2324

24-
// If an error occurs, aggregate it
2525
if let Err(err) = result {
2626
suppressed_error = Some(match suppressed_error {
2727
None => err,
28-
Some(previous) => {
29-
// Create a SuppressedError
30-
create_suppressed_error(err, &previous, context)
31-
}
28+
Some(previous) => create_suppressed_error(err, &previous, context),
3229
});
3330
}
3431
}
3532
}
3633

37-
// Pop the disposal scope depth marker
38-
context.vm.pop_disposal_scope();
39-
40-
// If there were any errors, throw the aggregated error
4134
if let Some(err) = suppressed_error {
4235
return Err(err);
4336
}
@@ -62,7 +55,6 @@ fn create_suppressed_error(
6255
// TODO: Implement proper SuppressedError builtin in Phase 2
6356
let message = format!("An error was suppressed during disposal: {suppressed}");
6457

65-
// Attach the original error as a property
6658
// This is a temporary solution until SuppressedError is implemented
6759
JsNativeError::error().with_message(message).into()
6860
}
Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
mod add_disposable;
22
mod dispose_resources;
3-
mod push_scope;
43

54
pub(crate) use add_disposable::*;
65
pub(crate) use dispose_resources::*;
7-
pub(crate) use push_scope::*;

core/engine/src/vm/opcode/disposal/push_scope.rs

Lines changed: 0 additions & 21 deletions
This file was deleted.

core/engine/src/vm/opcode/mod.rs

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2207,18 +2207,11 @@ generate_opcodes! {
22072207
/// Dispose all resources in the current disposal stack.
22082208
///
22092209
/// This opcode implements the DisposeResources abstract operation.
2210-
/// It calls all dispose methods in reverse order (LIFO).
2210+
/// It calls the last `count` dispose methods in reverse order (LIFO).
2211+
/// The count is statically determined by the bytecompiler.
22112212
///
22122213
/// - Stack: **=>**
2213-
DisposeResources,
2214-
2215-
/// Push a new disposal scope.
2216-
///
2217-
/// This marks the current disposal stack depth for a new scope.
2218-
/// When DisposeResources is called, it will dispose resources back to this depth.
2219-
///
2220-
/// - Stack: **=>**
2221-
PushDisposalScope,
2214+
DisposeResources { count: IndexOperand },
22222215
/// Reserved [`Opcode`].
22232216
Reserved4 => Reserved,
22242217
/// Reserved [`Opcode`].
@@ -2323,4 +2316,6 @@ generate_opcodes! {
23232316
Reserved54 => Reserved,
23242317
/// Reserved [`Opcode`].
23252318
Reserved55 => Reserved,
2319+
/// Reserved [`Opcode`].
2320+
Reserved56 => Reserved,
23262321
}

0 commit comments

Comments
 (0)