Ensure that data_p_ is typed with the underlying type of data_ (#267)

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*`

Author: DavisVaughan

inst/include/cpp11/R.hpp

@@ -37,6 +37,13 @@ constexpr R_xlen_t operator"" _xl(unsigned long long int value) { return value;
 
 }  // namespace literals
 
+namespace traits {
+template <typename T>
+struct get_underlying_type {
+  using type = T;
+};
+}  // namespace traits
+
 template <typename T>
 inline T na();
 

inst/include/cpp11/doubles.hpp

@@ -34,7 +34,8 @@ inline double r_vector<double>::operator[](const R_xlen_t pos) const {
 }
 
 template <>
-inline double* r_vector<double>::get_p(bool is_altrep, SEXP data) {
+inline typename r_vector<double>::underlying_type* r_vector<double>::get_p(bool is_altrep,
+                                                                           SEXP data) {
   if (is_altrep) {
     return nullptr;
   } else {

inst/include/cpp11/integers.hpp

@@ -35,7 +35,8 @@ inline int r_vector<int>::operator[](const R_xlen_t pos) const {
 }
 
 template <>
-inline int* r_vector<int>::get_p(bool is_altrep, SEXP data) {
+inline typename r_vector<int>::underlying_type* r_vector<int>::get_p(bool is_altrep,
+                                                                     SEXP data) {
   if (is_altrep) {
     return nullptr;
   } else {

inst/include/cpp11/list.hpp

@@ -45,7 +45,7 @@ inline SEXP r_vector<SEXP>::operator[](const r_string& name) const {
 }
 
 template <>
-inline SEXP* r_vector<SEXP>::get_p(bool, SEXP) {
+inline typename r_vector<SEXP>::underlying_type* r_vector<SEXP>::get_p(bool, SEXP) {
   return nullptr;
 }
 

inst/include/cpp11/logicals.hpp

@@ -29,22 +29,23 @@ inline SEXP r_vector<r_bool>::valid_type(SEXP data) {
 
 template <>
 inline r_bool r_vector<r_bool>::operator[](const R_xlen_t pos) const {
-  return is_altrep_ ? static_cast<r_bool>(LOGICAL_ELT(data_, pos)) : data_p_[pos];
+  return is_altrep_ ? LOGICAL_ELT(data_, pos) : data_p_[pos];
 }
 
 template <>
-inline r_bool* r_vector<r_bool>::get_p(bool is_altrep, SEXP data) {
+inline typename r_vector<r_bool>::underlying_type* r_vector<r_bool>::get_p(bool is_altrep,
+                                                                           SEXP data) {
   if (is_altrep) {
     return nullptr;
   } else {
-    return reinterpret_cast<r_bool*>(LOGICAL(data));
+    return LOGICAL(data);
   }
 }
 
 template <>
 inline void r_vector<r_bool>::const_iterator::fill_buf(R_xlen_t pos) {
   length_ = std::min(64_xl, data_->size() - pos);
-  LOGICAL_GET_REGION(data_->data_, pos, length_, reinterpret_cast<int*>(buf_.data()));
+  LOGICAL_GET_REGION(data_->data_, pos, length_, buf_.data());
   block_start_ = pos;
 }
 
@@ -66,7 +67,7 @@ inline typename r_vector<r_bool>::proxy& r_vector<r_bool>::proxy::operator=(
 template <>
 inline r_vector<r_bool>::proxy::operator r_bool() const {
   if (p_ == nullptr) {
-    return static_cast<r_bool>(LOGICAL_ELT(data_, index_));
+    return LOGICAL_ELT(data_, index_);
   } else {
     return *p_;
   }
@@ -100,7 +101,7 @@ inline r_vector<r_bool>::r_vector(std::initializer_list<named_arg> il)
       ++n_protected;
       auto it = il.begin();
       for (R_xlen_t i = 0; i < capacity_; ++i, ++it) {
-        data_p_[i] = static_cast<r_bool>(LOGICAL_ELT(it->value(), 0));
+        data_p_[i] = LOGICAL_ELT(it->value(), 0);
         SET_STRING_ELT(names, i, Rf_mkCharCE(it->name(), CE_UTF8));
       }
       UNPROTECT(n_protected);
@@ -121,7 +122,7 @@ inline void r_vector<r_bool>::reserve(R_xlen_t new_capacity) {
 
   preserved.release(old_protect);
 
-  data_p_ = reinterpret_cast<r_bool*>(LOGICAL(data_));
+  data_p_ = LOGICAL(data_);
   capacity_ = new_capacity;
 }
 

inst/include/cpp11/r_bool.hpp

@@ -71,4 +71,11 @@ inline r_bool na() {
   return NA_LOGICAL;
 }
 
+namespace traits {
+template <>
+struct get_underlying_type<r_bool> {
+  using type = int;
+};
+}  // namespace traits
+
 }  // namespace cpp11

inst/include/cpp11/r_string.hpp

@@ -93,4 +93,11 @@ inline r_string na() {
   return NA_STRING;
 }
 
+namespace traits {
+template <>
+struct get_underlying_type<r_string> {
+  using type = SEXP;
+};
+}  // namespace traits
+
 }  // namespace cpp11

inst/include/cpp11/r_vector.hpp

@@ -56,6 +56,8 @@ class r_vector {
   typedef T* pointer;
   typedef T& reference;
 
+  using underlying_type = typename traits::get_underlying_type<T>::type;
+
   r_vector() noexcept = default;
 
   r_vector(SEXP data);
@@ -173,7 +175,7 @@ class r_vector {
     void fill_buf(R_xlen_t pos);
 
     R_xlen_t pos_;
-    std::array<T, 64 * 64> buf_;
+    std::array<underlying_type, 64 * 64> buf_;
     R_xlen_t block_start_ = 0;
     R_xlen_t length_ = 0;
   };
@@ -193,10 +195,10 @@ class r_vector {
   SEXP data_ = R_NilValue;
   SEXP protect_ = R_NilValue;
   bool is_altrep_ = false;
-  T* data_p_ = nullptr;
+  underlying_type* data_p_ = nullptr;
   R_xlen_t length_ = 0;
 
-  static T* get_p(bool is_altrep, SEXP data);
+  static underlying_type* get_p(bool is_altrep, SEXP data);
 
   static SEXP valid_type(SEXP data);
 
@@ -216,6 +218,8 @@ class r_vector : public cpp11::r_vector<T> {
 
   // These are necessary because type names are not directly accessible in
   // template inheritance
+  using typename cpp11::r_vector<T>::underlying_type;
+
   using cpp11::r_vector<T>::data_;
   using cpp11::r_vector<T>::data_p_;
   using cpp11::r_vector<T>::is_altrep_;
@@ -228,11 +232,11 @@ class r_vector : public cpp11::r_vector<T> {
    private:
     const SEXP data_;
     const R_xlen_t index_;
-    T* const p_;
+    underlying_type* const p_;
     bool is_altrep_;
 
    public:
-    proxy(SEXP data, const R_xlen_t index, T* const p, bool is_altrep);
+    proxy(SEXP data, const R_xlen_t index, underlying_type* const p, bool is_altrep);
 
     proxy& operator=(const T& rhs);
     proxy& operator+=(const T& rhs);
@@ -572,9 +576,9 @@ inline typename cpp11::r_vector<T>::const_iterator cpp11::r_vector<T>::find(
 template <typename T>
 inline T r_vector<T>::const_iterator::operator*() const {
   if (data_->is_altrep()) {
-    return buf_[pos_ - block_start_];
+    return static_cast<T>(buf_[pos_ - block_start_]);
   } else {
-    return data_->data_p_[pos_];
+    return static_cast<T>(data_->data_p_[pos_]);
   }
 }
 
@@ -598,13 +602,17 @@ inline T r_vector<T>::operator[](size_type pos) const {
 namespace writable {
 
 template <typename T>
-r_vector<T>::proxy::proxy(SEXP data, const R_xlen_t index, T* const p, bool is_altrep)
+r_vector<T>::proxy::proxy(SEXP data, const R_xlen_t index,
+                          typename r_vector<T>::underlying_type* const p, bool is_altrep)
     : data_(data), index_(index), p_(p), is_altrep_(is_altrep) {}
 
 template <typename T>
 inline typename r_vector<T>::proxy r_vector<T>::iterator::operator*() const {
   if (data_.is_altrep()) {
-    return proxy(data_.data(), pos_, const_cast<T*>(&buf_[pos_ - block_start_]), true);
+    return proxy(
+        data_.data(), pos_,
+        const_cast<typename r_vector<T>::underlying_type*>(&buf_[pos_ - block_start_]),
+        true);
   } else {
     return proxy(data_.data(), pos_,
                  data_.data_p_ != nullptr ? &data_.data_p_[pos_] : nullptr, false);

inst/include/cpp11/raws.hpp

@@ -16,6 +16,13 @@
 
 namespace cpp11 {
 
+namespace traits {
+template <>
+struct get_underlying_type<uint8_t> {
+  using type = Rbyte;
+};
+}  // namespace traits
+
 template <>
 inline SEXP r_vector<uint8_t>::valid_type(SEXP data) {
   if (data == nullptr) {
@@ -34,20 +41,20 @@ inline uint8_t r_vector<uint8_t>::operator[](const R_xlen_t pos) const {
 }
 
 template <>
-inline uint8_t* r_vector<uint8_t>::get_p(bool is_altrep, SEXP data) {
+inline typename r_vector<uint8_t>::underlying_type* r_vector<uint8_t>::get_p(
+    bool is_altrep, SEXP data) {
   if (is_altrep) {
     return nullptr;
   } else {
-    return reinterpret_cast<uint8_t*>(RAW(data));
+    return RAW(data);
   }
 }
 
 template <>
 inline void r_vector<uint8_t>::const_iterator::fill_buf(R_xlen_t pos) {
   using namespace cpp11::literals;
   length_ = std::min(64_xl, data_->size() - pos);
-  unwind_protect(
-      [&] { RAW_GET_REGION(data_->data_, pos, length_, (uint8_t*)buf_.data()); });
+  unwind_protect([&] { RAW_GET_REGION(data_->data_, pos, length_, buf_.data()); });
   block_start_ = pos;
 }
 
@@ -124,7 +131,7 @@ inline void r_vector<uint8_t>::reserve(R_xlen_t new_capacity) {
   protect_ = preserved.insert(data_);
   preserved.release(old_protect);
 
-  data_p_ = reinterpret_cast<uint8_t*>(RAW(data_));
+  data_p_ = RAW(data_);
   capacity_ = new_capacity;
 }
 

inst/include/cpp11/strings.hpp

@@ -34,7 +34,8 @@ inline r_string r_vector<r_string>::operator[](const R_xlen_t pos) const {
 }
 
 template <>
-inline r_string* r_vector<r_string>::get_p(bool, SEXP) {
+inline typename r_vector<r_string>::underlying_type* r_vector<r_string>::get_p(bool,
+                                                                               SEXP) {
   return nullptr;
 }