From c7d62dc006ecbd326266a35c07e8486f3731bfe7 Mon Sep 17 00:00:00 2001 From: CGMossa Date: Thu, 9 May 2024 10:23:25 +0200 Subject: [PATCH 1/7] Replaces `SEXPTYPE` and `Rboolean` to the new `enum`-versions (#742) * replaces `SEXPTYPE` and `Rboolean` to the new enum-versions from latest `libR-sys`. * Cargo.toml: Update version of `libR-sys`, otherwise it doesn't work * build.rs: Added a feature for using `OBJSXP` vs `S4SXP` The cfg-feature might be excessive, but handling S4 object will have to depend on R versions eventually, so we are starting this branching now... * MSRV: Updated to 1.70 due to a change in bindgen, which uses `which`, and which uses `home` which silently updated their msrv to 1.70. --- Cargo.toml | 4 +- extendr-api/Cargo.toml | 2 +- extendr-api/build.rs | 4 + extendr-api/src/functions.rs | 6 +- extendr-api/src/graphics/device_driver.rs | 46 +++++------- extendr-api/src/graphics/mod.rs | 52 ++++++------- extendr-api/src/io/load.rs | 2 +- extendr-api/src/io/mod.rs | 8 +- extendr-api/src/iter.rs | 15 ++-- extendr-api/src/lib.rs | 22 ++++-- extendr-api/src/ownership.rs | 39 +++++++++- extendr-api/src/robj/into_robj.rs | 54 +++++++------- extendr-api/src/robj/mod.rs | 20 +++-- extendr-api/src/robj/rinternals.rs | 90 +++++++++++------------ extendr-api/src/thread_safety.rs | 2 +- extendr-api/src/wrapper/altrep.rs | 52 ++++++++----- extendr-api/src/wrapper/complexes.rs | 1 + extendr-api/src/wrapper/doubles.rs | 1 + extendr-api/src/wrapper/environment.rs | 2 +- extendr-api/src/wrapper/expr.rs | 2 +- extendr-api/src/wrapper/function.rs | 2 +- extendr-api/src/wrapper/integers.rs | 1 + extendr-api/src/wrapper/list.rs | 6 +- extendr-api/src/wrapper/logicals.rs | 1 + extendr-api/src/wrapper/mod.rs | 2 +- extendr-api/src/wrapper/pairlist.rs | 4 +- extendr-api/src/wrapper/promise.rs | 2 +- extendr-api/src/wrapper/raw.rs | 2 +- extendr-api/src/wrapper/strings.rs | 6 +- extendr-api/src/wrapper/symbol.rs | 4 +- extendr-api/tests/io_tests.rs | 6 +- 31 files changed, 259 insertions(+), 201 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 8873347ce4..774f4aa6e5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,13 +17,13 @@ authors = [ edition = "2021" license = "MIT" repository = "https://github.com/extendr/extendr" -rust-version = "1.60" +rust-version = "1.70" [workspace.dependencies] # When updating extendr's version, this version also needs to be updated extendr-macros = { path = "./extendr-macros", version = "0.6.0" } -libR-sys = "0.6.0" +libR-sys = "0.7.0" [patch.crates-io] # When uncommenting this, do not forget to uncomment the same line in diff --git a/extendr-api/Cargo.toml b/extendr-api/Cargo.toml index 382a267d0a..f1df1e0656 100644 --- a/extendr-api/Cargo.toml +++ b/extendr-api/Cargo.toml @@ -8,7 +8,7 @@ license.workspace = true repository.workspace = true # Note: it seems cargo-msrv doesn't support rust-version.workspace = true. -rust-version = "1.64" +rust-version = "1.70" [dependencies] libR-sys = { workspace = true } diff --git a/extendr-api/build.rs b/extendr-api/build.rs index ff884961fa..cace92cf49 100644 --- a/extendr-api/build.rs +++ b/extendr-api/build.rs @@ -30,4 +30,8 @@ fn main() { if &*major >= "4" && &*minor >= "3" { println!("cargo:rustc-cfg=use_r_altlist"); } + + if &*major >= "4" && &*minor >= "4" { + println!("cargo:rustc-cfg=use_objsxp"); + } } diff --git a/extendr-api/src/functions.rs b/extendr-api/src/functions.rs index 2228d3db32..bd306c2759 100644 --- a/extendr-api/src/functions.rs +++ b/extendr-api/src/functions.rs @@ -209,12 +209,12 @@ 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 status_ptr = &mut status as *mut _; let codeobj: Robj = code.into(); let parsed = Robj::from_sexp(R_ParseVector(codeobj.get(), -1, status_ptr, R_NilValue)); match status { - 1 => parsed.try_into(), + ParseStatus::PARSE_OK => parsed.try_into(), _ => Err(Error::ParseError(code.into())), } }) diff --git a/extendr-api/src/graphics/device_driver.rs b/extendr-api/src/graphics/device_driver.rs index 4dc2377fae..62aa2a5d37 100644 --- a/extendr-api/src/graphics/device_driver.rs +++ b/extendr-api/src/graphics/device_driver.rs @@ -488,7 +488,7 @@ pub trait DeviceDriver: std::marker::Sized { // It seems `NA` is just treated as `true`. Probably it doesn't matter much here. // c.f. https://github.com/wch/r-source/blob/6b22b60126646714e0f25143ac679240be251dbe/src/library/grDevices/src/devPS.c#L4235 - let winding = winding != 0; + let winding = winding != Rboolean::FALSE; data.path(coords, winding, *gc, *dd); } @@ -519,7 +519,7 @@ pub trait DeviceDriver: std::marker::Sized { rot, // It seems `NA` is just treated as `true`. Probably it doesn't matter much here. // c.f. https://github.com/wch/r-source/blob/6b22b60126646714e0f25143ac679240be251dbe/src/library/grDevices/src/devPS.c#L4062 - interpolate != 0, + interpolate != Rboolean::FALSE, *gc, *dd, ); @@ -589,11 +589,7 @@ pub trait DeviceDriver: std::marker::Sized { dd: pDevDesc, ) -> Rboolean { let data = ((*dd).deviceSpecific as *mut T).as_mut().unwrap(); - if let Ok(confirm) = data.new_frame_confirm(*dd).try_into() { - confirm - } else { - false.into() - } + data.new_frame_confirm(*dd).into() } unsafe extern "C" fn device_driver_holdflush( @@ -610,11 +606,7 @@ pub trait DeviceDriver: std::marker::Sized { dd: pDevDesc, ) -> Rboolean { let data = ((*dd).deviceSpecific as *mut T).as_mut().unwrap(); - if let Ok(success) = data.locator(x, y, *dd).try_into() { - success - } else { - false.into() - } + data.locator(x, y, *dd).into() } unsafe extern "C" fn device_driver_eventHelper(dd: pDevDesc, code: c_int) { @@ -753,12 +745,12 @@ pub trait DeviceDriver: std::marker::Sized { (*p_dev_desc).gamma = 1.0; (*p_dev_desc).canClip = match ::CLIPPING_STRATEGY { - ClippingStrategy::Engine => 0, - _ => 1, + ClippingStrategy::Engine => Rboolean::FALSE, + _ => Rboolean::TRUE, }; // As described above, gamma is not supported. - (*p_dev_desc).canChangeGamma = 0; + (*p_dev_desc).canChangeGamma = Rboolean::FALSE; (*p_dev_desc).canHAdj = CanHAdjOption::VariableAdjustment as _; @@ -773,14 +765,14 @@ pub trait DeviceDriver: std::marker::Sized { // A raw pointer to the data specific to the device. (*p_dev_desc).deviceSpecific = deviceSpecific; - (*p_dev_desc).displayListOn = if ::USE_PLOT_HISTORY { 1 } else { 0 }; + (*p_dev_desc).displayListOn = ::USE_PLOT_HISTORY.into(); // These are currently not used, so just set FALSE. - (*p_dev_desc).canGenMouseDown = 0; - (*p_dev_desc).canGenMouseMove = 0; - (*p_dev_desc).canGenMouseUp = 0; - (*p_dev_desc).canGenKeybd = 0; - (*p_dev_desc).canGenIdle = 0; + (*p_dev_desc).canGenMouseDown = Rboolean::FALSE; + (*p_dev_desc).canGenMouseMove = Rboolean::FALSE; + (*p_dev_desc).canGenMouseUp = Rboolean::FALSE; + (*p_dev_desc).canGenKeybd = Rboolean::FALSE; + (*p_dev_desc).canGenIdle = Rboolean::FALSE; // The header file says: // @@ -788,7 +780,7 @@ pub trait DeviceDriver: std::marker::Sized { // // It seems no implementation sets this, so this is probably what is // modified on the engine's side. - (*p_dev_desc).gettingEvent = 0; + (*p_dev_desc).gettingEvent = Rboolean::FALSE; (*p_dev_desc).activate = Some(device_driver_activate::); (*p_dev_desc).circle = Some(device_driver_circle::); @@ -829,7 +821,7 @@ pub trait DeviceDriver: std::marker::Sized { (*p_dev_desc).newFrameConfirm = Some(device_driver_new_frame_confirm::); // UTF-8 support - (*p_dev_desc).hasTextUTF8 = if ::ACCEPT_UTF8_TEXT { 1 } else { 0 }; + (*p_dev_desc).hasTextUTF8 = ::ACCEPT_UTF8_TEXT.into(); (*p_dev_desc).textUTF8 = if ::ACCEPT_UTF8_TEXT { Some(device_driver_text::) } else { @@ -840,7 +832,7 @@ pub trait DeviceDriver: std::marker::Sized { } else { None }; - (*p_dev_desc).wantSymbolUTF8 = if ::ACCEPT_UTF8_TEXT { 1 } else { 0 }; + (*p_dev_desc).wantSymbolUTF8 = ::ACCEPT_UTF8_TEXT.into(); // R internals says: // @@ -850,7 +842,7 @@ pub trait DeviceDriver: std::marker::Sized { // // It seems this is used only by plot3d, so FALSE should be appropriate in // most of the cases. - (*p_dev_desc).useRotatedTextInContour = 0; + (*p_dev_desc).useRotatedTextInContour = Rboolean::FALSE; (*p_dev_desc).eventEnv = empty_env().get(); (*p_dev_desc).eventHelper = Some(device_driver_eventHelper::); @@ -900,8 +892,8 @@ pub trait DeviceDriver: std::marker::Sized { (*p_dev_desc).deviceVersion = R_GE_definitions as _; (*p_dev_desc).deviceClip = match ::CLIPPING_STRATEGY { - ClippingStrategy::Device => 1, - _ => 0, + ClippingStrategy::Device => Rboolean::TRUE, + _ => Rboolean::FALSE, }; } diff --git a/extendr-api/src/graphics/mod.rs b/extendr-api/src/graphics/mod.rs index 154255da6b..965ff1ba46 100644 --- a/extendr-api/src/graphics/mod.rs +++ b/extendr-api/src/graphics/mod.rs @@ -179,22 +179,22 @@ pub enum FontFace { Symbol, } -impl LineEnd { - fn to_u32(&self) -> u32 { - match self { - Self::Round => 1, - Self::Butt => 2, - Self::Square => 3, +impl From for R_GE_lineend { + fn from(value: LineEnd) -> Self { + match value { + LineEnd::Round => Self::GE_ROUND_CAP, + LineEnd::Butt => Self::GE_BUTT_CAP, + LineEnd::Square => Self::GE_SQUARE_CAP, } } } -impl LineJoin { - fn to_u32(&self) -> u32 { - match self { - Self::Round => 1, - Self::Mitre => 2, - Self::Bevel => 3, +impl From for R_GE_linejoin { + fn from(value: LineJoin) -> Self { + match value { + LineJoin::Round => Self::GE_ROUND_JOIN, + LineJoin::Mitre => Self::GE_MITRE_JOIN, + LineJoin::Bevel => Self::GE_BEVEL_JOIN, } } } @@ -227,10 +227,10 @@ impl FontFace { fn unit_to_ge(unit: Unit) -> GEUnit { match unit { - Unit::Device => GEUnit_GE_DEVICE, - Unit::Normalized => GEUnit_GE_NDC, - Unit::Inches => GEUnit_GE_INCHES, - Unit::CM => GEUnit_GE_CM, + Unit::Device => GEUnit::GE_DEVICE, + Unit::Normalized => GEUnit::GE_NDC, + Unit::Inches => GEUnit::GE_INCHES, + Unit::CM => GEUnit::GE_CM, } } @@ -255,8 +255,8 @@ impl Context { gamma: 1.0, lwd: 1.0, lty: 0, - lend: R_GE_lineend_GE_ROUND_CAP, - ljoin: R_GE_linejoin_GE_ROUND_JOIN, + lend: R_GE_lineend::GE_ROUND_CAP, + ljoin: R_GE_linejoin::GE_ROUND_JOIN, lmitre: 10.0, cex: 1.0, ps: 14.0, @@ -330,7 +330,7 @@ impl Context { /// LineEnd::SquareCap /// ``` pub fn line_end(&mut self, lend: LineEnd) -> &mut Self { - self.context.lend = lend.to_u32(); + self.context.lend = lend.into(); self } @@ -341,7 +341,7 @@ impl Context { /// LineJoin::BevelJoin /// ``` pub fn line_join(&mut self, ljoin: LineJoin) -> &mut Self { - self.context.ljoin = ljoin.to_u32(); + self.context.ljoin = ljoin.into(); self } @@ -693,7 +693,7 @@ impl Device { yptr, nper.len() as std::os::raw::c_int, nperptr, - if winding { 1 } else { 0 }, + winding.into(), gc.context(), self.inner(), ) @@ -724,7 +724,7 @@ impl Device { let raster = pixels.as_ptr() as *mut u32; let w = w as i32; let h = h as i32; - let interpolate = if interpolate { 1 } else { 0 }; + let interpolate = interpolate.into(); GERaster( raster, w, @@ -755,7 +755,7 @@ impl Device { let (x, y) = gc.t(pos); let (xc, yc) = gc.trel(center); let text = std::ffi::CString::new(text.as_ref()).unwrap(); - let enc = cetype_t_CE_UTF8; + let enc = cetype_t::CE_UTF8; GEText( x, y, @@ -802,21 +802,21 @@ impl Device { /// Get the width of a unicode string. pub fn text_width>(&self, text: T, gc: &Context) -> f64 { let text = std::ffi::CString::new(text.as_ref()).unwrap(); - let enc = cetype_t_CE_UTF8; + let enc = cetype_t::CE_UTF8; unsafe { gc.its(GEStrWidth(text.as_ptr(), enc, gc.context(), self.inner())) } } /// Get the height of a unicode string. pub fn text_height>(&self, text: T, gc: &Context) -> f64 { let text = std::ffi::CString::new(text.as_ref()).unwrap(); - let enc = cetype_t_CE_UTF8; + let enc = cetype_t::CE_UTF8; unsafe { gc.its(GEStrHeight(text.as_ptr(), enc, gc.context(), self.inner())) } } /// Get the metrics for a unicode string. pub fn text_metric>(&self, text: T, gc: &Context) -> TextMetric { let text = std::ffi::CString::new(text.as_ref()).unwrap(); - let enc = cetype_t_CE_UTF8; + let enc = cetype_t::CE_UTF8; unsafe { let mut res = TextMetric { ascent: 0.0, diff --git a/extendr-api/src/io/load.rs b/extendr-api/src/io/load.rs index e655febe32..260c22acb5 100644 --- a/extendr-api/src/io/load.rs +++ b/extendr-api/src/io/load.rs @@ -58,7 +58,7 @@ pub trait Load { // pub type R_inpstream_t = *mut R_inpstream_st; let mut state = R_inpstream_st { data, - type_: format as R_pstream_format_t, + type_: format, InChar: Some(inchar::), InBytes: Some(inbytes::), InPersistHookFunc: hook_func, diff --git a/extendr-api/src/io/mod.rs b/extendr-api/src/io/mod.rs index 4aeb24cc24..b3a348905a 100644 --- a/extendr-api/src/io/mod.rs +++ b/extendr-api/src/io/mod.rs @@ -1,10 +1,4 @@ -pub enum PstreamFormat { - AnyFormat = 0, - AsciiFormat = 1, - BinaryFormat = 2, - XdrFormat = 3, - AsciihexFormat = 4, -} +pub type PstreamFormat = libR_sys::R_pstream_format_t; mod load; mod save; diff --git a/extendr-api/src/iter.rs b/extendr-api/src/iter.rs index 1cbbb90ae8..36596bacbe 100644 --- a/extendr-api/src/iter.rs +++ b/extendr-api/src/iter.rs @@ -64,7 +64,7 @@ fn str_from_strsxp<'a>(sexp: SEXP, index: isize) -> &'a str { let charsxp = STRING_ELT(sexp, index); if charsxp == R_NaString { <&str>::na() - } else if TYPEOF(charsxp) == CHARSXP as i32 { + } else if TYPEOF(charsxp) == SEXPTYPE::CHARSXP { let ptr = R_CHAR(charsxp) as *const u8; let slice = std::slice::from_raw_parts(ptr, Rf_xlength(charsxp) as usize); std::str::from_utf8_unchecked(slice) @@ -89,12 +89,13 @@ impl Iterator for StrIter { let vector = self.vector.get(); if i >= self.len { None - } else if TYPEOF(vector) as u32 == STRSXP { + } else if TYPEOF(vector) == SEXPTYPE::STRSXP { Some(str_from_strsxp(vector, i as isize)) - } else if TYPEOF(vector) as u32 == INTSXP && TYPEOF(self.levels) as u32 == STRSXP { + } else if TYPEOF(vector) == SEXPTYPE::INTSXP && TYPEOF(self.levels) == SEXPTYPE::STRSXP + { let j = *(INTEGER(vector).add(i)); Some(str_from_strsxp(self.levels, j as isize - 1)) - } else if TYPEOF(vector) as u32 == NILSXP { + } else if TYPEOF(vector) == SEXPTYPE::NILSXP { Some(<&str>::na()) } else { None @@ -167,7 +168,7 @@ pub trait AsStrIter: GetSexp + Types + Length + Attributes + Rinternals { let i = 0; let len = self.len(); match self.sexptype() { - STRSXP => unsafe { + SEXPTYPE::STRSXP => unsafe { Some(StrIter { vector: self.as_robj().clone(), i, @@ -175,9 +176,9 @@ pub trait AsStrIter: GetSexp + Types + Length + Attributes + Rinternals { levels: R_NilValue, }) }, - INTSXP => unsafe { + SEXPTYPE::INTSXP => unsafe { if let Some(levels) = self.get_attrib(levels_symbol()) { - if self.is_factor() && levels.sexptype() == STRSXP { + if self.is_factor() && levels.sexptype() == SEXPTYPE::STRSXP { Some(StrIter { vector: self.as_robj().clone(), i, diff --git a/extendr-api/src/lib.rs b/extendr-api/src/lib.rs index ce77dcff8a..9458ba38e5 100644 --- a/extendr-api/src/lib.rs +++ b/extendr-api/src/lib.rs @@ -488,8 +488,8 @@ pub unsafe fn register_call_methods(info: *mut libR_sys::DllInfo, metadata: Meta ); // This seems to allow both symbols and strings, - libR_sys::R_useDynamicSymbols(info, 0); - libR_sys::R_forceSymbols(info, 0); + libR_sys::R_useDynamicSymbols(info, Rboolean::FALSE); + libR_sys::R_forceSymbols(info, Rboolean::FALSE); } /// Type of R objects used by [Robj::rtype]. @@ -556,9 +556,10 @@ pub enum Rany<'a> { /// Convert extendr's Rtype to R's SEXPTYPE. /// Panics if the type is Unknown. -pub fn rtype_to_sxp(rtype: Rtype) -> i32 { +pub fn rtype_to_sxp(rtype: Rtype) -> SEXPTYPE { use Rtype::*; - (match rtype { + use SEXPTYPE::*; + match rtype { Null => NILSXP, Symbol => SYMSXP, Pairlist => LISTSXP, @@ -582,15 +583,19 @@ pub fn rtype_to_sxp(rtype: Rtype) -> i32 { ExternalPtr => EXTPTRSXP, WeakRef => WEAKREFSXP, Raw => RAWSXP, + #[cfg(not(use_objsxp))] S4 => S4SXP, + #[cfg(use_objsxp)] + S4 => OBJSXP, Unknown => panic!("attempt to use Unknown Rtype"), - }) as i32 + } } /// Convert R's SEXPTYPE to extendr's Rtype. -pub fn sxp_to_rtype(sxptype: i32) -> Rtype { +pub fn sxp_to_rtype(sxptype: SEXPTYPE) -> Rtype { use Rtype::*; - match sxptype as u32 { + use SEXPTYPE::*; + match sxptype { NILSXP => Null, SYMSXP => Symbol, LISTSXP => Pairlist, @@ -614,7 +619,10 @@ pub fn sxp_to_rtype(sxptype: i32) -> Rtype { EXTPTRSXP => ExternalPtr, WEAKREFSXP => WeakRef, RAWSXP => Raw, + #[cfg(not(use_objsxp))] S4SXP => S4, + #[cfg(use_objsxp)] + OBJSXP => S4, _ => Unknown, } } diff --git a/extendr-api/src/ownership.rs b/extendr-api/src/ownership.rs index 9115c161de..9d11b548c2 100644 --- a/extendr-api/src/ownership.rs +++ b/extendr-api/src/ownership.rs @@ -14,7 +14,7 @@ use std::sync::Mutex; use libR_sys::{ R_NilValue, R_PreserveObject, R_ReleaseObject, R_xlen_t, Rf_allocVector, Rf_protect, - Rf_unprotect, LENGTH, SET_VECTOR_ELT, SEXP, VECSXP, VECTOR_ELT, + Rf_unprotect, LENGTH, SET_VECTOR_ELT, SEXP, SEXPTYPE, VECTOR_ELT, }; static OWNERSHIP: Lazy> = Lazy::new(|| Mutex::new(Ownership::new())); @@ -55,7 +55,8 @@ struct Ownership { impl Ownership { fn new() -> Self { unsafe { - let preservation = Rf_allocVector(VECSXP, INITIAL_PRESERVATION_SIZE as R_xlen_t); + let preservation = + Rf_allocVector(SEXPTYPE::VECSXP, INITIAL_PRESERVATION_SIZE as R_xlen_t); R_PreserveObject(preservation); Ownership { preservation: preservation as usize, @@ -66,6 +67,38 @@ impl Ownership { } } + // Garbage collect the tracking structures. + unsafe fn garbage_collect(&mut self) { + let new_size = self.cur_index * 2 + EXTRA_PRESERVATION_SIZE; + let new_sexp = Rf_allocVector(SEXPTYPE::VECSXP, new_size as R_xlen_t); + R_PreserveObject(new_sexp); + let old_sexp = self.preservation.inner(); + + let mut new_objects = HashMap::with_capacity(new_size); + + // copy non-null elements to new vector and hashmap. + let mut j = 0; + for (addr, object) in self.objects.iter() { + if object.refcount != 0 { + SET_VECTOR_ELT(new_sexp, j as R_xlen_t, addr.inner()); + new_objects.insert( + addr.clone(), + Object { + refcount: object.refcount, + index: j, + }, + ); + j += 1; + } + } + + R_ReleaseObject(old_sexp); + self.preservation = (new_sexp).into(); + self.cur_index = j; + self.max_index = new_size; + self.objects = new_objects; + } + unsafe fn protect(&mut self, sexp: SEXP) { Rf_protect(sexp); @@ -306,7 +339,7 @@ mod test { let test_size = INITIAL_PRESERVATION_SIZE + EXTRA_PRESERVATION_SIZE * 5; // Make some test objects. - let sexp_pres = Rf_allocVector(VECSXP, test_size as R_xlen_t); + let sexp_pres = Rf_allocVector(SEXPTYPE::VECSXP, test_size as R_xlen_t); Rf_protect(sexp_pres); let sexps = (0..test_size).map(|i| { diff --git a/extendr-api/src/robj/into_robj.rs b/extendr-api/src/robj/into_robj.rs index 60d8d1f305..2ba6eabf39 100644 --- a/extendr-api/src/robj/into_robj.rs +++ b/extendr-api/src/robj/into_robj.rs @@ -13,7 +13,7 @@ pub(crate) fn str_to_character(s: &str) -> SEXP { Rf_mkCharLenCE( s.as_ptr() as *const raw::c_char, s.len() as i32, - cetype_t_CE_UTF8, + cetype_t::CE_UTF8, ) }) } @@ -159,7 +159,7 @@ where /// to `collect_robj()`. pub trait ToVectorValue { fn sexptype() -> SEXPTYPE { - 0 + SEXPTYPE::NILSXP } fn to_real(&self) -> f64 @@ -209,7 +209,7 @@ macro_rules! impl_real_tvv { ($t: ty) => { impl ToVectorValue for $t { fn sexptype() -> SEXPTYPE { - REALSXP + SEXPTYPE::REALSXP } fn to_real(&self) -> f64 { @@ -219,7 +219,7 @@ macro_rules! impl_real_tvv { impl ToVectorValue for &$t { fn sexptype() -> SEXPTYPE { - REALSXP + SEXPTYPE::REALSXP } fn to_real(&self) -> f64 { @@ -229,7 +229,7 @@ macro_rules! impl_real_tvv { impl ToVectorValue for Option<$t> { fn sexptype() -> SEXPTYPE { - REALSXP + SEXPTYPE::REALSXP } fn to_real(&self) -> f64 { @@ -257,7 +257,7 @@ macro_rules! impl_complex_tvv { ($t: ty) => { impl ToVectorValue for $t { fn sexptype() -> SEXPTYPE { - CPLXSXP + SEXPTYPE::CPLXSXP } fn to_complex(&self) -> Rcomplex { @@ -267,7 +267,7 @@ macro_rules! impl_complex_tvv { impl ToVectorValue for &$t { fn sexptype() -> SEXPTYPE { - CPLXSXP + SEXPTYPE::CPLXSXP } fn to_complex(&self) -> Rcomplex { @@ -285,7 +285,7 @@ macro_rules! impl_integer_tvv { ($t: ty) => { impl ToVectorValue for $t { fn sexptype() -> SEXPTYPE { - INTSXP + SEXPTYPE::INTSXP } fn to_integer(&self) -> i32 { @@ -295,7 +295,7 @@ macro_rules! impl_integer_tvv { impl ToVectorValue for &$t { fn sexptype() -> SEXPTYPE { - INTSXP + SEXPTYPE::INTSXP } fn to_integer(&self) -> i32 { @@ -305,7 +305,7 @@ macro_rules! impl_integer_tvv { impl ToVectorValue for Option<$t> { fn sexptype() -> SEXPTYPE { - INTSXP + SEXPTYPE::INTSXP } fn to_integer(&self) -> i32 { @@ -326,7 +326,7 @@ impl_integer_tvv!(u16); impl ToVectorValue for u8 { fn sexptype() -> SEXPTYPE { - RAWSXP + SEXPTYPE::RAWSXP } fn to_raw(&self) -> u8 { @@ -336,7 +336,7 @@ impl ToVectorValue for u8 { impl ToVectorValue for &u8 { fn sexptype() -> SEXPTYPE { - RAWSXP + SEXPTYPE::RAWSXP } fn to_raw(&self) -> u8 { @@ -348,7 +348,7 @@ macro_rules! impl_str_tvv { ($t: ty) => { impl ToVectorValue for $t { fn sexptype() -> SEXPTYPE { - STRSXP + SEXPTYPE::STRSXP } fn to_sexp(&self) -> SEXP @@ -361,7 +361,7 @@ macro_rules! impl_str_tvv { impl ToVectorValue for &$t { fn sexptype() -> SEXPTYPE { - STRSXP + SEXPTYPE::STRSXP } fn to_sexp(&self) -> SEXP @@ -374,7 +374,7 @@ macro_rules! impl_str_tvv { impl ToVectorValue for Option<$t> { fn sexptype() -> SEXPTYPE { - STRSXP + SEXPTYPE::STRSXP } fn to_sexp(&self) -> SEXP @@ -396,7 +396,7 @@ impl_str_tvv! {String} impl ToVectorValue for bool { fn sexptype() -> SEXPTYPE { - LGLSXP + SEXPTYPE::LGLSXP } fn to_logical(&self) -> i32 @@ -409,7 +409,7 @@ impl ToVectorValue for bool { impl ToVectorValue for &bool { fn sexptype() -> SEXPTYPE { - LGLSXP + SEXPTYPE::LGLSXP } fn to_logical(&self) -> i32 @@ -422,7 +422,7 @@ impl ToVectorValue for &bool { impl ToVectorValue for Rbool { fn sexptype() -> SEXPTYPE { - LGLSXP + SEXPTYPE::LGLSXP } fn to_logical(&self) -> i32 @@ -435,7 +435,7 @@ impl ToVectorValue for Rbool { impl ToVectorValue for &Rbool { fn sexptype() -> SEXPTYPE { - LGLSXP + SEXPTYPE::LGLSXP } fn to_logical(&self) -> i32 @@ -448,7 +448,7 @@ impl ToVectorValue for &Rbool { impl ToVectorValue for Option { fn sexptype() -> SEXPTYPE { - LGLSXP + SEXPTYPE::LGLSXP } fn to_logical(&self) -> i32 { @@ -470,40 +470,40 @@ where single_threaded(|| unsafe { // Length of the vector is known in advance. let sexptype = I::Item::sexptype(); - if sexptype != 0 { + if sexptype != SEXPTYPE::NILSXP { let res = Robj::alloc_vector(sexptype, len); let sexp = res.get(); match sexptype { - REALSXP => { + SEXPTYPE::REALSXP => { let ptr = REAL(sexp); for (i, v) in iter.enumerate() { *ptr.add(i) = v.to_real(); } } - CPLXSXP => { + SEXPTYPE::CPLXSXP => { let ptr = COMPLEX(sexp); for (i, v) in iter.enumerate() { *ptr.add(i) = v.to_complex(); } } - INTSXP => { + SEXPTYPE::INTSXP => { let ptr = INTEGER(sexp); for (i, v) in iter.enumerate() { *ptr.add(i) = v.to_integer(); } } - LGLSXP => { + SEXPTYPE::LGLSXP => { let ptr = LOGICAL(sexp); for (i, v) in iter.enumerate() { *ptr.add(i) = v.to_logical(); } } - STRSXP => { + SEXPTYPE::STRSXP => { for (i, v) in iter.enumerate() { SET_STRING_ELT(sexp, i as isize, v.to_sexp()); } } - RAWSXP => { + SEXPTYPE::RAWSXP => { let ptr = RAW(sexp); for (i, v) in iter.enumerate() { *ptr.add(i) = v.to_raw(); diff --git a/extendr-api/src/robj/mod.rs b/extendr-api/src/robj/mod.rs index d70369deb1..fc66712223 100644 --- a/extendr-api/src/robj/mod.rs +++ b/extendr-api/src/robj/mod.rs @@ -236,8 +236,8 @@ impl Robj { pub trait Types: GetSexp { #[doc(hidden)] /// Get the XXXSXP type of the object. - fn sexptype(&self) -> u32 { - unsafe { TYPEOF(self.get()) as u32 } + fn sexptype(&self) -> SEXPTYPE { + unsafe { TYPEOF(self.get()) } } /// Get the type of an R object. @@ -263,6 +263,7 @@ pub trait Types: GetSexp { /// } /// ``` fn rtype(&self) -> Rtype { + use SEXPTYPE::*; match self.sexptype() { NILSXP => Rtype::Null, SYMSXP => Rtype::Symbol, @@ -287,12 +288,16 @@ pub trait Types: GetSexp { EXTPTRSXP => Rtype::ExternalPtr, WEAKREFSXP => Rtype::WeakRef, RAWSXP => Rtype::Raw, + #[cfg(not(use_objsxp))] S4SXP => Rtype::S4, + #[cfg(use_objsxp)] + OBJSXP => Rtype::S4, _ => Rtype::Unknown, } } fn as_any(&self) -> Rany { + use SEXPTYPE::*; unsafe { match self.sexptype() { NILSXP => Rany::Null(std::mem::transmute(self.as_robj())), @@ -318,7 +323,10 @@ pub trait Types: GetSexp { EXTPTRSXP => Rany::ExternalPtr(std::mem::transmute(self.as_robj())), WEAKREFSXP => Rany::WeakRef(std::mem::transmute(self.as_robj())), RAWSXP => Rany::Raw(std::mem::transmute(self.as_robj())), + #[cfg(not(use_objsxp))] S4SXP => Rany::S4(std::mem::transmute(self.as_robj())), + #[cfg(use_objsxp)] + OBJSXP => Rany::S4(std::mem::transmute(self.as_robj())), _ => Rany::Unknown(std::mem::transmute(self.as_robj())), } } @@ -346,6 +354,7 @@ impl Robj { } else { unsafe { let sexp = self.get(); + use SEXPTYPE::*; match self.sexptype() { STRSXP => STRING_ELT(sexp, 0) == libR_sys::R_NaString, INTSXP => *(INTEGER(sexp)) == libR_sys::R_NaInt, @@ -787,6 +796,7 @@ macro_rules! make_typed_slice { } } +use SEXPTYPE::*; make_typed_slice!(Rbool, INTEGER, LGLSXP); make_typed_slice!(i32, INTEGER, INTSXP); make_typed_slice!(u32, INTEGER, INTSXP); @@ -818,7 +828,7 @@ pub trait Attributes: Types + Length { Robj: From + 'a, { let name = Robj::from(name); - if self.sexptype() == CHARSXP { + if self.sexptype() == SEXPTYPE::CHARSXP { None } else { // FIXME: this attribute does not need protection @@ -838,7 +848,7 @@ pub trait Attributes: Types + Length { Robj: From + 'a, { let name = Robj::from(name); - if self.sexptype() == CHARSXP { + if self.sexptype() == SEXPTYPE::CHARSXP { false } else { unsafe { Rf_getAttrib(self.get(), name.get()) != R_NilValue } @@ -1068,7 +1078,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 288196ab56..3ab90cdfae 100644 --- a/extendr-api/src/robj/rinternals.rs +++ b/extendr-api/src/robj/rinternals.rs @@ -7,57 +7,57 @@ 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()) != 0 } + unsafe { Rf_isNull(self.get()).into() } } /// Return true if this is a symbol. fn is_symbol(&self) -> bool { - unsafe { Rf_isSymbol(self.get()) != 0 } + unsafe { Rf_isSymbol(self.get()).into() } } /// Return true if this is a boolean (logical) vector fn is_logical(&self) -> bool { - unsafe { Rf_isLogical(self.get()) != 0 } + unsafe { Rf_isLogical(self.get()).into() } } /// Return true if this is a real (f64) vector. fn is_real(&self) -> bool { - unsafe { Rf_isReal(self.get()) != 0 } + unsafe { Rf_isReal(self.get()).into() } } /// Return true if this is a complex vector. fn is_complex(&self) -> bool { - unsafe { Rf_isComplex(self.get()) != 0 } + unsafe { Rf_isComplex(self.get()).into() } } /// Return true if this is an expression. fn is_expressions(&self) -> bool { - unsafe { Rf_isExpression(self.get()) != 0 } + unsafe { Rf_isExpression(self.get()).into() } } /// Return true if this is an environment. fn is_environment(&self) -> bool { - unsafe { Rf_isEnvironment(self.get()) != 0 } + unsafe { Rf_isEnvironment(self.get()).into() } } /// Return true if this is an environment. fn is_promise(&self) -> bool { - self.sexptype() == PROMSXP + self.sexptype() == SEXPTYPE::PROMSXP } /// Return true if this is a string. fn is_string(&self) -> bool { - unsafe { Rf_isString(self.get()) != 0 } + unsafe { Rf_isString(self.get()).into() } } /// Return true if this is an object (ie. has a class attribute). fn is_object(&self) -> bool { - unsafe { Rf_isObject(self.get()) != 0 } + unsafe { Rf_isObject(self.get()).into() } } /// Return true if this is a S4 object. fn is_s4(&self) -> bool { - unsafe { Rf_isS4(self.get()) != 0 } + unsafe { Rf_isS4(self.get()).into() } } /// Return true if this is an expression. @@ -81,10 +81,8 @@ pub trait Rinternals: Types + Conversions { } /// Convert to vectors of many kinds. - fn coerce_vector(&self, sexptype: u32) -> Robj { - single_threaded(|| unsafe { - Robj::from_sexp(Rf_coerceVector(self.get(), sexptype as SEXPTYPE)) - }) + fn coerce_vector(&self, sexptype: SEXPTYPE) -> Robj { + single_threaded(|| unsafe { Robj::from_sexp(Rf_coerceVector(self.get(), sexptype)) }) } /// Convert a pairlist (LISTSXP) to a vector list (VECSXP). @@ -274,7 +272,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. @@ -292,108 +290,108 @@ pub trait Rinternals: Types + Conversions { } /// Allocated an owned object of a certain type. - fn alloc_vector(sexptype: u32, len: usize) -> Robj { + fn alloc_vector(sexptype: SEXPTYPE, len: usize) -> Robj { single_threaded(|| unsafe { Robj::from_sexp(Rf_allocVector(sexptype, len as R_xlen_t)) }) } /// Return true if two arrays have identical dims. fn conformable(a: &Robj, b: &Robj) -> bool { - single_threaded(|| unsafe { Rf_conformable(a.get(), b.get()) != 0 }) + single_threaded(|| unsafe { Rf_conformable(a.get(), b.get()).into() }) } /// Return true if this is an array. fn is_array(&self) -> bool { - unsafe { Rf_isArray(self.get()) != 0 } + unsafe { Rf_isArray(self.get()).into() } } /// Return true if this is factor. fn is_factor(&self) -> bool { - unsafe { Rf_isFactor(self.get()) != 0 } + unsafe { Rf_isFactor(self.get()).into() } } /// Return true if this is a data frame. fn is_frame(&self) -> bool { - unsafe { Rf_isFrame(self.get()) != 0 } + unsafe { Rf_isFrame(self.get()).into() } } /// Return true if this is a function or a primitive (CLOSXP, BUILTINSXP or SPECIALSXP) fn is_function(&self) -> bool { - unsafe { Rf_isFunction(self.get()) != 0 } + unsafe { Rf_isFunction(self.get()).into() } } /// Return true if this is an integer vector (INTSXP) but not a factor. fn is_integer(&self) -> bool { - unsafe { Rf_isInteger(self.get()) != 0 } + unsafe { Rf_isInteger(self.get()).into() } } /// Return true if this is a language object (LANGSXP). fn is_language(&self) -> bool { - unsafe { Rf_isLanguage(self.get()) != 0 } + unsafe { Rf_isLanguage(self.get()).into() } } /// Return true if this is NILSXP or LISTSXP. fn is_pairlist(&self) -> bool { - unsafe { Rf_isList(self.get()) != 0 } + unsafe { Rf_isList(self.get()).into() } } /// Return true if this is a matrix. fn is_matrix(&self) -> bool { - unsafe { Rf_isMatrix(self.get()) != 0 } + unsafe { Rf_isMatrix(self.get()).into() } } /// Return true if this is NILSXP or VECSXP. fn is_list(&self) -> bool { - unsafe { Rf_isNewList(self.get()) != 0 } + unsafe { Rf_isNewList(self.get()).into() } } /// Return true if this is INTSXP, LGLSXP or REALSXP but not a factor. fn is_number(&self) -> bool { - unsafe { Rf_isNumber(self.get()) != 0 } + unsafe { Rf_isNumber(self.get()).into() } } /// Return true if this is a primitive function BUILTINSXP, SPECIALSXP. fn is_primitive(&self) -> bool { - unsafe { Rf_isPrimitive(self.get()) != 0 } + unsafe { Rf_isPrimitive(self.get()).into() } } /// Return true if this is a time series vector (see tsp). fn is_ts(&self) -> bool { - unsafe { Rf_isTs(self.get()) != 0 } + unsafe { Rf_isTs(self.get()).into() } } /// Return true if this is a user defined binop. fn is_user_binop(&self) -> bool { - unsafe { Rf_isUserBinop(self.get()) != 0 } + unsafe { Rf_isUserBinop(self.get()).into() } } /// Return true if this is a valid string. fn is_valid_string(&self) -> bool { - unsafe { Rf_isValidString(self.get()) != 0 } + unsafe { Rf_isValidString(self.get()).into() } } /// Return true if this is a valid string. fn is_valid_string_f(&self) -> bool { - unsafe { Rf_isValidStringF(self.get()) != 0 } + unsafe { Rf_isValidStringF(self.get()).into() } } /// Return true if this is a vector. fn is_vector(&self) -> bool { - unsafe { Rf_isVector(self.get()) != 0 } + unsafe { Rf_isVector(self.get()).into() } } /// Return true if this is an atomic vector. fn is_vector_atomic(&self) -> bool { - unsafe { Rf_isVectorAtomic(self.get()) != 0 } + unsafe { Rf_isVectorAtomic(self.get()).into() } } /// Return true if this is a vector list. fn is_vector_list(&self) -> bool { - unsafe { Rf_isVectorList(self.get()) != 0 } + unsafe { Rf_isVectorList(self.get()).into() } } /// Return true if this is can be made into a vector. fn is_vectorizable(&self) -> bool { - unsafe { Rf_isVectorizable(self.get()) != 0 } + unsafe { Rf_isVectorizable(self.get()).into() } } /// Return true if this is RAWSXP. @@ -410,7 +408,7 @@ pub trait Rinternals: Types + Conversions { /// This is used to wrap R objects. #[doc(hidden)] fn check_external_ptr_type(&self) -> bool { - if self.sexptype() == libR_sys::EXTPTRSXP { + if self.sexptype() == SEXPTYPE::EXTPTRSXP { let tag = unsafe { self.external_ptr_tag() }; if tag.as_str() == Some(std::any::type_name::()) { return true; @@ -428,7 +426,7 @@ pub trait Rinternals: Types + Conversions { } fn is_package_env(&self) -> bool { - unsafe { R_IsPackageEnv(self.get()) != 0 } + unsafe { R_IsPackageEnv(self.get()).into() } } fn package_env_name(&self) -> Robj { @@ -436,7 +434,7 @@ pub trait Rinternals: Types + Conversions { } fn is_namespace_env(&self) -> bool { - unsafe { R_IsNamespaceEnv(self.get()) != 0 } + unsafe { R_IsNamespaceEnv(self.get()).into() } } fn namespace_env_spec(&self) -> Robj { @@ -450,33 +448,33 @@ pub trait Rinternals: Types + Conversions { /// Returns `true` if this is an integer ALTREP object. fn is_altinteger(&self) -> bool { - unsafe { ALTREP(self.get()) != 0 && TYPEOF(self.get()) == INTSXP as i32 } + unsafe { ALTREP(self.get()) != 0 && TYPEOF(self.get()) == SEXPTYPE::INTSXP } } /// Returns `true` if this is an real ALTREP object. fn is_altreal(&self) -> bool { - unsafe { ALTREP(self.get()) != 0 && TYPEOF(self.get()) == REALSXP as i32 } + unsafe { ALTREP(self.get()) != 0 && TYPEOF(self.get()) == SEXPTYPE::REALSXP } } /// Returns `true` if this is an logical ALTREP object. fn is_altlogical(&self) -> bool { - unsafe { ALTREP(self.get()) != 0 && TYPEOF(self.get()) == LGLSXP as i32 } + unsafe { ALTREP(self.get()) != 0 && TYPEOF(self.get()) == SEXPTYPE::LGLSXP } } /// Returns `true` if this is a raw ALTREP object. fn is_altraw(&self) -> bool { - unsafe { ALTREP(self.get()) != 0 && TYPEOF(self.get()) == RAWSXP as i32 } + unsafe { ALTREP(self.get()) != 0 && TYPEOF(self.get()) == SEXPTYPE::RAWSXP } } /// Returns `true` if this is an integer ALTREP object. fn is_altstring(&self) -> bool { - unsafe { ALTREP(self.get()) != 0 && TYPEOF(self.get()) == STRSXP as i32 } + unsafe { ALTREP(self.get()) != 0 && TYPEOF(self.get()) == SEXPTYPE::STRSXP } } /// Returns `true` if this is an integer ALTREP object. #[cfg(use_r_altlist)] fn is_altlist(&self) -> bool { - unsafe { ALTREP(self.get()) != 0 && TYPEOF(self.get()) == VECSXP as i32 } + unsafe { ALTREP(self.get()) != 0 && TYPEOF(self.get()) == SEXPTYPE::VECSXP } } /// Generate a text representation of this object. diff --git a/extendr-api/src/thread_safety.rs b/extendr-api/src/thread_safety.rs index bf8114cfc5..fc3c515d59 100644 --- a/extendr-api/src/thread_safety.rs +++ b/extendr-api/src/thread_safety.rs @@ -100,7 +100,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 be316263ca..6a42a27777 100644 --- a/extendr-api/src/wrapper/altrep.rs +++ b/extendr-api/src/wrapper/altrep.rs @@ -155,22 +155,22 @@ fn manifest(x: SEXP) -> SEXP { single_threaded(|| unsafe { Rf_protect(x); let len = XLENGTH_EX(x); - let data2 = Rf_allocVector(TYPEOF(x) as u32, len as R_xlen_t); + let data2 = Rf_allocVector(TYPEOF(x), len as R_xlen_t); Rf_protect(data2); - match TYPEOF(x) as u32 { - INTSXP => { + match TYPEOF(x) { + SEXPTYPE::INTSXP => { INTEGER_GET_REGION(x, 0, len as R_xlen_t, INTEGER(data2)); } - LGLSXP => { + SEXPTYPE::LGLSXP => { LOGICAL_GET_REGION(x, 0, len as R_xlen_t, LOGICAL(data2)); } - REALSXP => { + SEXPTYPE::REALSXP => { REAL_GET_REGION(x, 0, len as R_xlen_t, REAL(data2)); } - RAWSXP => { + SEXPTYPE::RAWSXP => { RAW_GET_REGION(x, 0, len as R_xlen_t, RAW(data2)); } - CPLXSXP => { + SEXPTYPE::CPLXSXP => { COMPLEX_GET_REGION(x, 0, len as R_xlen_t, COMPLEX(data2)); } _ => panic!("unsupported ALTREP type."), @@ -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); @@ -586,7 +586,7 @@ impl Altrep { unsafe extern "C" fn altrep_Coerce( x: SEXP, - ty: c_int, + ty: SEXPTYPE, ) -> SEXP { ::coerce(x, sxp_to_rtype(ty)).get() } @@ -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/complexes.rs b/extendr-api/src/wrapper/complexes.rs index 8cff9d1246..939294acee 100644 --- a/extendr-api/src/wrapper/complexes.rs +++ b/extendr-api/src/wrapper/complexes.rs @@ -17,6 +17,7 @@ pub struct Complexes { pub(crate) robj: Robj, } +use libR_sys::SEXPTYPE::CPLXSXP; crate::wrapper::macros::gen_vector_wrapper_impl!( vector_type: Complexes, scalar_type: Rcplx, diff --git a/extendr-api/src/wrapper/doubles.rs b/extendr-api/src/wrapper/doubles.rs index 2be0b2cb8b..ff61f376c1 100644 --- a/extendr-api/src/wrapper/doubles.rs +++ b/extendr-api/src/wrapper/doubles.rs @@ -20,6 +20,7 @@ pub struct Doubles { pub(crate) robj: Robj, } +use libR_sys::SEXPTYPE::REALSXP; crate::wrapper::macros::gen_vector_wrapper_impl!( vector_type: Doubles, scalar_type: Rfloat, 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/src/wrapper/expr.rs b/extendr-api/src/wrapper/expr.rs index 35c21fca63..b80af50bfd 100644 --- a/extendr-api/src/wrapper/expr.rs +++ b/extendr-api/src/wrapper/expr.rs @@ -27,7 +27,7 @@ impl Expressions { V::Item: Into, { Self { - robj: make_vector(EXPRSXP, values), + robj: make_vector(SEXPTYPE::EXPRSXP, values), } } diff --git a/extendr-api/src/wrapper/function.rs b/extendr-api/src/wrapper/function.rs index a007873708..e741839f23 100644 --- a/extendr-api/src/wrapper/function.rs +++ b/extendr-api/src/wrapper/function.rs @@ -42,7 +42,7 @@ impl Function { /// ``` pub fn from_parts(formals: Pairlist, body: Language, env: Environment) -> Result { single_threaded(|| unsafe { - let sexp = Rf_allocSExp(CLOSXP); + let sexp = Rf_allocSExp(SEXPTYPE::CLOSXP); let robj = Robj::from_sexp(sexp); SET_FORMALS(sexp, formals.get()); SET_BODY(sexp, body.get()); diff --git a/extendr-api/src/wrapper/integers.rs b/extendr-api/src/wrapper/integers.rs index 559be0fb9c..01875346a6 100644 --- a/extendr-api/src/wrapper/integers.rs +++ b/extendr-api/src/wrapper/integers.rs @@ -20,6 +20,7 @@ pub struct Integers { pub(crate) robj: Robj, } +use libR_sys::SEXPTYPE::INTSXP; crate::wrapper::macros::gen_vector_wrapper_impl!( vector_type: Integers, // Implements for scalar_type: Rint, // Element type diff --git a/extendr-api/src/wrapper/list.rs b/extendr-api/src/wrapper/list.rs index 0b8a3a744d..4b4e8cfc97 100644 --- a/extendr-api/src/wrapper/list.rs +++ b/extendr-api/src/wrapper/list.rs @@ -25,7 +25,7 @@ impl List { /// } /// ``` pub fn new(size: usize) -> Self { - let robj = Robj::alloc_vector(VECSXP, size); + let robj = Robj::alloc_vector(SEXPTYPE::VECSXP, size); Self { robj } } @@ -45,7 +45,7 @@ impl List { V::Item: Into, { Self { - robj: make_vector(VECSXP, values), + robj: make_vector(SEXPTYPE::VECSXP, values), } } @@ -384,7 +384,7 @@ impl> FromIterator for List { let len = iter_collect.len(); crate::single_threaded(|| unsafe { - let mut robj = Robj::alloc_vector(VECSXP, len); + let mut robj = Robj::alloc_vector(SEXPTYPE::VECSXP, len); for (i, v) in iter_collect.into_iter().enumerate() { // We don't PROTECT each element here, as they will be immediately // placed into a list which will protect them: diff --git a/extendr-api/src/wrapper/logicals.rs b/extendr-api/src/wrapper/logicals.rs index 0b1c79d657..7c69273494 100644 --- a/extendr-api/src/wrapper/logicals.rs +++ b/extendr-api/src/wrapper/logicals.rs @@ -21,6 +21,7 @@ pub struct Logicals { pub(crate) robj: Robj, } +use SEXPTYPE::LGLSXP; crate::wrapper::macros::gen_vector_wrapper_impl!( vector_type: Logicals, // Implements for scalar_type: Rbool, // Element type diff --git a/extendr-api/src/wrapper/mod.rs b/extendr-api/src/wrapper/mod.rs index 9b1ca2f511..f9f2d16d9a 100644 --- a/extendr-api/src/wrapper/mod.rs +++ b/extendr-api/src/wrapper/mod.rs @@ -62,7 +62,7 @@ pub(crate) fn make_symbol(name: &str) -> SEXP { unsafe { libR_sys::Rf_install(name.as_ptr()) } } -pub(crate) fn make_vector(sexptype: u32, values: T) -> Robj +pub(crate) fn make_vector(sexptype: SEXPTYPE, values: T) -> Robj where T: IntoIterator, T::IntoIter: ExactSizeIterator, diff --git a/extendr-api/src/wrapper/pairlist.rs b/extendr-api/src/wrapper/pairlist.rs index 54b77b25cf..b37e487860 100644 --- a/extendr-api/src/wrapper/pairlist.rs +++ b/extendr-api/src/wrapper/pairlist.rs @@ -137,9 +137,9 @@ impl Iterator for PairlistIter { let tag = TAG(sexp); let value = Robj::from_sexp(CAR(sexp)); self.list_elem = CDR(sexp); - if TYPEOF(tag) == SYMSXP as i32 { + if TYPEOF(tag) == SEXPTYPE::SYMSXP { let printname = PRINTNAME(tag); - assert!(TYPEOF(printname) as u32 == CHARSXP); + assert!(TYPEOF(printname) == SEXPTYPE::CHARSXP); let name = to_str(R_CHAR(printname) as *const u8); Some((std::mem::transmute(name), value)) } else { diff --git a/extendr-api/src/wrapper/promise.rs b/extendr-api/src/wrapper/promise.rs index 61a4012be3..249825be01 100644 --- a/extendr-api/src/wrapper/promise.rs +++ b/extendr-api/src/wrapper/promise.rs @@ -19,7 +19,7 @@ impl Promise { /// ``` pub fn from_parts(code: Robj, environment: Environment) -> Result { single_threaded(|| unsafe { - let sexp = Rf_allocSExp(PROMSXP); + let sexp = Rf_allocSExp(SEXPTYPE::PROMSXP); let robj = Robj::from_sexp(sexp); SET_PRCODE(sexp, code.get()); SET_PRENV(sexp, environment.robj.get()); diff --git a/extendr-api/src/wrapper/raw.rs b/extendr-api/src/wrapper/raw.rs index b4319d3727..3b9fbbc344 100644 --- a/extendr-api/src/wrapper/raw.rs +++ b/extendr-api/src/wrapper/raw.rs @@ -17,7 +17,7 @@ pub struct Raw { } fn init_raw(len: usize, filler: F) -> Raw { - let mut robj = Robj::alloc_vector(RAWSXP, len); + let mut robj = Robj::alloc_vector(SEXPTYPE::RAWSXP, len); let slice = robj.as_raw_slice_mut().unwrap(); filler(slice); Raw { robj } diff --git a/extendr-api/src/wrapper/strings.rs b/extendr-api/src/wrapper/strings.rs index e243673c95..5c5f8bd730 100644 --- a/extendr-api/src/wrapper/strings.rs +++ b/extendr-api/src/wrapper/strings.rs @@ -25,7 +25,7 @@ impl Strings { /// } /// ``` pub fn new(size: usize) -> Strings { - let robj = Robj::alloc_vector(STRSXP, size); + let robj = Robj::alloc_vector(SEXPTYPE::STRSXP, size); Self { robj } } @@ -47,7 +47,7 @@ impl Strings { single_threaded(|| unsafe { let values = values.into_iter(); let maxlen = values.len(); - let mut robj = Robj::alloc_vector(STRSXP, maxlen); + let mut robj = Robj::alloc_vector(SEXPTYPE::STRSXP, maxlen); let sexp = robj.get_mut(); for (i, v) in values.into_iter().take(maxlen).enumerate() { let v = v.as_ref(); @@ -109,7 +109,7 @@ impl> FromIterator for Strings { let iter_collect: Vec<_> = iter.into_iter().collect(); let len = iter_collect.len(); - let mut robj = Strings::alloc_vector(STRSXP, len); + let mut robj = Strings::alloc_vector(SEXPTYPE::STRSXP, len); crate::single_threaded(|| unsafe { for (i, v) in iter_collect.into_iter().enumerate() { SET_STRING_ELT(robj.get_mut(), i as isize, str_to_character(v.as_ref())); diff --git a/extendr-api/src/wrapper/symbol.rs b/extendr-api/src/wrapper/symbol.rs index 334f4f9742..007e676e6f 100644 --- a/extendr-api/src/wrapper/symbol.rs +++ b/extendr-api/src/wrapper/symbol.rs @@ -35,7 +35,7 @@ impl Symbol { // Internal conversion for constant symbols. fn from_sexp(sexp: SEXP) -> Symbol { unsafe { - assert!(TYPEOF(sexp) == SYMSXP as i32); + assert!(TYPEOF(sexp) == SEXPTYPE::SYMSXP); } Symbol { robj: Robj::from_sexp(sexp), @@ -53,7 +53,7 @@ impl Symbol { unsafe { let sexp = self.robj.get(); let printname = PRINTNAME(sexp); - assert!(TYPEOF(printname) as u32 == CHARSXP); + assert!(TYPEOF(printname) == SEXPTYPE::CHARSXP); to_str(R_CHAR(printname) as *const u8) } } diff --git a/extendr-api/tests/io_tests.rs b/extendr-api/tests/io_tests.rs index 20683015f3..dd0d51f556 100644 --- a/extendr-api/tests/io_tests.rs +++ b/extendr-api/tests/io_tests.rs @@ -5,11 +5,11 @@ fn test_save() { use extendr_api::{io::PstreamFormat, io::Save, test, Result, Robj}; test! { let mut w = Vec::new(); - Robj::from(1).to_writer(&mut w, PstreamFormat::AsciiFormat, 3, None)?; + Robj::from(1).to_writer(&mut w, PstreamFormat::R_pstream_ascii_format, 3, None)?; assert!(w[0] == b'A'); let mut w = Vec::new(); - Robj::from(1).to_writer(&mut w, PstreamFormat::BinaryFormat, 3, None)?; + Robj::from(1).to_writer(&mut w, PstreamFormat::R_pstream_binary_format, 3, None)?; assert!(w[0] == b'B'); // let path : std::path::PathBuf = "/tmp/1".into(); @@ -40,7 +40,7 @@ UTF-8 let mut c = std::io::Cursor::new(text); - let res = Robj::from_reader(&mut c, PstreamFormat::AsciiFormat, None); + let res = Robj::from_reader(&mut c, PstreamFormat::R_pstream_ascii_format, None); assert_eq!(res, Ok(Robj::from(1_i32))); } } From 87a7f3e5c3e4dd94cbaab5f6996de2219f076bdb Mon Sep 17 00:00:00 2001 From: CGMossa Date: Fri, 28 Jun 2024 16:35:35 +0200 Subject: [PATCH 2/7] [urgent] Update non-API for CRAN (#809) * Cargo.toml: update libR-sys to latest commit * added more `non-api` cfg guards * moved doctests to their own module * wp: another * wp: another one * this test is disabled, yet it still compiles.. * typo * removed more tests * update changelog * update note on CRAN * `cargo extendr fmt` [skip ci] --------- Co-authored-by: Josiah Parry --- CHANGELOG.md | 31 ++++++- Cargo.toml | 2 +- extendr-api/Cargo.toml | 4 + extendr-api/src/functions.rs | 12 +-- extendr-api/src/lib.rs | 18 ---- extendr-api/src/prelude.rs | 6 +- extendr-api/src/rmacros.rs | 19 ---- extendr-api/src/robj/mod.rs | 4 +- extendr-api/src/robj/rinternals.rs | 14 +-- extendr-api/src/wrapper/altrep.rs | 2 +- extendr-api/src/wrapper/environment.rs | 32 +------ extendr-api/src/wrapper/function.rs | 1 + extendr-api/src/wrapper/primitive.rs | 10 +-- extendr-api/src/wrapper/promise.rs | 17 +++- extendr-api/tests/call_tests.rs | 1 + extendr-api/tests/non_api_tests.rs | 102 ++++++++++++++++++++++ tests/extendrtests/src/rust/src/matrix.rs | 31 +++++++ 17 files changed, 200 insertions(+), 106 deletions(-) create mode 100644 extendr-api/tests/non_api_tests.rs create mode 100644 tests/extendrtests/src/rust/src/matrix.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index a1d8db1f13..5aa78634f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,36 @@ ## Changed -- Potentially breaking: `RArray::from_parts` no longer requires a pointer to the underlying data vector [[#657]](https://github.com/extendr/extendr/pull/657) +- [_Potentially breaking_]: `RArray::from_parts` no longer requires a pointer to the underlying data + vector [[#657]](https://github.com/extendr/extendr/pull/657) +- `#[extendr(use_try_from = true)` is now the default setting, therefore the option `use_try_from` has been removed [[#759]](https://github.com/extendr/extendr/pull/759) + +#### Breaking changes + +- R-devel Non-API changes: + - R's C API is being formalized. While the changes are formalized, non-API functions are hidden behind a feature flag to prevent removal from CRAN. + - Non-API [changes are in flux in R-devel](https://github.com/r-devel/r-svn/blob/71afe1e304b11f7febaa536e96817c63a7c1c7ab/src/library/tools/R/sotools.R#L564), however, CRAN has set a July 9th date to remove any package that uses non-API functions. This includes almost every extendr based package on CRAN. + - See [[Rd] clarifying and adjusting the C API for R](https://stat.ethz.ch/pipermail/r-devel/2024-June/083449.html) + - [nonAPI.txt](https://github.com/r-devel/r-svn/blob/f36c203d3a53a74d56a81d4f97a68d24993e0652/src/library/tools/R/sotools.R#L564) functions are hidden behind the `non-api` feature flag. + - Removed from default include (but may not be limited to): + - `global_var()`, `local_var()`, `base_env()`, various `Environment`, `Function`, `Primitive`, and `Promise` methods. +- `Attributes` trait now returns a mutable reference + to `Self`. [[#745]](https://github.com/extendr/extendr/pull/745). Previously `.set_attrib()` would modify an object in + place, and then return an untyped owned pointer (Robj). Instead, now we return `&mut Self`. +- In `AltRep` the `unserialize_ex`, `set_parent`, `set_envflags` are +now hidden behind the feature flag `non-api`. Also, `Promise::from_parts` is marked as non-API. +- Floating point numbers with decimal part can no longer be converted to integer types via + rounding [[#757]](https://github.com/extendr/extendr/pull/757) +- You can no longer create an `Robj` from a reference `&T`, where `T` is an `extendr`-impl. [[#759]](https://github.com/extendr/extendr/pull/759) +- You can no longer use `from_robj`, as the trait `FromRobj` as been removed. Instead, use `try_from`. +- It is no longer possible to access an R integer vector as a `&[u32]` [[#767]](https://github.com/extendr/extendr/pull/767) + +### Fixed + +- returning `&Self` or `&mut Self` from a method in an `#[extendr]`-impl would +result in unintended cloning [[#614]](https://github.com/extendr/extendr/issues/614) +- `TryFrom<&Robj>` and `FromRobj` for integer scalars now correctly handles conversions + from `f64` [[#757]](https://github.com/extendr/extendr/pull/757) ## 0.6.0 diff --git a/Cargo.toml b/Cargo.toml index 774f4aa6e5..80373c22dd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,4 +29,4 @@ libR-sys = "0.7.0" # When uncommenting this, do not forget to uncomment the same line in # ./tests/extendrtests/src/rust/Cargo.toml, and "Run R integration tests using # {rextendr}" on .github/workflows/test.yml ! -libR-sys = { git = "https://github.com/extendr/libR-sys" } +libR-sys = { git = "https://github.com/extendr/libR-sys", rev = "975af4e555e4fed58084aef9596a9a71602666be" } diff --git a/extendr-api/Cargo.toml b/extendr-api/Cargo.toml index f1df1e0656..65d2386954 100644 --- a/extendr-api/Cargo.toml +++ b/extendr-api/Cargo.toml @@ -55,3 +55,7 @@ tests-all = ["tests", "graphics"] [package.metadata.docs.rs] features = ["full-functionality", "libR-sys/use-bindgen"] + +[[test]] +name = "non_api_tests" +required-features = ["non-api"] \ No newline at end of file diff --git a/extendr-api/src/functions.rs b/extendr-api/src/functions.rs index bd306c2759..ffb8e42133 100644 --- a/extendr-api/src/functions.rs +++ b/extendr-api/src/functions.rs @@ -1,6 +1,7 @@ use crate as extendr_api; use crate::*; +#[cfg(feature = "non-api")] /// Get a global variable from global_env() and ancestors. /// If the result is a promise, evaulate the promise. /// @@ -17,6 +18,7 @@ pub fn global_var>(key: K) -> Result { global_env().find_var(key)?.eval_promise() } +#[cfg(feature = "non-api")] /// Get a local variable from current_env() and ancestors. /// /// If the result is a promise, evaulate the promise. @@ -131,15 +133,7 @@ pub fn new_env(parent: Environment, hash: bool, capacity: i32) -> Environment { .unwrap() } -/// The base environment; formerly R_NilValue -/// -/// ``` -/// use extendr_api::prelude::*; -/// test! { -/// global_env().set_local(sym!(x), "hello"); -/// assert_eq!(base_env().local(sym!(+)), Ok(r!(Primitive::from_string("+")))); -/// } -/// ``` +/// The base environment; formerly `R_NilValue` pub fn base_env() -> Environment { unsafe { Robj::from_sexp(R_BaseEnv).try_into().unwrap() } } diff --git a/extendr-api/src/lib.rs b/extendr-api/src/lib.rs index 9458ba38e5..8f1a8a8cdc 100644 --- a/extendr-api/src/lib.rs +++ b/extendr-api/src/lib.rs @@ -175,24 +175,6 @@ //! } //! ``` //! -//! You can call R functions and primitives using the [call!] macro. -//! ``` -//! use extendr_api::prelude::*; -//! test! { -//! -//! // As one R! macro call -//! let confint1 = R!("confint(lm(weight ~ group - 1, PlantGrowth))")?; -//! -//! // As many parameterized calls. -//! let formula = lang!("~", sym!(weight), lang!("-", sym!(group), 1.0)).set_class(["formula"])?; -//! let plant_growth = global!(PlantGrowth)?; -//! let model = call!("lm", formula, plant_growth)?; -//! let confint2 = call!("confint", model)?; -//! -//! assert_eq!(confint1.as_real_vector(), confint2.as_real_vector()); -//! } -//! ``` -//! //! Rust has a concept of "Owned" and "Borrowed" objects. //! //! Owned objects, such as [Vec] and [String] allocate memory diff --git a/extendr-api/src/prelude.rs b/extendr-api/src/prelude.rs index e5c3238a83..76a2cb5944 100644 --- a/extendr-api/src/prelude.rs +++ b/extendr-api/src/prelude.rs @@ -15,10 +15,12 @@ pub use super::error::{Error, Result}; pub use super::functions::{ base_env, base_namespace, blank_scalar_string, blank_string, current_env, empty_env, eval_string, eval_string_with_params, find_namespace, find_namespaced_function, global_env, - global_function, global_var, local_var, na_string, namespace_registry, new_env, nil_value, - parse, srcref, + global_function, na_string, namespace_registry, new_env, nil_value, parse, srcref, }; +#[cfg(feature = "non-api")] +pub use super::functions::{global_var, local_var}; + pub use super::wrapper::symbol::{ base_symbol, brace_symbol, bracket_2_symbol, bracket_symbol, class_symbol, device_symbol, dim_symbol, dimnames_symbol, dollar_symbol, dot_defined, dot_method, dot_package_name, diff --git a/extendr-api/src/rmacros.rs b/extendr-api/src/rmacros.rs index bb91f9de88..98a4a01ff2 100644 --- a/extendr-api/src/rmacros.rs +++ b/extendr-api/src/rmacros.rs @@ -37,18 +37,6 @@ macro_rules! r { /// or a global variable if no such variable exists. /// /// Variables with embedded "." may not work. -/* -TODO: As of R 4.1.0, base env cannot be modifed, which makes it difficult to - test outside of R process, so this doc test is disabled for now. See #211 - for the details. -*/ -/// ```no_run -/// use extendr_api::prelude::*; -/// test! { -/// current_env().set_local(sym!(myvar), 1.0); -/// assert_eq!(var!(myvar), Ok(r!(1.0))); -/// } -/// ``` #[macro_export] macro_rules! var { ($($tokens: tt)*) => {{ @@ -59,13 +47,6 @@ macro_rules! var { /// Get a global variable. /// /// Variables with embedded "." may not work. -/// ``` -/// use extendr_api::prelude::*; -/// test! { -/// // The "iris" dataset is a dataframe. -/// assert_eq!(global!(iris)?.is_frame(), true); -/// } -/// ``` #[macro_export] macro_rules! global { ($($tokens: tt)*) => {{ diff --git a/extendr-api/src/robj/mod.rs b/extendr-api/src/robj/mod.rs index fc66712223..2523c0dc41 100644 --- a/extendr-api/src/robj/mod.rs +++ b/extendr-api/src/robj/mod.rs @@ -250,8 +250,6 @@ pub trait Types: GetSexp { /// assert_eq!(R!("function() {}")?.rtype(), Rtype::Function); /// assert_eq!(Environment::new_with_parent(global_env()).rtype(), Rtype::Environment); /// assert_eq!(lang!("+", 1, 2).rtype(), Rtype::Language); - /// assert_eq!(r!(Primitive::from_string("if")).rtype(), Rtype::Special); - /// assert_eq!(r!(Primitive::from_string("+")).rtype(), Rtype::Builtin); /// assert_eq!(r!(Rstr::from_string("hello")).rtype(), Rtype::Rstr); /// assert_eq!(r!(TRUE).rtype(), Rtype::Logicals); /// assert_eq!(r!(1).rtype(), Rtype::Integers); @@ -804,7 +802,7 @@ make_typed_slice!(Rint, INTEGER, INTSXP); make_typed_slice!(f64, REAL, REALSXP); make_typed_slice!(Rfloat, REAL, REALSXP); make_typed_slice!(u8, RAW, RAWSXP); -make_typed_slice!(Rstr, STRING_PTR, STRSXP); +make_typed_slice!(Rstr, STRING_PTR_RO, STRSXP); make_typed_slice!(c64, COMPLEX, CPLXSXP); make_typed_slice!(Rcplx, COMPLEX, CPLXSXP); make_typed_slice!(Rcomplex, COMPLEX, CPLXSXP); diff --git a/extendr-api/src/robj/rinternals.rs b/extendr-api/src/robj/rinternals.rs index 3ab90cdfae..91034ca140 100644 --- a/extendr-api/src/robj/rinternals.rs +++ b/extendr-api/src/robj/rinternals.rs @@ -163,24 +163,13 @@ pub trait Rinternals: Types + Conversions { /// /// Note that many common variables and functions are contained in promises /// which must be evaluated and this function may throw an R error. - /// ``` - /// use extendr_api::prelude::*; - /// test! { - /// let iris_dataframe = global_env() - /// .find_var(sym!(iris)).unwrap().eval_promise().unwrap(); - /// assert_eq!(iris_dataframe.is_frame(), true); - /// assert_eq!(iris_dataframe.len(), 5); /// - /// // Note: this may crash on some versions of windows which don't support unwinding. - /// //assert_eq!(global_env().find_var(sym!(imnotasymbol)), None); - /// } - /// ``` fn find_var>(&self, key: K) -> Result { let key: Symbol = key.try_into()?; if !self.is_environment() { return Err(Error::NotFound(key.into())); } - // Alterative: + // Alternative: // let mut env: Robj = self.into(); // loop { // if let Some(var) = env.local(&key) { @@ -210,6 +199,7 @@ pub trait Rinternals: Types + Conversions { } } + #[cfg(feature = "non-api")] /// If this object is a promise, evaluate it, otherwise return the object. /// ``` /// use extendr_api::prelude::*; diff --git a/extendr-api/src/wrapper/altrep.rs b/extendr-api/src/wrapper/altrep.rs index 6a42a27777..3aa04dca63 100644 --- a/extendr-api/src/wrapper/altrep.rs +++ b/extendr-api/src/wrapper/altrep.rs @@ -154,7 +154,7 @@ pub trait AltrepImpl: Clone + std::fmt::Debug { fn manifest(x: SEXP) -> SEXP { single_threaded(|| unsafe { Rf_protect(x); - let len = XLENGTH_EX(x); + let len = XLENGTH(x); let data2 = Rf_allocVector(TYPEOF(x), len as R_xlen_t); Rf_protect(data2); match TYPEOF(x) { diff --git a/extendr-api/src/wrapper/environment.rs b/extendr-api/src/wrapper/environment.rs index c489d3c223..8c818d7bce 100644 --- a/extendr-api/src/wrapper/environment.rs +++ b/extendr-api/src/wrapper/environment.rs @@ -89,6 +89,7 @@ impl Environment { self } + #[cfg(feature = "non-api")] /// Get the environment flags. pub fn envflags(&self) -> i32 { unsafe { @@ -106,6 +107,7 @@ impl Environment { self } + #[cfg(feature = "non-api")] /// Iterate over an environment. pub fn iter(&self) -> EnvIter { unsafe { @@ -125,6 +127,7 @@ impl Environment { } } + #[cfg(feature = "non-api")] /// Get the names in an environment. /// ``` /// use extendr_api::prelude::*; @@ -170,13 +173,7 @@ impl Environment { pub fn local>(&self, key: K) -> Result { let key = key.into(); if key.is_symbol() { - unsafe { - Ok(Robj::from_sexp(Rf_findVarInFrame3( - self.get(), - key.get(), - Rboolean::TRUE, - ))) - } + unsafe { Ok(Robj::from_sexp(Rf_findVarInFrame(self.get(), key.get()))) } } else { Err(Error::NotFound(key)) } @@ -185,27 +182,6 @@ impl Environment { /// Iterator over the names and values of an environment /// -/// ``` -/// use extendr_api::prelude::*; -/// test! { -/// let names_and_values = (0..100).map(|i| (format!("n{}", i), i)); -/// let env = Environment::from_pairs(global_env(), names_and_values); -/// let robj = r!(env); -/// let names_and_values = robj.as_environment().unwrap().iter().collect::>(); -/// assert_eq!(names_and_values.len(), 100); -/// -/// let small_env = Environment::new_with_capacity(global_env(), 1); -/// small_env.set_local(sym!(x), 1); -/// let names_and_values = small_env.as_environment().unwrap().iter().collect::>(); -/// assert_eq!(names_and_values, vec![("x", r!(1))]); -/// -/// let large_env = Environment::new_with_capacity(global_env(), 1000); -/// large_env.set_local(sym!(x), 1); -/// let names_and_values = large_env.as_environment().unwrap().iter().collect::>(); -/// assert_eq!(names_and_values, vec![("x", r!(1))]); -/// } -/// -/// ``` #[derive(Clone)] pub struct EnvIter { hash_table: ListIter, diff --git a/extendr-api/src/wrapper/function.rs b/extendr-api/src/wrapper/function.rs index e741839f23..12f574f6e5 100644 --- a/extendr-api/src/wrapper/function.rs +++ b/extendr-api/src/wrapper/function.rs @@ -29,6 +29,7 @@ pub struct Function { } impl Function { + #[cfg(feature = "non-api")] /// Make a function from parts. /// ``` /// use extendr_api::prelude::*; diff --git a/extendr-api/src/wrapper/primitive.rs b/extendr-api/src/wrapper/primitive.rs index 14a3bcc288..56a70b38de 100644 --- a/extendr-api/src/wrapper/primitive.rs +++ b/extendr-api/src/wrapper/primitive.rs @@ -2,14 +2,7 @@ use super::*; /// Wrapper for creating primitive objects. /// -/// Make a primitive object, or NULL if not available. -/// ``` -/// use extendr_api::prelude::*; -/// test! { -/// let builtin = r!(Primitive::from_string("+")); -/// let special = r!(Primitive::from_string("if")); -/// } -/// ``` +/// Make a primitive object, or `NULL` if not available /// #[derive(PartialEq, Clone)] pub struct Primitive { @@ -17,6 +10,7 @@ pub struct Primitive { } impl Primitive { + #[cfg(feature = "non-api")] /// Make a Primitive object from a string. /// ``` /// use extendr_api::prelude::*; diff --git a/extendr-api/src/wrapper/promise.rs b/extendr-api/src/wrapper/promise.rs index 249825be01..362338a37b 100644 --- a/extendr-api/src/wrapper/promise.rs +++ b/extendr-api/src/wrapper/promise.rs @@ -28,6 +28,7 @@ impl Promise { }) } + #[cfg(feature = "non-api")] /// Get the code to be executed from the promise. pub fn code(&self) -> Robj { unsafe { @@ -36,6 +37,7 @@ impl Promise { } } + #[cfg(feature = "non-api")] /// Get the environment for the execution from the promise. pub fn environment(&self) -> Environment { unsafe { @@ -44,6 +46,7 @@ impl Promise { } } + #[cfg(feature = "non-api")] /// Get the value of the promise, once executed. pub fn value(&self) -> Robj { unsafe { @@ -52,6 +55,7 @@ impl Promise { } } + #[cfg(feature = "non-api")] /// Get the seen flag (avoids recursion). pub fn seen(&self) -> i32 { unsafe { @@ -60,6 +64,7 @@ impl Promise { } } + #[cfg(feature = "non-api")] /// If this promise has not been evaluated, evaluate it, otherwise return the value. /// ``` /// use extendr_api::prelude::*; @@ -81,9 +86,13 @@ impl Promise { impl std::fmt::Debug for Promise { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("Promise") - .field("code", &self.code()) - .field("environment", &self.environment()) - .finish() + let mut result = f.debug_struct("Promise"); + + #[cfg(feature = "non-api")] + { + let result = result.field("code", &self.code()); + let result = result.field("environment", &self.environment()); + } + result.finish() } } diff --git a/extendr-api/tests/call_tests.rs b/extendr-api/tests/call_tests.rs index ff1ee0a579..47c5177b5e 100644 --- a/extendr-api/tests/call_tests.rs +++ b/extendr-api/tests/call_tests.rs @@ -1,6 +1,7 @@ use extendr_api::prelude::*; #[test] +#[cfg(feature = "non-api")] fn formula_test() { test! { // As one R! macro call diff --git a/extendr-api/tests/non_api_tests.rs b/extendr-api/tests/non_api_tests.rs new file mode 100644 index 0000000000..4b331ea643 --- /dev/null +++ b/extendr-api/tests/non_api_tests.rs @@ -0,0 +1,102 @@ +use extendr_api::prelude::*; +use extendr_engine::with_r; + +#[cfg(test)] +fn non_api_promise() { + with_r(|| { + let special = r!(Primitive::from_string("if")); + let builtin = r!(Primitive::from_string("+")); + }); +} + +#[cfg(test)] +fn environment() { + with_r(|| { + let names_and_values = (0..100).map(|i| (format!("n{}", i), i)); + let env = Environment::from_pairs(global_env(), names_and_values); + let robj = r!(env); + let names_and_values = robj.as_environment().unwrap().iter().collect::>(); + assert_eq!(names_and_values.len(), 100); + + let small_env = Environment::new_with_capacity(global_env(), 1); + small_env.set_local(sym!(x), 1); + let names_and_values = small_env + .as_environment() + .unwrap() + .iter() + .collect::>(); + assert_eq!(names_and_values, vec![("x", r!(1))]); + + let large_env = Environment::new_with_capacity(global_env(), 1000); + large_env.set_local(sym!(x), 1); + let names_and_values = large_env + .as_environment() + .unwrap() + .iter() + .collect::>(); + assert_eq!(names_and_values, vec![("x", r!(1))]); + }); +} + +#[cfg(test)] +fn non_api_rinternals_promise() { + with_r(|| { + let iris_dataframe = global_env() + .find_var(sym!(iris)) + .unwrap() + .eval_promise() + .unwrap(); + assert_eq!(iris_dataframe.is_frame(), true); + assert_eq!(iris_dataframe.len(), 5); + + // Note: this may crash on some versions of windows which don't support unwinding. + //assert_eq!(global_env().find_var(sym!(imnotasymbol)), None); + }); +} + +#[cfg(test)] +fn non_api_getsexp_rtype() { + with_r(|| { + assert_eq!(r!(Primitive::from_string("if")).rtype(), Rtype::Special); + assert_eq!(r!(Primitive::from_string("+")).rtype(), Rtype::Builtin); + }); +} + +#[cfg(test)] +fn non_api_rmacros() { + with_r(|| { + // The "iris" dataset is a dataframe. + assert_eq!(global!(iris)?.is_frame(), true); + }); +} + +/// In `extendr-api/lib.rs` there was a section with this +/// +/// > You can call R functions and primitives using the [call!] macro. +#[cfg(test)] +fn non_api_lib_README() { + with_r(|| { + // As one R! macro call + let confint1 = R!("confint(lm(weight ~ group - 1, PlantGrowth))")?; + + // As many parameterized calls. + let mut formula = lang!("~", sym!(weight), lang!("-", sym!(group), 1.0)); + formula.set_class(["formula"])?; + let plant_growth = global!(PlantGrowth)?; + let model = call!("lm", formula, plant_growth)?; + let confint2 = call!("confint", model)?; + + assert_eq!(confint1.as_real_vector(), confint2.as_real_vector()); + }); +} + +#[cfg(test)] +fn non_api_base_env() { + with_r(|| { + global_env().set_local(sym!(x), "hello"); + assert_eq!( + base_env().local(sym!(+)), + Ok(r!(Primitive::from_string("+"))) + ); + }); +} diff --git a/tests/extendrtests/src/rust/src/matrix.rs b/tests/extendrtests/src/rust/src/matrix.rs new file mode 100644 index 0000000000..892c667859 --- /dev/null +++ b/tests/extendrtests/src/rust/src/matrix.rs @@ -0,0 +1,31 @@ +use extendr_api::prelude::*; + +#[extendr] +fn fetch_dimnames(x: RMatrix) -> List { + x.get_dimnames() +} + +#[extendr] +fn fetch_rownames(x: RMatrix) -> Robj { + x.get_rownames() +} + +#[extendr] +fn fetch_colnames(x: RMatrix) -> Robj { + x.get_colnames() +} + +#[extendr] +fn change_dimnames(mut x: RMatrix) -> Robj { + let rownames = Strings::from_values(["AA", "BB", "CC"]); + x.set_dimnames(list!(rownames, NULL)); + x.to_owned() +} + +extendr_module! { + mod matrix; + fn fetch_dimnames; + fn fetch_colnames; + fn fetch_rownames; + fn change_dimnames; +} From 1fc408de9491598d202d553d743c64eba535f437 Mon Sep 17 00:00:00 2001 From: eitsupi <50911393+eitsupi@users.noreply.github.com> Date: Thu, 13 Mar 2025 15:22:39 +0000 Subject: [PATCH 3/7] fix: remove duplicate function --- extendr-api/src/ownership.rs | 34 ---------------------------------- 1 file changed, 34 deletions(-) diff --git a/extendr-api/src/ownership.rs b/extendr-api/src/ownership.rs index 9d11b548c2..38a7eed208 100644 --- a/extendr-api/src/ownership.rs +++ b/extendr-api/src/ownership.rs @@ -186,40 +186,6 @@ impl Ownership { } } - // Garbage collect the tracking structures. - unsafe fn garbage_collect(&mut self) { - // println!("garbage_collect {} {}", self.cur_index, self.max_index); - let new_size = self.cur_index * 2 + EXTRA_PRESERVATION_SIZE; - let new_sexp = Rf_allocVector(VECSXP, new_size as R_xlen_t); - R_PreserveObject(new_sexp); - let old_sexp = self.preservation as SEXP; - - let mut new_objects = HashMap::with_capacity(new_size); - - // copy non-null elements to new vector and hashmap. - let mut j = 0; - for (addr, object) in self.objects.iter() { - if object.refcount != 0 { - SET_VECTOR_ELT(new_sexp, j as R_xlen_t, *addr as SEXP); - new_objects.insert( - *addr, - Object { - refcount: object.refcount, - index: j, - }, - ); - j += 1; - } - } - // println!("j={}", j); - - R_ReleaseObject(old_sexp); - self.preservation = new_sexp as usize; - self.cur_index = j; - self.max_index = new_size; - self.objects = new_objects; - } - // Check the consistency of the model. #[allow(dead_code)] unsafe fn check_objects(&mut self) { From 1c643642cb58188b2a535d99508354df2979eb11 Mon Sep 17 00:00:00 2001 From: Josiah Parry Date: Tue, 3 Sep 2024 12:28:49 -0700 Subject: [PATCH 4/7] hide is_valid_string behind non-api flag (#842) --- extendr-api/src/robj/rinternals.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/extendr-api/src/robj/rinternals.rs b/extendr-api/src/robj/rinternals.rs index 91034ca140..203e3e3895 100644 --- a/extendr-api/src/robj/rinternals.rs +++ b/extendr-api/src/robj/rinternals.rs @@ -354,11 +354,13 @@ pub trait Rinternals: Types + Conversions { unsafe { Rf_isUserBinop(self.get()).into() } } + #[cfg(feature = "non-api")] /// Return true if this is a valid string. fn is_valid_string(&self) -> bool { unsafe { Rf_isValidString(self.get()).into() } } + #[cfg(feature = "non-api")] /// Return true if this is a valid string. fn is_valid_string_f(&self) -> bool { unsafe { Rf_isValidStringF(self.get()).into() } From 3c6717590f9a31aba5344a4eca9af9df8b0963fc Mon Sep 17 00:00:00 2001 From: CGMossa Date: Sat, 20 Apr 2024 14:06:34 +0200 Subject: [PATCH 5/7] Added unsafe `SendSEXP` to `ownership`-module (#666) * Added a `SendSEXP` instead of conversion to usize * `cargo fmt`. * remove unnecessary Deref/DerefMut impl * add comments to ownership * typos * `cargo fmt` * comments * remove set_inner() method from SendSEXP * rename sexp_usize to send_sexp to make the type: --------- Co-authored-by: Josiah Parry --- extendr-api/src/ownership.rs | 74 ++++++++++++++++++++++++++++-------- 1 file changed, 59 insertions(+), 15 deletions(-) diff --git a/extendr-api/src/ownership.rs b/extendr-api/src/ownership.rs index 38a7eed208..661f4944e2 100644 --- a/extendr-api/src/ownership.rs +++ b/extendr-api/src/ownership.rs @@ -17,6 +17,38 @@ use libR_sys::{ Rf_unprotect, LENGTH, SET_VECTOR_ELT, SEXP, SEXPTYPE, VECTOR_ELT, }; +mod send_sexp { + //! Provide a wrapper around R's pointer type `SEXP` that is `Send`. + //! + //! This can lead to soundness issues, therefore accessing the `SEXP` has + //! to happen through the unsafe method [`SendSEXP::inner`]. + //! + use libR_sys::SEXP; + + /// A wrapper around R's pointer type `SEXP` that is `Send`. + #[derive(Debug, Clone, PartialEq, Eq, Hash)] + pub struct SendSEXP(SEXP); + + impl From for SendSEXP { + fn from(value: SEXP) -> Self { + Self(value) + } + } + + // Allows SendSEXP to be sent between threads even though unsafe + // Requires that the SEXP is not accessed concurrently. + unsafe impl Send for SendSEXP {} + + impl SendSEXP { + /// Get the inner `SEXP` + pub unsafe fn inner(&self) -> SEXP { + self.0 + } + } +} + +use self::send_sexp::SendSEXP; + static OWNERSHIP: Lazy> = Lazy::new(|| Mutex::new(Ownership::new())); pub(crate) unsafe fn protect(sexp: SEXP) { @@ -32,15 +64,20 @@ pub(crate) unsafe fn unprotect(sexp: SEXP) { pub const INITIAL_PRESERVATION_SIZE: usize = 100000; pub const EXTRA_PRESERVATION_SIZE: usize = 100000; +// `Object` is a manual reference counting mechanism that is used for each SEXP. +// `refcount` is the number of times the SEXP is accessed. +// `index` is the index of the SEXP in the preservation vector. +#[derive(Debug)] struct Object { refcount: usize, index: usize, } // A reference counted object with an index in the preservation vector. +#[derive(Debug)] struct Ownership { // A growable vector containing all owned objects. - preservation: usize, + preservation: SendSEXP, // An incrementing count of objects through the vector. cur_index: usize, @@ -49,7 +86,7 @@ struct Ownership { max_index: usize, // A hash map from SEXP address to object. - objects: HashMap, + objects: HashMap, } impl Ownership { @@ -59,7 +96,7 @@ impl Ownership { Rf_allocVector(SEXPTYPE::VECSXP, INITIAL_PRESERVATION_SIZE as R_xlen_t); R_PreserveObject(preservation); Ownership { - preservation: preservation as usize, + preservation: preservation.into(), cur_index: 0, max_index: INITIAL_PRESERVATION_SIZE, objects: HashMap::with_capacity(INITIAL_PRESERVATION_SIZE), @@ -100,13 +137,21 @@ impl Ownership { } unsafe fn protect(&mut self, sexp: SEXP) { + // This protects the SEXP. Is this necessary? + // Because the Ownership object already protects an SEXP in the `preservation` field. + // The new `sexp` is inserted into the preservation list via `SET_VECTOR_ELT` below. + // If list is protected then so are all of its elements. + // + // > Protecting an R object automatically protects all the R objects + // > pointed to in the corresponding SEXPREC, for example all elements + // > of a protected list are automatically protected." 5.9.1 Rf_protect(sexp); if self.cur_index == self.max_index { self.garbage_collect(); } - let sexp_usize = sexp as usize; + let send_sexp = sexp.into(); let Ownership { ref mut preservation, ref mut cur_index, @@ -114,8 +159,8 @@ impl Ownership { ref mut objects, } = *self; - let mut entry = objects.entry(sexp_usize); - let preservation_sexp = *preservation as SEXP; + let mut entry = objects.entry(send_sexp); + let preservation_sexp = preservation.inner(); match entry { Entry::Occupied(ref mut occupied) => { if occupied.get().refcount == 0 { @@ -138,15 +183,15 @@ impl Ownership { } pub unsafe fn unprotect(&mut self, sexp: SEXP) { - let sexp_usize = sexp as usize; + let send_sexp = sexp.into(); let Ownership { preservation, cur_index: _, max_index: _, ref mut objects, - } = *self; + } = self; - let mut entry = objects.entry(sexp_usize); + let mut entry = objects.entry(send_sexp); match entry { Entry::Occupied(ref mut occupied) => { let object = occupied.get_mut(); @@ -158,7 +203,7 @@ impl Ownership { // Clear the preservation vector, but keep the hash table entry. // It is hard to clear the hash table entry here because we don't // have a ref to objects anymore and it is faster to clear them up en-masse. - let preservation_sexp = preservation as SEXP; + let preservation_sexp = preservation.inner(); SET_VECTOR_ELT(preservation_sexp, object.index as R_xlen_t, R_NilValue); } } @@ -178,8 +223,7 @@ impl Ownership { ref mut objects, } = *self; - let sexp_usize = sexp as usize; - let mut entry = objects.entry(sexp_usize); + let mut entry = objects.entry(sexp.into()); match entry { Entry::Occupied(ref mut occupied) => occupied.get().refcount, Entry::Vacant(_) => 0, @@ -189,7 +233,7 @@ impl Ownership { // Check the consistency of the model. #[allow(dead_code)] unsafe fn check_objects(&mut self) { - let preservation_sexp = self.preservation as SEXP; + let preservation_sexp = self.preservation.inner(); assert_eq!(self.max_index, LENGTH(preservation_sexp) as usize); // println!("\ncheck"); @@ -203,7 +247,7 @@ impl Ownership { // ); if object.refcount != 0 { // A non-zero refcount implies the object is in the vector. - assert_eq!(elt, *addr as SEXP); + assert_eq!(elt, addr.inner()); } else { // A zero refcount implies the object is NULL in the vector. assert_eq!(elt, R_NilValue); @@ -227,7 +271,7 @@ impl Ownership { mod test { use super::*; use crate::*; - use libR_sys::{Rf_ScalarInteger, Rf_protect, Rf_unprotect}; + use libR_sys::{Rf_ScalarInteger, Rf_protect}; #[test] fn basic_test() { From 24ec0933892fe6d926e3d293b92fc2eeb22b06ad Mon Sep 17 00:00:00 2001 From: CGMossa Date: Sat, 11 May 2024 22:30:43 +0200 Subject: [PATCH 6/7] Various maintenance (#754) * xtask: add helpful message when missing dependencies for `r-cmd-check` * Makevars: color is auto, not always * README: add instruction to use developer extendr in rextendr. * extendr-api: added `non-api` flag * ci: temporary hack to get past this circular testing BS * Cargo.toml: we will no longer rely on `patch.crates-io`. * ci: added ` REXTENDR_SKIP_DEV_TESTS: TRUE` --- .github/workflows/test.yml | 3 +- Cargo.toml | 3 -- extendr-api/Cargo.toml | 5 +++ extendr-api/src/wrapper/altrep.rs | 5 ++- extendr-api/src/wrapper/environment.rs | 4 ++- extendr-api/src/wrapper/promise.rs | 1 + extendr-api/tests/debug_tests.rs | 2 ++ tests/extendrtests/README.md | 20 +++++++++++ tests/extendrtests/src/Makevars | 2 +- xtask/src/commands/r_cmd_check.rs | 46 ++++++++++++++++++++++++++ 10 files changed, 84 insertions(+), 7 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a52bfb8790..b8a0b2575c 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -45,6 +45,7 @@ jobs: env: R_REMOTES_NO_ERRORS_FROM_WARNINGS: true + REXTENDR_SKIP_DEV_TESTS: TRUE # This environment variable enables support for pseudo multi-target cargo builds. # Current stable Rust does not support multi-targeting, @@ -238,7 +239,7 @@ jobs: "\" }"), # uncomment this line when we need to depend on the dev version of libR-sys - 'libR-sys = { git = "https://github.com/extendr/libR-sys" }', + # 'libR-sys = { git = "https://github.com/extendr/libR-sys", features = array("non-api", 1) }', sep = ";") diff --git a/Cargo.toml b/Cargo.toml index 80373c22dd..32c0be815d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,9 +23,6 @@ rust-version = "1.70" # When updating extendr's version, this version also needs to be updated extendr-macros = { path = "./extendr-macros", version = "0.6.0" } -libR-sys = "0.7.0" - -[patch.crates-io] # When uncommenting this, do not forget to uncomment the same line in # ./tests/extendrtests/src/rust/Cargo.toml, and "Run R integration tests using # {rextendr}" on .github/workflows/test.yml ! diff --git a/extendr-api/Cargo.toml b/extendr-api/Cargo.toml index 65d2386954..a1ff60a42c 100644 --- a/extendr-api/Cargo.toml +++ b/extendr-api/Cargo.toml @@ -38,6 +38,11 @@ result_condition = [] # but do not add functionality (such as `libR-sys/use-bindgen`) are excluded full-functionality = ["graphics", "either", "ndarray", "num-complex", "serde"] +# Parts of the R-API are locked behind non-API, as CRAN frowns upon the presence +# of non-API items in packages. You may enable this feature, to generate +# bindings to these non-API items, and their usage in extendr +non-api = ["libR-sys/non-api"] + # libc is needed to allocate a DevDesc (c.f., https://bugs.r-project.org/show_bug.cgi?id=18292) graphics = ["libc"] diff --git a/extendr-api/src/wrapper/altrep.rs b/extendr-api/src/wrapper/altrep.rs index 3aa04dca63..8d723ab3da 100644 --- a/extendr-api/src/wrapper/altrep.rs +++ b/extendr-api/src/wrapper/altrep.rs @@ -49,8 +49,9 @@ pub struct Altrep { /// Implement one or more of these methods to generate an Altrep class. /// This is likely to be unstable for a while. pub trait AltrepImpl: Clone + std::fmt::Debug { + #[cfg(feature = "non-api")] /// Constructor that is called when loading an Altrep object from a file. - fn unserialize_ex( + unsafe fn unserialize_ex( class: Robj, state: Robj, attributes: Robj, @@ -554,6 +555,7 @@ impl Altrep { use std::os::raw::c_int; use std::os::raw::c_void; + #[cfg(feature = "non-api")] unsafe extern "C" fn altrep_UnserializeEX( class: SEXP, state: SEXP, @@ -677,6 +679,7 @@ impl Altrep { _ => panic!("expected Altvec compatible type"), }; + #[cfg(feature = "non-api")] R_set_altrep_UnserializeEX_method(class_ptr, Some(altrep_UnserializeEX::)); R_set_altrep_Unserialize_method(class_ptr, Some(altrep_Unserialize::)); R_set_altrep_Serialized_state_method( diff --git a/extendr-api/src/wrapper/environment.rs b/extendr-api/src/wrapper/environment.rs index 8c818d7bce..0b906fe867 100644 --- a/extendr-api/src/wrapper/environment.rs +++ b/extendr-api/src/wrapper/environment.rs @@ -80,6 +80,7 @@ impl Environment { } } + #[cfg(feature = "non-api")] /// Set the enclosing (parent) environment. pub fn set_parent(&mut self, parent: Environment) -> &mut Self { single_threaded(|| unsafe { @@ -98,8 +99,9 @@ impl Environment { } } + #[cfg(feature = "non-api")] /// Set the environment flags. - pub fn set_envflags(&mut self, flags: i32) -> &mut Self { + pub unsafe fn set_envflags(&mut self, flags: i32) -> &mut Self { single_threaded(|| unsafe { let sexp = self.robj.get_mut(); SET_ENVFLAGS(sexp, flags); diff --git a/extendr-api/src/wrapper/promise.rs b/extendr-api/src/wrapper/promise.rs index 362338a37b..8b14f90628 100644 --- a/extendr-api/src/wrapper/promise.rs +++ b/extendr-api/src/wrapper/promise.rs @@ -17,6 +17,7 @@ impl Promise { /// assert_eq!(promise.value(), r!(1)); /// } /// ``` + #[cfg(feature = "non-api")] pub fn from_parts(code: Robj, environment: Environment) -> Result { single_threaded(|| unsafe { let sexp = Rf_allocSExp(SEXPTYPE::PROMSXP); diff --git a/extendr-api/tests/debug_tests.rs b/extendr-api/tests/debug_tests.rs index df985f829b..eaec8d1d92 100644 --- a/extendr-api/tests/debug_tests.rs +++ b/extendr-api/tests/debug_tests.rs @@ -19,7 +19,9 @@ fn test_debug() { assert_eq!(format!("{:?}", r), "base_env()"); let r = Environment::new_with_parent(global_env()); assert_eq!(format!("{:?}", r), ""); + #[cfg(feature = "non-api")] let r = Promise::from_parts(r!(1), global_env())?; + #[cfg(feature = "non-api")] assert_eq!(format!("{:?}", r), "Promise { code: 1, environment: global_env() }"); let r : Language = lang!("x").try_into()?; assert_eq!(format!("{:?}", r), "lang!(sym!(x))"); diff --git a/tests/extendrtests/README.md b/tests/extendrtests/README.md index b25ab1b16b..048cce06f9 100644 --- a/tests/extendrtests/README.md +++ b/tests/extendrtests/README.md @@ -12,6 +12,26 @@ The wrapper scripts calling the Rust functions are located here: The test functions that verify that the wrapper and Rust functions work correctly are located here: +## Using local extendr packages + +While testing new features, you may want to explore these features using +`rextendr::rust_function` / `rextendr::rust_source`, you may run this + +```r +options( + rextendr.patch.crates_io = + list( + `extendr-api` = list(path = "extendr-api" |> normalizePath()), + `extendr-macros` = list(path = "extendr-macros" |> normalizePath()), + `extendr-engine` = list(path = "extendr-engine" |> normalizePath()) + # uncomment this, if you have locally sourced libR-sys crate.. + # ,`libR-sys` = list(path = "libR-sys" |> normalizePath()) + ) +) +``` + +to ensure that the R-session knows about local extendr. + ## Running tests locally The `Cargo.toml` file hard-codes the relative path of the `extendr` libraries. You can build and install `extendrtests` from RStudio as normal using the menu items in the "Build" menu. However, "Check" does not work. To check this project locally you need to run: diff --git a/tests/extendrtests/src/Makevars b/tests/extendrtests/src/Makevars index 645d29af88..15d20edeff 100644 --- a/tests/extendrtests/src/Makevars +++ b/tests/extendrtests/src/Makevars @@ -8,7 +8,7 @@ all: C_clean $(SHLIB): $(STATLIB) $(STATLIB): - cargo build --lib --manifest-path=./rust/Cargo.toml --target-dir $(TARGET_DIR) --color=always + cargo build --lib --manifest-path=./rust/Cargo.toml --target-dir $(TARGET_DIR) --color=auto C_clean: rm -Rf $(SHLIB) $(STATLIB) $(OBJECTS) diff --git a/xtask/src/commands/r_cmd_check.rs b/xtask/src/commands/r_cmd_check.rs index feeca5af14..b6be6f2b7b 100644 --- a/xtask/src/commands/r_cmd_check.rs +++ b/xtask/src/commands/r_cmd_check.rs @@ -59,6 +59,21 @@ fn construct_check_dir_path, P: AsRef>( Ok(path.adjust_for_r()) } +#[derive(Debug)] +enum RCmdCheckError { + MissingRPackages, +} +impl std::fmt::Display for RCmdCheckError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + RCmdCheckError::MissingRPackages => { + write!(f, "Missing required R-packages, please install them.") + } + } + } +} +impl Error for RCmdCheckError {} + fn run_r_cmd_check( shell: &Shell, no_build_vignettes: bool, @@ -79,6 +94,37 @@ fn run_r_cmd_check( Some(cd) => format!("'{}'", cd), _ => "NULL".to_string(), }; + + let has_prerequisites = shell + .cmd("Rscript") + .args([ + "-e", + r#"requireNamespace("devtools"); + requireNamespace("rcmdcheck"); + requireNamespace("patrick"); + requireNamespace("lobstr"); + requireNamespace("rextendr")"#, + ]) + .run() + .is_ok(); + + if !has_prerequisites { + println!( + r#"R installation is missing necessary packages. +RScript -e 'options(repos = list(CRAN="http://cran.rstudio.com/"))' + -e 'install.packages("devtools")' + -e 'install.packages("rcmdcheck")' + -e 'install.packages("patrick")' + -e 'install.packages("lobstr")' + -e 'install.packages("rextendr")' + +Alternatively, install development version on rextendr +Rscript -e 'options(repos = list(CRAN="http://cran.rstudio.com/"))' + -e 'remotes::install_github("extendr/rextendr")'"# + ); + return Err(RCmdCheckError::MissingRPackages.into()); + } + shell .cmd("Rscript") .arg("-e") From 53bc326c5cdb4ac8684219d283d0ad21450072c1 Mon Sep 17 00:00:00 2001 From: CGMossa Date: Sat, 29 Jun 2024 23:26:42 +0200 Subject: [PATCH 7/7] Various maintenance (#754) * xtask: add helpful message when missing dependencies for `r-cmd-check` * Makevars: color is auto, not always * README: add instruction to use developer extendr in rextendr. * extendr-api: added `non-api` flag * ci: temporary hack to get past this circular testing BS * Cargo.toml: we will no longer rely on `patch.crates-io`. * ci: added ` REXTENDR_SKIP_DEV_TESTS: TRUE` --- .gitignore | 6 ++++++ Cargo.toml | 6 +++--- extendr-api/Cargo.toml | 3 ++- extendr-macros/Cargo.toml | 2 +- tests/extendrtests/src/rust/Cargo.toml | 9 +++------ xtask/Cargo.toml | 6 +++--- 6 files changed, 18 insertions(+), 14 deletions(-) diff --git a/.gitignore b/.gitignore index 245aec3d38..2f08e91ee7 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,9 @@ +### macOS ### +# General +.DS_Store +.AppleDouble +.LSOverride + # Generated by Cargo # will have compiled files and executables **/target/ diff --git a/Cargo.toml b/Cargo.toml index 32c0be815d..ace9aa9ab9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ resolver = "2" members = ["extendr-api", "extendr-engine", "extendr-macros", "xtask"] [workspace.package] -version = "0.6.0" +version = "0.7.0" authors = [ "andy-thomason ", "Thomas Down", @@ -21,9 +21,9 @@ rust-version = "1.70" [workspace.dependencies] # When updating extendr's version, this version also needs to be updated -extendr-macros = { path = "./extendr-macros", version = "0.6.0" } +extendr-macros = { path = "./extendr-macros", version = "0.7.0" } # When uncommenting this, do not forget to uncomment the same line in # ./tests/extendrtests/src/rust/Cargo.toml, and "Run R integration tests using # {rextendr}" on .github/workflows/test.yml ! -libR-sys = { git = "https://github.com/extendr/libR-sys", rev = "975af4e555e4fed58084aef9596a9a71602666be" } +libR-sys = { version = "0.7" } diff --git a/extendr-api/Cargo.toml b/extendr-api/Cargo.toml index a1ff60a42c..cf3b9e6484 100644 --- a/extendr-api/Cargo.toml +++ b/extendr-api/Cargo.toml @@ -20,6 +20,7 @@ libc = { version = "0.2", optional = true } ndarray = { version = "0.15.3", optional = true } num-complex = { version = "0.4", optional = true } serde = { version = "1.0", features = ["derive"], optional = true } +faer = { version = "0.19", optional = true } [dev-dependencies] extendr-engine = { path = "../extendr-engine" } @@ -63,4 +64,4 @@ features = ["full-functionality", "libR-sys/use-bindgen"] [[test]] name = "non_api_tests" -required-features = ["non-api"] \ No newline at end of file +required-features = ["non-api"] diff --git a/extendr-macros/Cargo.toml b/extendr-macros/Cargo.toml index 939f13f9ee..5c2d4cbb54 100644 --- a/extendr-macros/Cargo.toml +++ b/extendr-macros/Cargo.toml @@ -12,7 +12,7 @@ proc_macro = true [dependencies] syn = { version = "2.0", features = ["full", "extra-traits"] } -quote = "1.0.6" +quote = "1.0" proc-macro2 = { version = "1.0" } [dev-dependencies] diff --git a/tests/extendrtests/src/rust/Cargo.toml b/tests/extendrtests/src/rust/Cargo.toml index 93136c806e..90ae999ff8 100644 --- a/tests/extendrtests/src/rust/Cargo.toml +++ b/tests/extendrtests/src/rust/Cargo.toml @@ -3,7 +3,7 @@ resolver = "2" [package] name = "extendrtests" -version = "0.6.0" +version = "0.7.0" authors = [ "andy-thomason ", "Claus O. Wilke ", @@ -16,11 +16,8 @@ publish = false crate-type = ["staticlib"] [dependencies] -extendr-api = { version = "*", features = ["graphics", "ndarray", "either"] } - -# TODO: I couldn't find any nice way to add the condition based on the R version -# except for using libR-sys just for "DEP_R_*" envvars. -libR-sys = "*" +extendr-api = { version = "*", features = ["graphics", "ndarray", "faer", "either"] } +faer = "0.19" [patch.crates-io] ## This is configured to work with RStudio features. diff --git a/xtask/Cargo.toml b/xtask/Cargo.toml index a67d44eb9b..017a33f38a 100644 --- a/xtask/Cargo.toml +++ b/xtask/Cargo.toml @@ -10,6 +10,6 @@ authors.workspace = true publish = false [dependencies] -clap = { version = "4.4.6", features = ["derive"] } -toml_edit = "0.22.6" -xshell = "0.2.5" +clap = { version = "4.4", features = ["derive"] } +toml_edit = "0.22" +xshell = "0.2"