Named accessor ops (#965)

Implement get/set accessors for object wrap ops.

```rust
#[op2]
impl DOMPoint {
  #[getter]
  fn x(&self) -> f64 {
    self.x
  }
  
  #[setter]
  fn x(&self, x: f64) {}
}
```

```js
point.x;
point.x = 10; 
```

core/extensions.rs

@@ -188,12 +188,20 @@ pub struct OpMethodDecl {
   pub static_methods: &'static [OpDecl],
 }
 
+#[derive(Clone, Copy, PartialEq)]
+pub enum AccessorType {
+  Getter,
+  Setter,
+  None,
+}
+
 #[derive(Clone, Copy)]
 pub struct OpDecl {
   pub name: &'static str,
   pub name_fast: FastStaticString,
   pub is_async: bool,
   pub is_reentrant: bool,
+  pub accessor_type: AccessorType,
   pub arg_count: u8,
   pub no_side_effects: bool,
   /// The slow dispatch call. If metrics are disabled, the `v8::Function` is created with this callback.
@@ -220,6 +228,7 @@ impl OpDecl {
     no_side_effects: bool,
     slow_fn: OpFnRef,
     slow_fn_with_metrics: OpFnRef,
+    accessor_type: AccessorType,
     fast_fn: Option<CFunction>,
     fast_fn_with_metrics: Option<CFunction>,
     metadata: OpMetadata,
@@ -234,12 +243,17 @@ impl OpDecl {
       no_side_effects,
       slow_fn,
       slow_fn_with_metrics,
+      accessor_type,
       fast_fn,
       fast_fn_with_metrics,
       metadata,
     }
   }
 
+  pub fn is_accessor(&self) -> bool {
+    self.accessor_type != AccessorType::None
+  }
+
   /// Returns a copy of this `OpDecl` that replaces underlying functions
   /// with noops.
   pub fn disable(self) -> Self {

core/lib.rs

@@ -71,6 +71,7 @@ pub use crate::convert::ToV8;
 pub use crate::cppgc::GarbageCollected;
 pub use crate::error::GetErrorClassFn;
 pub use crate::error::JsErrorCreateFn;
+pub use crate::extensions::AccessorType;
 pub use crate::extensions::Extension;
 pub use crate::extensions::ExtensionFileSource;
 pub use crate::extensions::ExtensionFileSourceCode;

core/runtime/bindings.rs

@@ -1,6 +1,7 @@
 // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
 use anyhow::Context;
 use std::cell::RefCell;
+use std::collections::HashMap;
 use std::mem::MaybeUninit;
 use std::os::raw::c_void;
 use std::path::PathBuf;
@@ -28,6 +29,7 @@ use crate::ops::OpCtx;
 use crate::ops::OpMethodCtx;
 use crate::runtime::InitMode;
 use crate::runtime::JsRealm;
+use crate::AccessorType;
 use crate::FastStaticString;
 use crate::FastString;
 use crate::JsRuntime;
@@ -387,10 +389,10 @@ pub(crate) fn initialize_deno_core_ops_bindings<'s>(
     let prototype = tmpl.prototype_template(scope);
     let key = op_method_ctx.constructor.decl.name_fast.v8_string(scope);
 
+    let accessor_store = create_accessor_store(op_method_ctx);
+
     for method in op_method_ctx.methods.iter() {
-      let op_fn = op_ctx_template(scope, method);
-      let method_key = method.decl.name_fast.v8_string(scope);
-      prototype.set(method_key.into(), op_fn.into());
+      op_ctx_template_or_accessor(&accessor_store, scope, prototype, method);
     }
 
     for method in op_method_ctx.static_methods.iter() {
@@ -410,6 +412,63 @@ pub(crate) fn initialize_deno_core_ops_bindings<'s>(
   }
 }
 
+fn op_ctx_template_or_accessor<'s>(
+  accessor_store: &AccessorStore,
+  scope: &mut v8::HandleScope<'s>,
+  tmpl: v8::Local<'s, v8::ObjectTemplate>,
+  op_ctx: &OpCtx,
+) {
+  if !op_ctx.decl.is_accessor() {
+    let op_fn = op_ctx_template(scope, op_ctx);
+    let method_key = op_ctx.decl.name_fast.v8_string(scope);
+
+    tmpl.set(method_key.into(), op_fn.into());
+
+    return;
+  }
+
+  let op_ctx_ptr = op_ctx as *const OpCtx as *const c_void;
+  let external = v8::External::new(scope, op_ctx_ptr as *mut c_void);
+  let key = op_ctx.decl.name_fast.v8_string(scope).into();
+
+  if let Some((named_getter, named_setter)) =
+    accessor_store.get(op_ctx.decl.name)
+  {
+    let getter_raw = if named_getter.metrics_enabled() {
+      named_getter.decl.slow_fn_with_metrics
+    } else {
+      named_getter.decl.slow_fn
+    };
+
+    let getter_fn = v8::FunctionTemplate::builder_raw(getter_raw)
+      .data(external.into())
+      .build(scope);
+
+    let setter_fn = if let Some(setter) = named_setter {
+      let setter_raw = if setter.metrics_enabled() {
+        setter.decl.slow_fn_with_metrics
+      } else {
+        setter.decl.slow_fn
+      };
+
+      Some(
+        v8::FunctionTemplate::builder_raw(setter_raw)
+          .data(external.into())
+          .build(scope),
+      )
+    } else {
+      None
+    };
+
+    tmpl.set_accessor_property(
+      key,
+      Some(getter_fn),
+      setter_fn,
+      v8::PropertyAttribute::default(),
+    );
+  }
+}
+
 pub(crate) fn op_ctx_template<'s>(
   scope: &mut v8::HandleScope<'s>,
   op_ctx: &OpCtx,
@@ -456,6 +515,44 @@ fn op_ctx_function<'s>(
   v8fn
 }
 
