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.

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

Allow awaiting JsPromise from Rust code #3011

Merged
merged 3 commits into from
Jun 18, 2023
Merged

Allow awaiting JsPromise from Rust code #3011

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7fe6855
Allow awaiting `JsPromise` from Rust code
jedel1043 Jun 8, 2023
150da3b
Fix docs
jedel1043 Jun 8, 2023
80e9b89
Relink to docs.rs
jedel1043 Jun 17, 2023
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
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions boa_engine/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ criterion = "0.4.0"
float-cmp = "0.9.0"
indoc = "2.0.1"
textwrap = "0.16.0"
futures-lite = "1.13.0"

[target.x86_64-unknown-linux-gnu.dev-dependencies]
jemallocator = "0.5.0"
Expand Down
59 changes: 14 additions & 45 deletions boa_engine/src/builtins/promise/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2005,22 +2005,10 @@ impl Promise {
// 9. Return unused.
}

#[derive(Debug, Trace, Finalize)]
struct RejectResolveCaptures {
promise: JsObject,
#[unsafe_ignore_trace]
already_resolved: Rc<Cell<bool>>,
}

// 1. Let alreadyResolved be the Record { [[Value]]: false }.
let already_resolved = Rc::new(Cell::new(false));

// 5. Set resolve.[[Promise]] to promise.
// 6. Set resolve.[[AlreadyResolved]] to alreadyResolved.
let resolve_captures = RejectResolveCaptures {
already_resolved: already_resolved.clone(),
promise: promise.clone(),
};
let promise = Gc::new(Cell::new(Some(promise.clone())));

