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] - Allow passing owned HostHooks and JobQueues to Context #2811

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
Allow passing owned HostHooks and JobQueues to Context
  • Loading branch information
jedel1043 committed Apr 11, 2023
commit a957e68a0f62174c926dea9a93380a8fb5d81f51
4 changes: 2 additions & 2 deletions boa_cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,9 +302,9 @@ fn evaluate_files(args: &Opt, context: &mut Context<'_>) -> Result<(), io::Error
fn main() -> Result<(), io::Error> {
let args = Opt::parse();

let queue = Jobs::default();
let queue: &dyn JobQueue = &Jobs::default();
let mut context = ContextBuilder::new()
.job_queue(&queue)
.job_queue(queue)
.build()
.expect("cannot fail with default global object");

Expand Down
4 changes: 2 additions & 2 deletions boa_engine/src/builtins/intl/locale/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use icu_locid::{
locale, Locale,
};
use icu_plurals::provider::CardinalV1Marker;
use icu_provider::{DataLocale, DataProvider, DataRequest, DataRequestMetadata};
use icu_provider::{BufferProvider, DataLocale, DataProvider, DataRequest, DataRequestMetadata};

use crate::{
builtins::intl::{
Expand Down Expand Up @@ -73,7 +73,7 @@ impl Service for TestService {

#[test]
fn locale_resolution() {
let provider = boa_icu_provider::buffer();
let provider: &dyn BufferProvider = boa_icu_provider::buffer();
let icu = Icu::new(BoaProvider::Buffer(provider)).unwrap();
let mut default = default_locale(icu.locale_canonicalizer());
default
Expand Down
4 changes: 2 additions & 2 deletions boa_engine/src/builtins/intl/locale/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@ pub(in crate::builtins::intl) fn validate_extension<M: KeyedDataMarker>(
mod tests {
use icu_locid::{langid, locale, Locale};
use icu_plurals::provider::CardinalV1Marker;
use icu_provider::AsDeserializingBufferProvider;
use icu_provider::{AsDeserializingBufferProvider, BufferProvider};

use crate::{
builtins::intl::locale::utils::{
Expand Down Expand Up @@ -579,7 +579,7 @@ mod tests {

#[test]
fn lookup_match() {
let provider = boa_icu_provider::buffer();
let provider: &dyn BufferProvider = boa_icu_provider::buffer();
let icu = Icu::new(BoaProvider::Buffer(provider)).unwrap();

// requested: []
Expand Down
21 changes: 8 additions & 13 deletions boa_engine/src/builtins/promise/tests.rs
Original file line number Diff line number Diff line change
@@ -1,25 +1,20 @@
use crate::{context::ContextBuilder, job::SimpleJobQueue, run_test_actions_with, TestAction};
use crate::{run_test_actions, TestAction};
use indoc::indoc;

#[test]
fn promise() {
let queue = SimpleJobQueue::new();
let context = &mut ContextBuilder::new().job_queue(&queue).build().unwrap();
run_test_actions_with(
[
TestAction::run(indoc! {r#"
run_test_actions([
TestAction::run(indoc! {r#"
let count = 0;
const promise = new Promise((resolve, reject) => {
count += 1;
resolve(undefined);
}).then((_) => (count += 1));
count += 1;
"#}),
TestAction::assert_eq("count", 2),
#[allow(clippy::redundant_closure_for_method_calls)]
TestAction::inspect_context(|ctx| ctx.run_jobs()),
TestAction::assert_eq("count", 3),
],
context,
);
TestAction::assert_eq("count", 2),
#[allow(clippy::redundant_closure_for_method_calls)]
TestAction::inspect_context(|ctx| ctx.run_jobs()),
TestAction::assert_eq("count", 3),
]);
}
9 changes: 4 additions & 5 deletions boa_engine/src/context/icu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ where
M::Yokeable: ZeroFrom<'static, M::Yokeable> + MaybeSendSync,
{
fn load(&self, req: DataRequest<'_>) -> Result<DataResponse<M>, DataError> {
match *self {
match self {
BoaProvider::Buffer(provider) => provider.as_deserializing().load(req),
BoaProvider::Any(provider) => provider.as_downcasting().load(req),
}
Expand Down Expand Up @@ -125,8 +125,8 @@ impl BoaProvider<'_> {
}
}

/// Collection of tools initialized from a [`DataProvider`] that are used
/// for the functionality of `Intl`.
/// Collection of tools initialized from a [`DataProvider`] that are used for the functionality of
/// `Intl`.
pub(crate) struct Icu<'provider> {
provider: BoaProvider<'provider>,
locale_canonicalizer: LocaleCanonicalizer,
Expand All @@ -148,8 +148,7 @@ impl<'provider> Icu<'provider> {
///
/// # Errors
///
/// This method will return an error if any of the tools
/// required cannot be constructed.
/// Returns an error if any of the tools required cannot be constructed.
pub(crate) fn new(
provider: BoaProvider<'provider>,
) -> Result<Icu<'provider>, LocaleTransformError> {
Expand Down
42 changes: 42 additions & 0 deletions boa_engine/src/context/maybe_shared.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
use std::{ops::Deref, rc::Rc};

/// A [`Cow`][std::borrow::Cow]-like pointer where the `Owned` variant is an [`Rc`].
#[derive(Debug, PartialEq, Eq, PartialOrd, Ord)]
pub enum MaybeShared<'a, T: ?Sized> {
/// Borrowed data.
Borrowed(&'a T),
/// `Rc` shared data.
Shared(Rc<T>),
}

impl<T: ?Sized> Clone for MaybeShared<'_, T> {
fn clone(&self) -> Self {
match self {
Self::Borrowed(b) => Self::Borrowed(b),
Self::Shared(sh) => Self::Shared(sh.clone()),
}
}
}

impl<T: ?Sized> Deref for MaybeShared<'_, T> {
type Target = T;

fn deref(&self) -> &Self::Target {
match self {
MaybeShared::Borrowed(b) => b,
MaybeShared::Shared(sh) => sh,
}
}
}

impl<'a, T: ?Sized> From<&'a T> for MaybeShared<'a, T> {
fn from(value: &'a T) -> Self {
Self::Borrowed(value)
}
}

impl<T: ?Sized> From<Rc<T>> for MaybeShared<'static, T> {
fn from(value: Rc<T>) -> Self {
Self::Shared(value)
}
}
66 changes: 45 additions & 21 deletions boa_engine/src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,28 @@

mod hooks;
pub mod intrinsics;
mod maybe_shared;
pub use hooks::{DefaultHooks, HostHooks};
pub use maybe_shared::MaybeShared;
Razican marked this conversation as resolved.
Show resolved Hide resolved

#[cfg(feature = "intl")]
pub(crate) mod icu;

#[cfg(feature = "intl")]
pub use icu::BoaProvider;

use icu_provider::BufferProvider;
use intrinsics::Intrinsics;

use std::io::Read;
#[cfg(not(feature = "intl"))]
pub use std::marker::PhantomData;
use std::{io::Read, rc::Rc};

use crate::{
builtins,
bytecompiler::ByteCompiler,
class::{Class, ClassBuilder},
job::{IdleJobQueue, JobQueue, NativeJob},
job::{JobQueue, NativeJob, SimpleJobQueue},
native_function::NativeFunction,
object::{FunctionObjectBuilder, JsObject},
optimizer::{Optimizer, OptimizerOptions, OptimizerStatistics},
Expand Down Expand Up @@ -99,9 +102,9 @@ pub struct Context<'host> {
#[cfg(feature = "intl")]
icu: icu::Icu<'host>,

host_hooks: &'host dyn HostHooks,
host_hooks: MaybeShared<'host, dyn HostHooks>,

job_queue: &'host dyn JobQueue,
job_queue: MaybeShared<'host, dyn JobQueue>,

optimizer_options: OptimizerOptions,
}
Expand Down Expand Up @@ -494,12 +497,12 @@ impl<'host> Context<'host> {

/// Enqueues a [`NativeJob`] on the [`JobQueue`].
pub fn enqueue_job(&mut self, job: NativeJob) {
self.job_queue.enqueue_promise_job(job, self);
self.job_queue().enqueue_promise_job(job, self);
}

/// Runs all the jobs in the job queue.
pub fn run_jobs(&mut self) {
self.job_queue.run_jobs(self);
self.job_queue().run_jobs(self);
self.clear_kept_objects();
}

Expand All @@ -524,13 +527,13 @@ impl<'host> Context<'host> {
}

/// Gets the host hooks.
pub fn host_hooks(&self) -> &'host dyn HostHooks {
self.host_hooks
pub fn host_hooks(&self) -> MaybeShared<'host, dyn HostHooks> {
self.host_hooks.clone()
}

/// Gets the job queue.
pub fn job_queue(&mut self) -> &'host dyn JobQueue {
self.job_queue
pub fn job_queue(&self) -> MaybeShared<'host, dyn JobQueue> {
self.job_queue.clone()
}
}

Expand Down Expand Up @@ -558,8 +561,8 @@ impl<'host> Context<'host> {
#[derive(Default)]
pub struct ContextBuilder<'icu, 'hooks, 'queue> {
interner: Option<Interner>,
host_hooks: Option<&'hooks dyn HostHooks>,
job_queue: Option<&'queue dyn JobQueue>,
host_hooks: Option<MaybeShared<'hooks, dyn HostHooks>>,
job_queue: Option<MaybeShared<'queue, dyn JobQueue>>,
#[cfg(feature = "intl")]
icu: Option<icu::Icu<'icu>>,
#[cfg(not(feature = "intl"))]
Expand All @@ -570,10 +573,15 @@ pub struct ContextBuilder<'icu, 'hooks, 'queue> {

impl std::fmt::Debug for ContextBuilder<'_, '_, '_> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
#[derive(Clone, Copy, Debug)]
struct JobQueue;
#[derive(Clone, Copy, Debug)]
struct HostHooks;
let mut out = f.debug_struct("ContextBuilder");

out.field("interner", &self.interner)
.field("host_hooks", &"HostHooks");
.field("host_hooks", &self.host_hooks.as_ref().map(|_| HostHooks))
.field("job_queue", &self.job_queue.as_ref().map(|_| JobQueue));

#[cfg(feature = "intl")]
out.field("icu", &self.icu);
Expand Down Expand Up @@ -632,18 +640,27 @@ impl<'icu, 'hooks, 'queue> ContextBuilder<'icu, 'hooks, 'queue> {
///
/// [`Host Hooks`]: https://tc39.es/ecma262/#sec-host-hooks-summary
#[must_use]
pub fn host_hooks(self, host_hooks: &dyn HostHooks) -> ContextBuilder<'icu, '_, 'queue> {
pub fn host_hooks<'new_hooks, H>(
self,
host_hooks: H,
) -> ContextBuilder<'icu, 'new_hooks, 'queue>
where
H: Into<MaybeShared<'new_hooks, dyn HostHooks>>,
{
ContextBuilder {
host_hooks: Some(host_hooks),
host_hooks: Some(host_hooks.into()),
..self
}
}

/// Initializes the [`JobQueue`] for the context.
#[must_use]
pub fn job_queue(self, job_queue: &dyn JobQueue) -> ContextBuilder<'icu, 'hooks, '_> {
pub fn job_queue<'new_queue, Q>(self, job_queue: Q) -> ContextBuilder<'icu, 'hooks, 'new_queue>
where
Q: Into<MaybeShared<'new_queue, dyn JobQueue>>,
{
ContextBuilder {
job_queue: Some(job_queue),
job_queue: Some(job_queue.into()),
..self
}
}
Expand All @@ -666,8 +683,11 @@ impl<'icu, 'hooks, 'queue> ContextBuilder<'icu, 'hooks, 'queue> {
'hooks: 'host,
'queue: 'host,
{
let host_hooks = self.host_hooks.unwrap_or(&DefaultHooks);
let realm = Realm::create(host_hooks);
let host_hooks = self.host_hooks.unwrap_or_else(|| {
let hooks: &dyn HostHooks = &DefaultHooks;
hooks.into()
});
let realm = Realm::create(&*host_hooks);
let vm = Vm::new(realm.environment().clone());

let mut context = Context {
Expand All @@ -677,14 +697,18 @@ impl<'icu, 'hooks, 'queue> ContextBuilder<'icu, 'hooks, 'queue> {
strict: false,
#[cfg(feature = "intl")]
icu: self.icu.unwrap_or_else(|| {
let provider = BoaProvider::Buffer(boa_icu_provider::buffer());
let buffer: &dyn BufferProvider = boa_icu_provider::buffer();
let provider = BoaProvider::Buffer(buffer);
icu::Icu::new(provider).expect("Failed to initialize default icu data.")
}),
#[cfg(feature = "fuzz")]
instructions_remaining: self.instructions_remaining,
kept_alive: Vec::new(),
host_hooks,
job_queue: self.job_queue.unwrap_or(&IdleJobQueue),
job_queue: self.job_queue.unwrap_or_else(|| {
let queue: Rc<dyn JobQueue> = Rc::new(SimpleJobQueue::new());
queue.into()
}),
optimizer_options: OptimizerOptions::OPTIMIZE_ALL,
};

Expand Down
24 changes: 12 additions & 12 deletions boa_engine/src/job.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,10 +198,16 @@ pub trait JobQueue {

/// A job queue that does nothing.
///
/// This is the default job queue for the [`Context`], and is useful if you want to disable
/// the promise capabilities of the engine.
/// This queue is mostly useful if you want to disable the promise capabilities of the engine. This
/// can be done by passing a reference to it to the [`ContextBuilder`]:
///
/// ```
/// use boa_engine::{context::ContextBuilder, job::{JobQueue, IdleJobQueue}};
///
/// let queue: &dyn JobQueue = &IdleJobQueue;
/// let context = ContextBuilder::new().job_queue(queue).build();
/// ```
///
/// If you want to enable running promise jobs, see [`SimpleJobQueue`].
#[derive(Debug, Clone, Copy)]
pub struct IdleJobQueue;

Expand All @@ -215,16 +221,10 @@ impl JobQueue for IdleJobQueue {

/// A simple FIFO job queue that bails on the first error.
///
/// To enable running promise jobs on the engine, you need to pass it to the [`ContextBuilder`]:
///
/// ```
/// use boa_engine::{context::ContextBuilder, job::SimpleJobQueue};
///
/// let queue = SimpleJobQueue::new();
/// let context = ContextBuilder::new().job_queue(&queue).build();
/// ```
/// This is the default job queue for the [`Context`], but it is mostly pretty limited for
/// custom event queues.
///
/// [`ContextBuilder`]: crate::context::ContextBuilder
/// To disable running promise jobs on the engine, see [`IdleJobQueue`].
#[derive(Default)]
pub struct SimpleJobQueue(RefCell<VecDeque<NativeJob>>);

Expand Down
3 changes: 1 addition & 2 deletions boa_engine/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,15 +151,14 @@ pub mod error;
pub mod job;
pub mod native_function;
pub mod object;
pub mod optimizer;
pub mod property;
pub mod realm;
pub mod string;
pub mod symbol;
pub mod value;
pub mod vm;

pub mod optimizer;

#[cfg(feature = "console")]
pub mod console;

Expand Down
Loading