Skip to content

Commit

Permalink
Fix super() construction with default parameters (#3339)
Browse files Browse the repository at this point in the history
- Remove unused `PushDeclarativeEnvironment` opcode
  • Loading branch information
HalidOdat authored Oct 2, 2023
1 parent 4bdb6c6 commit ceaaec7
Show file tree
Hide file tree
Showing 8 changed files with 30 additions and 100 deletions.
4 changes: 2 additions & 2 deletions boa_engine/src/bytecompiler/declarations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
5 changes: 0 additions & 5 deletions boa_engine/src/environments/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,9 +291,4 @@ impl CompileTimeEnvironment {
pub(crate) fn outer(&self) -> Option<Rc<Self>> {
self.outer.clone()
}

/// Gets the environment index of this environment.
pub(crate) const fn environment_index(&self) -> u32 {
self.environment_index
}
}
55 changes: 0 additions & 55 deletions boa_engine/src/environments/runtime/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<CompileTimeEnvironment>,
) {
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
Expand Down
6 changes: 2 additions & 4 deletions boa_engine/src/vm/code_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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"),
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions boa_engine/src/vm/flowgraph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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"),
}
}

Expand Down
9 changes: 2 additions & 7 deletions boa_engine/src/vm/opcode/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -2183,6 +2176,8 @@ generate_opcodes! {
Reserved55 => Reserved,
/// Reserved [`Opcode`].
Reserved56 => Reserved,
/// Reserved [`Opcode`].
Reserved57 => Reserved,
}

/// Specific opcodes for bindings.
Expand Down
24 changes: 0 additions & 24 deletions boa_engine/src/vm/opcode/push/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<CompletionType> {
let compile_environments_index = context.vm.read::<u32>();
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:
Expand Down
21 changes: 21 additions & 0 deletions boa_engine/src/vm/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")),
]);
}

0 comments on commit ceaaec7

Please sign in to comment.