Skip to content

Commit

Permalink
Fix base objects in with statements (#3870)
Browse files Browse the repository at this point in the history
  • Loading branch information
raskad committed Jun 4, 2024
1 parent c7e3ec4 commit 4419e6d
Show file tree
Hide file tree
Showing 16 changed files with 150 additions and 7 deletions.
3 changes: 3 additions & 0 deletions core/engine/src/builtins/eval/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,13 +246,16 @@ impl Eval {
context,
)?;

let in_with = context.vm.environments.has_object_environment();

let mut compiler = ByteCompiler::new(
js_string!("<main>"),
body.strict(),
false,
var_env.clone(),
lex_env.clone(),
context.interner_mut(),
in_with,
);

compiler.current_open_environments_count += 1;
Expand Down
2 changes: 2 additions & 0 deletions core/engine/src/builtins/function/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -626,10 +626,12 @@ impl BuiltInFunctionObject {
body
};

let in_with = context.vm.environments.has_object_environment();
let code = FunctionCompiler::new()
.name(js_string!("anonymous"))
.generator(generator)
.r#async(r#async)
.in_with(in_with)
.compile(
&parameters,
&body,
Expand Down
2 changes: 2 additions & 0 deletions core/engine/src/builtins/json/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,15 @@ impl Json {
parser.set_json_parse();
let script = parser.parse_script(context.interner_mut())?;
let code_block = {
let in_with = context.vm.environments.has_object_environment();
let mut compiler = ByteCompiler::new(
js_string!("<main>"),
script.strict(),
true,
context.realm().environment().compile_env(),
context.realm().environment().compile_env(),
context.interner_mut(),
in_with,
);
compiler.compile_statement_list(script.statements(), true, false);
Gc::new(compiler.finish())
Expand Down
5 changes: 5 additions & 0 deletions core/engine/src/bytecompiler/class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ impl ByteCompiler<'_> {
self.variable_environment.clone(),
self.lexical_environment.clone(),
self.interner,
self.in_with,
);

compiler.code_block_flags |= CodeBlockFlags::IS_CLASS_CONSTRUCTOR;
Expand Down Expand Up @@ -288,6 +289,7 @@ impl ByteCompiler<'_> {
self.variable_environment.clone(),
self.lexical_environment.clone(),
self.interner,
self.in_with,
);

// Function environment
Expand Down Expand Up @@ -316,6 +318,7 @@ impl ByteCompiler<'_> {
self.variable_environment.clone(),
self.lexical_environment.clone(),
self.interner,
self.in_with,
);
let _ = field_compiler.push_compile_environment(true);
if let Some(node) = field {
Expand Down Expand Up @@ -354,6 +357,7 @@ impl ByteCompiler<'_> {
self.variable_environment.clone(),
self.lexical_environment.clone(),
self.interner,
self.in_with,
);
let _ = field_compiler.push_compile_environment(true);
if let Some(node) = field {
Expand Down Expand Up @@ -388,6 +392,7 @@ impl ByteCompiler<'_> {
self.variable_environment.clone(),
self.lexical_environment.clone(),
self.interner,
self.in_with,
);
let _ = compiler.push_compile_environment(true);

Expand Down
2 changes: 2 additions & 0 deletions core/engine/src/bytecompiler/declarations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,7 @@ impl ByteCompiler<'_> {
.generator(generator)
.r#async(r#async)
.strict(self.strict())
.in_with(self.in_with)
.binding_identifier(Some(name.sym().to_js_string(self.interner())))
.compile(
parameters,
Expand Down Expand Up @@ -954,6 +955,7 @@ impl ByteCompiler<'_> {
.generator(generator)
.r#async(r#async)
.strict(self.strict())
.in_with(self.in_with)
.binding_identifier(Some(name.sym().to_js_string(self.interner())))
.compile(
parameters,
Expand Down
9 changes: 9 additions & 0 deletions core/engine/src/bytecompiler/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ pub(crate) struct FunctionCompiler {
strict: bool,
arrow: bool,
method: bool,
in_with: bool,
binding_identifier: Option<JsString>,
}

Expand All @@ -35,6 +36,7 @@ impl FunctionCompiler {
strict: false,
arrow: false,
method: false,
in_with: false,
binding_identifier: None,
}
}
Expand Down Expand Up @@ -85,6 +87,12 @@ impl FunctionCompiler {
self
}

/// Indicate if the function is in a `with` statement.
pub(crate) const fn in_with(mut self, in_with: bool) -> Self {
self.in_with = in_with;
self
}

/// Compile a function statement list and it's parameters into bytecode.
pub(crate) fn compile(
mut self,
Expand All @@ -105,6 +113,7 @@ impl FunctionCompiler {
variable_environment,
lexical_environment,
interner,
self.in_with,
);
compiler.length = length;
compiler
Expand Down
21 changes: 20 additions & 1 deletion core/engine/src/bytecompiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,9 @@ pub struct ByteCompiler<'ctx> {
pub(crate) async_handler: Option<u32>,
json_parse: bool,

/// Whether the function is in a `with` statement.
pub(crate) in_with: bool,

pub(crate) interner: &'ctx mut Interner,

#[cfg(feature = "annex-b")]
Expand All @@ -324,6 +327,7 @@ impl<'ctx> ByteCompiler<'ctx> {
variable_environment: Rc<CompileTimeEnvironment>,
lexical_environment: Rc<CompileTimeEnvironment>,
interner: &'ctx mut Interner,
in_with: bool,
) -> ByteCompiler<'ctx> {
let mut code_block_flags = CodeBlockFlags::empty();
code_block_flags.set(CodeBlockFlags::STRICT, strict);
Expand Down Expand Up @@ -356,6 +360,7 @@ impl<'ctx> ByteCompiler<'ctx> {

#[cfg(feature = "annex-b")]
annex_b_function_names: Vec::new(),
in_with,
}
}

Expand Down Expand Up @@ -1320,6 +1325,7 @@ impl<'ctx> ByteCompiler<'ctx> {
.r#async(r#async)
.strict(self.strict())
.arrow(arrow)
.in_with(self.in_with)
.binding_identifier(binding_identifier)
.compile(
parameters,
Expand Down Expand Up @@ -1395,6 +1401,7 @@ impl<'ctx> ByteCompiler<'ctx> {
.strict(self.strict())
.arrow(arrow)
.method(true)
.in_with(self.in_with)
.binding_identifier(binding_identifier)
.compile(
parameters,
Expand Down Expand Up @@ -1442,6 +1449,7 @@ impl<'ctx> ByteCompiler<'ctx> {
.strict(true)
.arrow(arrow)
.method(true)
.in_with(self.in_with)
.binding_identifier(binding_identifier)
.compile(
parameters,
Expand Down Expand Up @@ -1481,8 +1489,19 @@ impl<'ctx> ByteCompiler<'ctx> {
if *ident == Sym::EVAL {
kind = CallKind::CallEval;
}

if self.in_with {
let name = self.resolve_identifier_expect(*ident);
let binding = self.lexical_environment.get_identifier_reference(name);
let index = self.get_or_insert_binding(binding.locator());
self.emit_with_varying_operand(Opcode::ThisForObjectEnvironmentName, index);
} else {
self.emit_opcode(Opcode::PushUndefined);
}
} else {
self.emit_opcode(Opcode::PushUndefined);
}
self.emit_opcode(Opcode::PushUndefined);

self.compile_expr(expr, true);
}
expr => {
Expand Down
3 changes: 3 additions & 0 deletions core/engine/src/bytecompiler/statement/with.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ impl ByteCompiler<'_> {
let _ = self.push_compile_environment(false);
self.emit_opcode(Opcode::PushObjectEnvironment);

let in_with = self.in_with;
self.in_with = true;
self.compile_stmt(with.statement(), use_expr, true);
self.in_with = in_with;

self.pop_compile_environment();
self.lexical_environment = old_lex_env;
Expand Down
51 changes: 51 additions & 0 deletions core/engine/src/environments/runtime/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,13 @@ impl EnvironmentStack {
}
names
}

/// Indicate if the current environment stack has an object environment.
pub(crate) fn has_object_environment(&self) -> bool {
self.stack
.iter()
.any(|env| matches!(env, Environment::Object(_)))
}
}

/// A binding locator contains all information about a binding that is needed to resolve it at runtime.
Expand Down Expand Up @@ -541,6 +548,50 @@ impl Context {
Ok(())
}

/// Finds the object environment that contains the binding and returns the `this` value of the object environment.
pub(crate) fn this_from_object_environment_binding(
&mut self,
locator: &BindingLocator,
) -> JsResult<Option<JsObject>> {
let current = self.vm.environments.current();
if let Some(env) = current.as_declarative() {
if !env.with() {
return Ok(None);
}
}

for env_index in (locator.environment_index..self.vm.environments.stack.len() as u32).rev()
{
match self.environment_expect(env_index) {
Environment::Declarative(env) => {
if env.poisoned() {
let compile = env.compile_env();
if compile.is_function() && compile.get_binding(locator.name()).is_some() {
break;
}
} else if !env.with() {
break;
}
}
Environment::Object(o) => {
let o = o.clone();
let key = locator.name().clone();
if o.has_property(key.clone(), self)? {
if let Some(unscopables) = o.get(JsSymbol::unscopables(), self)?.as_object()
{
if unscopables.get(key.clone(), self)?.to_boolean() {
continue;
}
}
return Ok(Some(o));
}
}
}
}

Ok(None)
}

/// Checks if the binding pointed by `locator` is initialized.
///
/// # Panics
Expand Down
1 change: 1 addition & 0 deletions core/engine/src/module/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1433,6 +1433,7 @@ impl SourceTextModule {
env.clone(),
env.clone(),
context.interner_mut(),
false,
);

compiler.code_block_flags |= CodeBlockFlags::IS_ASYNC;
Expand Down
1 change: 1 addition & 0 deletions core/engine/src/module/synthetic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ impl SyntheticModule {
module_compile_env.clone(),
module_compile_env.clone(),
context.interner_mut(),
false,
);

// 4. For each String exportName in module.[[ExportNames]], do
Expand Down
1 change: 1 addition & 0 deletions core/engine/src/script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ impl Script {
self.inner.realm.environment().compile_env(),
self.inner.realm.environment().compile_env(),
context.interner_mut(),
false,
);

#[cfg(feature = "annex-b")]
Expand Down
4 changes: 2 additions & 2 deletions core/engine/src/vm/code_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -611,6 +611,7 @@ impl CodeBlock {
| Instruction::Exception
| Instruction::MaybeException
| Instruction::This
| Instruction::ThisForObjectEnvironmentName { .. }
| Instruction::Super
| Instruction::CheckReturn
| Instruction::Return
Expand Down Expand Up @@ -719,8 +720,7 @@ impl CodeBlock {
| Instruction::Reserved50
| Instruction::Reserved51
| Instruction::Reserved52
| Instruction::Reserved53
| Instruction::Reserved54 => unreachable!("Reserved opcodes are unrechable"),
| Instruction::Reserved53 => unreachable!("Reserved opcodes are unrechable"),
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions core/engine/src/vm/flowgraph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,7 @@ impl CodeBlock {
| Instruction::ToPropertyKey
| Instruction::ToBoolean
| Instruction::This
| Instruction::ThisForObjectEnvironmentName { .. }
| Instruction::Super
| Instruction::IncrementLoopIteration
| Instruction::CreateForInIterator
Expand Down Expand Up @@ -515,8 +516,7 @@ impl CodeBlock {
| Instruction::Reserved50
| Instruction::Reserved51
| Instruction::Reserved52
| Instruction::Reserved53
| Instruction::Reserved54 => unreachable!("Reserved opcodes are unrechable"),
| Instruction::Reserved53 => unreachable!("Reserved opcodes are unrechable"),
}
}

Expand Down
39 changes: 39 additions & 0 deletions core/engine/src/vm/opcode/environment/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,45 @@ impl Operation for This {
}
}

/// `ThisForObjectEnvironmentName` implements the Opcode Operation for `Opcode::ThisForObjectEnvironmentName`
///
/// Operation:
/// - Pushes `this` value that is related to the object environment of the given binding.
#[derive(Debug, Clone, Copy)]
pub(crate) struct ThisForObjectEnvironmentName;

impl ThisForObjectEnvironmentName {
fn operation(context: &mut Context, index: usize) -> JsResult<CompletionType> {
let binding_locator = context.vm.frame().code_block.bindings[index].clone();
let this = context
.this_from_object_environment_binding(&binding_locator)?
.map_or(JsValue::undefined(), Into::into);
context.vm.push(this);
Ok(CompletionType::Normal)
}
}

impl Operation for ThisForObjectEnvironmentName {
const NAME: &'static str = "ThisForObjectEnvironmentName";
const INSTRUCTION: &'static str = "INST - ThisForObjectEnvironmentName";
const COST: u8 = 1;

fn execute(context: &mut Context) -> JsResult<CompletionType> {
let index = context.vm.read::<u8>();
Self::operation(context, index as usize)
}

fn execute_with_u16_operands(context: &mut Context) -> JsResult<CompletionType> {
let index = context.vm.read::<u16>() as usize;
Self::operation(context, index)
}

fn execute_with_u32_operands(context: &mut Context) -> JsResult<CompletionType> {
let index = context.vm.read::<u32>();
Self::operation(context, index as usize)
}
}

/// `Super` implements the Opcode Operation for `Opcode::Super`
///
/// Operation:
Expand Down
Loading

0 comments on commit 4419e6d

Please sign in to comment.