From bf75cce8aff9c9d94dcc7e12d9bc048059046b0e Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Thu, 25 Apr 2024 22:36:38 +0900 Subject: [PATCH 01/10] Replace `SETLENGTH` with `Rf_xlengthgets` --- savvy-ffi/src/lib.rs | 2 +- src/sexp/complex.rs | 2 +- src/sexp/integer.rs | 2 +- src/sexp/logical.rs | 2 +- src/sexp/real.rs | 2 +- src/sexp/string.rs | 2 +- xtask/src/main.rs | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/savvy-ffi/src/lib.rs b/savvy-ffi/src/lib.rs index 1c85e88b..3e20ae47 100644 --- a/savvy-ffi/src/lib.rs +++ b/savvy-ffi/src/lib.rs @@ -69,7 +69,7 @@ extern "C" { // Allocation and attributes extern "C" { pub fn Rf_xlength(arg1: SEXP) -> R_xlen_t; - pub fn SETLENGTH(x: SEXP, v: R_xlen_t); + pub fn Rf_xlengthgets(x: SEXP, v: R_xlen_t); pub fn Rf_allocVector(arg1: SEXPTYPE, arg2: R_xlen_t) -> SEXP; pub fn Rf_install(arg1: *const ::std::os::raw::c_char) -> SEXP; pub fn Rf_getAttrib(arg1: SEXP, arg2: SEXP) -> SEXP; diff --git a/src/sexp/complex.rs b/src/sexp/complex.rs index 0c991f3b..afe95e45 100644 --- a/src/sexp/complex.rs +++ b/src/sexp/complex.rs @@ -213,7 +213,7 @@ impl OwnedComplexSexp { let new_len = last_index + 1; if new_len != upper { unsafe { - savvy_ffi::SETLENGTH(out.inner, new_len as _); + savvy_ffi::Rf_xlengthgets(out.inner, new_len as _); } out.len = new_len; } diff --git a/src/sexp/integer.rs b/src/sexp/integer.rs index 3eedea41..e169f297 100644 --- a/src/sexp/integer.rs +++ b/src/sexp/integer.rs @@ -299,7 +299,7 @@ impl OwnedIntegerSexp { let new_len = last_index + 1; if new_len != upper { unsafe { - savvy_ffi::SETLENGTH(out.inner, new_len as _); + savvy_ffi::Rf_xlengthgets(out.inner, new_len as _); } out.len = new_len; } diff --git a/src/sexp/logical.rs b/src/sexp/logical.rs index 05978cb2..d4cd38bd 100644 --- a/src/sexp/logical.rs +++ b/src/sexp/logical.rs @@ -273,7 +273,7 @@ impl OwnedLogicalSexp { let new_len = last_index + 1; if new_len != upper { unsafe { - savvy_ffi::SETLENGTH(out.inner, new_len as _); + savvy_ffi::Rf_xlengthgets(out.inner, new_len as _); } out.len = new_len; } diff --git a/src/sexp/real.rs b/src/sexp/real.rs index 10767f33..e0f77e04 100644 --- a/src/sexp/real.rs +++ b/src/sexp/real.rs @@ -302,7 +302,7 @@ impl OwnedRealSexp { let new_len = last_index + 1; if new_len != upper { unsafe { - savvy_ffi::SETLENGTH(out.inner, new_len as _); + savvy_ffi::Rf_xlengthgets(out.inner, new_len as _); } out.len = new_len; } diff --git a/src/sexp/string.rs b/src/sexp/string.rs index 3aff3450..5d9377a9 100644 --- a/src/sexp/string.rs +++ b/src/sexp/string.rs @@ -203,7 +203,7 @@ impl OwnedStringSexp { let new_len = last_index + 1; if new_len != upper { unsafe { - savvy_ffi::SETLENGTH(out.inner, new_len as _); + savvy_ffi::Rf_xlengthgets(out.inner, new_len as _); } out.len = new_len; } diff --git a/xtask/src/main.rs b/xtask/src/main.rs index 450e1db4..ae0da807 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -84,7 +84,7 @@ fn show() -> Result<(), DynError> { .allowlist_function("R_IsNA") // Allocation and attributes .allowlist_function("Rf_xlength") - .allowlist_function("SETLENGTH") + .allowlist_function("Rf_xlengthgets") .allowlist_function("Rf_allocVector") .allowlist_function("Rf_install") .allowlist_function("Rf_getAttrib") From 27bb252b44711e1c41c56f1e934c8c98f04314ca Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Thu, 25 Apr 2024 22:44:17 +0900 Subject: [PATCH 02/10] Tweak --- .github/workflows/check-non-api.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/check-non-api.yaml b/.github/workflows/check-non-api.yaml index 1de37540..6d90312b 100644 --- a/.github/workflows/check-non-api.yaml +++ b/.github/workflows/check-non-api.yaml @@ -36,4 +36,6 @@ jobs: if [ -n "${NON_API}" ]; then echo 'Found what they call "non-API"!' exit 1 + else + echo "OK!" fi From 488aba855bc14dd7ecf11758e7cda337eb7dfc37 Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Thu, 25 Apr 2024 22:45:24 +0900 Subject: [PATCH 03/10] Ignore error --- .github/workflows/check-non-api.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/check-non-api.yaml b/.github/workflows/check-non-api.yaml index 6d90312b..f4a1a008 100644 --- a/.github/workflows/check-non-api.yaml +++ b/.github/workflows/check-non-api.yaml @@ -26,7 +26,7 @@ jobs: REGEX=$(Rscript tmp.R) - NON_API=$(grep -R -E "${REGEX}" ./savvy-ffi/src/) + NON_API=$(grep -R -E "${REGEX}" ./savvy-ffi/src/ || true) echo "Check result:" echo From 84c826a3702f21b1d26264ffd565dab05f458c84 Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Thu, 25 Apr 2024 22:48:42 +0900 Subject: [PATCH 04/10] Add CHANGELOG [skip ci] --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b54f1b78..7e98791d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,10 @@ ## [Unreleased] (ReleaseDate) +### Minor improvements + +* Now savvy no longer uses `SETLENGTH`, which is a so-called "non-API" thing. + ## [v0.6.0] (2024-04-20) ### Breaking changes From 806381108055778e92dff6111cac74701d0a551e Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Thu, 25 Apr 2024 22:58:34 +0900 Subject: [PATCH 05/10] Update argnames --- savvy-ffi/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/savvy-ffi/src/lib.rs b/savvy-ffi/src/lib.rs index 3e20ae47..680201ff 100644 --- a/savvy-ffi/src/lib.rs +++ b/savvy-ffi/src/lib.rs @@ -69,7 +69,7 @@ extern "C" { // Allocation and attributes extern "C" { pub fn Rf_xlength(arg1: SEXP) -> R_xlen_t; - pub fn Rf_xlengthgets(x: SEXP, v: R_xlen_t); + pub fn Rf_xlengthgets(arg1: SEXP, arg2: R_xlen_t); pub fn Rf_allocVector(arg1: SEXPTYPE, arg2: R_xlen_t) -> SEXP; pub fn Rf_install(arg1: *const ::std::os::raw::c_char) -> SEXP; pub fn Rf_getAttrib(arg1: SEXP, arg2: SEXP) -> SEXP; From 0acab870beeb5a868b8abca0da65e65191d3214e Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Thu, 25 Apr 2024 23:02:00 +0900 Subject: [PATCH 06/10] Update --- savvy-ffi/src/lib.rs | 2 +- src/sexp/complex.rs | 2 +- src/sexp/integer.rs | 2 +- src/sexp/logical.rs | 2 +- src/sexp/real.rs | 2 +- src/sexp/string.rs | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/savvy-ffi/src/lib.rs b/savvy-ffi/src/lib.rs index 680201ff..4769f37a 100644 --- a/savvy-ffi/src/lib.rs +++ b/savvy-ffi/src/lib.rs @@ -69,7 +69,7 @@ extern "C" { // Allocation and attributes extern "C" { pub fn Rf_xlength(arg1: SEXP) -> R_xlen_t; - pub fn Rf_xlengthgets(arg1: SEXP, arg2: R_xlen_t); + pub fn Rf_xlengthgets(arg1: SEXP, arg2: R_xlen_t) -> SEXP; pub fn Rf_allocVector(arg1: SEXPTYPE, arg2: R_xlen_t) -> SEXP; pub fn Rf_install(arg1: *const ::std::os::raw::c_char) -> SEXP; pub fn Rf_getAttrib(arg1: SEXP, arg2: SEXP) -> SEXP; diff --git a/src/sexp/complex.rs b/src/sexp/complex.rs index afe95e45..592562fe 100644 --- a/src/sexp/complex.rs +++ b/src/sexp/complex.rs @@ -213,7 +213,7 @@ impl OwnedComplexSexp { let new_len = last_index + 1; if new_len != upper { unsafe { - savvy_ffi::Rf_xlengthgets(out.inner, new_len as _); + out.inner = savvy_ffi::Rf_xlengthgets(out.inner, new_len as _); } out.len = new_len; } diff --git a/src/sexp/integer.rs b/src/sexp/integer.rs index e169f297..ed332c61 100644 --- a/src/sexp/integer.rs +++ b/src/sexp/integer.rs @@ -299,7 +299,7 @@ impl OwnedIntegerSexp { let new_len = last_index + 1; if new_len != upper { unsafe { - savvy_ffi::Rf_xlengthgets(out.inner, new_len as _); + out.inner = savvy_ffi::Rf_xlengthgets(out.inner, new_len as _); } out.len = new_len; } diff --git a/src/sexp/logical.rs b/src/sexp/logical.rs index d4cd38bd..8a8952fd 100644 --- a/src/sexp/logical.rs +++ b/src/sexp/logical.rs @@ -273,7 +273,7 @@ impl OwnedLogicalSexp { let new_len = last_index + 1; if new_len != upper { unsafe { - savvy_ffi::Rf_xlengthgets(out.inner, new_len as _); + out.inner = savvy_ffi::Rf_xlengthgets(out.inner, new_len as _); } out.len = new_len; } diff --git a/src/sexp/real.rs b/src/sexp/real.rs index e0f77e04..4062ce1d 100644 --- a/src/sexp/real.rs +++ b/src/sexp/real.rs @@ -302,7 +302,7 @@ impl OwnedRealSexp { let new_len = last_index + 1; if new_len != upper { unsafe { - savvy_ffi::Rf_xlengthgets(out.inner, new_len as _); + out.inner = savvy_ffi::Rf_xlengthgets(out.inner, new_len as _); } out.len = new_len; } diff --git a/src/sexp/string.rs b/src/sexp/string.rs index 5d9377a9..f9f39d2f 100644 --- a/src/sexp/string.rs +++ b/src/sexp/string.rs @@ -203,7 +203,7 @@ impl OwnedStringSexp { let new_len = last_index + 1; if new_len != upper { unsafe { - savvy_ffi::Rf_xlengthgets(out.inner, new_len as _); + out.inner = savvy_ffi::Rf_xlengthgets(out.inner, new_len as _); } out.len = new_len; } From 35a6af2a58417904141ceb7831c2232ed8daffcf Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Fri, 26 Apr 2024 09:36:40 +0900 Subject: [PATCH 07/10] Do not rely on Rf_xlengthgets() --- src/sexp/complex.rs | 27 +++++++++++++++------------ src/sexp/integer.rs | 28 ++++++++++++++++------------ src/sexp/logical.rs | 30 +++++++++++++++++------------- src/sexp/real.rs | 28 ++++++++++++++++------------ src/sexp/string.rs | 23 ++++++++++++----------- 5 files changed, 76 insertions(+), 60 deletions(-) diff --git a/src/sexp/complex.rs b/src/sexp/complex.rs index 592562fe..d39f5834 100644 --- a/src/sexp/complex.rs +++ b/src/sexp/complex.rs @@ -5,7 +5,7 @@ use savvy_ffi::CPLXSXP; use savvy_ffi::{COMPLEX, SEXP}; use super::{impl_common_sexp_ops, impl_common_sexp_ops_owned, utils::assert_len, Sexp}; -use crate::protect; +use crate::protect::{self, local_protect}; use crate::NotAvailableValue; // for na() /// An external SEXP of a complex vector. @@ -198,27 +198,30 @@ impl OwnedComplexSexp { // (e.g. `(0..10).filter(|x| x % 2 == 0)`), so it needs to be // truncated to the actual length at last. - let mut out = unsafe { Self::new_without_init(upper)? }; + let inner = crate::alloc_vector(CPLXSXP, upper as _)?; + local_protect(inner); + let raw = unsafe { COMPLEX(inner) }; let mut last_index = 0; for (i, v) in iter.enumerate() { // The upper bound of size_hint() is just for optimization - // and what we should not trust. So, we should't use - // `set_elt_unchecked()` here. - out.set_elt(i, v)?; + // and what we should not trust. + assert_len(upper, i)?; + unsafe { *(raw.add(i)) = v }; last_index = i; } let new_len = last_index + 1; - if new_len != upper { - unsafe { - out.inner = savvy_ffi::Rf_xlengthgets(out.inner, new_len as _); - } - out.len = new_len; + if new_len == upper { + Self::new_from_raw_sexp(inner, upper) + } else { + let mut out = unsafe { Self::new_without_init(new_len)? }; + let raw_slice = unsafe { std::slice::from_raw_parts(raw, new_len) }; // ignore the part over + out.as_mut_slice().copy_from_slice(raw_slice); + + Ok(out) } - - Ok(out) } (_, None) => { // When the length is not known at all, collect() it first. diff --git a/src/sexp/integer.rs b/src/sexp/integer.rs index ed332c61..33cc9f6b 100644 --- a/src/sexp/integer.rs +++ b/src/sexp/integer.rs @@ -4,7 +4,7 @@ use savvy_ffi::{INTEGER, INTSXP, SEXP}; use super::utils::assert_len; use super::{impl_common_sexp_ops, impl_common_sexp_ops_owned, Sexp}; -use crate::protect; +use crate::protect::{self, local_protect}; use crate::NotAvailableValue; // for na() /// An external SEXP of an integer vector. @@ -284,27 +284,31 @@ impl OwnedIntegerSexp { // (e.g. `(0..10).filter(|x| x % 2 == 0)`), so it needs to be // truncated to the actual length at last. - let mut out = unsafe { Self::new_without_init(upper)? }; + let inner = crate::alloc_vector(INTSXP, upper as _)?; + local_protect(inner); + let raw = unsafe { INTEGER(inner) }; let mut last_index = 0; for (i, v) in iter.enumerate() { // The upper bound of size_hint() is just for optimization - // and what we should not trust. So, we should't use - // `set_elt_unchecked()` here. - out.set_elt(i, v)?; + // and what we should not trust. + assert_len(upper, i)?; + unsafe { *(raw.add(i)) = v }; last_index = i; } let new_len = last_index + 1; - if new_len != upper { - unsafe { - out.inner = savvy_ffi::Rf_xlengthgets(out.inner, new_len as _); - } - out.len = new_len; + if new_len == upper { + Self::new_from_raw_sexp(inner, upper) + } else { + let out = unsafe { Self::new_without_init(new_len)? }; + let dst = unsafe { std::slice::from_raw_parts_mut(out.raw, new_len) }; + let src = unsafe { std::slice::from_raw_parts(raw, new_len) }; // ignore the part over + dst.copy_from_slice(src); + + Ok(out) } - - Ok(out) } (_, None) => { // When the length is not known at all, collect() it first. diff --git a/src/sexp/logical.rs b/src/sexp/logical.rs index 8a8952fd..294db484 100644 --- a/src/sexp/logical.rs +++ b/src/sexp/logical.rs @@ -1,7 +1,7 @@ use savvy_ffi::{R_NaInt, LGLSXP, LOGICAL, SET_LOGICAL_ELT, SEXP}; -use super::{impl_common_sexp_ops, impl_common_sexp_ops_owned, Sexp}; -use crate::protect; +use super::{impl_common_sexp_ops, impl_common_sexp_ops_owned, utils::assert_len, Sexp}; +use crate::protect::{self, local_protect}; /// An external SEXP of a logical vector. pub struct LogicalSexp(pub SEXP); @@ -258,27 +258,31 @@ impl OwnedLogicalSexp { // (e.g. `(0..10).filter(|x| x % 2 == 0)`), so it needs to be // truncated to the actual length at last. - let mut out = unsafe { Self::new_without_init(upper)? }; + let inner = crate::alloc_vector(LGLSXP, upper as _)?; + local_protect(inner); + let raw = unsafe { LOGICAL(inner) }; let mut last_index = 0; for (i, v) in iter.enumerate() { // The upper bound of size_hint() is just for optimization - // and what we should not trust. So, we should't use - // `set_elt_unchecked()` here. - out.set_elt(i, v)?; + // and what we should not trust. + assert_len(upper, i)?; + unsafe { *(raw.add(i)) = v as _ }; last_index = i; } let new_len = last_index + 1; - if new_len != upper { - unsafe { - out.inner = savvy_ffi::Rf_xlengthgets(out.inner, new_len as _); - } - out.len = new_len; + if new_len == upper { + Self::new_from_raw_sexp(inner, upper) + } else { + let out = unsafe { Self::new_without_init(new_len)? }; + let dst = unsafe { std::slice::from_raw_parts_mut(out.raw, new_len) }; + let src = unsafe { std::slice::from_raw_parts(raw, new_len) }; // ignore the part over + dst.copy_from_slice(src); + + Ok(out) } - - Ok(out) } (_, None) => { // When the length is not known at all, collect() it first. diff --git a/src/sexp/real.rs b/src/sexp/real.rs index 4062ce1d..077f0e49 100644 --- a/src/sexp/real.rs +++ b/src/sexp/real.rs @@ -4,7 +4,7 @@ use savvy_ffi::{REAL, REALSXP, SEXP}; use super::utils::assert_len; use super::{impl_common_sexp_ops, impl_common_sexp_ops_owned, Sexp}; -use crate::protect; +use crate::protect::{self, local_protect}; use crate::NotAvailableValue; // for na() /// An external SEXP of a real vector. @@ -287,27 +287,31 @@ impl OwnedRealSexp { // (e.g. `(0..10).filter(|x| x % 2 == 0)`), so it needs to be // truncated to the actual length at last. - let mut out = unsafe { Self::new_without_init(upper)? }; + let inner = crate::alloc_vector(REALSXP, upper as _)?; + local_protect(inner); + let raw = unsafe { REAL(inner) }; let mut last_index = 0; for (i, v) in iter.enumerate() { // The upper bound of size_hint() is just for optimization - // and what we should not trust. So, we should't use - // `set_elt_unchecked()` here. - out.set_elt(i, v)?; + // and what we should not trust. + assert_len(upper, i)?; + unsafe { *(raw.add(i)) = v }; last_index = i; } let new_len = last_index + 1; - if new_len != upper { - unsafe { - out.inner = savvy_ffi::Rf_xlengthgets(out.inner, new_len as _); - } - out.len = new_len; + if new_len == upper { + Self::new_from_raw_sexp(inner, upper) + } else { + let out = unsafe { Self::new_without_init(new_len)? }; + let dst = unsafe { std::slice::from_raw_parts_mut(out.raw, new_len) }; + let src = unsafe { std::slice::from_raw_parts(raw, new_len) }; // ignore the part over + dst.copy_from_slice(src); + + Ok(out) } - - Ok(out) } (_, None) => { // When the length is not known at all, collect() it first. diff --git a/src/sexp/string.rs b/src/sexp/string.rs index f9f39d2f..f127e34a 100644 --- a/src/sexp/string.rs +++ b/src/sexp/string.rs @@ -187,28 +187,29 @@ impl OwnedStringSexp { // (e.g. `(0..10).filter(|x| x % 2 == 0)`), so it needs to be // truncated to the actual length at last. - // string doesn't have new_without_init() method - let mut out = Self::new(upper)?; + let inner = crate::alloc_vector(STRSXP, upper as _)?; + local_protect(inner); let mut last_index = 0; for (i, v) in iter.enumerate() { // The upper bound of size_hint() is just for optimization - // and what we should not trust. So, we should't use - // `set_elt_unchecked()` here. - out.set_elt(i, v.as_ref())?; + // and what we should not trust. + assert_len(upper, i)?; + unsafe { SET_STRING_ELT(inner, i as _, str_to_charsxp(v.as_ref())?) }; last_index = i; } let new_len = last_index + 1; - if new_len != upper { - unsafe { - out.inner = savvy_ffi::Rf_xlengthgets(out.inner, new_len as _); + if new_len == upper { + Self::new_from_raw_sexp(inner, upper) + } else { + let mut out = Self::new(new_len)?; + for i in 0..new_len { + unsafe { out.set_elt_unchecked(i, STRING_ELT(inner, i as _)) }; } - out.len = new_len; + Ok(out) } - - Ok(out) } (_, None) => { // When the length is not known at all, collect() it first. From 7415f28753406efd624f52eb0bd4ddab90d66fa0 Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Fri, 26 Apr 2024 09:37:10 +0900 Subject: [PATCH 08/10] Remove Rf_xlengthgets() --- xtask/src/main.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/xtask/src/main.rs b/xtask/src/main.rs index ae0da807..d2704950 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -84,7 +84,6 @@ fn show() -> Result<(), DynError> { .allowlist_function("R_IsNA") // Allocation and attributes .allowlist_function("Rf_xlength") - .allowlist_function("Rf_xlengthgets") .allowlist_function("Rf_allocVector") .allowlist_function("Rf_install") .allowlist_function("Rf_getAttrib") From 4ac426c408c42bacef45a26a94f3b9944fcbd27c Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Fri, 26 Apr 2024 09:40:23 +0900 Subject: [PATCH 09/10] Remove Rf_xlengthgets --- savvy-ffi/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/savvy-ffi/src/lib.rs b/savvy-ffi/src/lib.rs index 4769f37a..23f78ab6 100644 --- a/savvy-ffi/src/lib.rs +++ b/savvy-ffi/src/lib.rs @@ -69,7 +69,6 @@ extern "C" { // Allocation and attributes extern "C" { pub fn Rf_xlength(arg1: SEXP) -> R_xlen_t; - pub fn Rf_xlengthgets(arg1: SEXP, arg2: R_xlen_t) -> SEXP; pub fn Rf_allocVector(arg1: SEXPTYPE, arg2: R_xlen_t) -> SEXP; pub fn Rf_install(arg1: *const ::std::os::raw::c_char) -> SEXP; pub fn Rf_getAttrib(arg1: SEXP, arg2: SEXP) -> SEXP; From 2bf829e039e2d8e668e7a65b122fb3881cc45a81 Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Fri, 26 Apr 2024 09:45:24 +0900 Subject: [PATCH 10/10] Add comments [skip ci] --- src/sexp/complex.rs | 6 +++++- src/sexp/integer.rs | 6 +++++- src/sexp/logical.rs | 6 +++++- src/sexp/real.rs | 6 +++++- src/sexp/string.rs | 3 +++ 5 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/sexp/complex.rs b/src/sexp/complex.rs index d39f5834..c55d2e05 100644 --- a/src/sexp/complex.rs +++ b/src/sexp/complex.rs @@ -214,10 +214,14 @@ impl OwnedComplexSexp { let new_len = last_index + 1; if new_len == upper { + // If the length is the same as expected, use it as it is. Self::new_from_raw_sexp(inner, upper) } else { + // If the length is shorter than expected, re-allocate a new + // SEXP and copy the values into it. let mut out = unsafe { Self::new_without_init(new_len)? }; - let raw_slice = unsafe { std::slice::from_raw_parts(raw, new_len) }; // ignore the part over + // `raw` is longer than new_len, but the elements over new_len are ignored + let raw_slice = unsafe { std::slice::from_raw_parts(raw, new_len) }; out.as_mut_slice().copy_from_slice(raw_slice); Ok(out) diff --git a/src/sexp/integer.rs b/src/sexp/integer.rs index 33cc9f6b..ac0f2db4 100644 --- a/src/sexp/integer.rs +++ b/src/sexp/integer.rs @@ -300,11 +300,15 @@ impl OwnedIntegerSexp { let new_len = last_index + 1; if new_len == upper { + // If the length is the same as expected, use it as it is. Self::new_from_raw_sexp(inner, upper) } else { + // If the length is shorter than expected, re-allocate a new + // SEXP and copy the values into it. let out = unsafe { Self::new_without_init(new_len)? }; let dst = unsafe { std::slice::from_raw_parts_mut(out.raw, new_len) }; - let src = unsafe { std::slice::from_raw_parts(raw, new_len) }; // ignore the part over + // `raw` is longer than new_len, but the elements over new_len are ignored + let src = unsafe { std::slice::from_raw_parts(raw, new_len) }; dst.copy_from_slice(src); Ok(out) diff --git a/src/sexp/logical.rs b/src/sexp/logical.rs index 294db484..76657f9e 100644 --- a/src/sexp/logical.rs +++ b/src/sexp/logical.rs @@ -274,11 +274,15 @@ impl OwnedLogicalSexp { let new_len = last_index + 1; if new_len == upper { + // If the length is the same as expected, use it as it is. Self::new_from_raw_sexp(inner, upper) } else { + // If the length is shorter than expected, re-allocate a new + // SEXP and copy the values into it. let out = unsafe { Self::new_without_init(new_len)? }; let dst = unsafe { std::slice::from_raw_parts_mut(out.raw, new_len) }; - let src = unsafe { std::slice::from_raw_parts(raw, new_len) }; // ignore the part over + // `raw` is longer than new_len, but the elements over new_len are ignored + let src = unsafe { std::slice::from_raw_parts(raw, new_len) }; dst.copy_from_slice(src); Ok(out) diff --git a/src/sexp/real.rs b/src/sexp/real.rs index 077f0e49..e4d87e53 100644 --- a/src/sexp/real.rs +++ b/src/sexp/real.rs @@ -303,11 +303,15 @@ impl OwnedRealSexp { let new_len = last_index + 1; if new_len == upper { + // If the length is the same as expected, use it as it is. Self::new_from_raw_sexp(inner, upper) } else { + // If the length is shorter than expected, re-allocate a new + // SEXP and copy the values into it. let out = unsafe { Self::new_without_init(new_len)? }; let dst = unsafe { std::slice::from_raw_parts_mut(out.raw, new_len) }; - let src = unsafe { std::slice::from_raw_parts(raw, new_len) }; // ignore the part over + // `raw` is longer than new_len, but the elements over new_len are ignored + let src = unsafe { std::slice::from_raw_parts(raw, new_len) }; dst.copy_from_slice(src); Ok(out) diff --git a/src/sexp/string.rs b/src/sexp/string.rs index f127e34a..e1908ff5 100644 --- a/src/sexp/string.rs +++ b/src/sexp/string.rs @@ -202,8 +202,11 @@ impl OwnedStringSexp { let new_len = last_index + 1; if new_len == upper { + // If the length is the same as expected, use it as it is. Self::new_from_raw_sexp(inner, upper) } else { + // If the length is shorter than expected, re-allocate a new + // SEXP and copy the values into it. let mut out = Self::new(new_len)?; for i in 0..new_len { unsafe { out.set_elt_unchecked(i, STRING_ELT(inner, i as _)) };