From a66ccd8a929313011412af1349a6b6d07ad62ed4 Mon Sep 17 00:00:00 2001 From: Aymeric Stamm Date: Thu, 8 May 2025 00:27:31 +0200 Subject: [PATCH 1/3] Remove unprotect calls that rchk claims to be over unprotecting. --- inst/include/cpp11/protect.hpp | 2 -- inst/include/cpp11/r_vector.hpp | 3 --- 2 files changed, 5 deletions(-) diff --git a/inst/include/cpp11/protect.hpp b/inst/include/cpp11/protect.hpp index 9cb4876a..363892a9 100644 --- a/inst/include/cpp11/protect.hpp +++ b/inst/include/cpp11/protect.hpp @@ -299,8 +299,6 @@ inline SEXP insert(SEXP x) { SETCDR(head, cell); SETCAR(next, cell); - UNPROTECT(2); - return cell; } diff --git a/inst/include/cpp11/r_vector.hpp b/inst/include/cpp11/r_vector.hpp index 576f4fe6..7a5b1cba 100644 --- a/inst/include/cpp11/r_vector.hpp +++ b/inst/include/cpp11/r_vector.hpp @@ -1339,7 +1339,6 @@ inline SEXP r_vector::reserve_data(SEXP x, bool is_altrep, R_xlen_t size) { // Does not look like it would ever error in our use cases, so no `safe[]`. Rf_copyMostAttrib(x, out); - UNPROTECT(2); return out; } @@ -1364,7 +1363,6 @@ inline SEXP r_vector::resize_data(SEXP x, bool is_altrep, R_xlen_t size) { } } - UNPROTECT(1); return out; } @@ -1386,7 +1384,6 @@ inline SEXP r_vector::resize_names(SEXP x, R_xlen_t size) { SET_STRING_ELT(out, i, R_BlankString); } - UNPROTECT(1); return out; } From 24cf9084d280635e8f1a36ea7ed00636f7efcdd2 Mon Sep 17 00:00:00 2001 From: Aymeric Stamm Date: Fri, 9 May 2025 16:47:03 +0200 Subject: [PATCH 2/3] Fix for insert(). --- inst/include/cpp11/protect.hpp | 10 +++------- inst/include/cpp11/r_vector.hpp | 6 ++++++ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/inst/include/cpp11/protect.hpp b/inst/include/cpp11/protect.hpp index 363892a9..259b7065 100644 --- a/inst/include/cpp11/protect.hpp +++ b/inst/include/cpp11/protect.hpp @@ -281,21 +281,17 @@ inline SEXP insert(SEXP x) { return R_NilValue; } - PROTECT(x); - SEXP list = get(); // Get references to the head of the preserve list and the next element - // after the head SEXP head = list; SEXP next = CDR(list); - // Add a new cell that points to the current head + next. - SEXP cell = PROTECT(Rf_cons(head, next)); + // Create the new cell + SEXP cell = Rf_cons(head, next); SET_TAG(cell, x); - // Update the head + next to point at the newly-created cell, - // effectively inserting that cell between the current head + next. + // Update the list structure SETCDR(head, cell); SETCAR(next, cell); diff --git a/inst/include/cpp11/r_vector.hpp b/inst/include/cpp11/r_vector.hpp index 7a5b1cba..71115920 100644 --- a/inst/include/cpp11/r_vector.hpp +++ b/inst/include/cpp11/r_vector.hpp @@ -1339,6 +1339,8 @@ inline SEXP r_vector::reserve_data(SEXP x, bool is_altrep, R_xlen_t size) { // Does not look like it would ever error in our use cases, so no `safe[]`. Rf_copyMostAttrib(x, out); + UNPROTECT(2); + return out; } @@ -1363,6 +1365,8 @@ inline SEXP r_vector::resize_data(SEXP x, bool is_altrep, R_xlen_t size) { } } + UNPROTECT(1); + return out; } @@ -1384,6 +1388,8 @@ inline SEXP r_vector::resize_names(SEXP x, R_xlen_t size) { SET_STRING_ELT(out, i, R_BlankString); } + UNPROTECT(1); + return out; } From 9f4e327703d9e3f2ed86eb99f8b5812a92e618e8 Mon Sep 17 00:00:00 2001 From: Aymeric Stamm Date: Fri, 9 May 2025 21:22:52 +0200 Subject: [PATCH 3/3] Fix protection stack manipulation issues in `r_vector.hpp`. --- inst/include/cpp11/r_vector.hpp | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/inst/include/cpp11/r_vector.hpp b/inst/include/cpp11/r_vector.hpp index 71115920..0381f388 100644 --- a/inst/include/cpp11/r_vector.hpp +++ b/inst/include/cpp11/r_vector.hpp @@ -1320,16 +1320,17 @@ inline typename r_vector::iterator r_vector::iterator::operator+(R_xlen_t template inline SEXP r_vector::reserve_data(SEXP x, bool is_altrep, R_xlen_t size) { // Resize core data - SEXP out = PROTECT(resize_data(x, is_altrep, size)); + SEXP out = resize_data(x, is_altrep, size); // Resize names, if required - // Protection seems needed to make rchk happy - SEXP names = PROTECT(Rf_getAttrib(x, R_NamesSymbol)); + SEXP names = Rf_getAttrib(x, R_NamesSymbol); if (names != R_NilValue) { if (Rf_xlength(names) != size) { - names = resize_names(names, size); + SEXP new_names = resize_names(names, size); + Rf_setAttrib(out, R_NamesSymbol, new_names); + } else { + Rf_setAttrib(out, R_NamesSymbol, names); } - Rf_setAttrib(out, R_NamesSymbol, names); } // Copy over "most" attributes, and set OBJECT bit and S4 bit as needed. @@ -1338,9 +1339,6 @@ inline SEXP r_vector::reserve_data(SEXP x, bool is_altrep, R_xlen_t size) { // as this is a vector. // Does not look like it would ever error in our use cases, so no `safe[]`. Rf_copyMostAttrib(x, out); - - UNPROTECT(2); - return out; } @@ -1348,7 +1346,7 @@ template inline SEXP r_vector::resize_data(SEXP x, bool is_altrep, R_xlen_t size) { underlying_type const* v_x = get_const_p(is_altrep, x); - SEXP out = PROTECT(safe[Rf_allocVector](get_sexptype(), size)); + SEXP out = safe[Rf_allocVector](get_sexptype(), size); underlying_type* v_out = get_p(ALTREP(out), out); const R_xlen_t x_size = Rf_xlength(x); @@ -1365,22 +1363,18 @@ inline SEXP r_vector::resize_data(SEXP x, bool is_altrep, R_xlen_t size) { } } - UNPROTECT(1); - return out; } template inline SEXP r_vector::resize_names(SEXP x, R_xlen_t size) { - const SEXP* v_x = STRING_PTR_RO(x); - - SEXP out = PROTECT(safe[Rf_allocVector](STRSXP, size)); + SEXP out = safe[Rf_allocVector](STRSXP, size); const R_xlen_t x_size = Rf_xlength(x); const R_xlen_t copy_size = (x_size > size) ? size : x_size; for (R_xlen_t i = 0; i < copy_size; ++i) { - SET_STRING_ELT(out, i, v_x[i]); + SET_STRING_ELT(out, i, STRING_ELT(x, i)); } // Ensure remaining names are initialized to `""` @@ -1388,8 +1382,6 @@ inline SEXP r_vector::resize_names(SEXP x, R_xlen_t size) { SET_STRING_ELT(out, i, R_BlankString); } - UNPROTECT(1); - return out; }