Skip to content

Commit d6b750b

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 d6b750b

File tree

10 files changed

+65
-25
lines changed

10 files changed

+65
-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: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ class r_vector {
5656
typedef T* pointer;
5757
typedef T& reference;
5858

59+
using underlying_type = typename traits::get_underlying_type<T>::type;
60+
5961
r_vector() noexcept = default;
6062

6163
r_vector(SEXP data);
@@ -173,7 +175,7 @@ class r_vector {
173175
void fill_buf(R_xlen_t pos);
174176

175177
R_xlen_t pos_;
176-
std::array<T, 64 * 64> buf_;
178+
std::array<underlying_type, 64 * 64> buf_;
177179
R_xlen_t block_start_ = 0;
178180
R_xlen_t length_ = 0;
179181
};
@@ -193,10 +195,10 @@ class r_vector {
193195
SEXP data_ = R_NilValue;
194196
SEXP protect_ = R_NilValue;
195197
bool is_altrep_ = false;
196-
T* data_p_ = nullptr;
198+
underlying_type* data_p_ = nullptr;
197199
R_xlen_t length_ = 0;
198200

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

201203
static SEXP valid_type(SEXP data);
202204

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

217219
// These are necessary because type names are not directly accessible in
218220
// template inheritance
221+
using typename cpp11::r_vector<T>::underlying_type;
222+
219223
using cpp11::r_vector<T>::data_;
220224
using cpp11::r_vector<T>::data_p_;
221225
using cpp11::r_vector<T>::is_altrep_;
@@ -228,11 +232,11 @@ class r_vector : public cpp11::r_vector<T> {
228232
private:
229233
const SEXP data_;
230234
const R_xlen_t index_;
231-
T* const p_;
235+
underlying_type* const p_;
232236
bool is_altrep_;
233237

234238
public:
235-
proxy(SEXP data, const R_xlen_t index, T* const p, bool is_altrep);
239+
proxy(SEXP data, const R_xlen_t index, underlying_type* const p, bool is_altrep);
236240

237241
proxy& operator=(const T& rhs);
238242
proxy& operator+=(const T& rhs);
@@ -572,9 +576,9 @@ inline typename cpp11::r_vector<T>::const_iterator cpp11::r_vector<T>::find(
572576
template <typename T>
573577
inline T r_vector<T>::const_iterator::operator*() const {
574578
if (data_->is_altrep()) {
575-
return buf_[pos_ - block_start_];
579+
return static_cast<T>(buf_[pos_ - block_start_]);
576580
} else {
577-
return data_->data_p_[pos_];
581+
return static_cast<T>(data_->data_p_[pos_]);
578582
}
579583
}
580584

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

600604
template <typename T>
601-
r_vector<T>::proxy::proxy(SEXP data, const R_xlen_t index, T* const p, bool is_altrep)
605+
r_vector<T>::proxy::proxy(SEXP data, const R_xlen_t index,
606+
typename r_vector<T>::underlying_type* const p, bool is_altrep)
602607
: data_(data), index_(index), p_(p), is_altrep_(is_altrep) {}
603608

604609
template <typename T>
605610
inline typename r_vector<T>::proxy r_vector<T>::iterator::operator*() const {
606611
if (data_.is_altrep()) {
607-
return proxy(data_.data(), pos_, const_cast<T*>(&buf_[pos_ - block_start_]), true);
612+
return proxy(
613+
data_.data(), pos_,
614+
const_cast<typename r_vector<T>::underlying_type*>(&buf_[pos_ - block_start_]),
615+
true);
608616
} else {
609617
return proxy(data_.data(), pos_,
610618
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)