Skip to content

Commit

Permalink
feat(engine): Support destructuring in catch parameter bindings (#511)
Browse files Browse the repository at this point in the history
  • Loading branch information
@aapoalas
aapoalas authored Jan 5, 2025
1 parent 5c1c447 commit f6da795
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 142 deletions.
47 changes: 40 additions & 7 deletions nova_vm/src/engine/bytecode/bytecode_compiler.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2766,26 +2766,59 @@ impl CompileEvaluation for ast::TryStatement<'_> {

let catch_clause = self.handler.as_ref().unwrap();
ctx.set_jump_target_here(jump_to_catch);

if let Some(exception_param) = &catch_clause.param {
let ast::BindingPatternKind::BindingIdentifier(identifier) =
&exception_param.pattern.kind
else {
todo!("{:?}", exception_param.pattern.kind);
};
// 1. Let oldEnv be the running execution context's LexicalEnvironment.
// 2. Let catchEnv be NewDeclarativeEnvironment(oldEnv).
// 4. Set the running execution context's LexicalEnvironment to catchEnv.
// Note: We skip the declarative environment if there is no catch
// param as it's not observable.
ctx.add_instruction(Instruction::EnterDeclarativeEnvironment);
if let Some(i) = ctx.current_depth_of_loop_scope.as_mut() {
*i += 1;
}
let identifier_string = String::from_str(ctx.agent, identifier.name.as_str(), ctx.gc);
ctx.add_instruction_with_identifier(Instruction::CreateCatchBinding, identifier_string);
// 3. For each element argName of the BoundNames of CatchParameter, do
// a. Perform ! catchEnv.CreateMutableBinding(argName, false).
exception_param.pattern.bound_names(&mut |arg_name| {
let arg_name = String::from_str(ctx.agent, arg_name.name.as_str(), ctx.gc);
ctx.add_instruction_with_identifier(Instruction::CreateMutableBinding, arg_name);
});
// 5. Let status be Completion(BindingInitialization of CatchParameter with arguments thrownValue and catchEnv).
// 6. If status is an abrupt completion, then
// a. Set the running execution context's LexicalEnvironment to oldEnv.
// b. Return ? status.
match &exception_param.pattern.kind {
ast::BindingPatternKind::BindingIdentifier(identifier) => {
let identifier_string = ctx.create_identifier(&identifier.name);
ctx.add_instruction_with_identifier(
Instruction::ResolveBinding,
identifier_string,
);
ctx.add_instruction(Instruction::InitializeReferencedBinding);
}
ast::BindingPatternKind::ObjectPattern(pattern) => {
ctx.add_instruction(Instruction::Load);
ctx.lexical_binding_state = true;
pattern.compile(ctx);
}
ast::BindingPatternKind::ArrayPattern(pattern) => {
ctx.add_instruction(Instruction::Load);
ctx.lexical_binding_state = true;
pattern.compile(ctx);
}
ast::BindingPatternKind::AssignmentPattern(_) => unreachable!(),
}
}
// 7. Let B be Completion(Evaluation of Block).
catch_clause.body.compile(ctx);
// 8. Set the running execution context's LexicalEnvironment to oldEnv.
if catch_clause.param.is_some() {
ctx.add_instruction(Instruction::ExitDeclarativeEnvironment);
if let Some(i) = ctx.current_depth_of_loop_scope.as_mut() {
*i -= 1;
}
}
// 9. Return ? B.
ctx.set_jump_target_here(jump_to_end);
}
}
Expand Down
44 changes: 1 addition & 43 deletions nova_vm/src/engine/bytecode/vm.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@ pub(crate) struct Vm {
iterator_stack: Vec<VmIterator>,
exception_jump_target_stack: Vec<ExceptionJumpTarget>,
result: Option<Value>,
exception: Option<Value>,
reference: Option<Reference<'static>>,
}

