Add impl<E: std::error::Error> From<E> for savvy::Error (#324)

CHANGELOG.md

@@ -3,12 +3,51 @@
 <!-- next-header -->
 ## [Unreleased] (ReleaseDate)
 
+### New feature
+
+Like [anyhow], now you can use `?` to propagate any error that implements the
+`std::error::Error` trait.
+
+[anyhow]: https://docs.rs/anyhow/latest/anyhow/index.html
+
+```rust
+#[savvy]
+fn no_such_file() -> savvy::Result<()> {
+    // previously, you had to write .map_err(|e| e.to_string().into())
+    let _ = std::fs::read_to_string("no_such_file")?;
+    Ok(())
+}
+```
+
+If you want to implement your own error type and the conversion to
+`savvy::Error`, please sepcify `use-custom-error` feature to opt-out the
+auto-conversion to avoid conflict with `impl From<dyn std::error::Error> for savvy::Error`
+
+```toml
+savvy = { version = "...", features = ["use-custom-error"] }
+```
+
+### Breaking change
+
+By introducing the anyhow-like conversion, savvy loses the error conversion from
+a string (e.g. `Err("foo".into())`). Instead, please use `savvy_err!()` macro,
+which is a shorthand of `Error::new(format!(...))`.
+
+```rust
+#[savvy]
+fn raise_error() -> savvy::Result<savvy::Sexp> {
+    Err(savvy_err!("This is my custom error"))
+}
+```
+
+### Minor improvements
+
 * `NumericScalar::as_usize()` and `NumericSexp::iter_usize()` now rejects
   numbers larger than `2^32` on 32-bit targets (i.e. webR). Thanks @eitsupi!
 
 ## [v0.7.2] (2024-10-27)
 
-### Minor Improvements
+### Minor improvements
 
 * savvy now generates a reduced number of R functions.
 
@@ -24,7 +63,7 @@
 
 ## [v0.7.0] (2024-10-20)
 
-### Breaking Change
+### Breaking change
 
 Removed `TryFrom<Sexp> for usize`, so the following code no longer compiles.
 
@@ -44,10 +83,10 @@ by yourself. Also, please be aware you need to handle NA as well.
 #[savvy]
 fn foo(x: i32) -> savvy::Result<()> {
     if x.is_na() {
-        return Err("cannot convert NA to usize".into())?;
+        return Err(savvy_err!("cannot convert NA to usize"));
     }
 
-    let x = <usize>::try_from(x).map_err(|e| e.to_string().into());
+    let x = <usize>::try_from(x)?;
 
     ...
 }
@@ -72,7 +111,7 @@ usize_to_string_scalar(2147483648)
 
 ## [v0.6.8] (2024-09-17)
 
-### Minor Improvements
+### Minor improvements
 
 * `savvy init` now generates
   * slightly better `configure` script that checks if `cargo` command is available
@@ -82,7 +121,7 @@ usize_to_string_scalar(2147483648)
 
 ## [v0.6.7] (2024-09-05)
 
-### Minor Improvements
+### Minor improvements
 
 * Remove the use of non-API call `Rf_findVarInFrame`.
 
@@ -115,7 +154,7 @@ usize_to_string_scalar(2147483648)
   binary data on Rust's side, your primary option should be to store it in an
   external pointer (of a struct you define) rather than an R's raw vector.
 
-### Minor Improvements
+### Minor improvements
 
 * Wrapper environment of a Rust struct or enum now cannot be modified by users.
 
@@ -158,7 +197,7 @@ usize_to_string_scalar(2147483648)
 
 * `r_print!()` and `r_eprint!()` now can print strings containing `%`.
 
-### Breaking Change
+### Breaking change
 
 * The notation for `savvy-cli test` is now changed to `#[cfg(feature =
   "savvy-test")]` from `#[cfg(savvy_test)]`. This is to avoid the upcoming

Cargo.toml

@@ -34,6 +34,12 @@ altrep = ["savvy-ffi/altrep"]
 # Support logger
 logger = ["log", "env_logger"]
 
+# By default, savvy provides `impl<E: std::error::Error + 'static> From<E> for Error`.
+# However, this conflicts if the user implements their custom error and the
+# conversion from it to savvy::Error. This flag removes the impl to allow such a
+# custom error.
+use-custom-error = []
+
 savvy-test = []
 
 [build-dependencies]

R-package/R/000-wrappers.R

@@ -523,6 +523,11 @@ NULL
 }
 
 
+`error_conversion` <- function() {
+  invisible(.Call(savvy_error_conversion__impl))
+}
+
+
 `do_call` <- function(`fun`, `args`) {
   .Call(savvy_do_call__impl, `fun`, `args`)
 }

R-package/src/init.c

@@ -479,6 +479,11 @@ SEXP savvy_safe_warn__impl(void) {
     return handle_result(res);
 }
 
