Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify preserve list approach #364

Merged
merged 5 commits into from
Jul 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
20 changes: 4 additions & 16 deletions cpp11test/src/protect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand All @@ -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
Expand All @@ -63,12 +51,12 @@
[[cpp11::register]] void protect_many_cpp11_(int n) {
std::vector<SEXP> 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();
}
}
Expand Down
13 changes: 0 additions & 13 deletions cpp11test/src/test-protect.cpp
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
8 changes: 4 additions & 4 deletions inst/include/cpp11/doubles.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ template <>
inline r_vector<double>::r_vector(std::initializer_list<named_arg> il)
: cpp11::r_vector<double>(safe[Rf_allocVector](REALSXP, il.size())),
capacity_(il.size()) {
protect_ = preserved.insert(data_);
protect_ = detail::store::insert(data_);
int n_protected = 0;

try {
Expand All @@ -100,7 +100,7 @@ inline r_vector<double>::r_vector(std::initializer_list<named_arg> il)
UNPROTECT(n_protected);
});
} catch (const unwind_exception& e) {
preserved.release(protect_);
detail::store::release(protect_);
UNPROTECT(n_protected);
throw e;
}
Expand All @@ -111,8 +111,8 @@ inline void r_vector<double>::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;
Expand Down
10 changes: 5 additions & 5 deletions inst/include/cpp11/integers.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -87,10 +87,10 @@ inline void r_vector<int>::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;
Expand All @@ -100,7 +100,7 @@ template <>
inline r_vector<int>::r_vector(std::initializer_list<named_arg> il)
: cpp11::r_vector<int>(safe[Rf_allocVector](INTSXP, il.size())),
capacity_(il.size()) {
protect_ = preserved.insert(data_);
protect_ = detail::store::insert(data_);
int n_protected = 0;

try {
Expand All @@ -116,7 +116,7 @@ inline r_vector<int>::r_vector(std::initializer_list<named_arg> il)
UNPROTECT(n_protected);
});
} catch (const unwind_exception& e) {
preserved.release(protect_);
detail::store::release(protect_);
UNPROTECT(n_protected);
throw e;
}
Expand Down
12 changes: 6 additions & 6 deletions inst/include/cpp11/list.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -78,7 +78,7 @@ template <>
inline r_vector<SEXP>::r_vector(std::initializer_list<SEXP> il)
: cpp11::r_vector<SEXP>(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);
Expand All @@ -89,7 +89,7 @@ template <>
inline r_vector<SEXP>::r_vector(std::initializer_list<named_arg> il)
: cpp11::r_vector<SEXP>(safe[Rf_allocVector](VECSXP, il.size())),
capacity_(il.size()) {
protect_ = preserved.insert(data_);
protect_ = detail::store::insert(data_);
int n_protected = 0;

try {
Expand All @@ -105,7 +105,7 @@ inline r_vector<SEXP>::r_vector(std::initializer_list<named_arg> il)
UNPROTECT(n_protected);
});
} catch (const unwind_exception& e) {
preserved.release(protect_);
detail::store::release(protect_);
UNPROTECT(n_protected);
throw e;
}
Expand All @@ -117,8 +117,8 @@ inline void r_vector<SEXP>::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;
}
Expand Down
12 changes: 6 additions & 6 deletions inst/include/cpp11/logicals.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -80,7 +80,7 @@ inline bool operator==(const r_vector<r_bool>::proxy& lhs, r_bool rhs) {
template <>
inline r_vector<r_bool>::r_vector(std::initializer_list<r_bool> il)
: cpp11::r_vector<r_bool>(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);
Expand All @@ -91,7 +91,7 @@ template <>
inline r_vector<r_bool>::r_vector(std::initializer_list<named_arg> il)
: cpp11::r_vector<r_bool>(safe[Rf_allocVector](LGLSXP, il.size())),
capacity_(il.size()) {
protect_ = preserved.insert(data_);
protect_ = detail::store::insert(data_);
int n_protected = 0;

try {
Expand All @@ -107,7 +107,7 @@ inline r_vector<r_bool>::r_vector(std::initializer_list<named_arg> il)
UNPROTECT(n_protected);
});
} catch (const unwind_exception& e) {
preserved.release(protect_);
detail::store::release(protect_);
UNPROTECT(n_protected);
throw e;
}
Expand All @@ -118,9 +118,9 @@ inline void r_vector<r_bool>::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;
Expand Down
Loading
Loading