From c9a4cd6b640856610d60ede2f55994c6bba61dba Mon Sep 17 00:00:00 2001 From: cgmossa Date: Sun, 28 Jan 2024 14:48:42 +0100 Subject: [PATCH] fix: use new `Rboolean` enum instead --- extendr-api/src/functions.rs | 10 ++-- extendr-api/src/lib.rs | 22 ++++---- extendr-api/src/robj/mod.rs | 2 +- extendr-api/src/robj/rinternals.rs | 70 ++++++++++++++------------ extendr-api/src/thread_safety.rs | 4 +- extendr-api/src/wrapper/altrep.rs | 36 +++++++++---- extendr-api/src/wrapper/environment.rs | 2 +- extendr-api/tests/set_class_tests.rs | 16 +++--- 8 files changed, 92 insertions(+), 70 deletions(-) diff --git a/extendr-api/src/functions.rs b/extendr-api/src/functions.rs index 2228d3db32..4a77201812 100644 --- a/extendr-api/src/functions.rs +++ b/extendr-api/src/functions.rs @@ -209,10 +209,14 @@ pub fn blank_scalar_string() -> Robj { pub fn parse(code: &str) -> Result { single_threaded(|| unsafe { use libR_sys::*; - let mut status = 0_u32; - let status_ptr = &mut status as _; + let mut status = ParseStatus_PARSE_NULL; let codeobj: Robj = code.into(); - let parsed = Robj::from_sexp(R_ParseVector(codeobj.get(), -1, status_ptr, R_NilValue)); + let parsed = Robj::from_sexp(R_ParseVector( + codeobj.get(), + -1, + &mut status as *mut _, + R_NilValue, + )); match status { 1 => parsed.try_into(), _ => Err(Error::ParseError(code.into())), diff --git a/extendr-api/src/lib.rs b/extendr-api/src/lib.rs index 02d863b6f1..66036842bb 100644 --- a/extendr-api/src/lib.rs +++ b/extendr-api/src/lib.rs @@ -482,17 +482,17 @@ pub unsafe fn register_call_methods(info: *mut libR_sys::DllInfo, metadata: Meta }); single_threaded(|| { - libR_sys::R_registerRoutines( - info, - std::ptr::null(), - rmethods.as_ptr(), - std::ptr::null(), - std::ptr::null(), - ); - - // This seems to allow both symbols and strings, - libR_sys::R_useDynamicSymbols(info, 0); - libR_sys::R_forceSymbols(info, 0); + libR_sys::R_registerRoutines( + info, + std::ptr::null(), + rmethods.as_ptr(), + std::ptr::null(), + std::ptr::null(), + ); + + // This seems to allow both symbols and strings, + libR_sys::R_useDynamicSymbols(info, Rboolean::FALSE); + libR_sys::R_forceSymbols(info, Rboolean::FALSE); }) } diff --git a/extendr-api/src/robj/mod.rs b/extendr-api/src/robj/mod.rs index 7ca781029e..d4476095b9 100644 --- a/extendr-api/src/robj/mod.rs +++ b/extendr-api/src/robj/mod.rs @@ -1068,7 +1068,7 @@ impl PartialEq for Robj { } // see https://github.com/hadley/r-internals/blob/master/misc.md - R_compute_identical(self.get(), rhs.get(), 16) != 0 + R_compute_identical(self.get(), rhs.get(), 16) != Rboolean::FALSE } } } diff --git a/extendr-api/src/robj/rinternals.rs b/extendr-api/src/robj/rinternals.rs index 04262549c7..11a71269de 100644 --- a/extendr-api/src/robj/rinternals.rs +++ b/extendr-api/src/robj/rinternals.rs @@ -7,37 +7,37 @@ use std::os::raw; pub trait Rinternals: Types + Conversions { /// Return true if this is the null object. fn is_null(&self) -> bool { - unsafe { Rf_isNull(self.get()) == Rboolean_TRUE } + unsafe { Rf_isNull(self.get()) == Rboolean::TRUE } } /// Return `true` if this is a symbol. fn is_symbol(&self) -> bool { - unsafe { Rf_isSymbol(self.get()) == Rboolean_TRUE } + unsafe { Rf_isSymbol(self.get()) == Rboolean::TRUE } } /// Return `true` if this is a boolean (`logical`) vector fn is_logical(&self) -> bool { - unsafe { Rf_isLogical(self.get()) == Rboolean_TRUE } + unsafe { Rf_isLogical(self.get()) == Rboolean::TRUE } } /// Return `true` if this is a real (`f64`) vector. fn is_real(&self) -> bool { - unsafe { Rf_isReal(self.get()) == Rboolean_TRUE } + unsafe { Rf_isReal(self.get()) == Rboolean::TRUE } } /// Return `true` if this is a complex vector. fn is_complex(&self) -> bool { - unsafe { Rf_isComplex(self.get()) == Rboolean_TRUE } + unsafe { Rf_isComplex(self.get()) == Rboolean::TRUE } } /// Return `true` if this is an expression. fn is_expressions(&self) -> bool { - unsafe { Rf_isExpression(self.get()) == Rboolean_TRUE } + unsafe { Rf_isExpression(self.get()) == Rboolean::TRUE } } /// Return `true` if this is an environment. fn is_environment(&self) -> bool { - unsafe { Rf_isEnvironment(self.get()) == Rboolean_TRUE } + unsafe { Rf_isEnvironment(self.get()) == Rboolean::TRUE } } /// Return `true` if this is an environment. @@ -47,17 +47,21 @@ pub trait Rinternals: Types + Conversions { /// Return `true` if this is a string. fn is_string(&self) -> bool { - unsafe { Rf_isString(self.get()) == Rboolean_TRUE } + unsafe { Rf_isString(self.get()) == Rboolean::TRUE } } /// Return `true` if this is an object (ie. has a class attribute). fn is_object(&self) -> bool { - unsafe { Rf_isObject(self.get()) == Rboolean_TRUE } + unsafe { Rf_isObject(self.get()) == Rboolean::TRUE } } /// Return `true` if this is a S4 object. fn is_s4(&self) -> bool { - unsafe { Rf_isS4(self.get()) == Rboolean_TRUE } + unsafe { + // calls IS_S4_OBJECT, returns 16 instead of 1.. + Rf_isS4(self.get()) != Rboolean::FALSE + // unsafe { Rf_isS4(self.get()) == Rboolean_TRUE } + } } /// Return `true` if this is an expression. @@ -274,7 +278,7 @@ pub trait Rinternals: Types + Conversions { unsafe fn register_c_finalizer(&self, func: R_CFinalizer_t) { // Use R_RegisterCFinalizerEx() and set onexit to 1 (TRUE) to invoke the // finalizer on a shutdown of the R session as well. - single_threaded(|| R_RegisterCFinalizerEx(self.get(), func, 1)); + single_threaded(|| R_RegisterCFinalizerEx(self.get(), func, Rboolean::TRUE)); } /// Copy a vector and resize it. @@ -298,102 +302,102 @@ pub trait Rinternals: Types + Conversions { /// Return `true` if two arrays have identical dims. fn conformable(a: &Robj, b: &Robj) -> bool { - single_threaded(|| unsafe { Rf_conformable(a.get(), b.get()) == Rboolean_TRUE }) + single_threaded(|| unsafe { Rf_conformable(a.get(), b.get()) == Rboolean::TRUE }) } /// Return `true` if this is an array. fn is_array(&self) -> bool { - unsafe { Rf_isArray(self.get()) == Rboolean_TRUE } + unsafe { Rf_isArray(self.get()) == Rboolean::TRUE } } /// Return `true` if this is factor. fn is_factor(&self) -> bool { - unsafe { Rf_isFactor(self.get()) == Rboolean_TRUE } + unsafe { Rf_isFactor(self.get()) == Rboolean::TRUE } } /// Return `true` if this is a data frame. fn is_frame(&self) -> bool { - unsafe { Rf_isFrame(self.get()) == Rboolean_TRUE } + unsafe { Rf_isFrame(self.get()) == Rboolean::TRUE } } /// Return `true` if this is a function or a primitive (`CLOSXP`, `BUILTINSXP` or `SPECIALSXP`) fn is_function(&self) -> bool { - unsafe { Rf_isFunction(self.get()) == Rboolean_TRUE } + unsafe { Rf_isFunction(self.get()) == Rboolean::TRUE } } /// Return `true` if this is an integer vector (`INTSXP`) but not a factor. fn is_integer(&self) -> bool { - unsafe { Rf_isInteger(self.get()) == Rboolean_TRUE } + unsafe { Rf_isInteger(self.get()) == Rboolean::TRUE } } /// Return `true` if this is a language object (`LANGSXP`). fn is_language(&self) -> bool { - unsafe { Rf_isLanguage(self.get()) == Rboolean_TRUE } + unsafe { Rf_isLanguage(self.get()) == Rboolean::TRUE } } /// Return `true` if this is `NILSXP` or `LISTSXP`. fn is_pairlist(&self) -> bool { - unsafe { Rf_isList(self.get()) == Rboolean_TRUE } + unsafe { Rf_isList(self.get()) == Rboolean::TRUE } } /// Return `true` if this is a matrix. fn is_matrix(&self) -> bool { - unsafe { Rf_isMatrix(self.get()) == Rboolean_TRUE } + unsafe { Rf_isMatrix(self.get()) == Rboolean::TRUE } } /// Return `true` if this is `NILSXP` or `VECSXP`. fn is_list(&self) -> bool { - unsafe { Rf_isNewList(self.get()) == Rboolean_TRUE } + unsafe { Rf_isNewList(self.get()) == Rboolean::TRUE } } /// Return `true` if this is `INTSXP`, `LGLSXP` or `REALSXP` but not a factor. fn is_number(&self) -> bool { - unsafe { Rf_isNumber(self.get()) == Rboolean_TRUE } + unsafe { Rf_isNumber(self.get()) == Rboolean::TRUE } } /// Return `true` if this is a primitive function `BUILTINSXP`, `SPECIALSXP`. fn is_primitive(&self) -> bool { - unsafe { Rf_isPrimitive(self.get()) == Rboolean_TRUE } + unsafe { Rf_isPrimitive(self.get()) == Rboolean::TRUE } } /// Return `true` if this is a time series vector (see tsp). fn is_ts(&self) -> bool { - unsafe { Rf_isTs(self.get()) == Rboolean_TRUE } + unsafe { Rf_isTs(self.get()) == Rboolean::TRUE } } /// Return `true` if this is a user defined binop. fn is_user_binop(&self) -> bool { - unsafe { Rf_isUserBinop(self.get()) == Rboolean_TRUE } + unsafe { Rf_isUserBinop(self.get()) == Rboolean::TRUE } } /// Return `true` if this is a valid string. fn is_valid_string(&self) -> bool { - unsafe { Rf_isValidString(self.get()) == Rboolean_TRUE } + unsafe { Rf_isValidString(self.get()) == Rboolean::TRUE } } /// Return `true` if this is a valid string. fn is_valid_string_f(&self) -> bool { - unsafe { Rf_isValidStringF(self.get()) == Rboolean_TRUE } + unsafe { Rf_isValidStringF(self.get()) == Rboolean::TRUE } } /// Return `true` if this is a vector. fn is_vector(&self) -> bool { - unsafe { Rf_isVector(self.get()) == Rboolean_TRUE } + unsafe { Rf_isVector(self.get()) == Rboolean::TRUE } } /// Return `true` if this is an atomic vector. fn is_vector_atomic(&self) -> bool { - unsafe { Rf_isVectorAtomic(self.get()) == Rboolean_TRUE } + unsafe { Rf_isVectorAtomic(self.get()) == Rboolean::TRUE } } /// Return `true` if this is a vector list. fn is_vector_list(&self) -> bool { - unsafe { Rf_isVectorList(self.get()) == Rboolean_TRUE } + unsafe { Rf_isVectorList(self.get()) == Rboolean::TRUE } } /// Return `true` if this is can be made into a vector. fn is_vectorizable(&self) -> bool { - unsafe { Rf_isVectorizable(self.get()) == Rboolean_TRUE } + unsafe { Rf_isVectorizable(self.get()) == Rboolean::TRUE } } /// Return `true` if this is RAWSXP. @@ -428,7 +432,7 @@ pub trait Rinternals: Types + Conversions { } fn is_package_env(&self) -> bool { - unsafe { R_IsPackageEnv(self.get()) == Rboolean_TRUE } + unsafe { R_IsPackageEnv(self.get()) == Rboolean::TRUE } } fn package_env_name(&self) -> Robj { @@ -436,7 +440,7 @@ pub trait Rinternals: Types + Conversions { } fn is_namespace_env(&self) -> bool { - unsafe { R_IsNamespaceEnv(self.get()) == Rboolean_TRUE } + unsafe { R_IsNamespaceEnv(self.get()) == Rboolean::TRUE } } fn namespace_env_spec(&self) -> Robj { diff --git a/extendr-api/src/thread_safety.rs b/extendr-api/src/thread_safety.rs index a8653c4a82..fe85ca294f 100644 --- a/extendr-api/src/thread_safety.rs +++ b/extendr-api/src/thread_safety.rs @@ -46,7 +46,7 @@ pub fn throw_r_error>(s: S) -> ! { let s = s.as_ref(); unsafe { R_ERROR_BUF = Some(std::ffi::CString::new(s).unwrap()); - libR_sys::Rf_error(R_ERROR_BUF.as_ref().unwrap().as_ptr()); + libR_sys::Rf_error(R_ERROR_BUF.as_ref().unwrap().as_ptr()) }; } @@ -78,7 +78,7 @@ where } unsafe extern "C" fn do_cleanup(_: *mut raw::c_void, jump: Rboolean) { - if jump != 0 { + if jump != Rboolean::FALSE { panic!("R has thrown an error."); } } diff --git a/extendr-api/src/wrapper/altrep.rs b/extendr-api/src/wrapper/altrep.rs index 6e5f754225..681270281e 100644 --- a/extendr-api/src/wrapper/altrep.rs +++ b/extendr-api/src/wrapper/altrep.rs @@ -512,7 +512,7 @@ impl Altrep { // Use R_RegisterCFinalizerEx() and set onexit to 1 (TRUE) to invoke // the finalizer on a shutdown of the R session as well. - R_RegisterCFinalizerEx(state, Some(finalizer::), 1); + R_RegisterCFinalizerEx(state, Some(finalizer::), Rboolean::TRUE); let class_ptr = R_altrep_class_t { ptr: class.get() }; let sexp = R_new_altrep(class_ptr, state, R_NilValue); @@ -595,14 +595,14 @@ impl Altrep { x: SEXP, deep: Rboolean, ) -> SEXP { - ::duplicate(x, deep == 1).get() + ::duplicate(x, deep == Rboolean::TRUE).get() } unsafe extern "C" fn altrep_DuplicateEX( x: SEXP, deep: Rboolean, ) -> SEXP { - ::duplicate_ex(x, deep == 1).get() + ::duplicate_ex(x, deep == Rboolean::TRUE).get() } unsafe extern "C" fn altrep_Inspect( @@ -625,7 +625,7 @@ impl Altrep { x: SEXP, writeable: Rboolean, ) -> *mut c_void { - ::dataptr(x, writeable != 0) as *mut c_void + ::dataptr(x, writeable != Rboolean::FALSE) as *mut c_void } unsafe extern "C" fn altvec_Dataptr_or_null( @@ -745,21 +745,27 @@ impl Altrep { x: SEXP, narm: Rboolean, ) -> SEXP { - Altrep::get_state::(x).sum(narm == 1).get() + Altrep::get_state::(x) + .sum(narm == Rboolean::TRUE) + .get() } unsafe extern "C" fn altinteger_Min( x: SEXP, narm: Rboolean, ) -> SEXP { - Altrep::get_state::(x).min(narm == 1).get() + Altrep::get_state::(x) + .min(narm == Rboolean::TRUE) + .get() } unsafe extern "C" fn altinteger_Max( x: SEXP, narm: Rboolean, ) -> SEXP { - Altrep::get_state::(x).max(narm == 1).get() + Altrep::get_state::(x) + .max(narm == Rboolean::TRUE) + .get() } R_set_altinteger_Elt_method(class_ptr, Some(altinteger_Elt::)); @@ -817,21 +823,27 @@ impl Altrep { x: SEXP, narm: Rboolean, ) -> SEXP { - Altrep::get_state::(x).sum(narm == 1).get() + Altrep::get_state::(x) + .sum(narm == Rboolean::TRUE) + .get() } unsafe extern "C" fn altreal_Min( x: SEXP, narm: Rboolean, ) -> SEXP { - Altrep::get_state::(x).min(narm == 1).get() + Altrep::get_state::(x) + .min(narm == Rboolean::TRUE) + .get() } unsafe extern "C" fn altreal_Max( x: SEXP, narm: Rboolean, ) -> SEXP { - Altrep::get_state::(x).max(narm == 1).get() + Altrep::get_state::(x) + .max(narm == Rboolean::TRUE) + .get() } R_set_altreal_Elt_method(class_ptr, Some(altreal_Elt::)); @@ -890,7 +902,9 @@ impl Altrep { x: SEXP, narm: Rboolean, ) -> SEXP { - Altrep::get_state::(x).sum(narm == 1).get() + Altrep::get_state::(x) + .sum(narm == Rboolean::TRUE) + .get() } R_set_altlogical_Elt_method(class_ptr, Some(altlogical_Elt::)); diff --git a/extendr-api/src/wrapper/environment.rs b/extendr-api/src/wrapper/environment.rs index 29ea0c91ff..c489d3c223 100644 --- a/extendr-api/src/wrapper/environment.rs +++ b/extendr-api/src/wrapper/environment.rs @@ -174,7 +174,7 @@ impl Environment { Ok(Robj::from_sexp(Rf_findVarInFrame3( self.get(), key.get(), - 1, + Rboolean::TRUE, ))) } } else { diff --git a/extendr-api/tests/set_class_tests.rs b/extendr-api/tests/set_class_tests.rs index e84efeefc7..2f5d9d1dda 100644 --- a/extendr-api/tests/set_class_tests.rs +++ b/extendr-api/tests/set_class_tests.rs @@ -1,7 +1,7 @@ use extendr_api::{Attributes, GetSexp}; use extendr_engine::with_r; use extendr_macros::list; -use libR_sys::{R_compute_identical, Rboolean_TRUE, Rf_PrintValue}; +use libR_sys::{R_compute_identical, Rboolean, Rf_PrintValue}; #[test] fn test_what_is_returned_from_set_class() { @@ -24,14 +24,14 @@ fn test_what_is_returned_from_set_class() { 32 = !IGNORE_SRCREF Default from R's default: 16 = (0 + 0 + 0 + 0 + 16 + 0) */ - assert!(R_compute_identical(a.get(), a_class.get(), 0) == Rboolean_TRUE); - // assert!(R_compute_identical(a.get(), a_class.get(), 1) == Rboolean_TRUE); - // assert!(R_compute_identical(a.get(), a_class.get(), 2) == Rboolean_TRUE); - // assert!(R_compute_identical(a.get(), a_class.get(), 4) == Rboolean_TRUE); - // assert!(R_compute_identical(a.get(), a_class.get(), 8) == Rboolean_TRUE); + assert!(R_compute_identical(a.get(), a_class.get(), 0) == Rboolean::TRUE); + // assert!(R_compute_identical(a.get(), a_class.get(), 1) == Rboolean::TRUE); + // assert!(R_compute_identical(a.get(), a_class.get(), 2) == Rboolean::TRUE); + // assert!(R_compute_identical(a.get(), a_class.get(), 4) == Rboolean::TRUE); + // assert!(R_compute_identical(a.get(), a_class.get(), 8) == Rboolean::TRUE); // // R default flag is 16 - // assert!(R_compute_identical(a.get(), a_class.get(), 16) == Rboolean_TRUE); - // assert!(R_compute_identical(a.get(), a_class.get(), 32) == Rboolean_TRUE); + // assert!(R_compute_identical(a.get(), a_class.get(), 16) == Rboolean::TRUE); + // assert!(R_compute_identical(a.get(), a_class.get(), 32) == Rboolean::TRUE); } }) }