Skip to content

refactor(engine): encapsulate Context VM field access behind wrapper methods #5149

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.

Sign up for GitHub

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

Open
hansl wants to merge 1 commit into
base: main
Choose a base branch
from
Open

refactor(engine): encapsulate Context VM field access behind wrapper methods #5149

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion core/engine/src/builtins/error/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ impl Error {
pub(crate) fn with_caller_position(tag: ErrorKind, context: &Context) -> Self {
Self {
tag,
position: IgnoreEq(context.vm.shadow_stack.caller_position()),
position: IgnoreEq(context.shadow_stack().caller_position()),
backtrace: IgnoreEq(None),
}
}
Expand Down
24 changes: 12 additions & 12 deletions core/engine/src/builtins/eval/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ impl Eval {
// 9. Let inClassFieldInitializer be false.
// a. Let thisEnvRec be GetThisEnvironment().
let flags = match {
let frame = context.vm.frame();
let frame = context.frame();
frame
.environments
.get_this_environment(frame.realm.environment())
Expand Down Expand Up @@ -212,22 +212,22 @@ impl Eval {
// Poison the last parent function environment, because it may contain new declarations after/during eval.
if !strict {
{
let frame = context.vm.frame_mut();
let frame = context.frame_mut();
let global = frame.realm.environment();
frame.environments.poison_until_last_function(global);
}
}

// Set the compile time environment to the current running environment and save the number of current environments.
let environments_len = context.vm.frame().environments.len();
let environments_len = context.frame().environments.len();

// Pop any added runtime environments that were not removed during the eval execution.
EnvStackAction::Truncate(environments_len)
} else {
// If the call to eval is indirect, the code is executed in the global environment.

// Pop all environments before the eval execution.
let saved = context.vm.frame_mut().environments.pop_to_global();
let saved = context.frame_mut().environments.pop_to_global();

// Restore all environments to the state from before the eval execution.
EnvStackAction::Restore(saved)
Expand All @@ -242,7 +242,7 @@ impl Eval {
});

let (var_environment, mut variable_scope) =
if let Some(e) = context.vm.frame().environments.outer_function_environment() {
if let Some(e) = context.frame().environments.outer_function_environment() {
(e.0, e.1)
} else {
(
Expand All @@ -269,7 +269,7 @@ impl Eval {
context,
)?;

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

let source_text = SourceText::new(source);
let spanned_source_text = SpannedSourceText::new_source_only(source_text);
Expand Down Expand Up @@ -328,10 +328,10 @@ impl Eval {
var_environment.extend_from_compile();
}

let env_fp = context.vm.frame().environments.len() as u32;
let environments = context.vm.frame().environments.clone();
let env_fp = context.frame().environments.len() as u32;
let environments = context.frame().environments.clone();
let realm = context.realm().clone();
context.vm.push_frame_with_stack(
context.push_frame_with_stack(
CallFrame::new(code_block.clone(), None, environments, realm)
.with_env_fp(env_fp)
.with_flags(CallFrameFlags::EXIT_EARLY),
Expand All @@ -345,7 +345,7 @@ impl Eval {
// in `eval_declaration_instantiation` get their proper
// environment stack.
{
let frame = context.vm.frame_mut();
let frame = context.frame_mut();
let global = frame.realm.environment();
frame
.environments
Expand All @@ -355,11 +355,11 @@ impl Eval {
context
.eval_declaration_instantiation(&code_block)
.inspect_err(|_| {
context.vm.pop_frame();
context.pop_frame();
})?;

let record = context.run();
context.vm.pop_frame();
context.pop_frame();

record.consume()
}
Expand Down
4 changes: 2 additions & 2 deletions core/engine/src/builtins/function/bound.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ fn bound_function_exotic_construct(
argument_count: usize,
context: &mut InternalMethodCallContext<'_>,
) -> JsResult<CallValue> {
let new_target = context.vm.stack.pop();
let new_target = context.stack_pop();

debug_assert!(new_target.is_object(), "new.target should be an object");

Expand Down Expand Up @@ -182,6 +182,6 @@ fn bound_function_exotic_construct(
};

// 6. Return ? Construct(target, args, newTarget).
context.vm.stack.push(new_target);
context.stack_push(new_target);
Ok(target.__construct__(bound_args.len() + argument_count))
}
61 changes: 26 additions & 35 deletions core/engine/src/builtins/function/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -653,7 +653,7 @@ impl BuiltInFunctionObject {
return Err(js_error!(SyntaxError: "failed to analyze function scope: {}", reason));
}

let in_with = context.vm.frame().environments.has_object_environment();
let in_with = context.frame().environments.has_object_environment();
let spanned_source_text = SpannedSourceText::new_empty();

let code = FunctionCompiler::new(spanned_source_text)
Expand All @@ -672,7 +672,7 @@ impl BuiltInFunctionObject {
context.interner_mut(),
);

let saved = context.vm.frame_mut().environments.pop_to_global();
let saved = context.frame_mut().environments.pop_to_global();
let function_object = crate::vm::create_function_object(code, prototype, context);
context
.vm
Expand Down Expand Up @@ -1027,48 +1027,42 @@ pub(crate) fn function_call(
.patch_last_native(native_source_info);
}

context.vm.push_frame(frame);
context.vm.set_return_value(JsValue::undefined());
context.push_frame(frame);
context.set_return_value(JsValue::undefined());

let context = context.context();

let lexical_this_mode = context.vm.frame().code_block.this_mode == ThisMode::Lexical;
let lexical_this_mode = context.frame().code_block.this_mode == ThisMode::Lexical;
let this = if lexical_this_mode {
ThisBindingStatus::Lexical
} else {
let this = context.vm.stack.get_this(context.vm.frame());
if context.vm.frame().code_block.strict() {
context.vm.frame_mut().flags |= CallFrameFlags::THIS_VALUE_CACHED;
let this = context.stack_get_this();
if context.frame().code_block.strict() {
context.frame_mut().flags |= CallFrameFlags::THIS_VALUE_CACHED;
ThisBindingStatus::Initialized(this)
} else if this.is_null_or_undefined() {
context.vm.frame_mut().flags |= CallFrameFlags::THIS_VALUE_CACHED;
context.frame_mut().flags |= CallFrameFlags::THIS_VALUE_CACHED;
let this: JsValue = context.realm().global_this().clone().into();
context.vm.stack.set_this(
context.vm.frames.last().js_expect("frame must exist")?,
this.clone(),
);
context.stack_set_this(this.clone());
ThisBindingStatus::Initialized(this)
} else {
let this: JsValue = this
.to_object(context)
.js_expect("conversion cannot fail")?
.into();
context.vm.frame_mut().flags |= CallFrameFlags::THIS_VALUE_CACHED;
context.vm.stack.set_this(
context.vm.frames.last().js_expect("frame must exist")?,
this.clone(),
);
context.frame_mut().flags |= CallFrameFlags::THIS_VALUE_CACHED;
context.stack_set_this(this.clone());
ThisBindingStatus::Initialized(this)
}
};

let mut last_env = 0;

let has_binding_identifier = context.vm.frame().code_block().has_binding_identifier();
let has_function_scope = context.vm.frame().code_block().has_function_scope();
let has_binding_identifier = context.frame().code_block().has_binding_identifier();
let has_function_scope = context.frame().code_block().has_function_scope();

if has_binding_identifier {
let frame = context.vm.frame_mut();
let frame = context.frame_mut();
let global = frame.realm.environment();
let index = frame.environments.push_lexical(1, global);
frame.environments.put_lexical_value(
Expand All @@ -1081,8 +1075,8 @@ pub(crate) fn function_call(
}

if has_function_scope {
let scope = context.vm.frame().code_block().constant_scope(last_env);
let frame = context.vm.frame_mut();
let scope = context.frame().code_block().constant_scope(last_env);
let frame = context.frame_mut();
let global = frame.realm.environment();
frame.environments.push_function(
scope,
Expand Down Expand Up @@ -1124,7 +1118,7 @@ fn function_construct(

let env_fp = environments.len() as u32;

let new_target = context.vm.stack.pop();
let new_target = context.stack_pop();

let this = if code.is_derived_constructor() {
None
Expand Down Expand Up @@ -1167,16 +1161,16 @@ fn function_construct(
.patch_last_native(native_source_info);
}

context.vm.push_frame(frame);
context.vm.set_return_value(JsValue::undefined());
context.push_frame(frame);
context.set_return_value(JsValue::undefined());

let mut last_env = 0;

let has_binding_identifier = context.vm.frame().code_block().has_binding_identifier();
let has_function_scope = context.vm.frame().code_block().has_function_scope();
let has_binding_identifier = context.frame().code_block().has_binding_identifier();
let has_function_scope = context.frame().code_block().has_function_scope();

if has_binding_identifier {
let frame = context.vm.frame_mut();
let frame = context.frame_mut();
let global = frame.realm.environment();
let index = frame.environments.push_lexical(1, global);
frame.environments.put_lexical_value(
Expand All @@ -1189,8 +1183,8 @@ fn function_construct(
}

if has_function_scope {
let scope = context.vm.frame().code_block().constant_scope(last_env);
let frame = context.vm.frame_mut();
let scope = context.frame().code_block().constant_scope(last_env);
let frame = context.frame_mut();
let global = frame.realm.environment();
frame.environments.push_function(
scope,
Expand All @@ -1211,10 +1205,7 @@ fn function_construct(
}

let context = context.context();
context.vm.stack.set_this(
context.vm.frames.last().js_expect("frame must exist")?,
this.map(JsValue::new).unwrap_or_default(),
);
context.stack_set_this(this.map(JsValue::new).unwrap_or_default());

Ok(CallValue::Ready)
}
20 changes: 10 additions & 10 deletions core/engine/src/builtins/generator/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,13 @@ pub(crate) struct GeneratorContext {
impl GeneratorContext {
/// Creates a new `GeneratorContext` from the current `Context` state.
pub(crate) fn from_current(context: &mut Context, async_generator: Option<JsObject>) -> Self {
let mut frame = context.vm.frame().clone();
frame.environments = context.vm.frame().environments.clone();
let mut frame = context.frame().clone();
frame.environments = context.frame().environments.clone();
frame.realm = context.realm().clone();

// Split the stack at fp. The split-off portion starts at what was fp,
// so adjust rp and fp to be relative to the new base.
let mut stack = context.vm.stack.split_off_frame(&frame);
let mut stack = context.stack_split_off_at(&frame);
frame.rp -= frame.fp;
frame.fp = 0;

Expand Down Expand Up @@ -101,28 +101,28 @@ impl GeneratorContext {
resume_kind: GeneratorResumeKind,
context: &mut Context,
) -> CompletionRecord {
std::mem::swap(&mut context.vm.stack, &mut self.stack);
context.stack_swap(&mut self.stack);
let Some(frame) = self.call_frame.take() else {
return CompletionRecord::Throw(PanicError::new("should have a call frame").into());
};
let fp = frame.fp;
let rp = frame.rp;
context.vm.push_frame(frame);
context.push_frame(frame);

let frame = context.vm.frame_mut();
let frame = context.frame_mut();
frame.fp = fp;
frame.rp = rp;
frame.set_exit_early(true);

if let Some(value) = value {
context.vm.stack.push(value);
context.stack_push(value);
}
context.vm.stack.push(resume_kind);
context.stack_push(resume_kind);

let result = context.run();

std::mem::swap(&mut context.vm.stack, &mut self.stack);
self.call_frame = context.vm.pop_frame();
context.stack_swap(&mut self.stack);
self.call_frame = context.pop_frame();
assert!(self.call_frame.is_some());
result
}
Expand Down
2 changes: 1 addition & 1 deletion core/engine/src/builtins/intl/date_time_format/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ impl BuiltInConstructor for DateTimeFormat {
// ChainDateTimeFormat ( dateTimeFormat, newTarget, this )
// <https://tc39.es/ecma402/#sec-chaindatetimeformat>

let this = context.vm.stack.get_this(context.vm.frame());
let this = context.stack_get_this();
let Some(this_obj) = this.as_object() else {
return Ok(date_time_format.into());
};
Expand Down
2 changes: 1 addition & 1 deletion core/engine/src/builtins/intl/number_format/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ impl BuiltInConstructor for NumberFormat {
// ChainNumberFormat ( numberFormat, newTarget, this )
// <https://tc39.es/ecma402/#sec-chainnumberformat>

let this = context.vm.stack.get_this(context.vm.frame());
let this = context.stack_get_this();
let Some(this_obj) = this.as_object() else {
return Ok(number_format.into());
};
Expand Down
6 changes: 3 additions & 3 deletions core/engine/src/builtins/is_html_dda.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,11 @@ fn is_html_dda_call(
.vm
.stack
.calling_convention_pop_arguments(argument_count);
let _func = context.vm.stack.pop();
let _this = context.vm.stack.pop();
let _func = context.stack_pop();
let _this = context.stack_pop();

// Push undefined as the return value.
context.vm.stack.push(JsValue::undefined());
context.stack_push(JsValue::undefined());

Ok(CallValue::Complete)
}
10 changes: 5 additions & 5 deletions core/engine/src/builtins/json/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ impl Json {
};

let code_block = {
let in_with = context.vm.frame().environments.has_object_environment();
let in_with = context.frame().environments.has_object_environment();
let spanned_source_text = SpannedSourceText::new_source_only(
crate::spanned_source_text::SourceText::new(source_text),
);
Expand All @@ -310,12 +310,12 @@ impl Json {

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

let env_fp = context.vm.frame().environments.len() as u32;
context.vm.push_frame_with_stack(
let env_fp = context.frame().environments.len() as u32;
context.push_frame_with_stack(
CallFrame::new(
code_block,
None,
context.vm.frame().environments.clone(),
context.frame().environments.clone(),
realm,
)
.with_env_fp(env_fp)
Expand All @@ -326,7 +326,7 @@ impl Json {

context.realm().resize_global_env();
let record = context.run();
context.vm.pop_frame();
context.pop_frame();

let unfiltered = record.consume()?;

Expand Down
Loading
Loading