diff --git a/crates/ark/src/debug.rs b/crates/ark/src/debug.rs index 5e457d3da..6b95ea64a 100644 --- a/crates/ark/src/debug.rs +++ b/crates/ark/src/debug.rs @@ -32,6 +32,7 @@ static _ARK_DISPLAY_VALUE: unsafe extern "C" fn(x: libr::SEXP) -> *const ffi::c_ // Implementations for entry points in `debug.c`. #[cfg_attr(not(test), no_mangle)] +#[allow(clippy::not_unsafe_ptr_arg_deref)] pub extern "C-unwind" fn ark_print_rs(x: libr::SEXP) -> *const ffi::c_char { capture_console_output(|| { unsafe { libr::Rf_PrintValue(x) }; @@ -81,6 +82,7 @@ pub extern "C-unwind" fn ark_trace_back_rs() -> *const ffi::c_char { } #[cfg_attr(not(test), no_mangle)] +#[allow(clippy::not_unsafe_ptr_arg_deref)] pub extern "C-unwind" fn ark_display_value_rs(x: libr::SEXP) -> *const ffi::c_char { let value = unsafe { let kind = tidy_kind(r_typeof(x)); diff --git a/crates/ark/src/sys/unix/console.rs b/crates/ark/src/sys/unix/console.rs index dd8ff2045..59223c335 100644 --- a/crates/ark/src/sys/unix/console.rs +++ b/crates/ark/src/sys/unix/console.rs @@ -155,6 +155,7 @@ pub extern "C-unwind" fn r_cleanup_for_tests(_save_act: i32, _status: i32, _run_ } /// On Unix, we assume that the buffer to write to the console is /// already in UTF-8 +#[allow(clippy::not_unsafe_ptr_arg_deref)] pub fn console_to_utf8(x: *const c_char) -> anyhow::Result { let x = unsafe { CStr::from_ptr(x) }; diff --git a/crates/ark/src/variables/r_variables.rs b/crates/ark/src/variables/r_variables.rs index bc79c515c..8ab598499 100644 --- a/crates/ark/src/variables/r_variables.rs +++ b/crates/ark/src/variables/r_variables.rs @@ -303,7 +303,7 @@ impl RVariables { let env = self.env.get().clone(); let result = RFunction::new("base", "rm") - .param("list", CharacterVector::create(variables).cast()) + .param("list", CharacterVector::create(variables)) .param("envir", env) .call(); diff --git a/crates/ark/src/variables/variable.rs b/crates/ark/src/variables/variable.rs index 8305fb4df..7589aee43 100644 --- a/crates/ark/src/variables/variable.rs +++ b/crates/ark/src/variables/variable.rs @@ -409,6 +409,7 @@ impl WorkspaceVariableDisplayType { /// - value: The R object to create the display type and type info for. /// - include_length: Whether to include the length of the object in the /// display type. + #[allow(clippy::not_unsafe_ptr_arg_deref)] pub fn from(value: SEXP, include_length: bool) -> Self { match Self::try_from_method(value, include_length) { Err(err) => log::error!( diff --git a/crates/harp/src/exec.rs b/crates/harp/src/exec.rs index 02b6063d6..1fb82c735 100644 --- a/crates/harp/src/exec.rs +++ b/crates/harp/src/exec.rs @@ -102,7 +102,7 @@ pub fn try_eval(expr: SEXP, env: SEXP) -> crate::Result { if let Err(Error::TryCatchError { ref mut call, .. }) = res { if call.is_none() { - *call = Some(unsafe { r_stringify(expr, "\n")? }); + *call = Some(r_stringify(expr, "\n")?); } } diff --git a/crates/harp/src/object.rs b/crates/harp/src/object.rs index 496cac478..e5d45a511 100644 --- a/crates/harp/src/object.rs +++ b/crates/harp/src/object.rs @@ -370,7 +370,7 @@ impl RObject { /// Address in hexadecimal format pub fn address(&self) -> String { - format!("{:p}", self.sexp as *const _) + format!("{:p}", self.sexp) } /// String accessor; get a string value from a vector of strings. diff --git a/crates/harp/src/r.rs b/crates/harp/src/r.rs index 776df14fd..45dd5eedd 100644 --- a/crates/harp/src/r.rs +++ b/crates/harp/src/r.rs @@ -118,7 +118,7 @@ pub fn attrib_for_each(x: SEXP, mut f: F) { ) -> SEXP { let f = &mut *(data as *mut F); f(tag, val); - std::ptr::null_mut() + SEXP::null() } let data = &mut f as *mut F as *mut std::ffi::c_void; libr::R_mapAttrib(x, Some(trampoline::), data); diff --git a/crates/harp/src/raii.rs b/crates/harp/src/raii.rs index 148a0d944..7ca0bd85d 100644 --- a/crates/harp/src/raii.rs +++ b/crates/harp/src/raii.rs @@ -49,6 +49,7 @@ impl RLocal where T: Copy, { + #[allow(clippy::not_unsafe_ptr_arg_deref)] pub fn new(new_value: T, variable: *mut T) -> RLocal { unsafe { let old_value = libr::get(variable); diff --git a/crates/harp/src/utils.rs b/crates/harp/src/utils.rs index 114147209..b4ebdae7e 100644 --- a/crates/harp/src/utils.rs +++ b/crates/harp/src/utils.rs @@ -347,22 +347,22 @@ pub fn r_formals(object: SEXP) -> Result> { Ok(arguments) } -pub unsafe fn r_envir_name(envir: SEXP) -> Result { +pub fn r_envir_name(envir: SEXP) -> Result { r_assert_type(envir, &[ENVSXP])?; if r_env_is_pkg_env(envir) { let name = RObject::from(r_pkg_env_name(envir)); - return name.to::(); + return unsafe { name.to::() }; } if r_env_is_ns_env(envir) { let name = RObject::from(r_ns_env_name(envir)); - return name.to::(); + return unsafe { name.to::() }; } - let name = Rf_getAttrib(envir, r_symbol!("name")); + let name = unsafe { Rf_getAttrib(envir, r_symbol!("name")) }; if r_typeof(name) == STRSXP { - let name = RObject::view(name).to::()?; + let name = unsafe { RObject::view(name).to::()? }; return Ok(name); } @@ -381,12 +381,12 @@ pub fn r_envir_get(symbol: &str, envir: SEXP) -> Option { } } -pub unsafe fn r_envir_set(symbol: &str, value: SEXP, envir: SEXP) { - Rf_defineVar(r_symbol!(symbol), value, envir); +pub fn r_envir_set(symbol: &str, value: SEXP, envir: SEXP) { + unsafe { Rf_defineVar(r_symbol!(symbol), value, envir) }; } -pub unsafe fn r_envir_remove(symbol: &str, envir: SEXP) { - R_removeVarFromFrame(r_symbol!(symbol), envir); +pub fn r_envir_remove(symbol: &str, envir: SEXP) { + unsafe { R_removeVarFromFrame(r_symbol!(symbol), envir) }; } /// Get names of a vector @@ -445,10 +445,10 @@ pub fn r_names2(x: SEXP) -> SEXP { out } -pub unsafe fn r_stringify(object: SEXP, delimiter: &str) -> Result { +pub fn r_stringify(object: SEXP, delimiter: &str) -> Result { // handle SYMSXPs upfront if r_typeof(object) == SYMSXP { - return RObject::view(object).to::(); + return unsafe { RObject::view(object).to::() }; } // call format on the object @@ -457,20 +457,24 @@ pub unsafe fn r_stringify(object: SEXP, delimiter: &str) -> Result { .call()?; // paste into a single string - let object = RFunction::new("base", "paste") - .add(object) - .param("collapse", delimiter) - .call()? - .to::()?; + let object = unsafe { + RFunction::new("base", "paste") + .add(object) + .param("collapse", delimiter) + .call()? + .to::()? + }; Ok(object) } -pub unsafe fn r_inspect(object: SEXP) { - let mut protect = RProtect::new(); - let inspect = protect.add(Rf_lang2(r_symbol!("inspect"), object)); - let internal = protect.add(Rf_lang2(r_symbol!(".Internal"), inspect)); - Rf_eval(internal, R_BaseEnv); +pub fn r_inspect(object: SEXP) { + unsafe { + let mut protect = RProtect::new(); + let inspect = protect.add(Rf_lang2(r_symbol!("inspect"), object)); + let internal = protect.add(Rf_lang2(r_symbol!(".Internal"), inspect)); + Rf_eval(internal, R_BaseEnv); + } } pub fn r_is_promise(x: SEXP) -> bool { @@ -490,9 +494,9 @@ pub fn r_promise_expr(x: SEXP) -> SEXP { unsafe { R_PromiseExpr(x) } } -pub unsafe fn r_promise_force(x: SEXP) -> harp::Result { +pub fn r_promise_force(x: SEXP) -> harp::Result { // Expect that the promise protects its own result - harp::try_eval(x, R_EmptyEnv) + harp::try_eval(x, unsafe { R_EmptyEnv }) } pub fn r_promise_force_with_rollback(x: SEXP) -> harp::Result { @@ -506,30 +510,30 @@ pub fn r_promise_force_with_rollback(x: SEXP) -> harp::Result { } } -pub unsafe fn r_promise_is_lazy_load_binding(x: SEXP) -> bool { +pub fn r_promise_is_lazy_load_binding(x: SEXP) -> bool { // `rlang:::promise_expr("across", asNamespace("dplyr"))` // returns: // `lazyLoadDBfetch(c(105202L, 4670L), datafile, compressed, envhook)` // We can take advantage of this to identify promises in namespaces // that correspond to symbols we should evaluate when generating completions. - let expr = PRCODE(x); + let expr = unsafe { PRCODE(x) }; if r_typeof(expr) != LANGSXP { return false; } - if Rf_xlength(expr) == 0 { + if unsafe { Rf_xlength(expr) } == 0 { return false; } - let expr = CAR(expr); + let expr = unsafe { CAR(expr) }; if r_typeof(expr) != SYMSXP { return false; } - expr == r_symbol!("lazyLoadDBfetch") + expr == unsafe { r_symbol!("lazyLoadDBfetch") } } pub fn r_bytecode_expr(x: SEXP) -> SEXP { @@ -563,46 +567,50 @@ pub fn r_env_binding_is_active(env: SEXP, sym: SEXP) -> harp::Result { } } -pub unsafe fn r_env_is_pkg_env(env: SEXP) -> bool { - R_IsPackageEnv(env) == Rboolean_TRUE || env == R_BaseEnv +pub fn r_env_is_pkg_env(env: SEXP) -> bool { + unsafe { R_IsPackageEnv(env) == Rboolean_TRUE || env == R_BaseEnv } } -pub unsafe fn r_pkg_env_name(env: SEXP) -> SEXP { - if env == R_BaseEnv { - // `R_BaseEnv` is not handled by `R_PackageEnvName()`, but most of the time we want to - // treat it like a package namespace - return r_char!("base"); - } +pub fn r_pkg_env_name(env: SEXP) -> SEXP { + unsafe { + if env == R_BaseEnv { + // `R_BaseEnv` is not handled by `R_PackageEnvName()`, but most of the time we want to + // treat it like a package namespace + return r_char!("base"); + } - let name = R_PackageEnvName(env); + let name = R_PackageEnvName(env); - if name == libr::R_NilValue { - // Should be very unlikely, but `NULL` can be returned - return r_char!(""); - } + if name == libr::R_NilValue { + // Should be very unlikely, but `NULL` can be returned + return r_char!(""); + } - STRING_ELT(name, 0) + STRING_ELT(name, 0) + } } -pub unsafe fn r_env_is_ns_env(env: SEXP) -> bool { +pub fn r_env_is_ns_env(env: SEXP) -> bool { // Does handle `R_BaseNamespace` // https://github.com/wch/r-source/blob/1cb35ff692d3eb3ab546e0db4761102b5ea4ac89/src/main/envir.c#L3689 - R_IsNamespaceEnv(env) == Rboolean_TRUE + unsafe { R_IsNamespaceEnv(env) == Rboolean_TRUE } } -pub unsafe fn r_ns_env_name(env: SEXP) -> SEXP { +pub fn r_ns_env_name(env: SEXP) -> SEXP { // Does handle `R_BaseNamespace` // https://github.com/wch/r-source/blob/1cb35ff692d3eb3ab546e0db4761102b5ea4ac89/src/main/envir.c#L3720 - let mut protect = RProtect::new(); + unsafe { + let mut protect = RProtect::new(); - let spec = protect.add(R_NamespaceEnvSpec(env)); + let spec = protect.add(R_NamespaceEnvSpec(env)); - if spec == libr::R_NilValue { - // Should be very unlikely, but `NULL` can be returned - return r_char!(""); - } + if spec == libr::R_NilValue { + // Should be very unlikely, but `NULL` can be returned + return r_char!(""); + } - STRING_ELT(spec, 0) + STRING_ELT(spec, 0) + } } /// Returns `true` if `f` returns `true` for any node of the pairlist diff --git a/crates/libr/src/r.rs b/crates/libr/src/r.rs index 81b6c0a16..0dd1a1836 100644 --- a/crates/libr/src/r.rs +++ b/crates/libr/src/r.rs @@ -473,219 +473,219 @@ constant_globals::generate! { pub static R_NaInt: std::ffi::c_int; #[doc = "The \"global\" environment"] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_GlobalEnv: SEXP; #[doc = "An empty environment at the root of the\nenvironment tree"] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_EmptyEnv: SEXP; #[doc = "The base environment; formerly R_NilValue"] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_BaseEnv: SEXP; #[doc = "The (fake) namespace for base"] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_BaseNamespace: SEXP; #[doc = "Registry for registered namespaces"] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_NamespaceRegistry: SEXP; #[doc = "The nil object"] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_NilValue: SEXP; #[doc = "Unbound marker"] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_UnboundValue: SEXP; #[doc = "Missing argument marker"] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_MissingArg: SEXP; #[doc = "To be found in BC interp. state\n(marker)"] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_InBCInterpreter: SEXP; #[doc = "Use current expression (marker)"] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_CurrentExpression: SEXP; #[doc = "Marker for restarted function calls"] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_RestartToken: SEXP; #[doc = "\"as.character\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_AsCharacterSymbol: SEXP; #[doc = "\"@\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_AtsignSymbol: SEXP; #[doc = "\"base\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_BaseSymbol: SEXP; #[doc = "\"{\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_BraceSymbol: SEXP; #[doc = "\"\\[\\[\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_Bracket2Symbol: SEXP; #[doc = "\"\\[\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_BracketSymbol: SEXP; #[doc = "\"class\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_ClassSymbol: SEXP; #[doc = "\".Device\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_DeviceSymbol: SEXP; #[doc = "\"dimnames\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_DimNamesSymbol: SEXP; #[doc = "\"dim\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_DimSymbol: SEXP; #[doc = "\"$\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_DollarSymbol: SEXP; #[doc = "\"...\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_DotsSymbol: SEXP; #[doc = "\"::\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_DoubleColonSymbol: SEXP; #[doc = "\"drop\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_DropSymbol: SEXP; #[doc = "\"eval\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_EvalSymbol: SEXP; #[doc = "\"function\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_FunctionSymbol: SEXP; #[doc = "\".Last.value\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_LastvalueSymbol: SEXP; #[doc = "\"levels\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_LevelsSymbol: SEXP; #[doc = "\"mode\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_ModeSymbol: SEXP; #[doc = "\"na.rm\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_NaRmSymbol: SEXP; #[doc = "\"name\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_NameSymbol: SEXP; #[doc = "\"names\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_NamesSymbol: SEXP; #[doc = "\".__NAMESPACE__.\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_NamespaceEnvSymbol: SEXP; #[doc = "\"package\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_PackageSymbol: SEXP; #[doc = "\"previous\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_PreviousSymbol: SEXP; #[doc = "\"quote\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_QuoteSymbol: SEXP; #[doc = "\"row.names\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_RowNamesSymbol: SEXP; #[doc = "\".Random.seed\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_SeedsSymbol: SEXP; #[doc = "\"sort.list\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_SortListSymbol: SEXP; #[doc = "\"source\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_SourceSymbol: SEXP; #[doc = "\"spec\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_SpecSymbol: SEXP; #[doc = "\":::\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_TripleColonSymbol: SEXP; #[doc = "\"srcfile\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_SrcfileSymbol: SEXP; #[doc = "\"srcref\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_SrcrefSymbol: SEXP; #[doc = "\"tsp\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_TspSymbol: SEXP; #[doc = "\".defined\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_dot_defined: SEXP; #[doc = "\".Method\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_dot_Method: SEXP; #[doc = "\".packageName\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_dot_packageName: SEXP; #[doc = "\".target\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_dot_target: SEXP; #[doc = "\".Generic\""] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_dot_Generic: SEXP; #[doc = "NA_STRING as a CHARSXP"] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_NaString: SEXP; #[doc = "\"\" as a CHARSXP"] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_BlankString: SEXP; #[doc = "\"\" as a STRSXP"] - #[default = std::ptr::null_mut()] + #[default = SEXP::null()] pub static R_BlankScalarString: SEXP; #[default = 4503599627370496] diff --git a/crates/libr/src/types.rs b/crates/libr/src/types.rs index adb7375d3..c5b5102e2 100644 --- a/crates/libr/src/types.rs +++ b/crates/libr/src/types.rs @@ -50,7 +50,37 @@ pub const FUNSXP: u32 = 99; pub struct SEXPREC { _unused: [u8; 0], } -pub type SEXP = *mut SEXPREC; + +/// SEXP as a newtype rather than a type alias for `*mut SEXPREC`. This way +/// clippy doesn't see it as a raw pointer, avoiding +/// `clippy::not_unsafe_ptr_arg_deref` on every public function that takes a +/// SEXP. `#[repr(transparent)]` guarantees the same ABI as `*mut SEXPREC` +/// across FFI boundaries. +#[repr(transparent)] +#[derive(Copy, Clone, PartialEq, Eq, Hash)] +pub struct SEXP(*mut SEXPREC); + +impl SEXP { + pub const fn null() -> Self { + Self(std::ptr::null_mut()) + } + + pub fn is_null(self) -> bool { + self.0.is_null() + } +} + +impl std::fmt::Debug for SEXP { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "SEXP({:?})", self.0) + } +} + +impl std::fmt::Pointer for SEXP { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + std::fmt::Pointer::fmt(&self.0, f) + } +} #[doc = "R 4.3 redefined `Rcomplex` to a union for compatibility with Fortran.\n But the old definition is compatible both the union version\n and the struct version.\n See: https://github.com/extendr/extendr/issues/524"] #[repr(C)]