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

Restrict ClosureFunction to only Copy closures #1518

Merged
merged 1 commit into from
Aug 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
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
7 changes: 7 additions & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions boa/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ ryu-js = "0.2.1"
chrono = "0.4.19"
fast-float = "0.2.0"
unicode-normalization = "0.1.19"
dyn-clone = "1.0.4"

# Optional Dependencies
measureme = { version = "9.1.2", optional = true }
Expand Down
7 changes: 4 additions & 3 deletions boa/examples/closures.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
use boa::{Context, JsString, JsValue};
use boa::{Context, JsValue};

fn main() -> Result<(), JsValue> {
let mut context = Context::new();

let variable = JsString::new("I am a captured variable");
let variable = "I am a captured variable";

// We register a global closure function that has the name 'closure' with length 0.
context.register_global_closure("closure", 0, move |_, _, _| {
// This value is captured from main function.
Ok(variable.clone().into())
println!("variable = {}", variable);
Ok(JsValue::new(variable))
})?;

assert_eq!(
Expand Down
58 changes: 38 additions & 20 deletions boa/src/builtins/function/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,25 +15,54 @@ use crate::object::PROTOTYPE;
use crate::{
builtins::{Array, BuiltIn},
environment::lexical_environment::Environment,
gc::{custom_trace, empty_trace, Finalize, Trace},
gc::{empty_trace, Finalize, Trace},
object::{ConstructorBuilder, FunctionBuilder, JsObject, Object, ObjectData},
property::{Attribute, PropertyDescriptor},
syntax::ast::node::{FormalParameter, RcStatementList},
BoaProfiler, Context, JsResult, JsValue,
};
use bitflags::bitflags;

use dyn_clone::DynClone;
use sealed::Sealed;
use std::fmt::{self, Debug};
use std::rc::Rc;

#[cfg(test)]
mod tests;

/// _fn(this, arguments, context) -> ResultValue_ - The signature of a native built-in function
// Allows restricting closures to only `Copy` ones.
// Used the sealed pattern to disallow external implementations
// of `DynCopy`.
mod sealed {
pub trait Sealed {}
impl<T: Copy> Sealed for T {}
}
pub trait DynCopy: Sealed {}
impl<T: Copy> DynCopy for T {}

/// Type representing a native built-in function a.k.a. function pointer.
///
/// Native functions need to have this signature in order to
/// be callable from Javascript.
pub type NativeFunction = fn(&JsValue, &[JsValue], &mut Context) -> JsResult<JsValue>;

/// _fn(this, arguments, context) -> ResultValue_ - The signature of a closure built-in function
pub type ClosureFunction = dyn Fn(&JsValue, &[JsValue], &mut Context) -> JsResult<JsValue>;
/// Trait representing a native built-in closure.
///
/// Closures need to have this signature in order to
/// be callable from Javascript, but most of the time the compiler
/// is smart enough to correctly infer the types.
pub trait ClosureFunction:
Fn(&JsValue, &[JsValue], &mut Context) -> JsResult<JsValue> + DynCopy + DynClone + 'static
{
}

// The `Copy` bound automatically infers `DynCopy` and `DynClone`
impl<T> ClosureFunction for T where
T: Fn(&JsValue, &[JsValue], &mut Context) -> JsResult<JsValue> + Copy + 'static
{
}

// Allows cloning Box<dyn ClosureFunction>
dyn_clone::clone_trait_object!(ClosureFunction);

#[derive(Clone, Copy, Finalize)]
pub struct BuiltInFunction(pub(crate) NativeFunction);
Expand Down Expand Up @@ -83,14 +112,15 @@ unsafe impl Trace for FunctionFlags {
/// FunctionBody is specific to this interpreter, it will either be Rust code or JavaScript code (AST Node)
///
/// <https://tc39.es/ecma262/#sec-ecmascript-function-objects>
#[derive(Finalize)]
#[derive(Trace, Finalize)]
pub enum Function {
Native {
function: BuiltInFunction,
constructable: bool,
},
Closure {
function: Rc<ClosureFunction>,
#[unsafe_ignore_trace]
function: Box<dyn ClosureFunction>,
constructable: bool,
},
Ordinary {
Expand All @@ -107,18 +137,6 @@ impl Debug for Function {
}
}

unsafe impl Trace for Function {
custom_trace!(this, {
match this {
Function::Native { .. } => {}
Function::Closure { .. } => {}
Function::Ordinary { environment, .. } => {
mark(environment);
}
}
});
}

impl Function {
// Adds the final rest parameters to the Environment as an array
pub(crate) fn add_rest_param(
Expand Down
2 changes: 1 addition & 1 deletion boa/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,7 @@ impl Context {
#[inline]
pub fn register_global_closure<F>(&mut self, name: &str, length: usize, body: F) -> JsResult<()>
where
F: Fn(&JsValue, &[JsValue], &mut Context) -> JsResult<JsValue> + 'static,
F: Fn(&JsValue, &[JsValue], &mut Context) -> JsResult<JsValue> + Copy + 'static,
{
let function = FunctionBuilder::closure(self, body)
.name(name)
Expand Down
3 changes: 1 addition & 2 deletions boa/src/object/gcobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ use std::{
collections::HashMap,
error::Error,
fmt::{self, Debug, Display},
rc::Rc,
result::Result as StdResult,
};

Expand All @@ -46,7 +45,7 @@ pub struct JsObject(Gc<GcCell<Object>>);
enum FunctionBody {
BuiltInFunction(NativeFunction),
BuiltInConstructor(NativeFunction),
Closure(Rc<ClosureFunction>),
Closure(Box<dyn ClosureFunction>),
Ordinary(RcStatementList),
}

Expand Down
7 changes: 3 additions & 4 deletions boa/src/object/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,12 @@ use crate::{
context::StandardConstructor,
gc::{Finalize, Trace},
property::{Attribute, PropertyDescriptor, PropertyKey},
BoaProfiler, Context, JsBigInt, JsString, JsSymbol, JsValue,
BoaProfiler, Context, JsBigInt, JsResult, JsString, JsSymbol, JsValue,
};
use std::{
any::Any,
fmt::{self, Debug, Display},
ops::{Deref, DerefMut},
rc::Rc,
};

#[cfg(test)]
Expand Down Expand Up @@ -1108,12 +1107,12 @@ impl<'context> FunctionBuilder<'context> {
#[inline]
pub fn closure<F>(context: &'context mut Context, function: F) -> Self
where
F: Fn(&JsValue, &[JsValue], &mut Context) -> Result<JsValue, JsValue> + 'static,
F: Fn(&JsValue, &[JsValue], &mut Context) -> JsResult<JsValue> + Copy + 'static,
{
Self {
context,
function: Some(Function::Closure {
function: Rc::new(function),
function: Box::new(function),
constructable: false,
}),
name: JsString::default(),
Expand Down