diff --git a/extendr-api/Cargo.toml b/extendr-api/Cargo.toml index 9379bd6d86..74fc091ea7 100644 --- a/extendr-api/Cargo.toml +++ b/extendr-api/Cargo.toml @@ -32,7 +32,10 @@ serde = { version = "1.0", features = ["derive"], optional = true } rstest = "0.16.0" [features] -default = [] +default = ["result_panic"] +result_panic =[] +result_list = [] +result_condition = [] # This dummy feature enables all features that increase the functionality of # extendr, via conversions or R features. Features that change behaviour diff --git a/extendr-api/README.md b/extendr-api/README.md index 9708ad94e3..cf0ce15eb1 100644 --- a/extendr-api/README.md +++ b/extendr-api/README.md @@ -220,9 +220,9 @@ test! { } ``` -## Feature gates + ## Feature gates -extendr-api has some optional features behind these feature gates: + extendr-api has some optional features behind these feature gates: * `ndarray`: provides the conversion between R's matrices and [ndarray](https://docs.rs/ndarray/latest/ndarray/). * `num-complex`: provides the conversion between R's complex numbers and [num-complex](https://docs.rs/num-complex/latest/num_complex/). @@ -230,6 +230,13 @@ extendr-api has some optional features behind these feature gates: * `graphics`: provides the functionality to control or implement graphics devices. * `either`: provides implementation of type conversion traits for `Either` from [either](https://docs.rs/either/latest/either/) if `L` and `R` both implement those traits. +extendr-api has three ways to return a Result to R. Only one behavior features can be picked. + - `result_panic`: Default behavior, return `Ok` as is, panic! on any `Err` + - `result_list`: return `Ok` as `list(ok=?, err=NULL)` or `Err` `list(ok=NULL, err=?)` + - `result_condition`: return `Ok` as is or `Err` as $value in an R error condition. + To choose either of these set e.g. `extendr-api = {..., default-features = false, features= ["result_condition"]}` + These features are experimental and may change. + ## License MIT diff --git a/extendr-api/src/lib.rs b/extendr-api/src/lib.rs index 3292429b18..9b99180c6f 100644 --- a/extendr-api/src/lib.rs +++ b/extendr-api/src/lib.rs @@ -253,6 +253,73 @@ //! } //! ``` //! +//! ## Returning Result to R +//! +//! Currently `throw_r_error()` does leak memory because it jumps to R without releasing +//! memory for rust objects. +//! +//! The memory safe way to do error handling with extendr is to return a Result +//! to R. By default any Err will trigger a panic! on rust side which unwinds the stack. +//! The rust error trace will be printed via stderr, not R terminal. Any Ok value is returned +//! as is. +//! +//! Alternatively two experimental non-leaking features `result_list` and `result_condition` +//! can be used to not cause panics on `Err`. Instead an `Err` `x` is returned respectively as +//! - list: `list(ok=NULL, err=x)` +//! - error condition: ``, with `x` placed in `condition$value` +//! +//! It is currently solely up to the user to handle any result on R side. +//! +//! The minimal overhead of calling an extendr function is in the ballpark of 2-4us. +//! Returning a condition or list increases the overhead to 4-8us. To check and handle the result +//! on R side will likely increase overall overhead to 8-16us, depending on how efficient the +//! result is handled. If you plan to call an extendr-functions a million times every 5 seconds, +//! this overhead matters. Otherwise the overhead is likely very negileble. +//! +//! ```ignore +//! use extendr_api::prelude::*; +//! // simple function always returning an Err string +//! #[extendr] +//! fn oups(a: i32) -> std::result::Result { +//! Err("I did it again".to_string()) +//! } +//! +//! // define exports using extendr_module +//! extendr_module! { +//! mod mymodule; +//! fn oups; +//! } +//! +//! ``` +//! +//! In R: +//! +//! ```ignore +//! #default result_panic feature +//! oups(1) +//! > ... long panic traceback from rust printed to stderr +//! +//! #result_list feature +//! lst <-oups(1) +//! print(lst) +//! > list(ok=NULL, err="I did it again") +//! +//! #result_condition feature +//! cnd = oups(1) +//! print(cnd) +//! > +//! print(cnd$value) +//! > "I did it again" +//! +//! #handling example for result_condition +//! oups_handled = function(a) { +//! val_or_err = oups(1) +//! if(inherits(val_or_err,"extendr_error")) stop(val_or_err) +//! val_or_err +//! } +//! +//! ``` +//! //! ## Feature gates //! //! extendr-api has some optional features behind these feature gates: @@ -261,6 +328,14 @@ //! - `num-complex`: provides the conversion between R's complex numbers and [num-complex](https://docs.rs/num-complex/latest/num_complex/). //! - `serde`: provides the [Serde](https://serde.rs/) support. //! - `graphics`: provides the functionality to control or implement graphics devices. +//! +//! extendr-api has three ways to return a Result to R. Only one behavior features can be picked. +//! - `result_panic`: Default behavior, return `Ok` as is, panic! on any `Err` +//! +//! To choose either if these set e.g. `extendr-api = {..., default-features = false, features= ["result_condition"]}` +//! These features are experimental and may change. +//! - `result_list`: return `Ok` as `list(ok=?, err=NULL)` or `Err` `list(ok=NULL, err=?)` +//! - `result_condition`: return `Ok` as is or `Err` as $value in an R error condition. #![doc( html_logo_url = "https://raw.githubusercontent.com/extendr/extendr/master/extendr-logo-256.png" diff --git a/extendr-api/src/robj/into_robj.rs b/extendr-api/src/robj/into_robj.rs index a412d14150..9de979b4b1 100644 --- a/extendr-api/src/robj/into_robj.rs +++ b/extendr-api/src/robj/into_robj.rs @@ -39,16 +39,157 @@ impl From<()> for Robj { /// assert_eq!(r!(my_func()), r!(1.0)); /// } /// ``` -impl From> for Robj +#[cfg(feature = "result_panic")] +impl From> for Robj where T: Into, + E: std::fmt::Debug, { - fn from(res: Result) -> Self { - // Force a panic on error. + fn from(res: std::result::Result) -> Self { res.unwrap().into() } } +/// Convert a Result to an Robj either an Ok value or the Err value wrapped in a +/// error condition. This is used to allow functions to use the ? operator +/// and return [Result] without panicking on an Err. T must impl IntoRobj. +/// +/// Returns ok-value as is. Return err wrapped in a R error condition. The err is placed in +/// $value. The condition messeage is simply +/// ``` +/// use extendr_api::prelude::*; +/// fn my_func() -> Result { +/// Ok(1.0) +/// } +/// +/// test! { +/// assert_eq!(r!(my_func()), r!(1.0)); +/// } +/// +/// //ok and err type is any IntoRobj +/// fn my_err_f() -> std::result::Result { +/// Err(42.0) // return err float +/// } +/// +/// test! { +/// assert_eq!( +/// r!(my_err_f()), +/// R!( +/// "structure(list(message = 'extendr_err', +/// value = 42.0), class = c('extendr_error', 'error', 'condition'))" +/// ).unwrap() +/// ); +/// } +/// +/// ``` +#[cfg(feature = "result_condition")] +impl From> for Robj +where + T: Into, + E: Into, +{ + fn from(res: std::result::Result) -> Self { + match res { + Ok(x) => x.into(), + Err(x) => { + let robj: Robj = x.into(); + List::from_names_and_values( + &["message", "value"], + &["extendr_err".into_robj(), robj], + ) + } + //can only imagine this would ever fail due memory allcation error, but then panicking is the right choice + .expect("internal error: failed to create an R list") + .set_class(["extendr_error", "error", "condition"]) + .expect("internal error: failed to set class"), + } + } +} + +/// Convert a Result to an R `List` with an `ok` and `err` element. +/// This is used to allow functions to use the ? operator +/// and return [std::result::Result or extendr_api::result::Result] +/// without panicking on an Err. +/// +/// +/// ``` +/// use extendr_api::prelude::*; +/// fn my_err_f() -> std::result::Result { +/// Err("We have water in the engine room!".to_string()) +/// } +/// fn my_ok_f() -> std::result::Result { +/// Ok(123.123) +/// } +/// +/// test! { +/// assert_eq!( +/// r!(my_err_f()), +/// R!("x=list(ok=NULL, err='We have water in the engine room!') +/// class(x)='extendr_result' +/// x" +/// ).unwrap() +/// ); +/// assert_eq!( +/// r!(my_ok_f()), +/// R!("x = list(ok=123.123, err=NULL) +/// class(x)='extendr_result' +/// x" +/// ).unwrap() +/// ); +/// } +/// +/// ``` +#[cfg(feature = "result_list")] +impl From> for Robj +where + T: Into, + E: Into, +{ + fn from(res: std::result::Result) -> Self { + match res { + Ok(x) => List::from_names_and_values(&["ok", "err"], &[x.into(), NULL.into()]), + Err(x) => { + let err_robj = x.into(); + if err_robj.is_null() { + panic!("Internal error: result_list not allowed to return NULL as err-value") + } + List::from_names_and_values(&["ok", "err"], &[NULL.into(), err_robj]) + } + } + //can only imagine this would ever fail due memory allcation error, but then panicking is the right choice + .expect("Internal error: failed to create an R list") + .set_class(&["extendr_result"]) + .expect("Internal error: failed to set class") + .into() + } +} + +// string conversions from Error trait to Robj and String +impl From for Robj { + fn from(res: Error) -> Self { + res.to_string().into() + } +} +impl From for String { + fn from(res: Error) -> Self { + res.to_string().into() + } +} + +// // ... and Err does not have to implement Display +// impl From> for Robj +// where +// T: Into, +// E: Into, +// { +// fn from(res: std::result::Result) -> Self { +// match res { +// Ok(x) => x.into(), +// Err(x) => x.into_robj().set_attrib("extendr_err", true).unwrap(), +// } +// } +// } + /// Convert an Robj reference into a borrowed Robj. impl From<&Robj> for Robj { // Note: we should probably have a much better reference diff --git a/extendr-macros/src/extendr_function.rs b/extendr-macros/src/extendr_function.rs index 25ba3e0abd..c8e1617382 100644 --- a/extendr-macros/src/extendr_function.rs +++ b/extendr-macros/src/extendr_function.rs @@ -26,7 +26,7 @@ pub fn parse_options(opts: &mut wrappers::ExtendrOptions, arg: &syn::NestedMeta) use syn::{Lit, LitBool, Meta, MetaNameValue, NestedMeta}; fn help_message() -> ! { - panic!("expected #[extendr(use_try_from=bool, r_name=\"name\")]"); + panic!("expected #[extendr(use_try_from=bool, r_name=\"name\", r_class_name=\"AnyChosenName\", r_super_class_name=\"AnySuperName\")]"); } match arg { @@ -35,6 +35,7 @@ pub fn parse_options(opts: &mut wrappers::ExtendrOptions, arg: &syn::NestedMeta) eq_token: _, lit, })) => { + //TODO refactor with match guards if path.is_ident("use_try_from") { if let Lit::Bool(LitBool { value, .. }) = lit { opts.use_try_from = *value; @@ -53,6 +54,18 @@ pub fn parse_options(opts: &mut wrappers::ExtendrOptions, arg: &syn::NestedMeta) } else { help_message(); } + } else if path.is_ident("r_class_name") { + if let Lit::Str(litstr) = lit { + opts.r_class_name = Some(litstr.value()); + } else { + help_message(); + } + } else if path.is_ident("r_super_class_name") { + if let Lit::Str(litstr) = lit { + opts.r_super_class_name = Some(litstr.value()); + } else { + help_message(); + } } else { help_message(); } diff --git a/extendr-macros/src/extendr_impl.rs b/extendr-macros/src/extendr_impl.rs index dbdf84d9f2..388791b4b7 100644 --- a/extendr-macros/src/extendr_impl.rs +++ b/extendr-macros/src/extendr_impl.rs @@ -36,7 +36,7 @@ use crate::wrappers; /// fn aux_func; /// } /// ``` -pub fn extendr_impl(mut item_impl: ItemImpl) -> TokenStream { +pub fn extendr_impl(args: Vec, mut item_impl: ItemImpl) -> TokenStream { // Only `impl name { }` allowed if item_impl.defaultness.is_some() { return quote! { compile_error!("default not allowed in #[extendr] impl"); }.into(); @@ -62,13 +62,33 @@ pub fn extendr_impl(mut item_impl: ItemImpl) -> TokenStream { return quote! { compile_error!("where clause not allowed in #[extendr] impl"); }.into(); } - let opts = wrappers::ExtendrOptions::default(); + let mut opts = wrappers::ExtendrOptions::default(); + for arg in &args { + crate::extendr_function::parse_options(&mut opts, arg); + } + let self_ty = item_impl.self_ty.as_ref(); - let self_ty_name = wrappers::type_name(self_ty); + let self_ty_name = opts + .r_class_name + .clone() + .unwrap_or_else(|| wrappers::type_name(self_ty)); + let r_super_class_name = opts + .r_super_class_name + .clone() + .unwrap_or_else(|| "no_super_class".into()); let prefix = format!("{}__", self_ty_name); let mut method_meta_names = Vec::new(); let doc_string = wrappers::get_doc_string(&item_impl.attrs); + dbg!( + "{:?}{:?}{:?}{:?}{:?}{:?}", + &opts, + &self_ty, + &self_ty_name, + &prefix, + &method_meta_names, + &doc_string + ); // Generate wrappers for methods. // eg. // ``` @@ -105,6 +125,8 @@ pub fn extendr_impl(mut item_impl: ItemImpl) -> TokenStream { let finalizer_name = format_ident!("__finalize__{}", self_ty_name); + dbg!(&meta_name, &finalizer_name); + let expanded = TokenStream::from(quote! { // The impl itself copied from the source. #item_impl @@ -139,10 +161,11 @@ pub fn extendr_impl(mut item_impl: ItemImpl) -> TokenStream { // Output conversion function for this type. impl From<#self_ty> for Robj { fn from(value: #self_ty) -> Self { + unsafe { let ptr = Box::into_raw(Box::new(value)); let res = Robj::make_external_ptr(ptr, Robj::from(())); - res.set_attrib(class_symbol(), #self_ty_name).unwrap(); + res.set_attrib(class_symbol(), vec!(#self_ty_name, #r_super_class_name)).unwrap(); res.register_c_finalizer(Some(#finalizer_name)); res } diff --git a/extendr-macros/src/lib.rs b/extendr-macros/src/lib.rs index 81d7aed4b8..e70266218b 100644 --- a/extendr-macros/src/lib.rs +++ b/extendr-macros/src/lib.rs @@ -76,7 +76,7 @@ pub fn extendr(attr: TokenStream, item: TokenStream) -> TokenStream { let args = parse_macro_input!(attr as syn::AttributeArgs); match parse_macro_input!(item as Item) { Item::Fn(func) => extendr_function::extendr_function(args, func), - Item::Impl(item_impl) => extendr_impl::extendr_impl(item_impl), + Item::Impl(item_impl) => extendr_impl::extendr_impl(args, item_impl), other_item => TokenStream::from(quote! {#other_item}), } } diff --git a/extendr-macros/src/wrappers.rs b/extendr-macros/src/wrappers.rs index 644cb2cf67..aac655440b 100644 --- a/extendr-macros/src/wrappers.rs +++ b/extendr-macros/src/wrappers.rs @@ -9,6 +9,8 @@ pub const WRAP_PREFIX: &str = "wrap__"; pub struct ExtendrOptions { pub use_try_from: bool, pub r_name: Option, + pub r_class_name: Option, + pub r_super_class_name: Option, pub mod_name: Option, }