#[extendr]-impl: Return the right reference

extendr-api/src/error.rs

@@ -80,6 +80,7 @@ pub enum Error {
 
     ExpectedExternalPtrType(Robj, String),
     ExpectedExternalNonNullPtr(Robj),
+    ExpectedExternalPtrReference,
     Other(String),
 
     #[cfg(feature = "ndarray")]
@@ -163,6 +164,9 @@ impl std::fmt::Display for Error {
                     robj
                 )
             }
+            Error::ExpectedExternalPtrReference => {
+                write!(f, "It is only possible to return a reference to self.")
+            }
             Error::NoGraphicsDevices(_robj) => write!(f, "No graphics devices active."),
             Error::Other(str) => write!(f, "{}", str),
 

extendr-api/src/lib.rs

@@ -422,9 +422,14 @@ pub const NA_LOGICAL: Rbool = Rbool::na_value();
 #[doc(hidden)]
 pub use std::collections::HashMap;
 
+/// This is needed for the generation of wrappers.
 #[doc(hidden)]
 pub use libR_sys::DllInfo;
 
+/// This is necessary for `#[extendr]`-impl
+#[doc(hidden)]
+pub use libR_sys::R_ExternalPtrAddr;
+
 #[doc(hidden)]
 pub use libR_sys::SEXP;
 

extendr-api/src/robj/rinternals.rs

@@ -1,5 +1,4 @@
 use crate::*;
-use std::os::raw;
 
 ///////////////////////////////////////////////////////////////
 /// The following impls wrap specific Rinternals.h functions.
