diff --git a/NEWS.md b/NEWS.md index c2a72541..d4f21208 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,17 @@ # cpp11 (development version) +* The approach for the protection list managed by cpp11 has been tweaked + slightly. In 0.4.6, we changed to an approach that creates one protection list + per compilation unit, but we now believe we've found an approach that is + guaranteed by the C++ standard to create one protection list per package, + which makes slightly more sense and still has all the benefits of the reduced + maintanence burden mentioned in the 0.4.6 news bullet (#364). + + A side effect of this new approach is that the `preserved` object exposed + through `protect.hpp` no longer exists. We don't believe that anyone was using + this. This also means you should no longer see "unused variable" warnings + about `preserved` (#249). + * Dropped support for gcc 4.8, mainly an issue for extremely old CentOS 7 systems which used that as their default compiler. As of June 2024, CentOS 7 is past its Vendor end of support date and therefore also out of scope for diff --git a/cpp11test/src/protect.cpp b/cpp11test/src/protect.cpp index 04ddf0ac..ad15c10d 100644 --- a/cpp11test/src/protect.cpp +++ b/cpp11test/src/protect.cpp @@ -18,8 +18,8 @@ [[cpp11::register]] void protect_one_cpp11_(SEXP x, int n) { for (R_xlen_t i = 0; i < n; ++i) { - SEXP p = cpp11::preserved.insert(x); - cpp11::preserved.release(p); + SEXP p = cpp11::detail::store::insert(x); + cpp11::detail::store::release(p); } } @@ -32,18 +32,6 @@ // The internal protections here are actually uneeded, but it is a useful way to benchmark // them -// -// clang-format off -#ifdef __clang__ -# pragma clang diagnostic push -# pragma clang diagnostic ignored "-Wunused-variable" -#endif - -#ifdef __GNUC__ -# pragma GCC diagnostic push -# pragma GCC diagnostic ignored "-Wunused-variable" -#endif -// clang-format on [[cpp11::register]] void protect_many_(int n) { #ifdef CPP11_BENCH @@ -63,12 +51,12 @@ [[cpp11::register]] void protect_many_cpp11_(int n) { std::vector res; for (R_xlen_t i = 0; i < n; ++i) { - res.push_back(cpp11::preserved.insert(Rf_ScalarInteger(n))); + res.push_back(cpp11::detail::store::insert(Rf_ScalarInteger(n))); } for (R_xlen_t i = n - 1; i >= 0; --i) { SEXP x = res[i]; - cpp11::preserved.release(x); + cpp11::detail::store::release(x); res.pop_back(); } } diff --git a/cpp11test/src/test-protect.cpp b/cpp11test/src/test-protect.cpp index 2bd2dadf..a0fd18ac 100644 --- a/cpp11test/src/test-protect.cpp +++ b/cpp11test/src/test-protect.cpp @@ -1,16 +1,3 @@ -// Avoid unused variable warnings regarding `cpp11::preserved` -// clang-format off -#ifdef __clang__ -# pragma clang diagnostic push -# pragma clang diagnostic ignored "-Wunused-variable" -#endif - -#ifdef __GNUC__ -# pragma GCC diagnostic push -# pragma GCC diagnostic ignored "-Wunused-variable" -#endif -// clang-format on - #define CPP11_USE_FMT #include "cpp11/protect.hpp" #include "testthat.h" diff --git a/inst/include/cpp11/doubles.hpp b/inst/include/cpp11/doubles.hpp index 9782346d..a0a92608 100644 --- a/inst/include/cpp11/doubles.hpp +++ b/inst/include/cpp11/doubles.hpp @@ -84,7 +84,7 @@ template <> inline r_vector::r_vector(std::initializer_list il) : cpp11::r_vector(safe[Rf_allocVector](REALSXP, il.size())), capacity_(il.size()) { - protect_ = preserved.insert(data_); + protect_ = detail::store::insert(data_); int n_protected = 0; try { @@ -100,7 +100,7 @@ inline r_vector::r_vector(std::initializer_list il) UNPROTECT(n_protected); }); } catch (const unwind_exception& e) { - preserved.release(protect_); + detail::store::release(protect_); UNPROTECT(n_protected); throw e; } @@ -111,8 +111,8 @@ inline void r_vector::reserve(R_xlen_t new_capacity) { data_ = data_ == R_NilValue ? safe[Rf_allocVector](REALSXP, new_capacity) : safe[Rf_xlengthgets](data_, new_capacity); SEXP old_protect = protect_; - protect_ = preserved.insert(data_); - preserved.release(old_protect); + protect_ = detail::store::insert(data_); + detail::store::release(old_protect); data_p_ = REAL(data_); capacity_ = new_capacity; diff --git a/inst/include/cpp11/integers.hpp b/inst/include/cpp11/integers.hpp index 1159e2f3..dd14d018 100644 --- a/inst/include/cpp11/integers.hpp +++ b/inst/include/cpp11/integers.hpp @@ -9,7 +9,7 @@ #include "cpp11/as.hpp" // for as_sexp #include "cpp11/attribute_proxy.hpp" // for attribute_proxy #include "cpp11/named_arg.hpp" // for named_arg -#include "cpp11/protect.hpp" // for preserved +#include "cpp11/protect.hpp" // for store #include "cpp11/r_vector.hpp" // for r_vector, r_vector<>::proxy #include "cpp11/sexp.hpp" // for sexp @@ -87,10 +87,10 @@ inline void r_vector::reserve(R_xlen_t new_capacity) { SEXP old_protect = protect_; // Protect the new data - protect_ = preserved.insert(data_); + protect_ = detail::store::insert(data_); // Release the old protection; - preserved.release(old_protect); + detail::store::release(old_protect); data_p_ = INTEGER(data_); capacity_ = new_capacity; @@ -100,7 +100,7 @@ template <> inline r_vector::r_vector(std::initializer_list il) : cpp11::r_vector(safe[Rf_allocVector](INTSXP, il.size())), capacity_(il.size()) { - protect_ = preserved.insert(data_); + protect_ = detail::store::insert(data_); int n_protected = 0; try { @@ -116,7 +116,7 @@ inline r_vector::r_vector(std::initializer_list il) UNPROTECT(n_protected); }); } catch (const unwind_exception& e) { - preserved.release(protect_); + detail::store::release(protect_); UNPROTECT(n_protected); throw e; } diff --git a/inst/include/cpp11/list.hpp b/inst/include/cpp11/list.hpp index 64dd987b..d8e1bb79 100644 --- a/inst/include/cpp11/list.hpp +++ b/inst/include/cpp11/list.hpp @@ -5,7 +5,7 @@ #include "cpp11/R.hpp" // for SEXP, SEXPREC, SET_VECTOR_ELT #include "cpp11/attribute_proxy.hpp" // for attribute_proxy #include "cpp11/named_arg.hpp" // for named_arg -#include "cpp11/protect.hpp" // for preserved +#include "cpp11/protect.hpp" // for store #include "cpp11/r_string.hpp" // for r_string #include "cpp11/r_vector.hpp" // for r_vector, r_vector<>::proxy #include "cpp11/sexp.hpp" // for sexp @@ -78,7 +78,7 @@ template <> inline r_vector::r_vector(std::initializer_list il) : cpp11::r_vector(safe[Rf_allocVector](VECSXP, il.size())), capacity_(il.size()) { - protect_ = preserved.insert(data_); + protect_ = detail::store::insert(data_); auto it = il.begin(); for (R_xlen_t i = 0; i < capacity_; ++i, ++it) { SET_VECTOR_ELT(data_, i, *it); @@ -89,7 +89,7 @@ template <> inline r_vector::r_vector(std::initializer_list il) : cpp11::r_vector(safe[Rf_allocVector](VECSXP, il.size())), capacity_(il.size()) { - protect_ = preserved.insert(data_); + protect_ = detail::store::insert(data_); int n_protected = 0; try { @@ -105,7 +105,7 @@ inline r_vector::r_vector(std::initializer_list il) UNPROTECT(n_protected); }); } catch (const unwind_exception& e) { - preserved.release(protect_); + detail::store::release(protect_); UNPROTECT(n_protected); throw e; } @@ -117,8 +117,8 @@ inline void r_vector::reserve(R_xlen_t new_capacity) { : safe[Rf_xlengthgets](data_, new_capacity); SEXP old_protect = protect_; - protect_ = preserved.insert(data_); - preserved.release(old_protect); + protect_ = detail::store::insert(data_); + detail::store::release(old_protect); capacity_ = new_capacity; } diff --git a/inst/include/cpp11/logicals.hpp b/inst/include/cpp11/logicals.hpp index 6a4e2cfa..8b1d74b2 100644 --- a/inst/include/cpp11/logicals.hpp +++ b/inst/include/cpp11/logicals.hpp @@ -7,7 +7,7 @@ #include "cpp11/R.hpp" // for SEXP, SEXPREC, Rf_all... #include "cpp11/attribute_proxy.hpp" // for attribute_proxy #include "cpp11/named_arg.hpp" // for named_arg -#include "cpp11/protect.hpp" // for preserved +#include "cpp11/protect.hpp" // for store #include "cpp11/r_bool.hpp" // for r_bool #include "cpp11/r_vector.hpp" // for r_vector, r_vector<>::proxy #include "cpp11/sexp.hpp" // for sexp @@ -80,7 +80,7 @@ inline bool operator==(const r_vector::proxy& lhs, r_bool rhs) { template <> inline r_vector::r_vector(std::initializer_list il) : cpp11::r_vector(Rf_allocVector(LGLSXP, il.size())), capacity_(il.size()) { - protect_ = preserved.insert(data_); + protect_ = detail::store::insert(data_); auto it = il.begin(); for (R_xlen_t i = 0; i < capacity_; ++i, ++it) { SET_LOGICAL_ELT(data_, i, *it); @@ -91,7 +91,7 @@ template <> inline r_vector::r_vector(std::initializer_list il) : cpp11::r_vector(safe[Rf_allocVector](LGLSXP, il.size())), capacity_(il.size()) { - protect_ = preserved.insert(data_); + protect_ = detail::store::insert(data_); int n_protected = 0; try { @@ -107,7 +107,7 @@ inline r_vector::r_vector(std::initializer_list il) UNPROTECT(n_protected); }); } catch (const unwind_exception& e) { - preserved.release(protect_); + detail::store::release(protect_); UNPROTECT(n_protected); throw e; } @@ -118,9 +118,9 @@ inline void r_vector::reserve(R_xlen_t new_capacity) { data_ = data_ == R_NilValue ? safe[Rf_allocVector](LGLSXP, new_capacity) : safe[Rf_xlengthgets](data_, new_capacity); SEXP old_protect = protect_; - protect_ = preserved.insert(data_); + protect_ = detail::store::insert(data_); - preserved.release(old_protect); + detail::store::release(old_protect); data_p_ = LOGICAL(data_); capacity_ = new_capacity; diff --git a/inst/include/cpp11/protect.hpp b/inst/include/cpp11/protect.hpp index e4473e6f..0f1cf944 100644 --- a/inst/include/cpp11/protect.hpp +++ b/inst/include/cpp11/protect.hpp @@ -245,84 +245,104 @@ void warning(const std::string& fmt, Args... args) { } #endif -/// A doubly-linked list of preserved objects, allowing O(1) insertion/release of -/// objects compared to O(N preserved) with R_PreserveObject. -static struct { - SEXP insert(SEXP obj) { - if (obj == R_NilValue) { - return R_NilValue; - } +namespace detail { - PROTECT(obj); +// A doubly-linked list of preserved objects, allowing O(1) insertion/release of objects +// compared to O(N preserved) with `R_PreserveObject()` and `R_ReleaseObject()`. +// +// We let R manage the memory of the list itself by calling `R_PreserveObject()` on it. +// +// cpp11 being a header only library makes creating a "global" preserve list a bit tricky. +// The trick we use here is that static local variables in inline extern functions are +// guaranteed by the standard to be unique across the whole program. Inline functions are +// extern by default, but `static inline` functions are not, so do not change these +// functions to `static`. If we did that, we would end up having one preserve list per +// compilation unit instead. As it stands today, we are fairly confident that we have 1 +// preserve list per package, which seems to work nicely. +// https://stackoverflow.com/questions/185624/what-happens-to-static-variables-in-inline-functions +// https://stackoverflow.com/questions/51612866/global-variables-in-header-only-library +// https://github.com/r-lib/cpp11/issues/330 +// +// > A static local variable in an extern inline function always refers to the +// same object. 7.1.2/4 - C++98/C++14 (n3797) +namespace store { + +inline SEXP init() { + SEXP out = Rf_cons(R_NilValue, Rf_cons(R_NilValue, R_NilValue)); + R_PreserveObject(out); + return out; +} - static SEXP list = get_preserve_list(); +inline SEXP get() { + // Note the `static` local variable in the inline extern function here! Guarantees we + // have 1 unique preserve list across all compilation units in the package. + static SEXP out = init(); + return out; +} - // Get references to the head of the precious list and the next element - // after the head - SEXP head = list; - SEXP next = CDR(list); +inline R_xlen_t count() { + const R_xlen_t head = 1; + const R_xlen_t tail = 1; + SEXP list = get(); + return Rf_xlength(list) - head - tail; +} - // Add a new cell that points to the current head + next. - SEXP cell = PROTECT(Rf_cons(head, next)); - SET_TAG(cell, obj); +inline SEXP insert(SEXP x) { + if (x == R_NilValue) { + return R_NilValue; + } - // Update the head + next to point at the newly-created cell, - // effectively inserting that cell between the current head + next. - SETCDR(head, cell); - SETCAR(next, cell); + PROTECT(x); - UNPROTECT(2); + SEXP list = get(); - return cell; - } + // Get references to the head of the preserve list and the next element + // after the head + SEXP head = list; + SEXP next = CDR(list); - void print() { - static SEXP list = get_preserve_list(); - for (SEXP cell = list; cell != R_NilValue; cell = CDR(cell)) { - REprintf("%p CAR: %p CDR: %p TAG: %p\n", reinterpret_cast(cell), - reinterpret_cast(CAR(cell)), reinterpret_cast(CDR(cell)), - reinterpret_cast(TAG(cell))); - } - REprintf("---\n"); - } + // Add a new cell that points to the current head + next. + SEXP cell = PROTECT(Rf_cons(head, next)); + SET_TAG(cell, x); - void release(SEXP cell) { - if (cell == R_NilValue) { - return; - } + // Update the head + next to point at the newly-created cell, + // effectively inserting that cell between the current head + next. + SETCDR(head, cell); + SETCAR(next, cell); - // Get a reference to the cells before and after the token. - SEXP lhs = CAR(cell); - SEXP rhs = CDR(cell); + UNPROTECT(2); - // Remove the cell from the precious list -- effectively, we do this - // by updating the 'lhs' and 'rhs' references to point at each-other, - // effectively removing any references to the cell in the pairlist. - SETCDR(lhs, rhs); - SETCAR(rhs, lhs); - } + return cell; +} - private: - // Each compilation unit purposefully gets its own preserve list. - // This avoids issues with sharing preserve list state across compilation units - // and across packages, which has historically caused many issues (#330). - static SEXP get_preserve_list() { - static SEXP out = init_preserve_list(); - return out; +inline void release(SEXP cell) { + if (cell == R_NilValue) { + return; } - static SEXP init_preserve_list() { - // Initialize the list exactly once per compilation unit, - // and let R manage its memory - SEXP out = new_preserve_list(); - R_PreserveObject(out); - return out; - } + // Get a reference to the cells before and after the token. + SEXP lhs = CAR(cell); + SEXP rhs = CDR(cell); - static SEXP new_preserve_list() { - return Rf_cons(R_NilValue, Rf_cons(R_NilValue, R_NilValue)); + // Remove the cell from the preserve list -- effectively, we do this + // by updating the 'lhs' and 'rhs' references to point at each-other, + // effectively removing any references to the cell in the pairlist. + SETCDR(lhs, rhs); + SETCAR(rhs, lhs); +} + +inline void print() { + SEXP list = get(); + for (SEXP cell = list; cell != R_NilValue; cell = CDR(cell)) { + REprintf("%p CAR: %p CDR: %p TAG: %p\n", reinterpret_cast(cell), + reinterpret_cast(CAR(cell)), reinterpret_cast(CDR(cell)), + reinterpret_cast(TAG(cell))); } + REprintf("---\n"); +} -} preserved; +} // namespace store + +} // namespace detail } // namespace cpp11 diff --git a/inst/include/cpp11/r_bool.hpp b/inst/include/cpp11/r_bool.hpp index a3bb18eb..4ea36da8 100644 --- a/inst/include/cpp11/r_bool.hpp +++ b/inst/include/cpp11/r_bool.hpp @@ -7,7 +7,7 @@ #include "R_ext/Boolean.h" // for Rboolean #include "cpp11/R.hpp" // for SEXP, SEXPREC, ... #include "cpp11/as.hpp" // for as_sexp -#include "cpp11/protect.hpp" // for unwind_protect, preserved +#include "cpp11/protect.hpp" // for unwind_protect #include "cpp11/r_vector.hpp" #include "cpp11/sexp.hpp" // for sexp diff --git a/inst/include/cpp11/r_vector.hpp b/inst/include/cpp11/r_vector.hpp index 4831c2f5..22739af2 100644 --- a/inst/include/cpp11/r_vector.hpp +++ b/inst/include/cpp11/r_vector.hpp @@ -15,7 +15,7 @@ #include "cpp11/R.hpp" // for R_xlen_t, SEXP, SEXPREC, Rf_xle... #include "cpp11/attribute_proxy.hpp" // for attribute_proxy -#include "cpp11/protect.hpp" // for preserved +#include "cpp11/protect.hpp" // for store #include "cpp11/r_string.hpp" // for r_string #include "cpp11/sexp.hpp" // for sexp @@ -82,12 +82,12 @@ class r_vector { SEXP old_protect = protect_; data_ = rhs.data_; - protect_ = preserved.insert(data_); + protect_ = detail::store::insert(data_); is_altrep_ = rhs.is_altrep_; data_p_ = rhs.data_p_; length_ = rhs.length_; - preserved.release(old_protect); + detail::store::release(old_protect); return *this; }; @@ -96,12 +96,12 @@ class r_vector { SEXP old_protect = protect_; data_ = rhs.data_; - protect_ = preserved.insert(data_); + protect_ = detail::store::insert(data_); is_altrep_ = rhs.is_altrep_; data_p_ = rhs.data_p_; length_ = rhs.length_; - preserved.release(old_protect); + detail::store::release(old_protect); }; r_vector(const writable::r_vector& rhs) : r_vector(static_cast(rhs)) {} @@ -189,7 +189,7 @@ class r_vector { const_iterator find(const r_string& name) const; - ~r_vector() { preserved.release(protect_); } + ~r_vector() { detail::store::release(protect_); } private: SEXP data_ = R_NilValue; @@ -371,7 +371,7 @@ class r_vector : public cpp11::r_vector { template inline r_vector::r_vector(const SEXP data) : data_(valid_type(data)), - protect_(preserved.insert(data)), + protect_(detail::store::insert(data)), is_altrep_(ALTREP(data)), data_p_(get_p(ALTREP(data), data)), length_(Rf_xlength(data)) {} @@ -379,7 +379,7 @@ inline r_vector::r_vector(const SEXP data) template inline r_vector::r_vector(const SEXP data, bool is_altrep) : data_(valid_type(data)), - protect_(preserved.insert(data)), + protect_(detail::store::insert(data)), is_altrep_(is_altrep), data_p_(get_p(is_altrep, data)), length_(Rf_xlength(data)) {} @@ -661,23 +661,25 @@ inline typename r_vector::iterator r_vector::end() const { template inline r_vector::r_vector(const SEXP& data) : cpp11::r_vector(safe[Rf_shallow_duplicate](data)), - protect_(preserved.insert(data_)), + protect_(detail::store::insert(data_)), capacity_(length_) {} template inline r_vector::r_vector(const SEXP& data, bool is_altrep) : cpp11::r_vector(safe[Rf_shallow_duplicate](data), is_altrep), - protect_(preserved.insert(data_)), + protect_(detail::store::insert(data_)), capacity_(length_) {} template inline r_vector::r_vector(SEXP&& data) - : cpp11::r_vector(data), protect_(preserved.insert(data_)), capacity_(length_) {} + : cpp11::r_vector(data), + protect_(detail::store::insert(data_)), + capacity_(length_) {} template inline r_vector::r_vector(SEXP&& data, bool is_altrep) : cpp11::r_vector(data, is_altrep), - protect_(preserved.insert(data_)), + protect_(detail::store::insert(data_)), capacity_(length_) {} template @@ -709,7 +711,7 @@ inline r_vector::r_vector(const R_xlen_t size) : r_vector() { template inline r_vector::~r_vector() { - preserved.release(protect_); + detail::store::release(protect_); } #ifdef LONG_VECTOR_SUPPORT @@ -792,7 +794,7 @@ inline typename r_vector::iterator r_vector::find(const r_string& name) co template inline r_vector::r_vector(const r_vector& rhs) : cpp11::r_vector(safe[Rf_shallow_duplicate](rhs)), - protect_(preserved.insert(data_)), + protect_(detail::store::insert(data_)), capacity_(rhs.capacity_) {} template @@ -805,7 +807,7 @@ inline r_vector::r_vector(r_vector&& rhs) template inline r_vector::r_vector(const cpp11::r_vector& rhs) : cpp11::r_vector(safe[Rf_shallow_duplicate](rhs)), - protect_(preserved.insert(data_)), + protect_(detail::store::insert(data_)), capacity_(rhs.length_) {} // We don't release the old object until the end in case we throw an exception @@ -821,9 +823,9 @@ inline r_vector& r_vector::operator=(const r_vector& rhs) { auto old_protect = protect_; data_ = safe[Rf_shallow_duplicate](rhs.data_); - protect_ = preserved.insert(data_); + protect_ = detail::store::insert(data_); - preserved.release(old_protect); + detail::store::release(old_protect); capacity_ = rhs.capacity_; @@ -841,9 +843,9 @@ inline r_vector& r_vector::operator=(r_vector&& rhs) { SEXP old_protect = protect_; data_ = rhs.data_; - protect_ = preserved.insert(data_); + protect_ = detail::store::insert(data_); - preserved.release(old_protect); + detail::store::release(old_protect); capacity_ = rhs.capacity_; diff --git a/inst/include/cpp11/raws.hpp b/inst/include/cpp11/raws.hpp index a5cb392f..e518f01e 100644 --- a/inst/include/cpp11/raws.hpp +++ b/inst/include/cpp11/raws.hpp @@ -8,7 +8,7 @@ #include "cpp11/R.hpp" // for RAW, SEXP, SEXPREC, Rf_allocVector #include "cpp11/attribute_proxy.hpp" // for attribute_proxy #include "cpp11/named_arg.hpp" // for named_arg -#include "cpp11/protect.hpp" // for preserved +#include "cpp11/protect.hpp" // for store #include "cpp11/r_vector.hpp" // for r_vector, r_vector<>::proxy #include "cpp11/sexp.hpp" // for sexp @@ -88,7 +88,7 @@ template <> inline r_vector::r_vector(std::initializer_list il) : cpp11::r_vector(safe[Rf_allocVector](RAWSXP, il.size())), capacity_(il.size()) { - protect_ = preserved.insert(data_); + protect_ = detail::store::insert(data_); auto it = il.begin(); for (R_xlen_t i = 0; i < capacity_; ++i, ++it) { data_p_[i] = *it; @@ -99,7 +99,7 @@ template <> inline r_vector::r_vector(std::initializer_list il) : cpp11::r_vector(safe[Rf_allocVector](RAWSXP, il.size())), capacity_(il.size()) { - protect_ = preserved.insert(data_); + protect_ = detail::store::insert(data_); int n_protected = 0; try { @@ -116,7 +116,7 @@ inline r_vector::r_vector(std::initializer_list il) UNPROTECT(n_protected); }); } catch (const unwind_exception& e) { - preserved.release(protect_); + detail::store::release(protect_); UNPROTECT(n_protected); throw e; } @@ -128,8 +128,8 @@ inline void r_vector::reserve(R_xlen_t new_capacity) { : safe[Rf_xlengthgets](data_, new_capacity); SEXP old_protect = protect_; - protect_ = preserved.insert(data_); - preserved.release(old_protect); + protect_ = detail::store::insert(data_); + detail::store::release(old_protect); data_p_ = RAW(data_); capacity_ = new_capacity; diff --git a/inst/include/cpp11/sexp.hpp b/inst/include/cpp11/sexp.hpp index 9f6f5e18..1000a6d4 100644 --- a/inst/include/cpp11/sexp.hpp +++ b/inst/include/cpp11/sexp.hpp @@ -6,7 +6,7 @@ #include "cpp11/R.hpp" // for SEXP, SEXPREC, REAL_ELT, R_NilV... #include "cpp11/attribute_proxy.hpp" // for attribute_proxy -#include "cpp11/protect.hpp" // for preserved +#include "cpp11/protect.hpp" // for store namespace cpp11 { @@ -19,14 +19,14 @@ class sexp { public: sexp() = default; - sexp(SEXP data) : data_(data), preserve_token_(preserved.insert(data_)) { + sexp(SEXP data) : data_(data), preserve_token_(detail::store::insert(data_)) { // REprintf("created %p %p\n", reinterpret_cast(data_), // reinterpret_cast(preserve_token_)); } sexp(const sexp& rhs) { data_ = rhs.data_; - preserve_token_ = preserved.insert(data_); + preserve_token_ = detail::store::insert(data_); // REprintf("copied %p new protect %p\n", reinterpret_cast(rhs.data_), // reinterpret_cast(preserve_token_)); } @@ -42,10 +42,10 @@ class sexp { } sexp& operator=(const sexp& rhs) { - preserved.release(preserve_token_); + detail::store::release(preserve_token_); data_ = rhs.data_; - preserve_token_ = preserved.insert(data_); + preserve_token_ = detail::store::insert(data_); // REprintf("assigned %p\n", reinterpret_cast(rhs.data_)); return *this; } @@ -56,7 +56,7 @@ class sexp { //*this = tmp; //} - ~sexp() { preserved.release(preserve_token_); } + ~sexp() { detail::store::release(preserve_token_); } attribute_proxy attr(const char* name) const { return attribute_proxy(*this, name); diff --git a/inst/include/cpp11/strings.hpp b/inst/include/cpp11/strings.hpp index e312c231..a9c56223 100644 --- a/inst/include/cpp11/strings.hpp +++ b/inst/include/cpp11/strings.hpp @@ -7,7 +7,7 @@ #include "cpp11/as.hpp" // for as_sexp #include "cpp11/attribute_proxy.hpp" // for attribute_proxy #include "cpp11/named_arg.hpp" // for named_arg -#include "cpp11/protect.hpp" // for preserved +#include "cpp11/protect.hpp" // for store #include "cpp11/r_string.hpp" // for r_string #include "cpp11/r_vector.hpp" // for r_vector, r_vector<>::proxy #include "cpp11/sexp.hpp" // for sexp @@ -95,7 +95,7 @@ inline SEXP alloc_if_charsxp(const SEXP data) { template <> inline r_vector::r_vector(const SEXP& data) : cpp11::r_vector(alloc_or_copy(data)), - protect_(preserved.insert(data_)), + protect_(detail::store::insert(data_)), capacity_(length_) { if (TYPEOF(data) == CHARSXP) { SET_STRING_ELT(data_, 0, data); @@ -105,7 +105,7 @@ inline r_vector::r_vector(const SEXP& data) template <> inline r_vector::r_vector(SEXP&& data) : cpp11::r_vector(alloc_if_charsxp(data)), - protect_(preserved.insert(data_)), + protect_(detail::store::insert(data_)), capacity_(length_) { if (TYPEOF(data) == CHARSXP) { SET_STRING_ELT(data_, 0, data); @@ -120,7 +120,7 @@ template <> inline r_vector::r_vector(std::initializer_list il) : cpp11::r_vector(safe[Rf_allocVector](STRSXP, il.size())), capacity_(il.size()) { - protect_ = preserved.insert(data_); + protect_ = detail::store::insert(data_); int n_protected = 0; try { @@ -136,7 +136,7 @@ inline r_vector::r_vector(std::initializer_list il) UNPROTECT(n_protected); }); } catch (const unwind_exception& e) { - preserved.release(protect_); + detail::store::release(protect_); UNPROTECT(n_protected); throw e; } @@ -148,8 +148,8 @@ inline void r_vector::reserve(R_xlen_t new_capacity) { : safe[Rf_xlengthgets](data_, new_capacity); SEXP old_protect = protect_; - protect_ = preserved.insert(data_); - preserved.release(old_protect); + protect_ = detail::store::insert(data_); + detail::store::release(old_protect); capacity_ = new_capacity; } diff --git a/vignettes/internals.Rmd b/vignettes/internals.Rmd index 66d86111..ece5809a 100644 --- a/vignettes/internals.Rmd +++ b/vignettes/internals.Rmd @@ -126,15 +126,16 @@ Each node in the list uses the head (`CAR`) part to point to the previous node, The `TAG` is used to point to the object being protected. The head and tail of the list have `R_NilValue` as their `CAR` and `CDR` pointers respectively. -Calling `preserved.insert()` with a regular R object will add a new node to the list and return a protect token corresponding to the node added. -Calling `preserved.release()` on this returned token will release the protection by unlinking the node from the linked list. +Calling `cpp11::detail::store::insert()` with a regular R object will add a new node to the list and return a protect token corresponding to the node added. +Calling `cpp11::detail::store::release()` on this returned token will release the protection by unlinking the node from the linked list. +These two functions are considered internal to cpp11, so do not use them in your packages. This scheme scales in O(1) time to release or insert an object vs O(N) or worse time with `R_PreserveObject()` / `R_ReleaseObject()`. -Each compilation unit has its own unique protection list, which avoids the need to manage a "global" protection list shared across compilation units within the same package and across packages. -A previous version of cpp11 used a global protection list, but this caused [multiple issues](https://github.com/r-lib/cpp11/issues/330). +Each package has its own unique protection list, which avoids the need to manage a "global" protection list shared across packages. +A previous version of cpp11 used a global protection list stored in an R global option, but this caused [multiple issues](https://github.com/r-lib/cpp11/issues/330). -These functions are defined in [protect.hpp](https://github.com/r-lib/cpp11/blob/main/inst/include/cpp11/protect.hpp) +These functions are defined in [protect.hpp](https://github.com/r-lib/cpp11/blob/main/inst/include/cpp11/protect.hpp). ### Unwind Protect