+SEXP savvy_error_conversion__impl(void) {
+    SEXP res = savvy_error_conversion__ffi();
+    return handle_result(res);
+}
+
 SEXP savvy_do_call__impl(SEXP c_arg__fun, SEXP c_arg__args) {
     SEXP res = savvy_do_call__ffi(c_arg__fun, c_arg__args);
     return handle_result(res);
@@ -809,6 +814,7 @@ static const R_CallMethodDef CallEntries[] = {
     {"savvy_raise_error__impl", (DL_FUNC) &savvy_raise_error__impl, 0},
     {"savvy_must_panic__impl", (DL_FUNC) &savvy_must_panic__impl, 0},
     {"savvy_safe_warn__impl", (DL_FUNC) &savvy_safe_warn__impl, 0},
+    {"savvy_error_conversion__impl", (DL_FUNC) &savvy_error_conversion__impl, 0},
     {"savvy_do_call__impl", (DL_FUNC) &savvy_do_call__impl, 2},
     {"savvy_call_with_args__impl", (DL_FUNC) &savvy_call_with_args__impl, 1},
     {"savvy_get_args__impl", (DL_FUNC) &savvy_get_args__impl, 1},

R-package/src/rust/api.h

@@ -87,6 +87,7 @@ SEXP savvy_safe_stop__ffi(void);
 SEXP savvy_raise_error__ffi(void);
 SEXP savvy_must_panic__ffi(void);
 SEXP savvy_safe_warn__ffi(void);
+SEXP savvy_error_conversion__ffi(void);
 SEXP savvy_do_call__ffi(SEXP c_arg__fun, SEXP c_arg__args);
 SEXP savvy_call_with_args__ffi(SEXP c_arg__fun);
 SEXP savvy_get_args__ffi(SEXP c_arg__args);

R-package/src/rust/src/altrep.rs

@@ -4,8 +4,8 @@ use savvy::altrep::{
     register_altstring_class, AltInteger, AltList, AltLogical, AltRaw, AltReal, AltString,
 };
 use savvy::{
-    r_println, savvy, savvy_init, IntegerSexp, ListSexp, LogicalSexp, NullSexp, RawSexp, RealSexp,
-    StringSexp,
+    r_println, savvy, savvy_err, savvy_init, IntegerSexp, ListSexp, LogicalSexp, NullSexp, RawSexp,
+    RealSexp, StringSexp,
 };
 
 // integer
@@ -47,7 +47,7 @@ fn print_altint(x: IntegerSexp) -> savvy::Result<()> {
         return Ok(());
     };
 
-    Err("Not a known ALTREP".into())
+    Err(savvy_err!("Not a known ALTREP"))
 }
 
 #[savvy]
@@ -60,7 +60,7 @@ fn tweak_altint(mut x: IntegerSexp) -> savvy::Result<()> {
         return Ok(());
     };
 
-    Err("Not a known ALTREP".into())
+    Err(savvy_err!("Not a known ALTREP"))
 }
 
 // real
@@ -102,7 +102,7 @@ fn print_altreal(x: RealSexp) -> savvy::Result<()> {
         return Ok(());
     };
 
-    Err("Not a known ALTREP".into())
+    Err(savvy_err!("Not a known ALTREP"))
 }
 
 #[savvy]
@@ -115,7 +115,7 @@ fn tweak_altreal(mut x: RealSexp) -> savvy::Result<()> {
         return Ok(());
     };
 
-    Err("Not a known ALTREP".into())
+    Err(savvy_err!("Not a known ALTREP"))
 }
 
 // logical
@@ -157,7 +157,7 @@ fn print_altlogical(x: LogicalSexp) -> savvy::Result<()> {
         return Ok(());
     };
 
-    Err("Not a known ALTREP".into())
+    Err(savvy_err!("Not a known ALTREP"))
 }
 
 #[savvy]
@@ -170,7 +170,7 @@ fn tweak_altlogical(mut x: LogicalSexp) -> savvy::Result<()> {
         return Ok(());
     };
 
-    Err("Not a known ALTREP".into())
+    Err(savvy_err!("Not a known ALTREP"))
 }
 
 // raw
@@ -212,7 +212,7 @@ fn print_altraw(x: RawSexp) -> savvy::Result<()> {
         return Ok(());
     };
 
-    Err("Not a known ALTREP".into())
+    Err(savvy_err!("Not a known ALTREP"))
 }
 
 #[savvy]
@@ -225,7 +225,7 @@ fn tweak_altraw(mut x: RawSexp) -> savvy::Result<()> {
         return Ok(());
     };
 
-    Err("Not a known ALTREP".into())
+    Err(savvy_err!("Not a known ALTREP"))
 }
 
 // string
@@ -267,7 +267,7 @@ fn print_altstring(x: StringSexp) -> savvy::Result<()> {
         return Ok(());
     };
 
