Skip to content

Commit

Permalink
Refactor ordinary VM calling
Browse files Browse the repository at this point in the history
- Prevent recursing in `__call__` and `__construct__` internal methods
  • Loading branch information
HalidOdat committed Oct 11, 2023
1 parent 72d2546 commit cbd24a5
Show file tree
Hide file tree
Showing 31 changed files with 1,039 additions and 751 deletions.
8 changes: 7 additions & 1 deletion boa_engine/src/builtins/eval/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,9 +256,15 @@ impl Eval {
}

let env_fp = context.vm.environments.len() as u32;
let environments = context.vm.environments.clone();
let realm = context.realm().clone();
context
.vm
.push_frame(CallFrame::new(code_block, None, None).with_env_fp(env_fp));
.push_frame(CallFrame::new(code_block, None, environments, realm).with_env_fp(env_fp));

context.vm.push(JsValue::undefined()); // Push `this` value.
context.vm.push(JsValue::undefined()); // No function object, so push undefined.

context.realm().resize_global_env();

let record = context.run();
Expand Down
24 changes: 6 additions & 18 deletions boa_engine/src/builtins/generator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
use crate::{
builtins::iterable::create_iter_result_object,
context::intrinsics::Intrinsics,
environments::EnvironmentStack,
error::JsNativeError,
js_string,
object::{JsObject, CONSTRUCTOR},
Expand Down Expand Up @@ -60,36 +59,28 @@ unsafe impl Trace for GeneratorState {
/// context/vm before the generator execution starts/resumes and after it has ended/yielded.
#[derive(Debug, Clone, Trace, Finalize)]
pub(crate) struct GeneratorContext {
pub(crate) environments: EnvironmentStack,
pub(crate) stack: Vec<JsValue>,
pub(crate) call_frame: Option<CallFrame>,
pub(crate) realm: Realm,
}

impl GeneratorContext {
/// Creates a new `GeneratorContext` from the raw `Context` state components.
pub(crate) fn new(
environments: EnvironmentStack,
stack: Vec<JsValue>,
call_frame: CallFrame,
realm: Realm,
) -> Self {
pub(crate) fn new(stack: Vec<JsValue>, call_frame: CallFrame) -> Self {
Self {
environments,
stack,
call_frame: Some(call_frame),
realm,
}
}

/// Creates a new `GeneratorContext` from the current `Context` state.
pub(crate) fn from_current(context: &mut Context<'_>) -> Self {
let mut frame = context.vm.frame().clone();
frame.environments = context.vm.environments.clone();
frame.realm = context.realm().clone();
let fp = context.vm.frame().fp as usize;
let this = Self {
environments: context.vm.environments.clone(),
call_frame: Some(context.vm.frame().clone()),
call_frame: Some(frame),
stack: context.vm.stack[fp..].to_vec(),
realm: context.realm().clone(),
};

context.vm.stack.truncate(fp);
Expand All @@ -104,14 +95,13 @@ impl GeneratorContext {
resume_kind: GeneratorResumeKind,
context: &mut Context<'_>,
) -> CompletionRecord {
std::mem::swap(&mut context.vm.environments, &mut self.environments);
std::mem::swap(&mut context.vm.stack, &mut self.stack);
context.swap_realm(&mut self.realm);
context
.vm
.push_frame(self.call_frame.take().expect("should have a call frame"));

context.vm.frame_mut().fp = 0;
context.vm.frame_mut().exit_early = true;

if let Some(value) = value {
context.vm.push(value);
Expand All @@ -120,9 +110,7 @@ impl GeneratorContext {

let result = context.run();

std::mem::swap(&mut context.vm.environments, &mut self.environments);
std::mem::swap(&mut context.vm.stack, &mut self.stack);
context.swap_realm(&mut self.realm);
self.call_frame = context.vm.pop_frame();
assert!(self.call_frame.is_some());
result
Expand Down
13 changes: 10 additions & 3 deletions boa_engine/src/builtins/json/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,17 @@ impl Json {
Gc::new(compiler.finish())
};

let realm = context.realm().clone();

let env_fp = context.vm.environments.len() as u32;
context
.vm
.push_frame(CallFrame::new(code_block, None, None).with_env_fp(env_fp));
context.vm.push_frame(
CallFrame::new(code_block, None, context.vm.environments.clone(), realm)
.with_env_fp(env_fp),
);

context.vm.push(JsValue::undefined()); // Push `this` value.
context.vm.push(JsValue::undefined()); // No function object, so push undefined.

context.realm().resize_global_env();
let record = context.run();
context.vm.pop_frame();
Expand Down
2 changes: 1 addition & 1 deletion boa_engine/src/bytecompiler/class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ impl ByteCompiler<'_, '_> {
compiler.emit_opcode(Opcode::PushUndefined);
} else if class.super_ref().is_some() {
compiler.emit_opcode(Opcode::SuperCallDerived);
compiler.emit_opcode(Opcode::BindThisValue);
} else {
compiler.emit_opcode(Opcode::RestParameterPop);
compiler.emit_opcode(Opcode::PushUndefined);
}
compiler.emit_opcode(Opcode::SetReturnValue);
Expand Down
7 changes: 3 additions & 4 deletions boa_engine/src/bytecompiler/declarations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1007,9 +1007,11 @@ impl ByteCompiler<'_, '_> {
// a. Perform ? IteratorBindingInitialization of formals with arguments iteratorRecord and undefined.
// 26. Else,
// a. Perform ? IteratorBindingInitialization of formals with arguments iteratorRecord and env.
for parameter in formals.as_ref() {
for (i, parameter) in formals.as_ref().iter().enumerate() {
if parameter.is_rest_param() {
self.emit_opcode(Opcode::RestParameterInit);
} else {
self.emit_with_varying_operand(Opcode::GetArgument, i as u32);
}
match parameter.variable().binding() {
Binding::Identifier(ident) => {
Expand All @@ -1030,9 +1032,6 @@ impl ByteCompiler<'_, '_> {
}
}
}
if !formals.has_rest_parameter() {
self.emit_opcode(Opcode::RestParameterPop);
}

if generator {
self.emit(Opcode::Generator, &[Operand::U8(self.in_async().into())]);
Expand Down
1 change: 1 addition & 0 deletions boa_engine/src/bytecompiler/expression/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,7 @@ impl ByteCompiler<'_, '_> {
super_call.arguments().len() as u32,
);
}
self.emit_opcode(Opcode::BindThisValue);

if !use_expr {
self.emit_opcode(Opcode::Pop);
Expand Down
7 changes: 6 additions & 1 deletion boa_engine/src/bytecompiler/jump_control.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,12 @@ impl JumpRecord {
//
// Note: If there is promise capability resolve or reject it based on pending exception.
(true, false) => compiler.emit_opcode(Opcode::CompletePromiseCapability),
(_, _) => {}
(false, false) => {
// TODO: We can omit checking for return, when constructing for functions,
// that cannot be constructed, like arrow functions.
compiler.emit_opcode(Opcode::CheckReturn);
}
(false, true) => {}
}

compiler.emit_opcode(Opcode::Return);
Expand Down
3 changes: 2 additions & 1 deletion boa_engine/src/bytecompiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,8 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> {
compile_environments: Vec::default(),
current_open_environments_count: 0,

current_stack_value_count: 0,
// This starts at two because the first value is the `this` value, then function object.
current_stack_value_count: 2,
code_block_flags,
handlers: ThinVec::default(),

Expand Down
34 changes: 15 additions & 19 deletions boa_engine/src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,6 @@ use self::intrinsics::StandardConstructor;
/// assert_eq!(value.as_number(), Some(12.0))
/// ```
pub struct Context<'host> {
/// realm holds both the global object and the environment
realm: Realm,

/// String interner in the context.
interner: Interner,

Expand Down Expand Up @@ -121,7 +118,7 @@ impl std::fmt::Debug for Context<'_> {
let mut debug = f.debug_struct("Context");

debug
.field("realm", &self.realm)
.field("realm", &self.vm.realm)
.field("interner", &self.interner)
.field("vm", &self.vm)
.field("strict", &self.strict)
Expand Down Expand Up @@ -268,7 +265,7 @@ impl<'host> Context<'host> {
length: usize,
body: NativeFunction,
) -> JsResult<()> {
let function = FunctionObjectBuilder::new(&self.realm, body)
let function = FunctionObjectBuilder::new(self.realm(), body)
.name(name.clone())
.length(length)
.constructor(true)
Expand Down Expand Up @@ -301,7 +298,7 @@ impl<'host> Context<'host> {
length: usize,
body: NativeFunction,
) -> JsResult<()> {
let function = FunctionObjectBuilder::new(&self.realm, body)
let function = FunctionObjectBuilder::new(self.realm(), body)
.name(name.clone())
.length(length)
.constructor(false)
Expand Down Expand Up @@ -335,7 +332,7 @@ impl<'host> Context<'host> {
/// context.register_global_class::<MyClass>()?;
/// ```
pub fn register_global_class<C: Class>(&mut self) -> JsResult<()> {
if self.realm.has_class::<C>() {
if self.realm().has_class::<C>() {
return Err(JsNativeError::typ()
.with_message("cannot register a class twice")
.into());
Expand All @@ -353,7 +350,7 @@ impl<'host> Context<'host> {

self.global_object()
.define_property_or_throw(js_string!(C::NAME), property, self)?;
self.realm.register_class::<C>(class);
self.realm().register_class::<C>(class);

Ok(())
}
Expand Down Expand Up @@ -384,20 +381,20 @@ impl<'host> Context<'host> {
pub fn unregister_global_class<C: Class>(&mut self) -> JsResult<Option<StandardConstructor>> {
self.global_object()
.delete_property_or_throw(js_string!(C::NAME), self)?;
Ok(self.realm.unregister_class::<C>())
Ok(self.realm().unregister_class::<C>())
}

/// Checks if the currently active realm has the global class `C` registered.
#[must_use]
pub fn has_global_class<C: Class>(&self) -> bool {
self.realm.has_class::<C>()
self.realm().has_class::<C>()
}

/// Gets the constructor and prototype of the global class `C` if the currently active realm has
/// that class registered.
#[must_use]
pub fn get_global_class<C: Class>(&self) -> Option<StandardConstructor> {
self.realm.get_class::<C>()
self.realm().get_class::<C>()
}

/// Gets the string interner.
Expand All @@ -417,21 +414,21 @@ impl<'host> Context<'host> {
#[inline]
#[must_use]
pub fn global_object(&self) -> JsObject {
self.realm.global_object().clone()
self.vm.realm.global_object().clone()
}

/// Returns the currently active intrinsic constructors and objects.
#[inline]
#[must_use]
pub fn intrinsics(&self) -> &Intrinsics {
self.realm.intrinsics()
self.vm.realm.intrinsics()
}

/// Returns the currently active realm.
#[inline]
#[must_use]
pub const fn realm(&self) -> &Realm {
&self.realm
&self.vm.realm
}

/// Set the value of trace on the context
Expand Down Expand Up @@ -510,7 +507,7 @@ impl<'host> Context<'host> {
self.vm
.environments
.replace_global(realm.environment().clone());
std::mem::replace(&mut self.realm, realm)
std::mem::replace(&mut self.vm.realm, realm)
}

/// Get the [`RootShape`].
Expand Down Expand Up @@ -565,7 +562,7 @@ impl<'host> Context<'host> {
impl Context<'_> {
/// Swaps the currently active realm with `realm`.
pub(crate) fn swap_realm(&mut self, realm: &mut Realm) {
std::mem::swap(&mut self.realm, realm);
std::mem::swap(&mut self.vm.realm, realm);
}

/// Increment and get the parser identifier.
Expand Down Expand Up @@ -794,7 +791,7 @@ impl Context<'_> {
}

if let Some(frame) = self.vm.frames.last() {
return frame.active_function.clone();
return frame.function(&self.vm);
}

None
Expand Down Expand Up @@ -987,7 +984,7 @@ impl<'icu, 'hooks, 'queue, 'module> ContextBuilder<'icu, 'hooks, 'queue, 'module
hooks.into()
});
let realm = Realm::create(&*host_hooks, &root_shape);
let vm = Vm::new(realm.environment().clone());
let vm = Vm::new(realm);

let module_loader = if let Some(loader) = self.module_loader {
loader
Expand All @@ -1005,7 +1002,6 @@ impl<'icu, 'hooks, 'queue, 'module> ContextBuilder<'icu, 'hooks, 'queue, 'module
};

let mut context = Context {
realm,
interner: self.interner.unwrap_or_default(),
vm,
strict: false,
Expand Down
Loading

0 comments on commit cbd24a5

Please sign in to comment.