+type AccessorStore<'a> = HashMap<String, (&'a OpCtx, Option<&'a OpCtx>)>;
+
+fn create_accessor_store(ctx: &OpMethodCtx) -> AccessorStore {
+  let mut store = AccessorStore::new();
+
+  for method in ctx.methods.iter() {
+    // Populate all setters first.
+    if method.decl.accessor_type == AccessorType::Setter {
+      let key = method.decl.name_fast.to_string();
+
+      // All setters must start with "__set_".
+      let key = key.strip_prefix("__set_").expect("Invalid setter name");
+
+      // There must be a getter for each setter.
+      let getter = ctx
+        .methods
+        .iter()
+        .find(|m| {
+          m.decl.name == key && m.decl.accessor_type == AccessorType::Getter
+        })
+        .expect("Getter not found for setter");
+
+      store.insert(key.to_string(), (getter, Some(method)));
+    }
+  }
+
+  // Populate getters without setters.
+  for method in ctx.methods.iter() {
+    if method.decl.accessor_type == AccessorType::Getter {
+      let key = method.decl.name_fast.to_string();
+
+      store.entry(key).or_insert((method, None));
+    }
+  }
+
+  store
+}
+
 pub extern "C" fn wasm_async_resolve_promise_callback(
   _isolate: *mut v8::Isolate,
   context: v8::Local<v8::Context>,
@@ -910,10 +1007,10 @@ pub fn create_exports_for_ops_virtual_module<'s>(
     let tmpl = op_ctx_template(scope, &ctx.constructor);
     let prototype = tmpl.prototype_template(scope);
 
+    let accessor_store = create_accessor_store(ctx);
+
     for method in ctx.methods.iter() {
-      let op_fn = op_ctx_template(scope, method);
-      let method_key = method.decl.name_fast.v8_string(scope);
-      prototype.set(method_key.into(), op_fn.into());
+      op_ctx_template_or_accessor(&accessor_store, scope, prototype, method);
     }
 
     for method in ctx.static_methods.iter() {

ops/op2/config.rs

@@ -30,6 +30,10 @@ pub(crate) struct MacroConfig {
   pub static_member: bool,
   /// Marks an op with no side effects.
   pub no_side_effects: bool,
+  /// Marks an op as a getter.
+  pub getter: bool,
+  /// Marks an op as a setter.
+  pub setter: bool,
 }
 
 impl MacroConfig {
@@ -75,6 +79,10 @@ impl MacroConfig {
         config.constructor = true;
       } else if flag == "static_method" {
         config.static_member = true;
+      } else if flag == "getter" {
+        config.getter = true;
+      } else if flag == "setter" {
+        config.setter = true;
       } else if flag == "fast" {
         config.fast = true;
       } else if flag.starts_with("fast(") {

ops/op2/dispatch_slow.rs

@@ -57,7 +57,7 @@ pub(crate) fn generate_dispatch_slow_call(
 }
 
 pub(crate) fn generate_dispatch_slow(
-  _config: &MacroConfig,
+  config: &MacroConfig,
   generator_state: &mut GeneratorState,
   signature: &ParsedSignature,
 ) -> Result<TokenStream, V8SignatureMappingError> {
@@ -68,9 +68,13 @@ pub(crate) fn generate_dispatch_slow(
   output.extend(gs_quote!(generator_state(result) => (let #result = {
     #args
   };)));
-  output.extend(return_value(generator_state, &signature.ret_val).map_err(
-    |s| V8SignatureMappingError::NoRetValMapping(s, signature.ret_val.clone()),
-  )?);
+  if !config.setter {
+    output.extend(return_value(generator_state, &signature.ret_val).map_err(
+      |s| {
+        V8SignatureMappingError::NoRetValMapping(s, signature.ret_val.clone())
+      },
+    )?);
+  }
 
   let with_stack_trace = if generator_state.needs_stack_trace {
     with_stack_trace(generator_state)
@@ -158,6 +162,7 @@ pub(crate) fn generate_dispatch_slow(
       extern "C" fn #slow_function_metrics<'s>(#info: *const deno_core::v8::FunctionCallbackInfo) {
         let info: &'s _ = unsafe { &*#info };
         let args = deno_core::v8::FunctionCallbackArguments::from_function_callback_info(info);
+
         let #opctx: &'s _ = unsafe {
           &*(deno_core::v8::Local::<deno_core::v8::External>::cast_unchecked(args.data()).value()
               as *const deno_core::_ops::OpCtx)

ops/op2/mod.rs

@@ -95,7 +95,7 @@ pub(crate) fn op2(
 
 pub(crate) fn generate_op2(
   config: MacroConfig,
-  func: ItemFn,
+  mut func: ItemFn,
 ) -> Result<TokenStream, Op2Error> {
   // Create a copy of the original function, named "call"
   let call = Ident::new("call", Span::call_site());
@@ -117,7 +117,10 @@ pub(crate) fn generate_op2(
       FnArg::Typed(ty) => ty.attrs.clear(),
     }
   }
-
+  if config.setter {
+    // Prepend "__set_" to the setter function name.
+    func.sig.ident = format_ident!("__set_{}", func.sig.ident);
+  }
   let signature = parse_signature(func.attrs, func.sig.clone())?;
   if let Some(ident) = signature.lifetime.as_ref().map(|s| format_ident!("{s}"))
   {
@@ -227,12 +230,16 @@ pub(crate) fn generate_op2(
       &signature,
     )? {
       Some((fast_definition, fast_metrics_definition, fast_fn)) => {
-        if !config.fast && !config.nofast && config.fast_alternatives.is_empty()
+        if !config.fast
+          && !config.nofast
+          && config.fast_alternatives.is_empty()
+          && !config.getter
+          && !config.setter
         {
           return Err(Op2Error::ShouldBeFast);
         }
         // nofast requires the function to be valid for fast
-        if config.nofast {
+        if config.nofast || config.getter || config.setter {
           (quote!(None), quote!(None), quote!())
         } else {
           (
@@ -243,7 +250,7 @@ pub(crate) fn generate_op2(
         }
       }
       None => {
-        if config.fast {
+        if config.fast || config.getter || config.setter {
           return Err(Op2Error::ShouldNotBeFast("fast"));
         }
         if config.nofast {
@@ -291,6 +298,14 @@ pub(crate) fn generate_op2(
     }
   };
 
+  let accessor_type = if config.getter {
+    quote!(::deno_core::AccessorType::Getter)
+  } else if config.setter {
+    quote!(::deno_core::AccessorType::Setter)
+  } else {
+    quote!(::deno_core::AccessorType::None)
+  };
+
   Ok(quote! {
     #[allow(non_camel_case_types)]
     #vis const fn #name <#(#generic : #bound),*> () -> ::deno_core::_ops::OpDecl {
@@ -311,6 +326,7 @@ pub(crate) fn generate_op2(
           /*no_side_effect*/ #no_side_effect,
           /*slow_fn*/ Self::#slow_function as _,
           /*slow_fn_metrics*/ Self::#slow_function_metrics as _,
+          /*accessor_type*/ #accessor_type,
           /*fast_fn*/ #fast_definition,
           /*fast_fn_metrics*/ #fast_definition_metrics,
           /*metadata*/ ::deno_core::OpMetadata {

ops/op2/object_wrap.rs

@@ -1,6 +1,7 @@
 // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
 
 use proc_macro2::TokenStream;
+use quote::format_ident;
 use quote::quote;
 use quote::ToTokens;
 use syn::ImplItem;
@@ -34,9 +35,15 @@ use crate::op2::Op2Error;
 //
 //    #[method]
 //    #[smi]
-//    fn x(&self) -> i32 {
+//    fn method(&self) -> i32 {
 //      // ...
 //    }
+//
+//    #[getter]
+//    fn x(&self) -> i32 {}
+//
+//    #[setter]
+//    fn x(&self, x: i32) {}
 // }
 //
 // The generated OpMethodDecl that can be passed to
@@ -53,12 +60,10 @@ use crate::op2::Op2Error;
 // import { MyObject } from "ext:core/ops";
 // ```
 //
-// Currently supported bindings:
+// Supported bindings:
 // - constructor
 // - methods
 // - static methods
-//
-// Planned support:
 // - getters
 // - setters
 //
@@ -100,11 +105,16 @@ pub(crate) fn generate_impl_ops(
       } else if config.static_member {
         static_methods.push(ident);
       } else {
-        methods.push(ident);
+        if config.setter {
+          methods.push(format_ident!("__set_{}", ident));
+        } else {
+          methods.push(ident);
+        }
+
         config.method = Some(self_ty_ident.clone());
       }
 
-      let op = generate_op2(config, func);
+      let op = generate_op2(config, func)?;
       tokens.extend(op);
     }
   }