-    Err("Not a known ALTREP".into())
+    Err(savvy_err!("Not a known ALTREP"))
 }
 
 #[savvy]
@@ -280,7 +280,7 @@ fn tweak_altstring(mut x: StringSexp) -> savvy::Result<()> {
         return Ok(());
     };
 
-    Err("Not a known ALTREP".into())
+    Err(savvy_err!("Not a known ALTREP"))
 }
 
 // list
@@ -337,7 +337,7 @@ fn print_altlist(x: ListSexp) -> savvy::Result<()> {
         return Ok(());
     };
 
-    Err("Not a known ALTREP".into())
+    Err(savvy_err!("Not a known ALTREP"))
 }
 
 #[savvy]
@@ -356,7 +356,7 @@ fn tweak_altlist(mut x: ListSexp) -> savvy::Result<()> {
         return Ok(());
     };
 
-    Err("Not a known ALTREP".into())
+    Err(savvy_err!("Not a known ALTREP"))
 }
 
 // initialization
@@ -400,6 +400,6 @@ fn print_altint_by_weird_way(x: IntegerSexp) -> savvy::Result<()> {
         r_println!("{out:?}");
         Ok(())
     } else {
-        Err("Not an altint".into())
+        Err(savvy_err!("Not an altint"))
     }
 }

R-package/src/rust/src/environment.rs

@@ -1,10 +1,10 @@
-use savvy::{savvy, EnvironmentSexp, Sexp};
+use savvy::{savvy, savvy_err, EnvironmentSexp, Sexp};
 
 #[savvy]
 fn get_var_in_env(name: &str, env: Option<EnvironmentSexp>) -> savvy::Result<Sexp> {
     let env = env.unwrap_or(EnvironmentSexp::global_env());
     let obj = env.get(name)?;
-    obj.ok_or("Not found".into())
+    obj.ok_or(savvy_err!("Not found"))
 }
 
 #[savvy]

R-package/src/rust/src/error_handling.rs

@@ -1,4 +1,4 @@
-use savvy::{savvy, savvy_init, NullSexp, Sexp};
+use savvy::{savvy, savvy_err, savvy_init, NullSexp, Sexp};
 use savvy_ffi::DllInfo;
 use std::ffi::CString;
 
@@ -8,10 +8,10 @@ static FOO_VALUE: OnceLock<Mutex<i32>> = OnceLock::new();
 
 #[savvy_init]
 fn init_foo_value(dll: *mut DllInfo) -> savvy::Result<()> {
-    match FOO_VALUE.set(Mutex::new(-1)) {
-        Ok(_) => Ok(()),
-        Err(_) => Err("Failed to set values".into()),
-    }
+    FOO_VALUE
+        .set(Mutex::new(-1))
+        .map_err(|_| savvy_err!("Failed to set values"))?;
+    Ok(())
 }
 
 struct Foo {}
@@ -38,7 +38,7 @@ impl Drop for Foo {
 fn get_foo_value() -> savvy::Result<Sexp> {
     match FOO_VALUE.get() {
         Some(x) => {
-            let v = *x.lock().unwrap();
+            let v = *x.lock()?;
             v.try_into()
         }
         None => NullSexp.into(),
@@ -61,7 +61,7 @@ fn safe_stop() -> savvy::Result<()> {
 
 #[savvy]
 fn raise_error() -> savvy::Result<savvy::Sexp> {
-    Err(savvy::Error::new("This is my custom error"))
+    Err(savvy_err!("This is my custom error"))
 }
 
 #[allow(clippy::out_of_bounds_indexing, unconditional_panic)]
@@ -80,3 +80,9 @@ fn safe_warn() -> savvy::Result<()> {
 
     Ok(())
 }
+
+#[savvy]
+fn error_conversion() -> savvy::Result<()> {
+    let _ = std::fs::read_to_string("no_such_file")?;
+    Ok(())
+}

book/src/altrep.md

@@ -147,7 +147,7 @@ fn print_altint(x: IntegerSexp) -> savvy::Result<()> {
         return Ok(());
     };
 
-    Err("Not a known ALTREP".into())
+    Err(savvy_err!("Not a known ALTREP"))
 }
 ```
 
@@ -183,7 +183,7 @@ fn tweak_altint(mut x: IntegerSexp) -> savvy::Result<()> {
         return Ok(());
     };
 
-    Err("Not a known ALTREP".into())
+    Err(savvy_err!("Not a known ALTREP"))
 }
 ```
 

book/src/enum.md

@@ -67,7 +67,7 @@ fn plot_line(x: IntegerSexp, y: IntegerSexp, line_type: &str) -> savvy::Result<(
             ...
         },
         _ => {
-            return Err("Unsupported line type!".into());
+            return Err(savvy_err!("Unsupported line type!"));
         }
     }
 }