Skip to content

Commit

Permalink
Replaces SEXPTYPE and Rboolean to the new enum-versions (#742)
Browse files Browse the repository at this point in the history 
* 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.
  • Loading branch information
@CGMossa
CGMossa committed May 9, 2024
1 parent 6923b8e commit 2a8fb6c
Show file tree
Hide file tree
Showing 31 changed files with 228 additions and 202 deletions.
4 changes: 2 additions & 2 deletions Cargo.toml
View file
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion extendr-api/Cargo.toml
View file
Expand Up @@ -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 }
Expand Down
4 changes: 4 additions & 0 deletions extendr-api/build.rs
View file
Expand Up @@ -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");
}
}
6 changes: 3 additions & 3 deletions extendr-api/src/functions.rs
View file
Expand Up @@ -209,12 +209,12 @@ pub fn blank_scalar_string() -> Robj {
pub fn parse(code: &str) -> Result<Expressions> {
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())),
}
})
Expand Down
46 changes: 19 additions & 27 deletions extendr-api/src/graphics/device_driver.rs
View file
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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,
);
Expand Down Expand Up @@ -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<T: DeviceDriver>(
Expand All @@ -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<T: DeviceDriver>(dd: pDevDesc, code: c_int) {
Expand Down Expand Up @@ -753,12 +745,12 @@ pub trait DeviceDriver: std::marker::Sized {
(*p_dev_desc).gamma = 1.0;

(*p_dev_desc).canClip = match <T>::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 _;

Expand All @@ -773,22 +765,22 @@ 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 <T>::USE_PLOT_HISTORY { 1 } else { 0 };
(*p_dev_desc).displayListOn = <T>::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:
//
// This is set while getGraphicsEvent is actively looking for events.
//
// 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::<T>);
(*p_dev_desc).circle = Some(device_driver_circle::<T>);
Expand Down Expand Up @@ -829,7 +821,7 @@ pub trait DeviceDriver: std::marker::Sized {
(*p_dev_desc).newFrameConfirm = Some(device_driver_new_frame_confirm::<T>);

// UTF-8 support
(*p_dev_desc).hasTextUTF8 = if <T>::ACCEPT_UTF8_TEXT { 1 } else { 0 };
(*p_dev_desc).hasTextUTF8 = <T>::ACCEPT_UTF8_TEXT.into();
(*p_dev_desc).textUTF8 = if <T>::ACCEPT_UTF8_TEXT {
Some(device_driver_text::<T>)
} else {
Expand All @@ -840,7 +832,7 @@ pub trait DeviceDriver: std::marker::Sized {
} else {
None
};
(*p_dev_desc).wantSymbolUTF8 = if <T>::ACCEPT_UTF8_TEXT { 1 } else { 0 };
(*p_dev_desc).wantSymbolUTF8 = <T>::ACCEPT_UTF8_TEXT.into();

// R internals says:
//
Expand All @@ -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::<T>);
Expand Down Expand Up @@ -900,8 +892,8 @@ pub trait DeviceDriver: std::marker::Sized {
(*p_dev_desc).deviceVersion = R_GE_definitions as _;

(*p_dev_desc).deviceClip = match <T>::CLIPPING_STRATEGY {
ClippingStrategy::Device => 1,
_ => 0,
ClippingStrategy::Device => Rboolean::TRUE,
_ => Rboolean::FALSE,
};
}

Expand Down
52 changes: 26 additions & 26 deletions extendr-api/src/graphics/mod.rs
View file
Expand Up @@ -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<LineEnd> 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<LineJoin> 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,
}
}
}
Expand Down Expand Up @@ -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,
}
}

Expand All @@ -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,
Expand Down Expand Up @@ -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
}

Expand All @@ -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
}

Expand Down Expand Up @@ -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(),
)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -802,21 +802,21 @@ impl Device {
/// Get the width of a unicode string.
pub fn text_width<T: AsRef<str>>(&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<T: AsRef<str>>(&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<T: AsRef<str>>(&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,
Expand Down
2 changes: 1 addition & 1 deletion extendr-api/src/io/load.rs
View file
Expand Up @@ -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::<R>),
InBytes: Some(inbytes::<R>),
InPersistHookFunc: hook_func,
Expand Down
8 changes: 1 addition & 7 deletions extendr-api/src/io/mod.rs
View file
@@ -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;
Expand Down
15 changes: 8 additions & 7 deletions extendr-api/src/iter.rs
View file
Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -167,17 +168,17 @@ 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,
len,
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,
Expand Down

0 comments on commit 2a8fb6c

Please sign in to comment.