From ceaaec72cb7dabf41a270d662d00ade1f634e645 Mon Sep 17 00:00:00 2001 From: Haled Odat <8566042+HalidOdat@users.noreply.github.com> Date: Tue, 3 Oct 2023 01:56:44 +0200 Subject: [PATCH] Fix super() construction with default parameters (#3339) - Remove unused `PushDeclarativeEnvironment` opcode --- boa_engine/src/bytecompiler/declarations.rs | 4 +- boa_engine/src/environments/compile.rs | 5 -- boa_engine/src/environments/runtime/mod.rs | 55 -------------------- boa_engine/src/vm/code_block.rs | 6 +-- boa_engine/src/vm/flowgraph/mod.rs | 6 +-- boa_engine/src/vm/opcode/mod.rs | 9 +--- boa_engine/src/vm/opcode/push/environment.rs | 24 --------- boa_engine/src/vm/tests.rs | 21 ++++++++ 8 files changed, 30 insertions(+), 100 deletions(-) diff --git a/boa_engine/src/bytecompiler/declarations.rs b/boa_engine/src/bytecompiler/declarations.rs index 27d94fc1cc..5ea9269d1b 100644 --- a/boa_engine/src/bytecompiler/declarations.rs +++ b/boa_engine/src/bytecompiler/declarations.rs @@ -1030,8 +1030,8 @@ impl ByteCompiler<'_, '_> { // visibility of declarations in the function body. // b. Let varEnv be NewDeclarativeEnvironment(env). // c. Set the VariableEnvironment of calleeContext to varEnv. - self.push_compile_environment(true); - env_label = Some(self.emit_opcode_with_operand(Opcode::PushFunctionEnvironment)); + self.push_compile_environment(false); + env_label = Some(self.emit_opcode_with_operand(Opcode::PushDeclarativeEnvironment)); // d. Let instantiatedVarNames be a new empty List. let mut instantiated_var_names = Vec::new(); diff --git a/boa_engine/src/environments/compile.rs b/boa_engine/src/environments/compile.rs index 2616ff1d73..f0dad91db2 100644 --- a/boa_engine/src/environments/compile.rs +++ b/boa_engine/src/environments/compile.rs @@ -291,9 +291,4 @@ impl CompileTimeEnvironment { pub(crate) fn outer(&self) -> Option> { self.outer.clone() } - - /// Gets the environment index of this environment. - pub(crate) const fn environment_index(&self) -> u32 { - self.environment_index - } } diff --git a/boa_engine/src/environments/runtime/mod.rs b/boa_engine/src/environments/runtime/mod.rs index dc5fc8a01e..d89a01a6b5 100644 --- a/boa_engine/src/environments/runtime/mod.rs +++ b/boa_engine/src/environments/runtime/mod.rs @@ -309,61 +309,6 @@ impl EnvironmentStack { ))); } - /// Push a function environment that inherits it's internal slots from the outer function - /// environment. - /// - /// # Panics - /// - /// Panics if no environment exists on the stack. - #[track_caller] - pub(crate) fn push_function_inherit( - &mut self, - compile_environment: Rc, - ) { - let num_bindings = compile_environment.num_bindings(); - - debug_assert!( - self.stack.len() as u32 == compile_environment.environment_index(), - "tried to push an invalid compile environment" - ); - - let (poisoned, with, slots) = { - let with = self - .stack - .last() - .expect("can only be called inside a function") - .as_declarative() - .is_none(); - - let (environment, slots) = self - .stack - .iter() - .rev() - .find_map(|env| { - if let Some(env) = env.as_declarative() { - if let DeclarativeEnvironmentKind::Function(fun) = env.kind() { - return Some((env, fun.slots().clone())); - } - } - None - }) - .expect("can only be called inside a function"); - (environment.poisoned(), with || environment.with(), slots) - }; - - self.stack.push(Environment::Declarative(Gc::new( - DeclarativeEnvironment::new( - DeclarativeEnvironmentKind::Function(FunctionEnvironment::new( - num_bindings, - poisoned, - with, - slots, - )), - compile_environment, - ), - ))); - } - /// Push a module environment on the environments stack. /// /// # Panics diff --git a/boa_engine/src/vm/code_block.rs b/boa_engine/src/vm/code_block.rs index 5b6f5b5f75..deb59f34bb 100644 --- a/boa_engine/src/vm/code_block.rs +++ b/boa_engine/src/vm/code_block.rs @@ -361,9 +361,6 @@ impl CodeBlock { | Instruction::ConcatToString { value_count: value } => value.value().to_string(), Instruction::PushDeclarativeEnvironment { compile_environments_index, - } - | Instruction::PushFunctionEnvironment { - compile_environments_index, } => compile_environments_index.to_string(), Instruction::CopyDataProperties { excluded_key_count: value1, @@ -643,7 +640,8 @@ impl CodeBlock { | Instruction::Reserved53 | Instruction::Reserved54 | Instruction::Reserved55 - | Instruction::Reserved56 => unreachable!("Reserved opcodes are unrechable"), + | Instruction::Reserved56 + | Instruction::Reserved57 => unreachable!("Reserved opcodes are unrechable"), } } } diff --git a/boa_engine/src/vm/flowgraph/mod.rs b/boa_engine/src/vm/flowgraph/mod.rs index 017670c050..98383a8052 100644 --- a/boa_engine/src/vm/flowgraph/mod.rs +++ b/boa_engine/src/vm/flowgraph/mod.rs @@ -226,8 +226,7 @@ impl CodeBlock { graph.add_node(previous_pc, NodeShape::None, label.into(), Color::None); graph.add_edge(previous_pc, pc, None, Color::None, EdgeStyle::Line); } - Instruction::PushDeclarativeEnvironment { .. } - | Instruction::PushFunctionEnvironment { .. } => { + Instruction::PushDeclarativeEnvironment { .. } => { let random = rand::random(); graph.add_node( @@ -522,7 +521,8 @@ impl CodeBlock { | Instruction::Reserved53 | Instruction::Reserved54 | Instruction::Reserved55 - | Instruction::Reserved56 => unreachable!("Reserved opcodes are unrechable"), + | Instruction::Reserved56 + | Instruction::Reserved57 => unreachable!("Reserved opcodes are unrechable"), } } diff --git a/boa_engine/src/vm/opcode/mod.rs b/boa_engine/src/vm/opcode/mod.rs index d51c1ca763..d63544674e 100644 --- a/boa_engine/src/vm/opcode/mod.rs +++ b/boa_engine/src/vm/opcode/mod.rs @@ -1757,13 +1757,6 @@ generate_opcodes! { /// Stack: object **=>** PushObjectEnvironment, - /// Push a function environment. - /// - /// Operands: compile_environments_index: `u32` - /// - /// Stack: **=>** - PushFunctionEnvironment { compile_environments_index: u32 }, - /// Pop the current environment. /// /// Operands: @@ -2183,6 +2176,8 @@ generate_opcodes! { Reserved55 => Reserved, /// Reserved [`Opcode`]. Reserved56 => Reserved, + /// Reserved [`Opcode`]. + Reserved57 => Reserved, } /// Specific opcodes for bindings. diff --git a/boa_engine/src/vm/opcode/push/environment.rs b/boa_engine/src/vm/opcode/push/environment.rs index 462c9930c8..e1776433b1 100644 --- a/boa_engine/src/vm/opcode/push/environment.rs +++ b/boa_engine/src/vm/opcode/push/environment.rs @@ -26,30 +26,6 @@ impl Operation for PushDeclarativeEnvironment { } } -/// `PushFunctionEnvironment` implements the Opcode Operation for `Opcode::PushFunctionEnvironment` -/// -/// Operation: -/// - Push a function environment. -#[derive(Debug, Clone, Copy)] -pub(crate) struct PushFunctionEnvironment; - -impl Operation for PushFunctionEnvironment { - const NAME: &'static str = "PushFunctionEnvironment"; - const INSTRUCTION: &'static str = "INST - PushFunctionEnvironment"; - - fn execute(context: &mut Context<'_>) -> JsResult { - let compile_environments_index = context.vm.read::(); - let compile_environment = context.vm.frame().code_block.compile_environments - [compile_environments_index as usize] - .clone(); - context - .vm - .environments - .push_function_inherit(compile_environment); - Ok(CompletionType::Normal) - } -} - /// `PushObjectEnvironment` implements the Opcode Operation for `Opcode::PushObjectEnvironment` /// /// Operation: diff --git a/boa_engine/src/vm/tests.rs b/boa_engine/src/vm/tests.rs index 8d8f43a5a7..5cea152af7 100644 --- a/boa_engine/src/vm/tests.rs +++ b/boa_engine/src/vm/tests.rs @@ -366,3 +366,24 @@ fn truncate_environments_on_non_caught_native_error() { TestAction::assert_native_error(source, JsNativeErrorKind::Reference, "a is not defined"), ]); } + +#[test] +fn super_construction_with_paramater_expression() { + run_test_actions([ + TestAction::run(indoc! {r#" + class Person { + constructor(name) { + this.name = name; + } + } + + class Student extends Person { + constructor(name = 'unknown') { + super(name); + } + } + "#}), + TestAction::assert_eq("new Student().name", js_string!("unknown")), + TestAction::assert_eq("new Student('Jack').name", js_string!("Jack")), + ]); +}