Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix base objects in with statements #3870

Merged
merged 1 commit into from
Jun 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading