Add FunctionBuilder (#512)

v8::Function and v8::FunctionTemplate have a lot of options that can be
configured. The builder() associated functions on those types help with
that.

src/binding.cc

@@ -7,6 +7,7 @@
 #include "v8/include/v8-inspector.h"
 #include "v8/include/v8-platform.h"
 #include "v8/include/v8-profiler.h"
+#include "v8/include/v8-fast-api-calls.h"
 #include "v8/include/v8.h"
 #include "v8/src/execution/isolate-utils-inl.h"
 #include "v8/src/execution/isolate-utils.h"
@@ -56,6 +57,9 @@ static_assert(sizeof(v8::Location) == sizeof(size_t) * 1,
 static_assert(sizeof(v8::SnapshotCreator) == sizeof(size_t) * 1,
               "SnapshotCreator size mismatch");
 
+static_assert(sizeof(v8::CFunction) == sizeof(size_t) * 2,
+              "CFunction size mismatch");
+
 static_assert(sizeof(three_pointers_t) == sizeof(v8_inspector::StringView),
               "StringView size mismatch");
 
@@ -1268,12 +1272,17 @@ const v8::StackTrace* v8__Exception__GetStackTrace(const v8::Value& exception) {
   return local_to_ptr(v8::Exception::GetStackTrace(ptr_to_local(&exception)));
 }
 
-const v8::Function* v8__Function__New(const v8::Context& context,
+const v8::Function* v8__Function__New(
-                                      v8::FunctionCallback callback,
+    const v8::Context& context,
-                                      const v8::Value* maybe_data) {
+    v8::FunctionCallback callback,
+    const v8::Value* data_or_null,
+    int length,
+    v8::ConstructorBehavior constructor_behavior,
+    v8::SideEffectType side_effect_type) {
   return maybe_local_to_ptr(
       v8::Function::New(ptr_to_local(&context), callback,
-                        ptr_to_local(maybe_data)));
+                        ptr_to_local(data_or_null), length,
+                        constructor_behavior, side_effect_type));
 }
 
 const v8::Value* v8__Function__Call(const v8::Function& self,
@@ -1294,8 +1303,20 @@ const v8::Object* v8__Function__NewInstance(const v8::Function& self,
 }
 
 const v8::FunctionTemplate* v8__FunctionTemplate__New(
-    v8::Isolate* isolate, v8::FunctionCallback callback = nullptr) {
+    v8::Isolate* isolate,
-  return local_to_ptr(v8::FunctionTemplate::New(isolate, callback));
+    v8::FunctionCallback callback,
+    const v8::Value* data_or_null,
+    const v8::Signature* signature_or_null,
+    int length,
+    v8::ConstructorBehavior constructor_behavior,
+    v8::SideEffectType side_effect_type,
+    const v8::CFunction* c_function_or_null) {
+  return local_to_ptr(
+      v8::FunctionTemplate::New(isolate, callback,
+                                ptr_to_local(data_or_null),
+                                ptr_to_local(signature_or_null), length,
+                                constructor_behavior, side_effect_type,
+                                c_function_or_null));
 }
 
 const v8::Function* v8__FunctionTemplate__GetFunction(

src/function.rs

@@ -14,13 +14,17 @@ use crate::HandleScope;
 use crate::Local;
 use crate::Name;
 use crate::Object;
+use crate::Signature;
 use crate::Value;
 
 extern "C" {
   fn v8__Function__New(
     context: *const Context,
     callback: FunctionCallback,
-    data: *const Value,
+    data_or_null: *const Value,
+    length: i32,
+    constructor_behavior: ConstructorBehavior,
+    side_effect_type: SideEffectType,
   ) -> *const Function;
   fn v8__Function__Call(
     this: *const Function,
@@ -63,6 +67,38 @@ extern "C" {
   fn v8__ReturnValue__Get(this: *const ReturnValue) -> *const Value;
 }
 
+// Ad-libbed - V8 does not document ConstructorBehavior.
+/// ConstructorBehavior::Allow creates a regular API function.
+///
+/// ConstructorBehavior::Throw creates a "concise" API function, a function
+/// without a ".prototype" property, that is somewhat faster to create and has
+/// a smaller footprint. Functionally equivalent to ConstructorBehavior::Allow
+/// followed by a call to FunctionTemplate::RemovePrototype().
+#[repr(C)]
+pub enum ConstructorBehavior {
+  Throw,
+  Allow,
+}
+
+/// Options for marking whether callbacks may trigger JS-observable side
+/// effects. Side-effect-free callbacks are allowlisted during debug evaluation
+/// with throwOnSideEffect. It applies when calling a Function,
+/// FunctionTemplate, or an Accessor callback. For Interceptors, please see
+/// PropertyHandlerFlags's kHasNoSideEffect.
+/// Callbacks that only cause side effects to the receiver are allowlisted if
+/// invoked on receiver objects that are created within the same debug-evaluate
+/// call, as these objects are temporary and the side effect does not escape.
+#[repr(C)]
+pub enum SideEffectType {
+  HasSideEffect,
+  HasNoSideEffect,
+  HasSideEffectToReceiver,
+}
+
+#[repr(C)]
+#[derive(Default)]
+pub(crate) struct CFunction([usize; 2]);
+
 // Note: the 'cb lifetime is required because the ReturnValue object must not
 // outlive the FunctionCallbackInfo/PropertyCallbackInfo object from which it
 // is derived.
@@ -286,41 +322,96 @@ where
   }
 }
 
+/// A builder to construct the properties of a Function or FunctionTemplate.
+pub struct FunctionBuilder<'s, T> {
+  pub(crate) callback: FunctionCallback,
+  pub(crate) data: Option<Local<'s, Value>>,
+  pub(crate) signature: Option<Local<'s, Signature>>,
+  pub(crate) length: i32,
+  pub(crate) constructor_behavior: ConstructorBehavior,
+  pub(crate) side_effect_type: SideEffectType,
+  phantom: PhantomData<T>,
+}
+
+impl<'s, T> FunctionBuilder<'s, T> {
+  /// Create a new FunctionBuilder.
+  pub fn new(callback: impl MapFnTo<FunctionCallback>) -> Self {
+    Self {
+      callback: callback.map_fn_to(),
+      data: None,
+      signature: None,
+      length: 0,
+      constructor_behavior: ConstructorBehavior::Allow,
+      side_effect_type: SideEffectType::HasSideEffect,
+      phantom: PhantomData,
+    }
+  }
+
+  /// Set the associated data. The default is no associated data.
+  pub fn data(mut self, data: Local<'s, Value>) -> Self {
+    self.data = Some(data);
+    self
+  }
+
+  /// Set the function length. The default is 0.
+  pub fn length(mut self, length: i32) -> Self {
+    self.length = length;
+    self
+  }
+
+  /// Set the constructor behavior. The default is ConstructorBehavior::Allow.
+  pub fn constructor_behavior(
+    mut self,
+    constructor_behavior: ConstructorBehavior,
+  ) -> Self {
+    self.constructor_behavior = constructor_behavior;
+    self
+  }
+
+  /// Set the side effect type. The default is SideEffectType::HasSideEffect.
+  pub fn side_effect_type(mut self, side_effect_type: SideEffectType) -> Self {
+    self.side_effect_type = side_effect_type;
+    self
+  }
+}
+
+impl<'s> FunctionBuilder<'s, Function> {
+  /// Create the function in the current execution context.
+  pub fn build(
+    self,
+    scope: &mut HandleScope<'s>,
+  ) -> Option<Local<'s, Function>> {
+    unsafe {
+      scope.cast_local(|sd| {
+        v8__Function__New(
+          sd.get_current_context(),
+          self.callback,
+          self.data.map_or_else(null, |p| &*p),
+          self.length,
+          self.constructor_behavior,
+          self.side_effect_type,
+        )
+      })
+    }
+  }
+}
+
 impl Function {
-  // TODO: add remaining arguments from C++
+  /// Create a FunctionBuilder to configure a Function.
+  /// This is the same as FunctionBuilder::<Function>::new().
+  pub fn builder<'s>(
+    callback: impl MapFnTo<FunctionCallback>,
+  ) -> FunctionBuilder<'s, Self> {
+    FunctionBuilder::new(callback)
+  }
+
   /// Create a function in the current execution context
   /// for a given FunctionCallback.
   pub fn new<'s>(
     scope: &mut HandleScope<'s>,
     callback: impl MapFnTo<FunctionCallback>,
   ) -> Option<Local<'s, Function>> {
-    unsafe {
+    Self::builder(callback).build(scope)
-      scope.cast_local(|sd| {
-        v8__Function__New(
-          sd.get_current_context(),
-          callback.map_fn_to(),
-          null(),
-        )
-      })
-    }
-  }
-
-  /// Create a function in the current execution context
-  /// for a given FunctionCallback and associated data.
-  pub fn new_with_data<'s>(
-    scope: &mut HandleScope<'s>,
-    data: Local<Value>,
-    callback: impl MapFnTo<FunctionCallback>,
-  ) -> Option<Local<'s, Function>> {
-    unsafe {
-      scope.cast_local(|sd| {
-        v8__Function__New(
-          sd.get_current_context(),
-          callback.map_fn_to(),
-          &*data,
-        )
-      })
-    }
   }
 
   pub fn call<'s>(

src/template.rs

@@ -6,16 +6,23 @@ use crate::data::Template;
 use crate::isolate::Isolate;
 use crate::support::int;
 use crate::support::MapFnTo;
+use crate::CFunction;
+use crate::ConstructorBehavior;
 use crate::Context;
 use crate::Function;
+use crate::FunctionBuilder;
 use crate::FunctionCallback;
 use crate::HandleScope;
 use crate::Local;
 use crate::Object;
 use crate::PropertyAttribute;
+use crate::SideEffectType;
+use crate::Signature;
 use crate::String;
+use crate::Value;
 use crate::NONE;
 use std::convert::TryFrom;
+use std::ptr::null;
 
 extern "C" {
   fn v8__Template__Set(
@@ -28,6 +35,12 @@ extern "C" {
   fn v8__FunctionTemplate__New(
     isolate: *mut Isolate,
     callback: FunctionCallback,
+    data_or_null: *const Value,
+    signature_or_null: *const Signature,
+    length: i32,
+    constructor_behavior: ConstructorBehavior,
+    side_effect_type: SideEffectType,
+    c_function_or_null: *const CFunction,
   ) -> *const FunctionTemplate;
   fn v8__FunctionTemplate__GetFunction(
     this: *const FunctionTemplate,
@@ -72,18 +85,51 @@ impl Template {
   }
 }
 
+impl<'s> FunctionBuilder<'s, FunctionTemplate> {
+  /// Set the function call signature. The default is no signature.
+  pub fn signature(mut self, signature: Local<'s, Signature>) -> Self {
+    self.signature = Some(signature);
+    self
+  }
+
+  /// Creates the function template.
+  pub fn build(
+    self,
+    scope: &mut HandleScope<'s, ()>,
+  ) -> Local<'s, FunctionTemplate> {
+    unsafe {
+      scope.cast_local(|sd| {
+        v8__FunctionTemplate__New(
+          sd.get_isolate_ptr(),
+          self.callback,
+          self.data.map_or_else(null, |p| &*p),
+          self.signature.map_or_else(null, |p| &*p),
+          self.length,
+          self.constructor_behavior,
+          self.side_effect_type,
+          null(),
+        )
+      })
+    }
+    .unwrap()
+  }
+}
+
 impl FunctionTemplate {
+  /// Create a FunctionBuilder to configure a FunctionTemplate.
+  /// This is the same as FunctionBuilder::<FunctionTemplate>::new().
+  pub fn builder<'s>(
+    callback: impl MapFnTo<FunctionCallback>,
+  ) -> FunctionBuilder<'s, Self> {
+    FunctionBuilder::new(callback)
+  }
+
   /// Creates a function template.
   pub fn new<'s>(
     scope: &mut HandleScope<'s, ()>,
     callback: impl MapFnTo<FunctionCallback>,
   ) -> Local<'s, FunctionTemplate> {
-    unsafe {
+    Self::builder(callback).build(scope)
-      scope.cast_local(|sd| {
-        v8__FunctionTemplate__New(sd.get_isolate_ptr(), callback.map_fn_to())
-      })
-    }
-    .unwrap()
   }
 
   /// Returns the unique function instance in the current execution context.

tests/test_api.rs

@@ -1637,6 +1637,7 @@ fn function() {
     let scope = &mut v8::ContextScope::new(scope, context);
     let global = context.global(scope);
     let recv: v8::Local<v8::Value> = global.into();
+
     // create function using template
     let fn_template = v8::FunctionTemplate::new(scope, fn_callback);
     let function = fn_template
@@ -1648,6 +1649,7 @@ fn function() {
     function
       .call(scope, recv, &[])
       .expect("Function call failed");
+
     // create function without a template
     let function = v8::Function::new(scope, fn_callback2)
       .expect("Unable to create function");
@@ -1656,19 +1658,33 @@ fn function() {
     let value = function
       .call(scope, recv, &[arg1.into(), arg2.into()])
       .unwrap();
-    let value_str = value.to_rust_string_lossy(scope);
+    let value_str = value.to_string(scope).unwrap();
-    assert_eq!(value_str, "Hello callback!".to_string());
+    let rust_str = value_str.to_rust_string_lossy(scope);
+    assert_eq!(rust_str, "Hello callback!".to_string());
+
     // create a function with associated data
     let true_data = v8::Boolean::new(scope, true);
-    let function = v8::Function::new_with_data(
+    let function = v8::Function::builder(data_is_true_callback)
-      scope,
+      .data(true_data.into())
-      true_data.into(),
+      .build(scope)
-      data_is_true_callback,
+      .expect("Unable to create function with data");
-    )
-    .expect("Unable to create function with data");
     function
       .call(scope, recv, &[])
       .expect("Function call failed");
+
+    // create a prototype-less function that throws on new
+    let function = v8::Function::builder(fn_callback)
+      .length(42)
+      .constructor_behavior(v8::ConstructorBehavior::Throw)
+      .build(scope)
+      .unwrap();
+    let name = v8::String::new(scope, "f").unwrap();
+    global.set(scope, name.into(), function.into()).unwrap();
+    let result = eval(scope, "f.length").unwrap();
+    assert_eq!(42, result.integer_value(scope).unwrap());
+    let result = eval(scope, "f.prototype").unwrap();
+    assert!(result.is_undefined());
+    assert!(eval(scope, "new f()").is_none()); // throws
   }
 }