Skip to content

Commit 84ff462

Browse files
committed
Ensure that data_p_ is typed with the underlying type of data_
Previously, a `reinterpret_cast()` was used before assigning to `data_` so that: - For raws, `RAW()` was reinterpreted from `Rbyte* -> uint8_t*` - For logicals, `LOGICAL()` was reinterpreted from `int* -> r_bool*`
1 parent 322e5ee commit 84ff462

File tree

10 files changed

+64
-25
lines changed

10 files changed

+64
-25
lines changed

inst/include/cpp11/R.hpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,13 @@ constexpr R_xlen_t operator"" _xl(unsigned long long int value) { return value;
3333

3434
} // namespace literals
3535

36+
namespace traits {
37+
template <typename T>
38+
struct get_underlying_type {
39+
using type = T;
40+
};
41+
} // namespace traits
42+
3643
template <typename T>
3744
inline T na();
3845

inst/include/cpp11/doubles.hpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ inline double r_vector<double>::operator[](const R_xlen_t pos) const {
3434
}
3535

3636
template <>
37-
inline double* r_vector<double>::get_p(bool is_altrep, SEXP data) {
37+
inline typename r_vector<double>::underlying_type* r_vector<double>::get_p(bool is_altrep,
38+
SEXP data) {
3839
if (is_altrep) {
3940
return nullptr;
4041
} else {

inst/include/cpp11/integers.hpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ inline int r_vector<int>::operator[](const R_xlen_t pos) const {
3535
}
3636

3737
template <>
38-
inline int* r_vector<int>::get_p(bool is_altrep, SEXP data) {
38+
inline typename r_vector<int>::underlying_type* r_vector<int>::get_p(bool is_altrep,
39+
SEXP data) {
3940
if (is_altrep) {
4041
return nullptr;
4142
} else {

inst/include/cpp11/list.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ inline SEXP r_vector<SEXP>::operator[](const r_string& name) const {
4545
}
4646

4747
template <>
48-
inline SEXP* r_vector<SEXP>::get_p(bool, SEXP) {
48+
inline typename r_vector<SEXP>::underlying_type* r_vector<SEXP>::get_p(bool, SEXP) {
4949
return nullptr;
5050
}
5151

inst/include/cpp11/logicals.hpp

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,22 +29,23 @@ inline SEXP r_vector<r_bool>::valid_type(SEXP data) {
2929

3030
template <>
3131
inline r_bool r_vector<r_bool>::operator[](const R_xlen_t pos) const {
32-
return is_altrep_ ? static_cast<r_bool>(LOGICAL_ELT(data_, pos)) : data_p_[pos];
32+
return is_altrep_ ? LOGICAL_ELT(data_, pos) : data_p_[pos];
3333
}
3434

3535
template <>
36-
inline r_bool* r_vector<r_bool>::get_p(bool is_altrep, SEXP data) {
36+
inline typename r_vector<r_bool>::underlying_type* r_vector<r_bool>::get_p(bool is_altrep,
37+
SEXP data) {
3738
if (is_altrep) {
3839
return nullptr;
3940
} else {
40-
return reinterpret_cast<r_bool*>(LOGICAL(data));
41+
return LOGICAL(data);
4142
}
4243
}
4344

4445
template <>
4546
inline void r_vector<r_bool>::const_iterator::fill_buf(R_xlen_t pos) {
4647
length_ = std::min(64_xl, data_->size() - pos);
47-
LOGICAL_GET_REGION(data_->data_, pos, length_, reinterpret_cast<int*>(buf_.data()));
48+
LOGICAL_GET_REGION(data_->data_, pos, length_, buf_.data());
4849
block_start_ = pos;
4950
}
5051

@@ -66,7 +67,7 @@ inline typename r_vector<r_bool>::proxy& r_vector<r_bool>::proxy::operator=(
6667
template <>
6768
inline r_vector<r_bool>::proxy::operator r_bool() const {
6869
if (p_ == nullptr) {
69-
return static_cast<r_bool>(LOGICAL_ELT(data_, index_));
70+
return LOGICAL_ELT(data_, index_);
7071
} else {
7172
return *p_;
7273
}
@@ -100,7 +101,7 @@ inline r_vector<r_bool>::r_vector(std::initializer_list<named_arg> il)
100101
++n_protected;
101102
auto it = il.begin();
102103
for (R_xlen_t i = 0; i < capacity_; ++i, ++it) {
103-
data_p_[i] = static_cast<r_bool>(LOGICAL_ELT(it->value(), 0));
104+
data_p_[i] = LOGICAL_ELT(it->value(), 0);
104105
SET_STRING_ELT(names, i, Rf_mkCharCE(it->name(), CE_UTF8));
105106
}
106107
UNPROTECT(n_protected);
@@ -121,7 +122,7 @@ inline void r_vector<r_bool>::reserve(R_xlen_t new_capacity) {
121122

122123
preserved.release(old_protect);
123124

124-
data_p_ = reinterpret_cast<r_bool*>(LOGICAL(data_));
125+
data_p_ = LOGICAL(data_);
125126
capacity_ = new_capacity;
126127
}
127128

inst/include/cpp11/r_bool.hpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,4 +71,11 @@ inline r_bool na() {
7171
return NA_LOGICAL;
7272
}
7373

74+
namespace traits {
75+
template <>
76+
struct get_underlying_type<r_bool> {
77+
using type = int;
78+
};
79+
} // namespace traits
80+
7481
} // namespace cpp11

inst/include/cpp11/r_string.hpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,4 +93,11 @@ inline r_string na() {
9393
return NA_STRING;
9494
}
9595

96+
namespace traits {
97+
template <>
98+
struct get_underlying_type<r_string> {
99+
using type = SEXP;
100+
};
101+
} // namespace traits
102+
96103
} // namespace cpp11

inst/include/cpp11/r_vector.hpp

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ class r_vector {
5555
typedef T value_type;
5656
typedef T* pointer;
5757
typedef T& reference;
58+
using underlying_type = typename traits::get_underlying_type<T>::type;
5859

5960
r_vector() noexcept = default;
6061

@@ -173,7 +174,7 @@ class r_vector {
173174
void fill_buf(R_xlen_t pos);
174175

175176
R_xlen_t pos_;
176-
std::array<T, 64 * 64> buf_;
177+
std::array<underlying_type, 64 * 64> buf_;
177178
R_xlen_t block_start_ = 0;
178179
R_xlen_t length_ = 0;
179180
};
@@ -193,10 +194,10 @@ class r_vector {
193194
SEXP data_ = R_NilValue;
194195
SEXP protect_ = R_NilValue;
195196
bool is_altrep_ = false;
196-
T* data_p_ = nullptr;
197+
underlying_type* data_p_ = nullptr;
197198
R_xlen_t length_ = 0;
198199

199-
static T* get_p(bool is_altrep, SEXP data);
200+
static underlying_type* get_p(bool is_altrep, SEXP data);
200201

201202
static SEXP valid_type(SEXP data);
202203

@@ -216,6 +217,8 @@ class r_vector : public cpp11::r_vector<T> {
216217

217218
// These are necessary because type names are not directly accessible in
218219
// template inheritance
220+
using typename cpp11::r_vector<T>::underlying_type;
221+
219222
using cpp11::r_vector<T>::data_;
220223
using cpp11::r_vector<T>::data_p_;
221224
using cpp11::r_vector<T>::is_altrep_;
@@ -228,11 +231,11 @@ class r_vector : public cpp11::r_vector<T> {
228231
private:
229232
const SEXP data_;
230233
const R_xlen_t index_;
231-
T* const p_;
234+
underlying_type* const p_;
232235
bool is_altrep_;
233236

234237
public:
235-
proxy(SEXP data, const R_xlen_t index, T* const p, bool is_altrep);
238+
proxy(SEXP data, const R_xlen_t index, underlying_type* const p, bool is_altrep);
236239

237240
proxy& operator=(const T& rhs);
238241
proxy& operator+=(const T& rhs);
@@ -572,9 +575,9 @@ inline typename cpp11::r_vector<T>::const_iterator cpp11::r_vector<T>::find(
572575
template <typename T>
573576
inline T r_vector<T>::const_iterator::operator*() const {
574577
if (data_->is_altrep()) {
575-
return buf_[pos_ - block_start_];
578+
return static_cast<T>(buf_[pos_ - block_start_]);
576579
} else {
577-
return data_->data_p_[pos_];
580+
return static_cast<T>(data_->data_p_[pos_]);
578581
}
579582
}
580583

@@ -598,13 +601,17 @@ inline T r_vector<T>::operator[](size_type pos) const {
598601
namespace writable {
599602

600603
template <typename T>
601-
r_vector<T>::proxy::proxy(SEXP data, const R_xlen_t index, T* const p, bool is_altrep)
604+
r_vector<T>::proxy::proxy(SEXP data, const R_xlen_t index,
605+
typename r_vector<T>::underlying_type* const p, bool is_altrep)
602606
: data_(data), index_(index), p_(p), is_altrep_(is_altrep) {}
603607

604608
template <typename T>
605609
inline typename r_vector<T>::proxy r_vector<T>::iterator::operator*() const {
606610
if (data_.is_altrep()) {
607-
return proxy(data_.data(), pos_, const_cast<T*>(&buf_[pos_ - block_start_]), true);
611+
return proxy(
612+
data_.data(), pos_,
613+
const_cast<typename r_vector<T>::underlying_type*>(&buf_[pos_ - block_start_]),
614+
true);
608615
} else {
609616
return proxy(data_.data(), pos_,
610617
data_.data_p_ != nullptr ? &data_.data_p_[pos_] : nullptr, false);

inst/include/cpp11/raws.hpp

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,13 @@
1616

1717
namespace cpp11 {
1818

19+
namespace traits {
20+
template <>
21+
struct get_underlying_type<uint8_t> {
22+
using type = Rbyte;
23+
};
24+
} // namespace traits
25+
1926
template <>
2027
inline SEXP r_vector<uint8_t>::valid_type(SEXP data) {
2128
if (data == nullptr) {
@@ -34,20 +41,20 @@ inline uint8_t r_vector<uint8_t>::operator[](const R_xlen_t pos) const {
3441
}
3542

3643
template <>
37-
inline uint8_t* r_vector<uint8_t>::get_p(bool is_altrep, SEXP data) {
44+
inline typename r_vector<uint8_t>::underlying_type* r_vector<uint8_t>::get_p(
45+
bool is_altrep, SEXP data) {
3846
if (is_altrep) {
3947
return nullptr;
4048
} else {
41-
return reinterpret_cast<uint8_t*>(RAW(data));
49+
return RAW(data);
4250
}
4351
}
4452

4553
template <>
4654
inline void r_vector<uint8_t>::const_iterator::fill_buf(R_xlen_t pos) {
4755
using namespace cpp11::literals;
4856
length_ = std::min(64_xl, data_->size() - pos);
49-
unwind_protect(
50-
[&] { RAW_GET_REGION(data_->data_, pos, length_, (uint8_t*)buf_.data()); });
57+
unwind_protect([&] { RAW_GET_REGION(data_->data_, pos, length_, buf_.data()); });
5158
block_start_ = pos;
5259
}
5360

@@ -124,7 +131,7 @@ inline void r_vector<uint8_t>::reserve(R_xlen_t new_capacity) {
124131
protect_ = preserved.insert(data_);
125132
preserved.release(old_protect);
126133

127-
data_p_ = reinterpret_cast<uint8_t*>(RAW(data_));
134+
data_p_ = RAW(data_);
128135
capacity_ = new_capacity;
129136
}
130137

inst/include/cpp11/strings.hpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ inline r_string r_vector<r_string>::operator[](const R_xlen_t pos) const {
3434
}
3535

3636
template <>
37-
inline r_string* r_vector<r_string>::get_p(bool, SEXP) {
37+
inline typename r_vector<r_string>::underlying_type* r_vector<r_string>::get_p(bool,
38+
SEXP) {
3839
return nullptr;
3940
}
4041

0 commit comments

Comments
 (0)