Expand Down Expand Up @@ -190,7 +189,6 @@ impl<'a> Vm {
iterator_stack: Vec::new(),
exception_jump_target_stack: Vec::new(),
result: None,
exception: None,
reference: None,
}
}
Expand All @@ -217,7 +215,6 @@ impl<'a> Vm {
iterator_stack: suspended.iterator_stack.into_vec(),
exception_jump_target_stack: suspended.exception_jump_target_stack.into_vec(),
result: None,
exception: None,
reference: None,
}
}
Expand Down Expand Up @@ -356,7 +353,7 @@ impl<'a> Vm {
.as_mut()
.unwrap()
.lexical_environment = ejt.lexical_environment;
self.exception = Some(err.value());
self.result = Some(err.value());
true
} else {
false
Expand Down Expand Up @@ -1752,41 +1749,6 @@ impl<'a> Vm {
.create_immutable_binding(agent, name, true, gc.nogc())
.unwrap();
}
Instruction::CreateCatchBinding => {
let lex_env = agent
.running_execution_context()
.ecmascript_code
.as_ref()
.unwrap()
.lexical_environment;
let name =
executable.fetch_identifier(agent, instr.args[0].unwrap() as usize, gc.nogc());
unwrap_try(lex_env.try_create_mutable_binding(agent, name, false, gc.nogc()))
.unwrap();

if let Err(result) = lex_env.initialize_binding(
agent,
name.unbind(),
vm.exception.take().unwrap(),
gc,
) {
let old_env = agent
.running_execution_context()
.ecmascript_code
.as_ref()
.unwrap()
.lexical_environment
.get_outer_env(agent)
.unwrap();
agent
.running_execution_context_mut()
.ecmascript_code
.as_mut()
.unwrap()
.lexical_environment = old_env;
return Err(result);
}
}
Instruction::Throw => {
let result = vm.result.take().unwrap();
return Err(JsError::new(result));
Expand Down Expand Up @@ -2434,15 +2396,13 @@ impl HeapMarkAndSweep for Vm {
iterator_stack,
exception_jump_target_stack,
result,
exception,
reference,
} = self;
stack.as_slice().mark_values(queues);
reference_stack.as_slice().mark_values(queues);
iterator_stack.as_slice().mark_values(queues);
exception_jump_target_stack.as_slice().mark_values(queues);
result.mark_values(queues);
exception.mark_values(queues);
reference.mark_values(queues);
}

