Unify push_back() implementations

r-lib · Aug 9, 2024 · c54765c · c54765c
1 parent 888afdb
commit c54765c
Show file tree

Hide file tree

Showing 7 changed files with 23 additions and 77 deletions.
diff --git a/inst/include/cpp11/doubles.hpp b/inst/include/cpp11/doubles.hpp
@@ -57,6 +57,7 @@ namespace writable {
 template <>
 inline void r_vector<double>::set_elt(SEXP x, R_xlen_t i,
                                       typename r_vector::underlying_type value) {
+  // NOPROTECT: Likely too costly to unwind protect every set elt
   SET_REAL_ELT(x, i, value);
 }
 
@@ -84,19 +85,6 @@ inline r_vector<double>::r_vector(std::initializer_list<named_arg> il)
   }
 }
 
-template <>
-inline void r_vector<double>::push_back(double value) {
-  while (length_ >= capacity_) {
-    reserve(capacity_ == 0 ? 1 : capacity_ *= 2);
-  }
-  if (is_altrep_) {
-    SET_REAL_ELT(data_, length_, value);
-  } else {
-    data_p_[length_] = value;
-  }
-  ++length_;
-}
-
 typedef r_vector<double> doubles;
 
 }  // namespace writable

diff --git a/inst/include/cpp11/integers.hpp b/inst/include/cpp11/integers.hpp
@@ -58,6 +58,7 @@ namespace writable {
 template <>
 inline void r_vector<int>::set_elt(SEXP x, R_xlen_t i,
                                    typename r_vector::underlying_type value) {
+  // NOPROTECT: Likely too costly to unwind protect every set elt
   SET_INTEGER_ELT(x, i, value);
 }
 
@@ -85,20 +86,6 @@ inline r_vector<int>::r_vector(std::initializer_list<named_arg> il)
   }
 }
 
-template <>
-inline void r_vector<int>::push_back(int value) {
-  while (length_ >= capacity_) {
-    reserve(capacity_ == 0 ? 1 : capacity_ *= 2);
-  }
-  if (is_altrep_) {
-    // NOPROTECT: likely too costly to unwind protect every elt
-    SET_INTEGER_ELT(data_, length_, value);
-  } else {
-    data_p_[length_] = value;
-  }
-  ++length_;
-}
-
 typedef r_vector<int> integers;
 
 }  // namespace writable

diff --git a/inst/include/cpp11/list.hpp b/inst/include/cpp11/list.hpp
@@ -68,6 +68,7 @@ namespace writable {
 template <>
 inline void r_vector<SEXP>::set_elt(SEXP x, R_xlen_t i,
                                     typename r_vector::underlying_type value) {
+  // NOPROTECT: Likely too costly to unwind protect every set elt
   SET_VECTOR_ELT(x, i, value);
 }
 
@@ -95,15 +96,6 @@ inline r_vector<SEXP>::r_vector(std::initializer_list<named_arg> il)
   }
 }
 
-template <>
-inline void r_vector<SEXP>::push_back(SEXP value) {
-  while (length_ >= capacity_) {
-    reserve(capacity_ == 0 ? 1 : capacity_ *= 2);
-  }
-  SET_VECTOR_ELT(data_, length_, value);
-  ++length_;
-}
-
 typedef r_vector<SEXP> list;
 
 }  // namespace writable

diff --git a/inst/include/cpp11/logicals.hpp b/inst/include/cpp11/logicals.hpp
@@ -57,6 +57,7 @@ namespace writable {
 template <>
 inline void r_vector<r_bool>::set_elt(SEXP x, R_xlen_t i,
                                       typename r_vector::underlying_type value) {
+  // NOPROTECT: Likely too costly to unwind protect every set elt
   SET_LOGICAL_ELT(x, i, value);
 }
 
@@ -99,19 +100,6 @@ inline r_vector<r_bool>::r_vector(std::initializer_list<named_arg> il)
   }
 }
 
