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

[Merged by Bors] - Implement CompletionRecords for the Vm #2618

Closed
wants to merge 23 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
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 boa_engine/src/builtins/async_generator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,7 @@ impl AsyncGenerator {
drop(generator_context_mut);

// 8. Assert: result is never an abrupt completion.
assert!(result.is_ok());
assert!(!result.is_throw_completion());
jedel1043 marked this conversation as resolved.
Show resolved Hide resolved

// 9. Assert: When we return here, genContext has already been removed from the execution context stack and callerContext is the currently running execution context.
// 10. Return unused.
Expand Down
27 changes: 13 additions & 14 deletions boa_engine/src/builtins/generator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use crate::{
property::Attribute,
symbol::JsSymbol,
value::JsValue,
vm::{CallFrame, GeneratorResumeKind, ReturnType},
vm::{CallFrame, CompletionRecord, GeneratorResumeKind},
Context, JsArgs, JsError, JsResult,
};
use boa_gc::{Finalize, Gc, GcRefCell, Trace};
Expand Down Expand Up @@ -227,7 +227,7 @@ impl Generator {

context.vm.frame_mut().generator_resume_kind = GeneratorResumeKind::Normal;

let result = context.run();
let record = context.run();

generator_context.call_frame = context
.vm
Expand All @@ -244,23 +244,22 @@ impl Generator {
.as_generator_mut()
.expect("already checked this object type");

match result {
Ok((value, ReturnType::Yield)) => {
match record {
CompletionRecord::Return(value) => {
generator.state = GeneratorState::SuspendedYield;
drop(generator_context);
generator.context = Some(generator_context_cell);
Ok(create_iter_result_object(value, false, context))
}
Ok((value, _)) => {
CompletionRecord::Normal(value) => {
generator.state = GeneratorState::Completed;
Ok(create_iter_result_object(value, true, context))
}
Err(value) => {
CompletionRecord::Throw(err) => {
generator.state = GeneratorState::Completed;
Err(value)
Err(err)
}
}

// 8. Push genContext onto the execution context stack; genContext is now the running execution context.
// 9. Resume the suspended evaluation of genContext using NormalCompletion(value) as the result of the operation that suspended it. Let result be the value returned by the resumed computation.
// 10. Assert: When we return here, genContext has already been removed from the execution context stack and methodContext is the currently running execution context.
Expand Down Expand Up @@ -340,7 +339,7 @@ impl Generator {
std::mem::swap(&mut context.vm.stack, &mut generator_context.stack);
context.vm.push_frame(generator_context.call_frame.clone());

let result = match abrupt_completion {
let completion_record = match abrupt_completion {
Ok(value) => {
context.vm.push(value);
context.vm.frame_mut().generator_resume_kind = GeneratorResumeKind::Return;
Expand Down Expand Up @@ -368,20 +367,20 @@ impl Generator {
.as_generator_mut()
.expect("already checked this object type");

match result {
Ok((value, ReturnType::Yield)) => {
match completion_record {
CompletionRecord::Return(value) => {
generator.state = GeneratorState::SuspendedYield;
drop(generator_context);
generator.context = Some(generator_context_cell);
Ok(create_iter_result_object(value, false, context))
}
Ok((value, _)) => {
CompletionRecord::Normal(value) => {
generator.state = GeneratorState::Completed;
Ok(create_iter_result_object(value, true, context))
}
Err(value) => {
CompletionRecord::Throw(err) => {
generator.state = GeneratorState::Completed;
Err(value)
Err(err)
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions boa_engine/src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,11 +260,11 @@ impl Context<'_> {
self.vm.push_frame(CallFrame::new(code_block));

self.realm.set_global_binding_number();
let result = self.run();
let record = self.run();
self.vm.pop_frame();
self.clear_kept_objects();

result.map(|r| r.0)
record.consume()
}

/// Register a global property.
Expand Down
14 changes: 14 additions & 0 deletions boa_engine/src/vm/call_frame/abrupt_record.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ pub(crate) enum AbruptKind {
Continue,
Break,
Throw,
Return,
}

/// The `AbruptCompletionRecord` tracks the current `AbruptCompletion` and target address of completion.
Expand Down Expand Up @@ -40,6 +41,14 @@ impl AbruptCompletionRecord {
}
}

/// Creates an `AbruptCompletionRecord` for an abrupt `Return`.
pub(crate) const fn new_return() -> Self {
Self {
kind: AbruptKind::Return,
target: u32::MAX,
}
}

/// Set the target field of the `AbruptCompletionRecord`.
pub(crate) const fn with_initial_target(mut self, target: u32) -> Self {
self.target = target;
Expand All @@ -59,6 +68,11 @@ impl AbruptCompletionRecord {
self.kind == AbruptKind::Continue
}

/// Returns a boolean value for whether `AbruptCompletionRecord` is a return.
pub(crate) fn is_return(self) -> bool {
self.kind == AbruptKind::Return
}

/// Returns a boolean value for whether `AbruptCompletionRecord` is a throw.
pub(crate) fn is_throw(self) -> bool {
self.kind == AbruptKind::Throw
Expand Down
51 changes: 16 additions & 35 deletions boa_engine/src/vm/call_frame/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,16 @@ mod env_stack;

pub(crate) use abrupt_record::AbruptCompletionRecord;
pub(crate) use env_stack::EnvStackEntry;

/// A `CallFrame` holds the state of a function call.
#[derive(Clone, Debug, Finalize, Trace)]
pub struct CallFrame {
pub(crate) code_block: Gc<CodeBlock>,
pub(crate) pc: usize,
#[unsafe_ignore_trace]
pub(crate) try_catch: Vec<FinallyAddresses>,
#[unsafe_ignore_trace]
pub(crate) finally_return: FinallyReturn,
pub(crate) fp: usize,
#[unsafe_ignore_trace]
pub(crate) abrupt_completion: Option<AbruptCompletionRecord>,
#[unsafe_ignore_trace]
pub(crate) early_return: Option<EarlyReturnType>,
pub(crate) pop_on_return: usize,
// Tracks the number of environments in environment entry.
// On abrupt returns this is used to decide how many environments need to be pop'ed.
Expand All @@ -45,11 +43,11 @@ impl CallFrame {
Self {
code_block,
pc: 0,
try_catch: Vec::new(),
finally_return: FinallyReturn::None,
fp: 0,
pop_on_return: 0,
env_stack: Vec::from([EnvStackEntry::new(0, max_length)]),
abrupt_completion: None,
early_return: None,
param_count: 0,
arg_count: 0,
generator_resume_kind: GeneratorResumeKind::Normal,
Expand All @@ -72,6 +70,10 @@ impl CallFrame {

/// ---- `CallFrame` stack methods ----
impl CallFrame {
pub(crate) fn set_frame_pointer(&mut self, pointer: usize) {
self.fp = pointer;
}

/// Tracks that one environment has been pushed in the current loop block.
pub(crate) fn inc_frame_env_stack(&mut self) {
self.env_stack
Expand All @@ -92,38 +94,17 @@ impl CallFrame {
}
}

/// Tracks the address that should be jumped to when an error is caught.
///
/// Additionally the address of a finally block is tracked, to allow for special handling if it exists.
#[derive(Copy, Clone, Debug)]
pub(crate) struct FinallyAddresses {
finally: Option<u32>,
}

impl FinallyAddresses {
pub(crate) const fn new(finally_address: Option<u32>) -> Self {
Self {
finally: finally_address,
}
}

pub(crate) const fn finally(self) -> Option<u32> {
self.finally
}
}

/// Indicates if a function should return or throw at the end of a finally block.
#[derive(Copy, Clone, Debug, PartialEq)]
pub(crate) enum FinallyReturn {
None,
Ok,
Err,
}

/// Indicates how a generator function that has been called/resumed should return.
#[derive(Copy, Clone, Debug, PartialEq)]
pub(crate) enum GeneratorResumeKind {
Normal,
Throw,
Return,
}

// An enum to mark whether a return is early due to Async or Yield
#[derive(Copy, Clone, Debug, PartialEq)]
pub(crate) enum EarlyReturnType {
Await,
Yield,
}
13 changes: 6 additions & 7 deletions boa_engine/src/vm/code_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -872,14 +872,13 @@ impl JsObject {
.with_arg_count(arg_count),
);

let result = context.run();
let record = context.run();
context.vm.pop_frame().expect("must have frame");

std::mem::swap(&mut environments, &mut context.realm.environments);
environments.truncate(environments_len);

let (result, _) = result?;
Ok(result)
record.consume()
}
Function::Async {
code,
Expand Down Expand Up @@ -1137,7 +1136,7 @@ impl JsObject {
}),
);

init_result?;
init_result.consume()?;

Ok(generator.into())
}
Expand Down Expand Up @@ -1288,7 +1287,7 @@ impl JsObject {
gen_context.call_frame.async_generator = Some(generator.clone());
}

init_result?;
init_result.consume()?;

Ok(generator.into())
}
Expand Down Expand Up @@ -1447,7 +1446,7 @@ impl JsObject {
.with_arg_count(arg_count),
);

let result = context.run();
let record = context.run();

context.vm.pop_frame();

Expand All @@ -1463,7 +1462,7 @@ impl JsObject {
environments.pop()
};

let (result, _) = result?;
let result = record.consume()?;

if let Some(result) = result.as_object() {
Ok(result.clone())
Expand Down
35 changes: 35 additions & 0 deletions boa_engine/src/vm/completion_record.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
//! An implementation of a `CompletionRecord` for Boa's VM.

use crate::{JsError, JsResult, JsValue};

/// An implementation of the ECMAScript's `CompletionRecord` [specification] for
/// Boa's VM output Completion and Result.
///
/// [specification]: https://tc39.es/ecma262/#sec-completion-record-specification-type
#[derive(Debug, Clone)]
Copy link
Member

@HalidOdat HalidOdat Mar 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#[derive(Debug, Clone)]
// spec: <https://tc39.es/ecma262/#sec-completion-record-specification-type>
#[derive(Debug, Clone)]

Would be nice to have a link to spec :)

pub(crate) enum CompletionRecord {
jedel1043 marked this conversation as resolved.
Show resolved Hide resolved
Normal(JsValue),
Return(JsValue),
Throw(JsError),
}

// ---- `CompletionRecord` methods ----
impl CompletionRecord {
pub(crate) const fn is_throw_completion(&self) -> bool {
matches!(self, CompletionRecord::Throw(_))
}

/// This function will consume the current `CompletionRecord` and return a `JsResult<JsValue>`
// NOTE: rustc bug around evaluating destructors that prevents this from being a const function.
// Related issue(s):
// - https://github.com/rust-lang/rust-clippy/issues/4041
// - https://github.com/rust-lang/rust/issues/60964
// - https://github.com/rust-lang/rust/issues/73255
#[allow(clippy::missing_const_for_fn)]
Copy link
Member

@HalidOdat HalidOdat Mar 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this lint allowed? If there is a valid reason could we have a comment explaining it :)

pub(crate) fn consume(self) -> JsResult<JsValue> {
match self {
Self::Throw(error) => Err(error),
Self::Normal(value) | Self::Return(value) => Ok(value),
}
}
}
Loading