// 2. Let stepsResolve be the algorithm steps defined in Promise Resolve Functions.
// 3. Let lengthResolve be the number of non-optional parameters of the function definition in Promise Resolve Functions.
Expand All @@ -2035,18 +2023,11 @@ impl Promise {
// 2. Assert: F has a [[Promise]] internal slot whose value is an Object.
// 3. Let promise be F.[[Promise]].
// 4. Let alreadyResolved be F.[[AlreadyResolved]].
let RejectResolveCaptures {
promise,
already_resolved,
} = captures;

// 5. If alreadyResolved.[[Value]] is true, return undefined.
if already_resolved.get() {
return Ok(JsValue::Undefined);
}

// 6. Set alreadyResolved.[[Value]] to true.
already_resolved.set(true);
let Some(promise) = captures.take() else {
return Ok(JsValue::undefined())
};

let resolution = args.get_or_undefined(0);

Expand All @@ -2058,7 +2039,7 @@ impl Promise {
.to_opaque(context);

// b. Perform RejectPromise(promise, selfResolutionError).
reject_promise(promise, self_resolution_error.into(), context);
reject_promise(&promise, self_resolution_error.into(), context);

// c. Return undefined.
return Ok(JsValue::Undefined);
Expand All @@ -2067,7 +2048,7 @@ impl Promise {
let Some(then) = resolution.as_object() else {
// 8. If Type(resolution) is not Object, then
// a. Perform FulfillPromise(promise, resolution).
fulfill_promise(promise, resolution.clone(), context);
fulfill_promise(&promise, resolution.clone(), context);

// b. Return undefined.
return Ok(JsValue::Undefined);
Expand All @@ -2078,7 +2059,7 @@ impl Promise {
// 10. If then is an abrupt completion, then
Err(e) => {
// a. Perform RejectPromise(promise, then.[[Value]]).
reject_promise(promise, e.to_opaque(context), context);
reject_promise(&promise, e.to_opaque(context), context);

// b. Return undefined.
return Ok(JsValue::Undefined);
Expand All @@ -2090,7 +2071,7 @@ impl Promise {
// 12. If IsCallable(thenAction) is false, then
let Some(then_action) = then_action.as_object().cloned().and_then(JsFunction::from_object) else {
// a. Perform FulfillPromise(promise, resolution).
fulfill_promise(promise, resolution.clone(), context);
fulfill_promise(&promise, resolution.clone(), context);

// b. Return undefined.
return Ok(JsValue::Undefined);
Expand All @@ -2113,7 +2094,7 @@ impl Promise {
// 16. Return undefined.
Ok(JsValue::Undefined)
},
resolve_captures,
promise.clone(),
),
)
.name("")
Expand All @@ -2123,11 +2104,6 @@ impl Promise {

// 10. Set reject.[[Promise]] to promise.
// 11. Set reject.[[AlreadyResolved]] to alreadyResolved.
let reject_captures = RejectResolveCaptures {
promise: promise.clone(),
already_resolved,
};

// 7. Let stepsReject be the algorithm steps defined in Promise Reject Functions.
// 8. Let lengthReject be the number of non-optional parameters of the function definition in Promise Reject Functions.
// 9. Let reject be CreateBuiltinFunction(stepsReject, lengthReject, "", « [[Promise]], [[AlreadyResolved]] »).
Expand All @@ -2141,26 +2117,19 @@ impl Promise {
// 2. Assert: F has a [[Promise]] internal slot whose value is an Object.
// 3. Let promise be F.[[Promise]].
// 4. Let alreadyResolved be F.[[AlreadyResolved]].
let RejectResolveCaptures {
promise,
already_resolved,
} = captures;

// 5. If alreadyResolved.[[Value]] is true, return undefined.
if already_resolved.get() {
return Ok(JsValue::Undefined);
}

// 6. Set alreadyResolved.[[Value]] to true.
already_resolved.set(true);
let Some(promise) = captures.take() else {
return Ok(JsValue::undefined());
};

// 7. Perform RejectPromise(promise, reason).
reject_promise(promise, args.get_or_undefined(0).clone(), context);
reject_promise(&promise, args.get_or_undefined(0).clone(), context);

// 8. Return undefined.
Ok(JsValue::Undefined)
},
reject_captures,
promise,
),
)
.name("")
Expand Down
64 changes: 35 additions & 29 deletions boa_engine/src/native_function.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,9 @@
//! [`NativeFunction`] is the main type of this module, providing APIs to create native callables
//! from native Rust functions and closures.

use std::future::Future;

use boa_gc::{custom_trace, Finalize, Gc, Trace};

use crate::{job::NativeJob, object::JsPromise, Context, JsResult, JsValue};
use crate::{object::JsPromise, Context, JsResult, JsValue};

/// The required signature for all native built-in function pointers.
///
Expand Down Expand Up @@ -115,14 +113,18 @@ impl NativeFunction {
}
}

/// Creates a `NativeFunction` from a function returning a [`Future`].
/// Creates a `NativeFunction` from a function returning a [`Future`]-like.
///
/// The returned `NativeFunction` will return an ECMAScript `Promise` that will be fulfilled
/// or rejected when the returned [`Future`] completes.
///
/// If you only need to convert a [`Future`]-like into a [`JsPromise`], see
/// [`JsPromise::from_future`].
///
/// # Caveats
///
/// Consider the next snippet:
/// Certain async functions need to be desugared for them to be `'static'`. For example, the
/// following won't compile:
///
/// ```compile_fail
/// # use boa_engine::{
Expand All @@ -144,14 +146,29 @@ impl NativeFunction {
/// NativeFunction::from_async_fn(test);
/// ```
///
/// Seems like a perfectly fine code, right? `args` is not used after the await point, which
/// in theory should make the whole future `'static` ... in theory ...
/// Even though `args` is only used before the first await point, Rust's async functions are
/// fully lazy, which makes `test` equivalent to something like:
///
/// ```
/// # use std::future::Future;
/// # use boa_engine::{JsValue, Context, JsResult};
/// fn test<'a>(
/// _this: &JsValue,
/// args: &'a [JsValue],
/// _context: &mut Context<'_>,
/// ) -> impl Future<Output = JsResult<JsValue>> + 'a {
/// async move {
/// let arg = args.get(0).cloned();
/// std::future::ready(()).await;
/// drop(arg);
/// Ok(JsValue::null())
/// }
/// }
/// ```
///
/// This code unfortunately fails to compile at the moment. This is because `rustc` currently
/// cannot determine that `args` can be dropped before the await point, which would trivially
/// make the future `'static`. Track [this issue] for more information.
/// Note that `args` is used inside the `async move`, making the whole future not `'static`.
///
/// In the meantime, a manual desugaring of the async function does the trick:
/// In those cases, you can manually restrict the lifetime of the arguments:
///
/// ```
/// # use std::future::Future;
Expand All @@ -175,29 +192,18 @@ impl NativeFunction {
/// }
/// NativeFunction::from_async_fn(test);
/// ```
/// [this issue]: https://github.com/rust-lang/rust/issues/69663
///
/// And this should always return a `'static` future.
///
/// [`Future`]: std::future::Future
pub fn from_async_fn<Fut>(f: fn(&JsValue, &[JsValue], &mut Context<'_>) -> Fut) -> Self
where
Fut: Future<Output = JsResult<JsValue>> + 'static,
Fut: std::future::IntoFuture<Output = JsResult<JsValue>> + 'static,
{
Self::from_copy_closure(move |this, args, context| {
let (promise, resolvers) = JsPromise::new_pending(context);

let future = f(this, args, context);
let future = async move {
let result = future.await;
NativeJob::new(move |ctx| match result {
Ok(v) => resolvers.resolve.call(&JsValue::undefined(), &[v], ctx),
Err(e) => {
let e = e.to_opaque(ctx);
resolvers.reject.call(&JsValue::undefined(), &[e], ctx)
}
})
};
context
.job_queue()
.enqueue_future_job(Box::pin(future), context);
Ok(promise.into())

Ok(JsPromise::from_future(future, context).into())
})
}

Expand Down
Loading