Skip to content

Commit e844fb3

Browse files
authored
Completely remove very unsafe Sxpinfo object (#540)
1 parent 3069e1f commit e844fb3

File tree

7 files changed

+7
-83
lines changed

7 files changed

+7
-83
lines changed

Cargo.lock

-21
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/ark/src/srcref.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ fn generate_source(
122122
}
123123

124124
// These don't deparse to a `function` call!
125-
if unsafe { IS_S4_OBJECT(old.sexp) != 0 } {
125+
if harp::utils::r_is_s4(old.sexp) {
126126
return Ok(None);
127127
}
128128

crates/harp/Cargo.toml

-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ rust-version.workspace = true
1212

1313
[dependencies]
1414
anyhow = "1.0.80"
15-
c2rust-bitfields = "0.17.0"
1615
cfg-if = "1.0.0"
1716
ctor = "0.1.26"
1817
harp-macros = { path = "./harp-macros" }

crates/harp/src/environment_iter.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ impl Binding {
7676

7777
let value = env.find(name)?;
7878

79-
if libr::ALTREP(value) != 0 {
79+
if r_is_altrep(value) {
8080
let value = BindingValue::Altrep {
8181
object: RObject::from(value),
8282
data1: RObject::from(R_altrep_data1(value)),

crates/harp/src/symbol.rs

-6
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@ use crate::object::r_length;
2222
use crate::r_symbol;
2323
use crate::utils::r_assert_type;
2424
use crate::utils::r_str_to_owned_utf8_unchecked;
25-
use crate::utils::Sxpinfo;
26-
use crate::utils::HASHASH_MASK;
2725

2826
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
2927
pub struct RSymbol {
@@ -39,10 +37,6 @@ impl RSymbol {
3937
r_assert_type(sexp, &[SYMSXP])?;
4038
Ok(Self::new_unchecked(sexp))
4139
}
42-
43-
pub fn has_hash(&self) -> bool {
44-
unsafe { (Sxpinfo::interpret(&PRINTNAME(self.sexp)).gp() & HASHASH_MASK) == 1 }
45-
}
4640
}
4741

4842
impl Ord for RSymbol {

crates/harp/src/utils.rs

+3-53
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
use std::ffi::CStr;
99
use std::ffi::CString;
1010

11-
use c2rust_bitfields::BitfieldStruct;
1211
use itertools::Itertools;
1312
use libr::*;
1413
use once_cell::sync::Lazy;
@@ -50,55 +49,6 @@ pub fn init_utils() {}
5049
static RE_SYNTACTIC_IDENTIFIER: Lazy<Regex> =
5150
Lazy::new(|| Regex::new(r"^[\p{L}\p{Nl}.][\p{L}\p{Nl}\p{Mn}\p{Mc}\p{Nd}\p{Pc}.]*$").unwrap());
5251

53-
#[derive(Copy, Clone, BitfieldStruct)]
54-
#[repr(C)]
55-
pub struct Sxpinfo {
56-
#[bitfield(name = "rtype", ty = "libc::c_uint", bits = "0..=4")]
57-
#[bitfield(name = "scalar", ty = "libc::c_uint", bits = "5..=5")]
58-
#[bitfield(name = "obj", ty = "libc::c_uint", bits = "6..=6")]
59-
#[bitfield(name = "alt", ty = "libc::c_uint", bits = "7..=7")]
60-
#[bitfield(name = "gp", ty = "libc::c_uint", bits = "8..=23")]
61-
#[bitfield(name = "mark", ty = "libc::c_uint", bits = "24..=24")]
62-
#[bitfield(name = "debug", ty = "libc::c_uint", bits = "25..=25")]
63-
#[bitfield(name = "trace", ty = "libc::c_uint", bits = "26..=26")]
64-
#[bitfield(name = "spare", ty = "libc::c_uint", bits = "27..=27")]
65-
#[bitfield(name = "gcgen", ty = "libc::c_uint", bits = "28..=28")]
66-
#[bitfield(name = "gccls", ty = "libc::c_uint", bits = "29..=31")]
67-
#[bitfield(name = "named", ty = "libc::c_uint", bits = "32..=47")]
68-
#[bitfield(name = "extra", ty = "libc::c_uint", bits = "48..=63")]
69-
pub rtype_scalar_obj_alt_gp_mark_debug_trace_spare_gcgen_gccls_named_extra: [u8; 8],
70-
}
71-
72-
pub static mut ACTIVE_BINDING_MASK: libc::c_uint = 1 << 15;
73-
pub static mut S4_OBJECT_MASK: libc::c_uint = 1 << 4;
74-
pub static mut HASHASH_MASK: libc::c_uint = 1;
75-
76-
impl Sxpinfo {
77-
pub fn interpret(x: &SEXP) -> &Self {
78-
unsafe { (*x as *mut Sxpinfo).as_ref().unwrap() }
79-
}
80-
81-
pub fn is_active(&self) -> bool {
82-
self.gp() & unsafe { ACTIVE_BINDING_MASK } != 0
83-
}
84-
85-
pub fn is_immediate(&self) -> bool {
86-
self.extra() != 0
87-
}
88-
89-
pub fn is_s4(&self) -> bool {
90-
self.gp() & unsafe { S4_OBJECT_MASK } != 0
91-
}
92-
93-
pub fn is_altrep(&self) -> bool {
94-
self.alt() != 0
95-
}
96-
97-
pub fn is_object(&self) -> bool {
98-
self.obj() != 0
99-
}
100-
}
101-
10252
#[harp::register]
10353
pub extern "C" fn harp_log_trace(msg: SEXP) -> crate::error::Result<SEXP> {
10454
let msg = String::try_from(RObject::view(msg))?;
@@ -176,15 +126,15 @@ pub fn r_is_null(object: SEXP) -> bool {
176126
}
177127

178128
pub fn r_is_altrep(object: SEXP) -> bool {
179-
Sxpinfo::interpret(&object).is_altrep()
129+
unsafe { libr::ALTREP(object) != 0 }
180130
}
181131

182132
pub fn r_is_object(object: SEXP) -> bool {
183-
Sxpinfo::interpret(&object).is_object()
133+
unsafe { libr::OBJECT(object) != 0 }
184134
}
185135

186136
pub fn r_is_s4(object: SEXP) -> bool {
187-
Sxpinfo::interpret(&object).is_s4()
137+
unsafe { libr::IS_S4_OBJECT(object) != 0 }
188138
}
189139

190140
pub fn r_is_unbound(object: SEXP) -> bool {

crates/libr/src/r.rs

+2
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,8 @@ functions::generate! {
188188

189189
pub fn Rf_xlength(arg1: SEXP) -> R_xlen_t;
190190

191+
pub fn OBJECT(x: SEXP) -> std::ffi::c_int;
192+
191193
pub fn ALTREP(x: SEXP) -> std::ffi::c_int;
192194

193195
pub fn ALTREP_CLASS(x: SEXP) -> SEXP;

0 commit comments

Comments
 (0)