From 1fee040e1269ce88ebc4e921461a709f34bcc776 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Thu, 26 Jan 2023 15:29:17 +0100 Subject: [PATCH 01/15] fix and test impl From> for RObject --- .../amalthea/crates/harp/src/object.rs | 20 +++++++++++++++++-- .../amalthea/crates/harp/src/utils.rs | 19 ++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/extensions/positron-r/amalthea/crates/harp/src/object.rs b/extensions/positron-r/amalthea/crates/harp/src/object.rs index 4c8a18139368..8199bea4c7e2 100644 --- a/extensions/positron-r/amalthea/crates/harp/src/object.rs +++ b/extensions/positron-r/amalthea/crates/harp/src/object.rs @@ -221,7 +221,7 @@ impl From> for RObject { let vector = Rf_protect(Rf_allocVector(STRSXP, n)); for i in 0..n { let string = value.get_unchecked(i as usize); - let element = Rf_mkCharLenCE(string.as_ptr() as *mut c_char, n as i32, cetype_t_CE_UTF8); + let element = Rf_mkCharLenCE(string.as_ptr() as *mut c_char, string.bytes().len() as i32, cetype_t_CE_UTF8); SET_STRING_ELT(vector, i as R_xlen_t, element); } Rf_unprotect(1); @@ -230,7 +230,6 @@ impl From> for RObject { } } - /// Convert RObject into other types. impl From for SEXP { @@ -347,3 +346,20 @@ impl TryFrom for HashMap { } } } + +#[cfg(test)] +mod tests { + use crate::{r_test, utils::{r_strings_eq}}; + + use super::RObject; + + #[test] + fn test_robject_from() { r_test! { + + let strings = vec![String::from("Apple"), String::from("Orange"), String::from("한")]; + let r_object : RObject = strings.into(); + + assert!(r_strings_eq(*r_object, vec!["Apple", "Orange", "한"])); + + }} +} \ No newline at end of file diff --git a/extensions/positron-r/amalthea/crates/harp/src/utils.rs b/extensions/positron-r/amalthea/crates/harp/src/utils.rs index 3eec9dc501ad..9143442ef193 100644 --- a/extensions/positron-r/amalthea/crates/harp/src/utils.rs +++ b/extensions/positron-r/amalthea/crates/harp/src/utils.rs @@ -48,6 +48,25 @@ pub unsafe fn r_assert_length(object: SEXP, expected: u32) -> Result { Ok(actual) } +pub unsafe fn r_strings_eq(object: SEXP, expected: Vec<&str>) -> bool { + let r_type = r_typeof(object); + match r_type { + STRSXP => { + let n = Rf_xlength(object) as isize; + if n != expected.len() as isize { + return false; + } + for i in 0..n { + if !expected[i as usize].eq(CStr::from_ptr(R_CHAR(STRING_ELT(object, i))).to_str().unwrap()) { + return false; + } + } + true + }, + _ => false + } +} + pub unsafe fn r_typeof(object: SEXP) -> u32 { TYPEOF(object) as u32 } From 75b9befabc6b437deb973f64dc91f1bec40ae35d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Thu, 26 Jan 2023 17:07:24 +0100 Subject: [PATCH 02/15] From> only borrows --- .../amalthea/crates/harp/src/object.rs | 48 +++++++++++++++---- .../amalthea/crates/harp/src/utils.rs | 2 +- 2 files changed, 39 insertions(+), 11 deletions(-) diff --git a/extensions/positron-r/amalthea/crates/harp/src/object.rs b/extensions/positron-r/amalthea/crates/harp/src/object.rs index 8199bea4c7e2..95f10179e98c 100644 --- a/extensions/positron-r/amalthea/crates/harp/src/object.rs +++ b/extensions/positron-r/amalthea/crates/harp/src/object.rs @@ -8,6 +8,7 @@ use std::collections::HashMap; use std::convert::TryFrom; use std::ffi::CStr; +use std::ffi::CString; use std::ops::Deref; use std::ops::DerefMut; use std::os::raw::c_char; @@ -214,14 +215,31 @@ impl From for RObject { } } -impl From> for RObject { - fn from(value: Vec) -> Self { +impl From<&Vec<&str>> for RObject { + fn from(value: &Vec<&str>) -> Self { + unsafe { + let n = value.len() as isize; + let vector = Rf_protect(Rf_allocVector(STRSXP, n)); + for i in 0..n { + let string = value[i as usize]; + let c_string = CString::new(string).unwrap(); + let element = Rf_mkCharLenCE(c_string.as_ptr(), string.len() as i32, cetype_t_CE_UTF8); + SET_STRING_ELT(vector, i as R_xlen_t, element); + } + Rf_unprotect(1); + return RObject::new(vector); + } + } +} + +impl From<&Vec> for RObject { + fn from(value: &Vec) -> Self { unsafe { let n = value.len() as isize; let vector = Rf_protect(Rf_allocVector(STRSXP, n)); for i in 0..n { let string = value.get_unchecked(i as usize); - let element = Rf_mkCharLenCE(string.as_ptr() as *mut c_char, string.bytes().len() as i32, cetype_t_CE_UTF8); + let element = Rf_mkCharLenCE(string.as_ptr() as *mut c_char, string.len() as i32, cetype_t_CE_UTF8); SET_STRING_ELT(vector, i as R_xlen_t, element); } Rf_unprotect(1); @@ -230,6 +248,7 @@ impl From> for RObject { } } + /// Convert RObject into other types. impl From for SEXP { @@ -354,12 +373,21 @@ mod tests { use super::RObject; #[test] - fn test_robject_from() { r_test! { - - let strings = vec![String::from("Apple"), String::from("Orange"), String::from("한")]; - let r_object : RObject = strings.into(); - - assert!(r_strings_eq(*r_object, vec!["Apple", "Orange", "한"])); - + #[allow(non_snake_case)] + fn test_RObject_from_ref_Vec_str() { r_test! { + let strings = vec!["Apple", "Orange", "한"]; + let strings2 : Vec = strings.iter().map(|&s| { String::from(s)}).collect(); + assert_eq!(strings, strings2); + + let r_strings = RObject::from(&strings); + let r_strings2 = RObject::from(&strings2); + + // just checking they were just borrowed + assert_eq!(strings.len(), 3); + assert_eq!(strings2.len(), 3); + + // check the contents + assert!(r_strings_eq(*r_strings, &strings)); + assert!(r_strings_eq(*r_strings2, &strings)); }} } \ No newline at end of file diff --git a/extensions/positron-r/amalthea/crates/harp/src/utils.rs b/extensions/positron-r/amalthea/crates/harp/src/utils.rs index 9143442ef193..a1211f7aad6d 100644 --- a/extensions/positron-r/amalthea/crates/harp/src/utils.rs +++ b/extensions/positron-r/amalthea/crates/harp/src/utils.rs @@ -48,7 +48,7 @@ pub unsafe fn r_assert_length(object: SEXP, expected: u32) -> Result { Ok(actual) } -pub unsafe fn r_strings_eq(object: SEXP, expected: Vec<&str>) -> bool { +pub unsafe fn r_strings_eq(object: SEXP, expected: &Vec<&str>) -> bool { let r_type = r_typeof(object); match r_type { STRSXP => { From df2dec1abcda75bb93b53db34d1ce8de0496888b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Thu, 26 Jan 2023 17:15:50 +0100 Subject: [PATCH 03/15] rm unnecessary brace --- extensions/positron-r/amalthea/crates/harp/src/object.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/positron-r/amalthea/crates/harp/src/object.rs b/extensions/positron-r/amalthea/crates/harp/src/object.rs index 95f10179e98c..ef28ee2b94eb 100644 --- a/extensions/positron-r/amalthea/crates/harp/src/object.rs +++ b/extensions/positron-r/amalthea/crates/harp/src/object.rs @@ -368,7 +368,7 @@ impl TryFrom for HashMap { #[cfg(test)] mod tests { - use crate::{r_test, utils::{r_strings_eq}}; + use crate::{r_test, utils::r_strings_eq}; use super::RObject; From 07d1f818b5a267d217ed941682603c659220eecc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Thu, 26 Jan 2023 17:18:30 +0100 Subject: [PATCH 04/15] + new line --- extensions/positron-r/amalthea/crates/harp/src/object.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/positron-r/amalthea/crates/harp/src/object.rs b/extensions/positron-r/amalthea/crates/harp/src/object.rs index ef28ee2b94eb..9aa214070d5d 100644 --- a/extensions/positron-r/amalthea/crates/harp/src/object.rs +++ b/extensions/positron-r/amalthea/crates/harp/src/object.rs @@ -390,4 +390,4 @@ mod tests { assert!(r_strings_eq(*r_strings, &strings)); assert!(r_strings_eq(*r_strings2, &strings)); }} -} \ No newline at end of file +} From 7603ae3bc6e87f740d795bbabf22b53b06941cf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Tue, 7 Feb 2023 15:37:47 +0100 Subject: [PATCH 05/15] impl From> and impl From> --- .../amalthea/crates/harp/src/object.rs | 23 ++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/extensions/positron-r/amalthea/crates/harp/src/object.rs b/extensions/positron-r/amalthea/crates/harp/src/object.rs index 9aa214070d5d..97c5c2434f6b 100644 --- a/extensions/positron-r/amalthea/crates/harp/src/object.rs +++ b/extensions/positron-r/amalthea/crates/harp/src/object.rs @@ -215,8 +215,8 @@ impl From for RObject { } } -impl From<&Vec<&str>> for RObject { - fn from(value: &Vec<&str>) -> Self { +impl From> for RObject { + fn from(value: Vec<&str>) -> Self { unsafe { let n = value.len() as isize; let vector = Rf_protect(Rf_allocVector(STRSXP, n)); @@ -232,8 +232,8 @@ impl From<&Vec<&str>> for RObject { } } -impl From<&Vec> for RObject { - fn from(value: &Vec) -> Self { +impl From> for RObject { + fn from(value: Vec) -> Self { unsafe { let n = value.len() as isize; let vector = Rf_protect(Rf_allocVector(STRSXP, n)); @@ -375,19 +375,16 @@ mod tests { #[test] #[allow(non_snake_case)] fn test_RObject_from_ref_Vec_str() { r_test! { - let strings = vec!["Apple", "Orange", "한"]; + let expected = vec!["Apple", "Orange", "한"]; + let strings = expected.clone(); let strings2 : Vec = strings.iter().map(|&s| { String::from(s)}).collect(); assert_eq!(strings, strings2); - let r_strings = RObject::from(&strings); - let r_strings2 = RObject::from(&strings2); - - // just checking they were just borrowed - assert_eq!(strings.len(), 3); - assert_eq!(strings2.len(), 3); + let r_strings = RObject::from(strings); + let r_strings2 = RObject::from(strings2); // check the contents - assert!(r_strings_eq(*r_strings, &strings)); - assert!(r_strings_eq(*r_strings2, &strings)); + assert!(r_strings_eq(*r_strings, &expected)); + assert!(r_strings_eq(*r_strings2, &expected)); }} } From b509f634765cfd075988061d8223b415af854e42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Tue, 7 Feb 2023 17:34:17 +0100 Subject: [PATCH 06/15] allow for assert_eq!(RObject, ) --- .../amalthea/crates/harp/src/object.rs | 43 +++++++---- .../amalthea/crates/harp/src/utils.rs | 72 +++++++++++++++---- 2 files changed, 88 insertions(+), 27 deletions(-) diff --git a/extensions/positron-r/amalthea/crates/harp/src/object.rs b/extensions/positron-r/amalthea/crates/harp/src/object.rs index 97c5c2434f6b..5cc9e361c8a4 100644 --- a/extensions/positron-r/amalthea/crates/harp/src/object.rs +++ b/extensions/positron-r/amalthea/crates/harp/src/object.rs @@ -102,6 +102,7 @@ unsafe fn unprotect(cell: SEXP) { } +#[derive(Debug)] pub struct RObject { pub sexp: SEXP, pub cell: SEXP, @@ -368,23 +369,39 @@ impl TryFrom for HashMap { #[cfg(test)] mod tests { - use crate::{r_test, utils::r_strings_eq}; + use crate::r_test; use super::RObject; #[test] #[allow(non_snake_case)] - fn test_RObject_from_ref_Vec_str() { r_test! { - let expected = vec!["Apple", "Orange", "한"]; - let strings = expected.clone(); - let strings2 : Vec = strings.iter().map(|&s| { String::from(s)}).collect(); - assert_eq!(strings, strings2); - - let r_strings = RObject::from(strings); - let r_strings2 = RObject::from(strings2); - - // check the contents - assert!(r_strings_eq(*r_strings, &expected)); - assert!(r_strings_eq(*r_strings2, &expected)); + fn test_RObject_from_Vec_str() { r_test! { + let expected = ["Apple", "Orange", "한"]; + + // RObject from Vec<&str> + let r_strings = RObject::from(expected.to_vec()); + assert_eq!(r_strings, expected); // [&str] + assert_eq!(r_strings, expected[..]); // [&str; const N] + assert_eq!(r_strings, expected.to_vec()); // Vec<&str> + + assert_eq!(expected , r_strings); // [&str] + assert_eq!(expected[..] , r_strings); // [&str; const N] + assert_eq!(expected.to_vec(), r_strings); // Vec<&str> + }} + + #[test] + #[allow(non_snake_case)] + fn test_RObject_from_Vec_String() { r_test! { + let expected = [String::from("Apple"), String::from("Orange"), String::from("한")]; + + // RObject from Vec + let r_strings = RObject::from(expected.to_vec()); + assert_eq!(r_strings, expected[..]); // [String] + assert_eq!(r_strings, expected); // [String; const N] + assert_eq!(r_strings, expected.to_vec()); // Vec + + assert_eq!(expected[..] , r_strings); // [String] + assert_eq!(expected , r_strings); // [String; const N] + assert_eq!(expected.to_vec(), r_strings); // Vec }} } diff --git a/extensions/positron-r/amalthea/crates/harp/src/utils.rs b/extensions/positron-r/amalthea/crates/harp/src/utils.rs index a1211f7aad6d..c2585e02b9ab 100644 --- a/extensions/positron-r/amalthea/crates/harp/src/utils.rs +++ b/extensions/positron-r/amalthea/crates/harp/src/utils.rs @@ -48,24 +48,68 @@ pub unsafe fn r_assert_length(object: SEXP, expected: u32) -> Result { Ok(actual) } -pub unsafe fn r_strings_eq(object: SEXP, expected: &Vec<&str>) -> bool { - let r_type = r_typeof(object); - match r_type { - STRSXP => { - let n = Rf_xlength(object) as isize; - if n != expected.len() as isize { - return false; - } - for i in 0..n { - if !expected[i as usize].eq(CStr::from_ptr(R_CHAR(STRING_ELT(object, i))).to_str().unwrap()) { - return false; +// used for [&str] and [String] +macro_rules! partial_eq_RObject_slice { + ($type:ty) => { + impl PartialEq<[$type]> for RObject { + fn eq(&self, other: &[$type]) -> bool { + unsafe { + let object = self.sexp; + if r_typeof(object) != STRSXP { + return false; + } + + let n = Rf_xlength(object) as isize; + if n != other.len() as isize { + return false; + } + for i in 0..n { + if !other[i as usize].eq(CStr::from_ptr(R_CHAR(STRING_ELT(object, i))).to_str().unwrap()) { + return false; + } + } + true } } - true - }, - _ => false + } + + }; +} + +partial_eq_RObject_slice!{ &str } +partial_eq_RObject_slice!{ String } + +macro_rules! partial_eq_RObject_slice2 { + ([$($vars:tt)*] $type:ty) => { + impl<$($vars)*> PartialEq<$type> for RObject { + fn eq(&self, other: &$type) -> bool { + self.eq(&other[..]) + } + } + } } +partial_eq_RObject_slice2!{ [const N: usize] [&str; N]} +partial_eq_RObject_slice2!{ [] Vec<&str> } +partial_eq_RObject_slice2!{ [const N: usize] [String; N]} +partial_eq_RObject_slice2!{ [] Vec} + +macro_rules! partial_eq_RObject_rhs { + ([$($vars:tt)*] $type:ty) => { + impl<$($vars)*> PartialEq for $type { + fn eq(&self, other: &RObject) -> bool { + other.eq(self) + } + } + } +} +partial_eq_RObject_rhs!{ [] [&str] } +partial_eq_RObject_rhs!{ [const N: usize] [&str; N] } +partial_eq_RObject_rhs!{ [] Vec<&str> } + +partial_eq_RObject_rhs!{ [] [String] } +partial_eq_RObject_rhs!{ [const N: usize] [String; N] } +partial_eq_RObject_rhs!{ [] Vec } pub unsafe fn r_typeof(object: SEXP) -> u32 { TYPEOF(object) as u32 From 3492f4d498c440fd88449961ffe5f07a912938f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Wed, 8 Feb 2023 13:04:01 +0100 Subject: [PATCH 07/15] + trait ToCharSxp, and use it to implement a more generic impl From> for RObject where S : ToCharSxp --- .../amalthea/crates/harp/src/object.rs | 37 +++++++++---------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/extensions/positron-r/amalthea/crates/harp/src/object.rs b/extensions/positron-r/amalthea/crates/harp/src/object.rs index 5cc9e361c8a4..e3d069a553c7 100644 --- a/extensions/positron-r/amalthea/crates/harp/src/object.rs +++ b/extensions/positron-r/amalthea/crates/harp/src/object.rs @@ -8,7 +8,6 @@ use std::collections::HashMap; use std::convert::TryFrom; use std::ffi::CStr; -use std::ffi::CString; use std::ops::Deref; use std::ops::DerefMut; use std::os::raw::c_char; @@ -216,32 +215,32 @@ impl From for RObject { } } -impl From> for RObject { - fn from(value: Vec<&str>) -> Self { +pub trait ToCharSxp { + fn to_charsxp(&self) -> SEXP; +} + +impl ToCharSxp for &str { + fn to_charsxp(&self) -> SEXP { unsafe { - let n = value.len() as isize; - let vector = Rf_protect(Rf_allocVector(STRSXP, n)); - for i in 0..n { - let string = value[i as usize]; - let c_string = CString::new(string).unwrap(); - let element = Rf_mkCharLenCE(c_string.as_ptr(), string.len() as i32, cetype_t_CE_UTF8); - SET_STRING_ELT(vector, i as R_xlen_t, element); - } - Rf_unprotect(1); - return RObject::new(vector); + Rf_mkCharLenCE(self.as_ptr() as *mut c_char, self.len() as i32, cetype_t_CE_UTF8) } } } -impl From> for RObject { - fn from(value: Vec) -> Self { +impl ToCharSxp for String { + fn to_charsxp(&self) -> SEXP { + (&self[..]).to_charsxp() + } +} + +impl From> for RObject where S : ToCharSxp { + fn from(value: Vec) -> Self { unsafe { let n = value.len() as isize; let vector = Rf_protect(Rf_allocVector(STRSXP, n)); for i in 0..n { let string = value.get_unchecked(i as usize); - let element = Rf_mkCharLenCE(string.as_ptr() as *mut c_char, string.len() as i32, cetype_t_CE_UTF8); - SET_STRING_ELT(vector, i as R_xlen_t, element); + SET_STRING_ELT(vector, i as R_xlen_t, string.to_charsxp()); } Rf_unprotect(1); return RObject::new(vector); @@ -249,7 +248,6 @@ impl From> for RObject { } } - /// Convert RObject into other types. impl From for SEXP { @@ -379,7 +377,8 @@ mod tests { let expected = ["Apple", "Orange", "한"]; // RObject from Vec<&str> - let r_strings = RObject::from(expected.to_vec()); + let vec = expected.to_vec(); + let r_strings = RObject::from(vec); assert_eq!(r_strings, expected); // [&str] assert_eq!(r_strings, expected[..]); // [&str; const N] assert_eq!(r_strings, expected.to_vec()); // Vec<&str> From e88d8de0b554fc80a064736c0976be4255a6e374 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Wed, 8 Feb 2023 13:04:31 +0100 Subject: [PATCH 08/15] minor --- extensions/positron-r/amalthea/crates/harp/src/object.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/extensions/positron-r/amalthea/crates/harp/src/object.rs b/extensions/positron-r/amalthea/crates/harp/src/object.rs index e3d069a553c7..cb24dbab61bc 100644 --- a/extensions/positron-r/amalthea/crates/harp/src/object.rs +++ b/extensions/positron-r/amalthea/crates/harp/src/object.rs @@ -377,8 +377,7 @@ mod tests { let expected = ["Apple", "Orange", "한"]; // RObject from Vec<&str> - let vec = expected.to_vec(); - let r_strings = RObject::from(vec); + let r_strings = RObject::from(expected.to_vec()); assert_eq!(r_strings, expected); // [&str] assert_eq!(r_strings, expected[..]); // [&str; const N] assert_eq!(r_strings, expected.to_vec()); // Vec<&str> From 2b98d6665e5baba55c64933fdc98676f418dc838 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Wed, 8 Feb 2023 14:28:25 +0100 Subject: [PATCH 09/15] implement PartialEq<{{strings}}> without macros, based on the CharSxpEq trait that is only implemented for &str and String --- .../amalthea/crates/harp/src/object.rs | 8 -- .../amalthea/crates/harp/src/utils.rs | 87 ++++++++----------- 2 files changed, 38 insertions(+), 57 deletions(-) diff --git a/extensions/positron-r/amalthea/crates/harp/src/object.rs b/extensions/positron-r/amalthea/crates/harp/src/object.rs index cb24dbab61bc..3a9e4d031b2c 100644 --- a/extensions/positron-r/amalthea/crates/harp/src/object.rs +++ b/extensions/positron-r/amalthea/crates/harp/src/object.rs @@ -381,10 +381,6 @@ mod tests { assert_eq!(r_strings, expected); // [&str] assert_eq!(r_strings, expected[..]); // [&str; const N] assert_eq!(r_strings, expected.to_vec()); // Vec<&str> - - assert_eq!(expected , r_strings); // [&str] - assert_eq!(expected[..] , r_strings); // [&str; const N] - assert_eq!(expected.to_vec(), r_strings); // Vec<&str> }} #[test] @@ -397,9 +393,5 @@ mod tests { assert_eq!(r_strings, expected[..]); // [String] assert_eq!(r_strings, expected); // [String; const N] assert_eq!(r_strings, expected.to_vec()); // Vec - - assert_eq!(expected[..] , r_strings); // [String] - assert_eq!(expected , r_strings); // [String; const N] - assert_eq!(expected.to_vec(), r_strings); // Vec }} } diff --git a/extensions/positron-r/amalthea/crates/harp/src/utils.rs b/extensions/positron-r/amalthea/crates/harp/src/utils.rs index c2585e02b9ab..6ce2577d0259 100644 --- a/extensions/positron-r/amalthea/crates/harp/src/utils.rs +++ b/extensions/positron-r/amalthea/crates/harp/src/utils.rs @@ -48,68 +48,57 @@ pub unsafe fn r_assert_length(object: SEXP, expected: u32) -> Result { Ok(actual) } -// used for [&str] and [String] -macro_rules! partial_eq_RObject_slice { - ($type:ty) => { - impl PartialEq<[$type]> for RObject { - fn eq(&self, other: &[$type]) -> bool { - unsafe { - let object = self.sexp; - if r_typeof(object) != STRSXP { - return false; - } - - let n = Rf_xlength(object) as isize; - if n != other.len() as isize { - return false; - } - for i in 0..n { - if !other[i as usize].eq(CStr::from_ptr(R_CHAR(STRING_ELT(object, i))).to_str().unwrap()) { - return false; - } - } - true - } - } +pub trait CharSxpEq { + fn eq_charsxp(&self, s: SEXP) -> bool; +} + +impl CharSxpEq for &str { + fn eq_charsxp(&self, s: SEXP) -> bool { + unsafe { + (*self).eq(CStr::from_ptr(R_CHAR(s)).to_str().unwrap()) } + } +} - }; +impl CharSxpEq for String { + fn eq_charsxp(&self, s: SEXP) -> bool { + (&self[..]).eq_charsxp(s) + } } -partial_eq_RObject_slice!{ &str } -partial_eq_RObject_slice!{ String } +impl PartialEq<[S]> for RObject where S : CharSxpEq { + fn eq(&self, other: &[S]) -> bool { + unsafe { + let object = self.sexp; + if r_typeof(object) != STRSXP { + return false; + } -macro_rules! partial_eq_RObject_slice2 { - ([$($vars:tt)*] $type:ty) => { - impl<$($vars)*> PartialEq<$type> for RObject { - fn eq(&self, other: &$type) -> bool { - self.eq(&other[..]) + let n = Rf_xlength(object) as isize; + if n != other.len() as isize { + return false; } + for i in 0..n { + if !other.get_unchecked(i as usize).eq_charsxp(STRING_ELT(object, i)) { + return false; + } + } + true } - } } -partial_eq_RObject_slice2!{ [const N: usize] [&str; N]} -partial_eq_RObject_slice2!{ [] Vec<&str> } -partial_eq_RObject_slice2!{ [const N: usize] [String; N]} -partial_eq_RObject_slice2!{ [] Vec} -macro_rules! partial_eq_RObject_rhs { - ([$($vars:tt)*] $type:ty) => { - impl<$($vars)*> PartialEq for $type { - fn eq(&self, other: &RObject) -> bool { - other.eq(self) - } - } +impl PartialEq<[S; N]> for RObject where S : CharSxpEq { + fn eq(&self, other: &[S; N]) -> bool { + self.eq(&other[..]) } } -partial_eq_RObject_rhs!{ [] [&str] } -partial_eq_RObject_rhs!{ [const N: usize] [&str; N] } -partial_eq_RObject_rhs!{ [] Vec<&str> } -partial_eq_RObject_rhs!{ [] [String] } -partial_eq_RObject_rhs!{ [const N: usize] [String; N] } -partial_eq_RObject_rhs!{ [] Vec } +impl PartialEq> for RObject where S : CharSxpEq { + fn eq(&self, other: &Vec) -> bool { + self.eq(&other[..]) + } +} pub unsafe fn r_typeof(object: SEXP) -> u32 { TYPEOF(object) as u32 From 5d8ddffab94d9dd1fac22ee8c9102fb0b63dee0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Wed, 8 Feb 2023 14:32:35 +0100 Subject: [PATCH 10/15] handle NA_STRING --- extensions/positron-r/amalthea/crates/harp/src/utils.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/positron-r/amalthea/crates/harp/src/utils.rs b/extensions/positron-r/amalthea/crates/harp/src/utils.rs index 6ce2577d0259..a32622e90b8e 100644 --- a/extensions/positron-r/amalthea/crates/harp/src/utils.rs +++ b/extensions/positron-r/amalthea/crates/harp/src/utils.rs @@ -55,7 +55,7 @@ pub trait CharSxpEq { impl CharSxpEq for &str { fn eq_charsxp(&self, s: SEXP) -> bool { unsafe { - (*self).eq(CStr::from_ptr(R_CHAR(s)).to_str().unwrap()) + s != R_NaString && (*self).eq(CStr::from_ptr(R_CHAR(s)).to_str().unwrap()) } } } From fc011a1983ca702abdc327b60a9c7aaa2a4a8a56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Wed, 8 Feb 2023 14:41:28 +0100 Subject: [PATCH 11/15] test for CharSxpEq --- .../positron-r/amalthea/crates/harp/src/object.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/extensions/positron-r/amalthea/crates/harp/src/object.rs b/extensions/positron-r/amalthea/crates/harp/src/object.rs index 3a9e4d031b2c..e8a7da28a186 100644 --- a/extensions/positron-r/amalthea/crates/harp/src/object.rs +++ b/extensions/positron-r/amalthea/crates/harp/src/object.rs @@ -367,10 +367,23 @@ impl TryFrom for HashMap { #[cfg(test)] mod tests { - use crate::r_test; + use libR_sys::STRING_ELT; + + use crate::{r_test, r_string, protect, utils::CharSxpEq}; use super::RObject; + #[test] + #[allow(non_snake_case)] + fn test_eq_charsxp() {r_test! { + let mut protect = protect::RProtect::new(); + let r_string = protect.add(r_string!("Apple")); + let apple = STRING_ELT(r_string, 0); + + assert!("Apple".eq_charsxp(apple)); + assert!(String::from("Apple").eq_charsxp(apple)); + }} + #[test] #[allow(non_snake_case)] fn test_RObject_from_Vec_str() { r_test! { From 2ef375cf6d4a3408cf174e434f55cd28b0e63d61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Wed, 8 Feb 2023 14:42:51 +0100 Subject: [PATCH 12/15] test NA_STRING case too --- extensions/positron-r/amalthea/crates/harp/src/object.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/extensions/positron-r/amalthea/crates/harp/src/object.rs b/extensions/positron-r/amalthea/crates/harp/src/object.rs index e8a7da28a186..ad48927c289e 100644 --- a/extensions/positron-r/amalthea/crates/harp/src/object.rs +++ b/extensions/positron-r/amalthea/crates/harp/src/object.rs @@ -367,7 +367,7 @@ impl TryFrom for HashMap { #[cfg(test)] mod tests { - use libR_sys::STRING_ELT; + use libR_sys::{STRING_ELT, R_NaString}; use crate::{r_test, r_string, protect, utils::CharSxpEq}; @@ -382,6 +382,9 @@ mod tests { assert!("Apple".eq_charsxp(apple)); assert!(String::from("Apple").eq_charsxp(apple)); + + assert!(!"Apple".eq_charsxp(R_NaString)); + assert!(!String::from("Apple").eq_charsxp(R_NaString)); }} #[test] From 6eb76e42988dc2c8150d9ccf3b2f4a8c6c5e2c85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Wed, 8 Feb 2023 15:06:19 +0100 Subject: [PATCH 13/15] handle From<[S]> for RObject and From<[S;N]> where S is string --- .../amalthea/crates/harp/src/object.rs | 40 ++++++++++++++++++- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/extensions/positron-r/amalthea/crates/harp/src/object.rs b/extensions/positron-r/amalthea/crates/harp/src/object.rs index ad48927c289e..5774fa5f225f 100644 --- a/extensions/positron-r/amalthea/crates/harp/src/object.rs +++ b/extensions/positron-r/amalthea/crates/harp/src/object.rs @@ -233,8 +233,8 @@ impl ToCharSxp for String { } } -impl From> for RObject where S : ToCharSxp { - fn from(value: Vec) -> Self { +impl From<&[S]> for RObject where S : ToCharSxp { + fn from(value: &[S]) -> Self { unsafe { let n = value.len() as isize; let vector = Rf_protect(Rf_allocVector(STRSXP, n)); @@ -248,6 +248,18 @@ impl From> for RObject where S : ToCharSxp { } } +impl From<&[S; N]> for RObject where S : ToCharSxp { + fn from(value: &[S; N]) -> Self { + RObject::from(&value[..]) + } +} + +impl From> for RObject where S : ToCharSxp { + fn from(value: Vec) -> Self { + RObject::from(&value[..]) + } +} + /// Convert RObject into other types. impl From for SEXP { @@ -392,6 +404,18 @@ mod tests { fn test_RObject_from_Vec_str() { r_test! { let expected = ["Apple", "Orange", "한"]; + // RObject from &[&str; 3] + let r_strings = RObject::from(&expected); + assert_eq!(r_strings, expected); // [&str] + assert_eq!(r_strings, expected[..]); // [&str; const N] + assert_eq!(r_strings, expected.to_vec()); // Vec<&str> + + // RObject from &[&str] + let r_strings = RObject::from(&expected[..]); + assert_eq!(r_strings, expected); // [&str] + assert_eq!(r_strings, expected[..]); // [&str; const N] + assert_eq!(r_strings, expected.to_vec()); // Vec<&str> + // RObject from Vec<&str> let r_strings = RObject::from(expected.to_vec()); assert_eq!(r_strings, expected); // [&str] @@ -404,6 +428,18 @@ mod tests { fn test_RObject_from_Vec_String() { r_test! { let expected = [String::from("Apple"), String::from("Orange"), String::from("한")]; + // RObject from &[String; 3] + let r_strings = RObject::from(&expected); + assert_eq!(r_strings, expected[..]); // [String] + assert_eq!(r_strings, expected); // [String; const N] + assert_eq!(r_strings, expected.to_vec()); // Vec + + // RObject from &[String; 3] + let r_strings = RObject::from(&expected[..]); + assert_eq!(r_strings, expected[..]); // [String] + assert_eq!(r_strings, expected); // [String; const N] + assert_eq!(r_strings, expected.to_vec()); // Vec + // RObject from Vec let r_strings = RObject::from(expected.to_vec()); assert_eq!(r_strings, expected[..]); // [String] From 58f4208fe83bd4b436282abb0a32a1ab91c0b882 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Thu, 9 Feb 2023 10:40:15 +0100 Subject: [PATCH 14/15] `.as_str()` --- extensions/positron-r/amalthea/crates/harp/src/object.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/positron-r/amalthea/crates/harp/src/object.rs b/extensions/positron-r/amalthea/crates/harp/src/object.rs index 5774fa5f225f..02ecc567bd8c 100644 --- a/extensions/positron-r/amalthea/crates/harp/src/object.rs +++ b/extensions/positron-r/amalthea/crates/harp/src/object.rs @@ -229,7 +229,7 @@ impl ToCharSxp for &str { impl ToCharSxp for String { fn to_charsxp(&self) -> SEXP { - (&self[..]).to_charsxp() + self.as_str().to_charsxp() } } From a149868cb8f16708bd3db32e6b79198bb825e2a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Thu, 9 Feb 2023 10:50:53 +0100 Subject: [PATCH 15/15] some comments around Rf_mkCharLenCE() call --- .../positron-r/amalthea/crates/harp/src/object.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/extensions/positron-r/amalthea/crates/harp/src/object.rs b/extensions/positron-r/amalthea/crates/harp/src/object.rs index 02ecc567bd8c..60b7d12cb079 100644 --- a/extensions/positron-r/amalthea/crates/harp/src/object.rs +++ b/extensions/positron-r/amalthea/crates/harp/src/object.rs @@ -222,6 +222,18 @@ pub trait ToCharSxp { impl ToCharSxp for &str { fn to_charsxp(&self) -> SEXP { unsafe { + /* + Rf_mkCharLenCE() will take care of allocating a nul terminated + string on the C side, so we don't need to worry about this here + + c = allocCharsxp(len); + memcpy(CHAR_RW(c), name, len); + + The only caveat is that this will error() if self embeds a nul + + rust strings being utf8, we can use cetype_t_CE_UTF8 and skip + worrying about various problems in Rf_mkCharLenCE() + */ Rf_mkCharLenCE(self.as_ptr() as *mut c_char, self.len() as i32, cetype_t_CE_UTF8) } }