-template <>
-inline void r_vector<r_bool>::push_back(r_bool value) {
-  while (length_ >= capacity_) {
-    reserve(capacity_ == 0 ? 1 : capacity_ *= 2);
-  }
-  if (is_altrep_) {
-    SET_LOGICAL_ELT(data_, length_, value);
-  } else {
-    data_p_[length_] = value;
-  }
-  ++length_;
-}
-
 typedef r_vector<r_bool> logicals;
 
 }  // namespace writable

diff --git a/inst/include/cpp11/r_vector.hpp b/inst/include/cpp11/r_vector.hpp
@@ -225,7 +225,6 @@ class r_vector : public cpp11::r_vector<T> {
   proxy at(const size_type pos) const;
   proxy at(const r_string& name) const;
 
-  /// Implemented in specialization
   void push_back(T value);
   /// Implemented in `strings.hpp`
   void push_back(const named_arg& value);
@@ -942,6 +941,21 @@ inline typename r_vector<T>::proxy r_vector<T>::at(const r_string& name) const {
   return operator[](name);
 }
 
+template <typename T>
+inline void r_vector<T>::push_back(T value) {
+  while (length_ >= capacity_) {
+    reserve(capacity_ == 0 ? 1 : capacity_ *= 2);
+  }
+
+  if (data_p_ != nullptr) {
+    data_p_[length_] = static_cast<underlying_type>(value);
+  } else {
+    set_elt(data_, length_, static_cast<underlying_type>(value));
+  }
+
+  ++length_;
+}
+
 template <typename T>
 inline void r_vector<T>::pop_back() {
   --length_;
@@ -1063,9 +1077,8 @@ inline typename r_vector<T>::proxy& r_vector<T>::proxy::operator=(const T& rhs)
   if (p_ != nullptr) {
     *p_ = elt;
   } else {
-    // Handles ALTREP, VECSXP, and STRSXP cases.
-    // NOPROTECT: Likely too costly to unwind protect every set elt.
-    r_vector<T>::set_elt(data_, index_, elt);
+    // Handles ALTREP, VECSXP, and STRSXP cases
+    set_elt(data_, index_, elt);
   }
 
   return *this;

diff --git a/inst/include/cpp11/raws.hpp b/inst/include/cpp11/raws.hpp
@@ -65,6 +65,7 @@ namespace writable {
 template <>
 inline void r_vector<uint8_t>::set_elt(SEXP x, R_xlen_t i,
                                        typename r_vector::underlying_type value) {
+  // NOPROTECT: Likely too costly to unwind protect every set elt
 #if R_VERSION >= R_Version(4, 2, 0)
   SET_RAW_ELT(x, i, value);
 #else
@@ -97,20 +98,6 @@ inline r_vector<uint8_t>::r_vector(std::initializer_list<named_arg> il)
   }
 }
 
-template <>
-inline void r_vector<uint8_t>::push_back(uint8_t value) {
-  while (length_ >= capacity_) {
-    reserve(capacity_ == 0 ? 1 : capacity_ *= 2);
-  }
-  if (is_altrep_) {
-    // NOPROTECT: likely too costly to unwind protect every elt
-    RAW(data_)[length_] = value;
-  } else {
-    data_p_[length_] = value;
-  }
-  ++length_;
-}
-
 typedef r_vector<uint8_t> raws;
 
 }  // namespace writable

diff --git a/inst/include/cpp11/strings.hpp b/inst/include/cpp11/strings.hpp
@@ -57,6 +57,7 @@ namespace writable {
 template <>
 inline void r_vector<r_string>::set_elt(SEXP x, R_xlen_t i,
                                         typename r_vector::underlying_type value) {
+  // NOPROTECT: Likely too costly to unwind protect every set elt
   SET_STRING_ELT(x, i, value);
 }
 
@@ -147,16 +148,6 @@ inline r_vector<r_string>::r_vector(std::initializer_list<named_arg> il)
   }
 }
 
-template <>
-inline void r_vector<r_string>::push_back(r_string value) {
-  while (length_ >= capacity_) {
-    reserve(capacity_ == 0 ? 1 : capacity_ *= 2);
-  }
-  // NOPROTECT: likely too costly to unwind protect every elt
-  SET_STRING_ELT(data_, length_, value);
-  ++length_;
-}
-
 typedef r_vector<r_string> strings;
 
 template <typename T>