@@ -67,7 +66,7 @@ pub trait Rinternals: Types + Conversions {
 
     /// Get the source ref.
     fn get_current_srcref(val: i32) -> Robj {
-        unsafe { Robj::from_sexp(R_GetCurrentSrcref(val as raw::c_int)) }
+        unsafe { Robj::from_sexp(R_GetCurrentSrcref(val as std::ffi::c_int)) }
     }
 
     /// Get the source filename.
@@ -253,7 +252,7 @@ pub trait Rinternals: Types + Conversions {
     /// Internal function used to implement `#[extendr]` impl
     #[doc(hidden)]
     unsafe fn external_ptr_addr<T>(&self) -> *mut T {
-        R_ExternalPtrAddr(self.get()) as *mut T
+        R_ExternalPtrAddr(self.get()).cast()
     }
 
     /// Internal function used to implement `#[extendr]` impl

extendr-api/src/wrapper/externalptr.rs

@@ -134,7 +134,7 @@ impl<T> ExternalPtr<T> {
 
             extern "C" fn finalizer<T>(x: SEXP) {
                 unsafe {
-                    let ptr = R_ExternalPtrAddr(x) as *mut T;
+                    let ptr = R_ExternalPtrAddr(x).cast::<T>();
 
                     // Free the `tag`, which is the type-name
                     R_SetExternalPtrTag(x, R_NilValue);

extendr-macros/src/wrappers.rs

@@ -91,11 +91,27 @@ pub(crate) fn make_function_wrappers(
         quote! { #rust_name }
     };
 
+    // arguments for the wrapper with type being `SEXP`
     let formal_args = inputs
         .iter()
         .map(|input| translate_formal(input, self_ty))
         .collect::<syn::Result<Punctuated<FnArg, Token![,]>>>()?;
 
+    // extract the names of the arguments only (`mut` are ignored in `formal_args` already)
+    let sexp_args = formal_args
+        .clone()
+        .into_iter()
+        .map(|x| match x {
+            // the wrapper doesn't use `self` arguments
+            FnArg::Receiver(_) => unreachable!(),
+            FnArg::Typed(ref typed) => match typed.pat.as_ref() {
+                syn::Pat::Ident(ref pat_ident) => pat_ident.ident.clone(),
+                _ => unreachable!(),
+            },
+        })
+        .collect::<Vec<Ident>>();
+
+    // arguments from R (`SEXP`s) are converted to `Robj`
     let convert_args: Vec<syn::Stmt> = inputs
         .iter()
         .map(translate_to_robj)
@@ -170,9 +186,16 @@ pub(crate) fn make_function_wrappers(
         // instead of converting &Self / &mut Self, pass on the passed
         // ExternalPtr<Self>
         quote!(
-            let _return_ref_to_self = #call_name(#actual_args);
-            //FIXME: find a less hardcoded way to write `_self_robj`
-            Ok(_self_robj)
+            let return_ref_to_self = #call_name(#actual_args);
+
+            #(
+            if std::ptr::addr_eq(
+                extendr_api::R_ExternalPtrAddr(#sexp_args),
+                std::ptr::from_ref(return_ref_to_self)) {
+                    return Ok(extendr_api::Robj::from_sexp(#sexp_args))
+                }
+            )*
+            Err(Error::ExpectedExternalPtrReference)
         )
     } else {
         quote!(Ok(extendr_api::Robj::from(#call_name(#actual_args))))
@@ -219,7 +242,7 @@ pub(crate) fn make_function_wrappers(
                     // included in panic. The advantage would be the panic cause could be included
                     // in the R terminal error message and not only via std-err.
                     // but it should be handled in a separate function and not in-lined here.
-                    let err_string = format!("user function panicked: {}",#r_name_str);
+                    let err_string = format!("User function panicked: {}", #r_name_str);
                     // cannot use throw_r_error here for some reason.
                     // handle_panic() exports err string differently than throw_r_error.
                     extendr_api::handle_panic(err_string.as_str(), || panic!());
@@ -445,6 +468,7 @@ fn translate_to_robj(input: &FnArg) -> syn::Result<syn::Stmt> {
             }
         }
         FnArg::Receiver(_) => {
+            // this is `mut`, in case of a mutable reference
             Ok(parse_quote! { let mut _self_robj = extendr_api::robj::Robj::from_sexp(_self); })
         }
     }

tests/extendrtests/NAMESPACE

@@ -4,13 +4,16 @@ S3method("$","__MyClass")
 S3method("$",MyClass)
 S3method("$",MyClassUnexported)
 S3method("$",MySubmoduleClass)
+S3method("$",Wrapper)
 S3method("[[","__MyClass")
 S3method("[[",MyClass)
 S3method("[[",MyClassUnexported)
 S3method("[[",MySubmoduleClass)
+S3method("[[",Wrapper)
 export("__00__special_function_name")
 export(MyClass)
 export(MySubmoduleClass)
+export(Wrapper)
 export(euclidean_dist)
 export(false)
 export(hello_submodule)

tests/extendrtests/R/extendr-wrappers.R

@@ -214,23 +214,52 @@ MySubmoduleClass$set_a <- function(x) invisible(.Call(wrap__MySubmoduleClass__se
 
 MySubmoduleClass$a <- function() .Call(wrap__MySubmoduleClass__a, self)
 
-MySubmoduleClass$me_owned <- function() .Call(wrap__MySubmoduleClass__me_owned, self)
+#' @rdname MySubmoduleClass
+#' @usage NULL
+#' @export
+`$.MySubmoduleClass` <- function (self, name) { func <- MySubmoduleClass[[name]]; environment(func) <- environment(); func }
+
+#' @export
+`[[.MySubmoduleClass` <- `$.MySubmoduleClass`
+
+#' Class for testing (exported)
+#' @examples
+#' x <- Wrapper$new()
+#' x$a()
+#' x$set_a(10)
+#' x$a()
+#' @export
+Wrapper <- new.env(parent = emptyenv())
 
-MySubmoduleClass$me_ref <- function() .Call(wrap__MySubmoduleClass__me_ref, self)
+Wrapper$new <- function() .Call(wrap__Wrapper__new)
 
-MySubmoduleClass$me_mut <- function() .Call(wrap__MySubmoduleClass__me_mut, self)
+Wrapper$set_a <- function(x) invisible(.Call(wrap__Wrapper__set_a, self, x))
 
-MySubmoduleClass$me_explicit_ref <- function() .Call(wrap__MySubmoduleClass__me_explicit_ref, self)
+Wrapper$a <- function() .Call(wrap__Wrapper__a, self)
 
-MySubmoduleClass$me_explicit_mut <- function() .Call(wrap__MySubmoduleClass__me_explicit_mut, self)
+Wrapper$me_owned <- function() .Call(wrap__Wrapper__me_owned, self)
 
-#' @rdname MySubmoduleClass
+Wrapper$me_ref <- function() .Call(wrap__Wrapper__me_ref, self)
+
+Wrapper$me_mut <- function() .Call(wrap__Wrapper__me_mut, self)
+
+Wrapper$me_explicit_ref <- function() .Call(wrap__Wrapper__me_explicit_ref, self)
+
+Wrapper$me_explicit_mut <- function() .Call(wrap__Wrapper__me_explicit_mut, self)
+
+Wrapper$max_ref <- function(other) .Call(wrap__Wrapper__max_ref, self, other)
+
+Wrapper$max_ref_offset <- function(other, `_offset`) .Call(wrap__Wrapper__max_ref_offset, self, other, `_offset`)
+
+Wrapper$max_ref2 <- function(other) .Call(wrap__Wrapper__max_ref2, self, other)
+
+#' @rdname Wrapper
 #' @usage NULL
 #' @export
-`$.MySubmoduleClass` <- function (self, name) { func <- MySubmoduleClass[[name]]; environment(func) <- environment(); func }
+`$.Wrapper` <- function (self, name) { func <- Wrapper[[name]]; environment(func) <- environment(); func }
 
 #' @export
-`[[.MySubmoduleClass` <- `$.MySubmoduleClass`
+`[[.Wrapper` <- `$.Wrapper`
 
 
 # nolint end

tests/extendrtests/src/rust/src/externalptr.rs

@@ -0,0 +1,96 @@
+use extendr_api::prelude::*;
+
+// Class for testing
+#[derive(Default, Debug, Clone, Copy)]
+struct Wrapper {
+    a: i32,
+}
+
+/// Class for testing (exported)
+/// @examples
+/// x <- Wrapper$new()
+/// x$a()
+/// x$set_a(10)
+/// x$a()
+/// @export
+#[extendr]
+impl Wrapper {
+    /// Method for making a new object.
+    fn new() -> Self {
+        Self { a: 0 }
+    }
+
+    /// Method for setting stuff.
+    /// @param x a number
+    fn set_a(&mut self, x: i32) {
+        self.a = x;
+    }
+
+    /// Method for getting stuff.
+    fn a(&self) -> i32 {
+        self.a
+    }
+
+    // NOTE: Cannot move ownership, as that concept is incompatible with bridging
+    // data from R to Rust
+    // fn myself(self) -> Self {
+    //     self
+    // }
+
+    /// Method for getting one's (by way of a copy) self.
+    fn me_owned(&self) -> Self {
+        // only possible due to `derive(Clone, Copy)`
+        *self
+    }
+
+    /// Method for getting one's (ref) self.
+    fn me_ref(&self) -> &Self {
+        self
+    }
+
+    /// Method for getting one's (ref mut) self.
+    fn me_mut(&mut self) -> &mut Self {
+        self
+    }
+
+    /// Method for getting one's ref (explicit) self.
+    fn me_explicit_ref(&self) -> &Wrapper {
+        self
+    }
+
+    /// Method for getting one's ref mut (explicit) self.
+    fn me_explicit_mut(&mut self) -> &mut Wrapper {
+        self
+    }
+
+    fn max_ref(&self, other: &'static Wrapper) -> &Self {
+        if self.a > other.a {
+            self
+        } else {
+            other
+        }
+    }
+
+    /// `offset` does nothing.
+    fn max_ref_offset(&self, other: &'static Wrapper, _offset: i32) -> &Self {
+        if self.a > other.a {
+            self
+        } else {
+            other
+        }
+    }
+
+    fn max_ref2(&self, other: &'static Self) -> &Self {
+        if self.a > other.a {
+            self
+        } else {
+            other
+        }
+    }
+}
+
+// Macro to generate exports
+extendr_module! {
+    mod externalptr;
+    impl Wrapper;
+}

tests/extendrtests/src/rust/src/lib.rs

@@ -3,6 +3,7 @@ use extendr_api::{graphics::*, prelude::*};
 mod altrep;
 mod attributes;
 mod dataframe;
+mod externalptr;
 mod graphic_device;
 mod memory_leaks;
 mod optional_either;
@@ -350,5 +351,5 @@ extendr_module! {
     use optional_faer;
     use raw_identifiers;
     use submodule;
-
+    use externalptr;
 }

tests/extendrtests/src/rust/src/submodule.rs

@@ -37,38 +37,6 @@ impl MySubmoduleClass {
     fn a(&self) -> i32 {
         self.a
     }
-
-    // NOTE: Cannot move ownership, as that concept is incompatible with bridging
-    // data from R to Rust
-    // fn myself(self) -> Self {
-    //     self
-    // }
-
-    /// Method for getting one's (by way of a copy) self.
-    fn me_owned(&self) -> Self {
-        // only possible due to `derive(Clone, Copy)`
-        *self
-    }
-
-    /// Method for getting one's (ref) self.
-    fn me_ref(&self) -> &Self {
-        self
-    }
-
-    /// Method for getting one's (ref mut) self.
-    fn me_mut(&mut self) -> &mut Self {
-        self
-    }
-
-    /// Method for getting one's ref (explicit) self.
-    fn me_explicit_ref(&self) -> &MySubmoduleClass {
-        self
-    }
-
-    /// Method for getting one's ref mut (explicit) self.
-    fn me_explicit_mut(&mut self) -> &mut MySubmoduleClass {
-        self
-    }
 }
 
 // Macro to generate exports