Expand All @@ -2454,7 +2414,6 @@ impl HeapMarkAndSweep for Vm {
iterator_stack,
exception_jump_target_stack,
result,
exception,
reference,
} = self;
stack.as_mut_slice().sweep_values(compactions);
Expand All @@ -2464,7 +2423,6 @@ impl HeapMarkAndSweep for Vm {
.as_mut_slice()
.sweep_values(compactions);
result.sweep_values(compactions);
exception.sweep_values(compactions);
reference.sweep_values(compactions);
}
}
Expand Down
90 changes: 0 additions & 90 deletions tests/expectations.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21536,98 +21536,8 @@
"language/statements/try/cptn-finally-skip-catch.js": "CRASH",
"language/statements/try/cptn-finally-wo-catch.js": "CRASH",
"language/statements/try/cptn-try.js": "FAIL",
"language/statements/try/dstr/ary-init-iter-close.js": "CRASH",
"language/statements/try/dstr/ary-init-iter-get-err-array-prototype.js": "CRASH",
"language/statements/try/dstr/ary-init-iter-get-err.js": "CRASH",
"language/statements/try/dstr/ary-init-iter-no-close.js": "CRASH",
"language/statements/try/dstr/ary-name-iter-val.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elem-ary-elem-init.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elem-ary-elem-iter.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elem-ary-elision-init.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elem-ary-elision-iter.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elem-ary-empty-init.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elem-ary-empty-iter.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elem-ary-rest-init.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elem-ary-rest-iter.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elem-ary-val-null.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elem-id-init-exhausted.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elem-id-init-fn-name-arrow.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elem-id-init-fn-name-class.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elem-id-init-fn-name-cover.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elem-id-init-fn-name-fn.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elem-id-init-fn-name-gen.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elem-id-init-hole.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elem-id-init-skipped.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elem-id-init-throws.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elem-id-init-undef.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elem-id-init-unresolvable.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elem-id-iter-complete.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elem-id-iter-done.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elem-id-iter-step-err.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elem-id-iter-val-array-prototype.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elem-id-iter-val-err.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elem-id-iter-val.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elem-obj-id-init.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elem-obj-id.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elem-obj-prop-id-init.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elem-obj-prop-id.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elem-obj-val-null.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elem-obj-val-undef.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elision-exhausted.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elision-step-err.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-elision.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-empty.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-rest-ary-elem.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-rest-ary-elision.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-rest-ary-empty.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-rest-ary-rest.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-rest-id-direct.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-rest-id-elision-next-err.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-rest-id-elision.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-rest-id-exhausted.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-rest-id-iter-step-err.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-rest-id-iter-val-err.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-rest-id.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-rest-obj-id.js": "CRASH",
"language/statements/try/dstr/ary-ptrn-rest-obj-prop-id.js": "CRASH",
"language/statements/try/dstr/obj-init-null.js": "CRASH",
"language/statements/try/dstr/obj-init-undefined.js": "CRASH",
"language/statements/try/dstr/obj-ptrn-empty.js": "CRASH",
"language/statements/try/dstr/obj-ptrn-id-get-value-err.js": "CRASH",
"language/statements/try/dstr/obj-ptrn-id-init-fn-name-arrow.js": "CRASH",
"language/statements/try/dstr/obj-ptrn-id-init-fn-name-class.js": "CRASH",
"language/statements/try/dstr/obj-ptrn-id-init-fn-name-cover.js": "CRASH",
"language/statements/try/dstr/obj-ptrn-id-init-fn-name-fn.js": "CRASH",
"language/statements/try/dstr/obj-ptrn-id-init-fn-name-gen.js": "CRASH",
"language/statements/try/dstr/obj-ptrn-id-init-skipped.js": "CRASH",
"language/statements/try/dstr/obj-ptrn-id-init-throws.js": "CRASH",
"language/statements/try/dstr/obj-ptrn-id-init-unresolvable.js": "CRASH",
"language/statements/try/dstr/obj-ptrn-id-trailing-comma.js": "CRASH",
"language/statements/try/dstr/obj-ptrn-list-err.js": "CRASH",
"language/statements/try/dstr/obj-ptrn-prop-ary-init.js": "CRASH",
"language/statements/try/dstr/obj-ptrn-prop-ary-trailing-comma.js": "CRASH",
"language/statements/try/dstr/obj-ptrn-prop-ary-value-null.js": "CRASH",
"language/statements/try/dstr/obj-ptrn-prop-ary.js": "CRASH",
"language/statements/try/dstr/obj-ptrn-prop-eval-err.js": "CRASH",
"language/statements/try/dstr/obj-ptrn-prop-id-get-value-err.js": "CRASH",
"language/statements/try/dstr/obj-ptrn-prop-id-init-skipped.js": "CRASH",
"language/statements/try/dstr/obj-ptrn-prop-id-init-throws.js": "CRASH",
"language/statements/try/dstr/obj-ptrn-prop-id-init-unresolvable.js": "CRASH",
"language/statements/try/dstr/obj-ptrn-prop-id-init.js": "CRASH",
"language/statements/try/dstr/obj-ptrn-prop-id-trailing-comma.js": "CRASH",
"language/statements/try/dstr/obj-ptrn-prop-id.js": "CRASH",
"language/statements/try/dstr/obj-ptrn-prop-obj-init.js": "CRASH",
"language/statements/try/dstr/obj-ptrn-prop-obj-value-null.js": "CRASH",
"language/statements/try/dstr/obj-ptrn-prop-obj-value-undef.js": "CRASH",
"language/statements/try/dstr/obj-ptrn-prop-obj.js": "CRASH",
"language/statements/try/dstr/obj-ptrn-rest-getter.js": "CRASH",
"language/statements/try/dstr/obj-ptrn-rest-skip-non-enumerable.js": "CRASH",
"language/statements/try/dstr/obj-ptrn-rest-val-obj.js": "CRASH",
"language/statements/try/early-catch-function.js": "FAIL",
"language/statements/try/optional-catch-binding-finally.js": "CRASH",
"language/statements/try/scope-catch-block-lex-open.js": "CRASH",
"language/statements/try/scope-catch-param-lex-open.js": "CRASH",
"language/statements/try/scope-catch-param-var-none.js": "CRASH",
"language/statements/try/static-init-await-binding-valid.js": "FAIL",
"language/statements/try/tco-catch-finally.js": "CRASH",
"language/statements/try/tco-catch.js": "CRASH",
Expand Down
4 changes: 2 additions & 2 deletions tests/metrics.json
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
{
"results": {
"crash": 14567,
"crash": 14477,
"fail": 7289,
"pass": 23387,
"pass": 23477,
"skip": 45,
"timeout": 3,
"unresolved": 0
Expand Down

0 comments on commit f6da795

Please sign in to comment.