From c65318ad537428590f39a44814d853e120bf539b Mon Sep 17 00:00:00 2001
From: Davis Vaughan <davis@posit.co>
Date: Fri, 9 Aug 2024 13:14:41 -0400
Subject: [PATCH] Unify `initializer_list<T>` constructors

---
 cpp11test/src/test-doubles.cpp  |  8 ++++++++
 cpp11test/src/test-integers.cpp |  7 +++++++
 cpp11test/src/test-list.cpp     | 13 +++++++++++++
 cpp11test/src/test-logicals.cpp | 21 +++++++++++++++++++++
 cpp11test/src/test-raws.cpp     |  7 +++++++
 cpp11test/src/test-strings.cpp  | 28 ++++++++++++++++++++++++++++
 inst/include/cpp11/doubles.hpp  | 10 ++++++----
 inst/include/cpp11/integers.hpp | 10 ++++++----
 inst/include/cpp11/list.hpp     | 16 ++++++----------
 inst/include/cpp11/logicals.hpp | 15 ++++++---------
 inst/include/cpp11/r_vector.hpp | 21 ++++++++++++++++++++-
 inst/include/cpp11/raws.hpp     | 21 +++++++++++----------
 inst/include/cpp11/strings.hpp  | 25 ++++++++++++++++++++++++-
 13 files changed, 163 insertions(+), 39 deletions(-)

diff --git a/cpp11test/src/test-doubles.cpp b/cpp11test/src/test-doubles.cpp
index d49cb8fa..d2f05545 100644
--- a/cpp11test/src/test-doubles.cpp
+++ b/cpp11test/src/test-doubles.cpp
@@ -175,6 +175,14 @@ context("doubles-C++") {
 
     UNPROTECT(1);
   }
+
+  test_that("writable::doubles(initializer_list<double>)") {
+    cpp11::writable::doubles x({1, 2.5, 3});
+    expect_true(x[0] == 1.0);
+    expect_true(x[1] == 2.5);
+    expect_true(x[2] == 3.0);
+  }
+
   test_that("writable::doubles(SEXP, bool)") {
     SEXP x = PROTECT(Rf_ScalarReal(5.));
     cpp11::writable::doubles y(x, false);
diff --git a/cpp11test/src/test-integers.cpp b/cpp11test/src/test-integers.cpp
index 6f876c57..f8097ec4 100644
--- a/cpp11test/src/test-integers.cpp
+++ b/cpp11test/src/test-integers.cpp
@@ -168,6 +168,13 @@ context("integers-C++") {
     UNPROTECT(1);
   }
 
+  test_that("writable::integers(initializer_list<int>)") {
+    cpp11::writable::integers x({1, 2, 3});
+    expect_true(x[0] == 1);
+    expect_true(x[1] == 2);
+    expect_true(x[2] == 3);
+  }
+
 #if defined(__APPLE__) && defined(R_VERSION) && R_VERSION >= R_Version(3, 5, 0)
   test_that("writable::integers(ALTREP_SEXP)") {
     // ALTREP compact-seq
diff --git a/cpp11test/src/test-list.cpp b/cpp11test/src/test-list.cpp
index 55f27c13..05aa9bd5 100644
--- a/cpp11test/src/test-list.cpp
+++ b/cpp11test/src/test-list.cpp
@@ -163,4 +163,17 @@ context("list-C++") {
     expect_true(Rf_xlength(y) == 0);
     expect_true(y != R_NilValue);
   }
+
+  test_that("writable::list(initializer_list<SEXP>)") {
+    SEXP x1 = PROTECT(Rf_allocVector(INTSXP, 1));
+    SEXP x2 = PROTECT(Rf_allocVector(REALSXP, 2));
+    SEXP x3 = PROTECT(Rf_allocVector(STRSXP, 3));
+
+    cpp11::writable::list x({x1, x2, x3});
+    expect_true(x[0] == x1);
+    expect_true(x[1] == x2);
+    expect_true(x[2] == x3);
+
+    UNPROTECT(3);
+  }
 }
diff --git a/cpp11test/src/test-logicals.cpp b/cpp11test/src/test-logicals.cpp
index 6c9b136a..062a77b0 100644
--- a/cpp11test/src/test-logicals.cpp
+++ b/cpp11test/src/test-logicals.cpp
@@ -127,6 +127,27 @@ context("logicals-C++") {
 
     UNPROTECT(1);
   }
+
+  test_that("writable::logicals(initializer_list<r_bool>)") {
+    cpp11::writable::logicals x(
+        {cpp11::r_bool(true), cpp11::r_bool(false), cpp11::r_bool(NA_INTEGER)});
+    expect_true(x[0] == cpp11::r_bool(true));
+    expect_true(x[1] == cpp11::r_bool(false));
+    expect_true(x[2] == cpp11::r_bool(NA_INTEGER));
+
+    // This works due to implicit conversion of `bool` to `r_bool`
+    cpp11::writable::logicals y({true, false, false});
+    expect_true(y[0] == cpp11::r_bool(true));
+    expect_true(y[1] == cpp11::r_bool(false));
+    expect_true(y[2] == cpp11::r_bool(false));
+
+    // This works due to implicit conversion of `Rboolean` to `r_bool`
+    cpp11::writable::logicals z({TRUE, FALSE, FALSE});
+    expect_true(z[0] == cpp11::r_bool(true));
+    expect_true(z[1] == cpp11::r_bool(false));
+    expect_true(z[2] == cpp11::r_bool(false));
+  }
+
   test_that("is_na(r_bool)") {
     cpp11::r_bool x = TRUE;
     expect_true(!cpp11::is_na(x));
diff --git a/cpp11test/src/test-raws.cpp b/cpp11test/src/test-raws.cpp
index ecae6db6..689c9755 100644
--- a/cpp11test/src/test-raws.cpp
+++ b/cpp11test/src/test-raws.cpp
@@ -128,6 +128,13 @@ context("raws-C++") {
     UNPROTECT(1);
   }
 
+  test_that("writable::raws(initializer_list<uint_8>)") {
+    cpp11::writable::raws x({1, 2, 255});
+    expect_true(x[0] == 1);
+    expect_true(x[1] == 2);
+    expect_true(x[2] == 255);
+  }
+
   // test_that("writable::raws(ALTREP_SEXP)") {
   // SEXP x = PROTECT(R_compact_uint8_trange(1, 5));
   //// Need to find (or create) an altrep class that implements duplicate.
diff --git a/cpp11test/src/test-strings.cpp b/cpp11test/src/test-strings.cpp
index 3a11e935..0509d311 100644
--- a/cpp11test/src/test-strings.cpp
+++ b/cpp11test/src/test-strings.cpp
@@ -133,6 +133,34 @@ context("strings-C++") {
     UNPROTECT(1);
   }
 
+  test_that("writable::strings(initializer_list<r_string>)") {
+    cpp11::r_string abc = cpp11::r_string("abc");
+    cpp11::r_string na = cpp11::r_string(NA_STRING);
+
+    cpp11::writable::strings x({abc, na, abc});
+    expect_true(x[0] == abc);
+    expect_true(x[1] == na);
+    expect_true(x[2] == abc);
+
+    // This works due to implicit conversion of `SEXP` to `r_string`
+    SEXP a = PROTECT(Rf_mkCharCE("a", CE_UTF8));
+    SEXP b = PROTECT(Rf_mkCharCE("b", CE_UTF8));
+    cpp11::writable::strings y({a, b});
+    expect_true(y[0] == cpp11::r_string("a"));
+    expect_true(y[1] == cpp11::r_string("b"));
+
+    // This works due to implicit conversion of `const char*` to `r_string`
+    cpp11::writable::strings z({"neat", "stuff"});
+    expect_true(z[0] == cpp11::r_string("neat"));
+    expect_true(z[1] == cpp11::r_string("stuff"));
+
+    cpp11::writable::strings w({std::string("neat"), std::string("stuff")});
+    expect_true(w[0] == cpp11::r_string("neat"));
+    expect_true(w[1] == cpp11::r_string("stuff"));
+
+    UNPROTECT(2);
+  }
+
   test_that("std::initializer_list<const char*>") {
     cpp11::writable::strings x{"foo"};
     expect_true(x.size() == 1);
diff --git a/inst/include/cpp11/doubles.hpp b/inst/include/cpp11/doubles.hpp
index 824c294c..f15cbf07 100644
--- a/inst/include/cpp11/doubles.hpp
+++ b/inst/include/cpp11/doubles.hpp
@@ -59,6 +59,12 @@ inline SEXPTYPE r_vector<double>::get_sexptype() {
   return REALSXP;
 }
 
+template <>
+inline void r_vector<double>::set_elt(
+    SEXP x, R_xlen_t i, typename traits::get_underlying_type<double>::type value) {
+  SET_REAL_ELT(x, i, value);
+}
+
 template <>
 inline typename r_vector<double>::proxy& r_vector<double>::proxy::operator=(
     const double& rhs) {
@@ -81,10 +87,6 @@ inline r_vector<double>::proxy::operator double() const {
   }
 }
 
-template <>
-inline r_vector<double>::r_vector(std::initializer_list<double> il)
-    : cpp11::r_vector<double>(as_sexp(il)), capacity_(il.size()) {}
-
 template <>
 inline r_vector<double>::r_vector(std::initializer_list<named_arg> il)
     : cpp11::r_vector<double>(safe[Rf_allocVector](REALSXP, il.size())),
diff --git a/inst/include/cpp11/integers.hpp b/inst/include/cpp11/integers.hpp
index 6e7ac8d7..3bf25203 100644
--- a/inst/include/cpp11/integers.hpp
+++ b/inst/include/cpp11/integers.hpp
@@ -60,6 +60,12 @@ inline SEXPTYPE r_vector<int>::get_sexptype() {
   return INTSXP;
 }
 
+template <>
+inline void r_vector<int>::set_elt(
+    SEXP x, R_xlen_t i, typename traits::get_underlying_type<int>::type value) {
+  SET_INTEGER_ELT(x, i, value);
+}
+
 template <>
 inline typename r_vector<int>::proxy& r_vector<int>::proxy::operator=(const int& rhs) {
   if (is_altrep_) {
@@ -81,10 +87,6 @@ inline r_vector<int>::proxy::operator int() const {
   }
 }
 
-template <>
-inline r_vector<int>::r_vector(std::initializer_list<int> il)
-    : cpp11::r_vector<int>(as_sexp(il)), capacity_(il.size()) {}
-
 template <>
 inline r_vector<int>::r_vector(std::initializer_list<named_arg> il)
     : cpp11::r_vector<int>(safe[Rf_allocVector](INTSXP, il.size())),
diff --git a/inst/include/cpp11/list.hpp b/inst/include/cpp11/list.hpp
index 2ee745e6..89c837a5 100644
--- a/inst/include/cpp11/list.hpp
+++ b/inst/include/cpp11/list.hpp
@@ -68,6 +68,12 @@ inline SEXPTYPE r_vector<SEXP>::get_sexptype() {
   return VECSXP;
 }
 
+template <>
+inline void r_vector<SEXP>::set_elt(
+    SEXP x, R_xlen_t i, typename traits::get_underlying_type<SEXP>::type value) {
+  SET_VECTOR_ELT(x, i, value);
+}
+
 template <>
 inline typename r_vector<SEXP>::proxy& r_vector<SEXP>::proxy::operator=(const SEXP& rhs) {
   SET_VECTOR_ELT(data_, index_, rhs);
@@ -79,16 +85,6 @@ inline r_vector<SEXP>::proxy::operator SEXP() const {
   return VECTOR_ELT(data_, index_);
 }
 
-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()) {
-  auto it = il.begin();
-  for (R_xlen_t i = 0; i < capacity_; ++i, ++it) {
-    SET_VECTOR_ELT(data_, i, *it);
-  }
-}
-
 template <>
 inline r_vector<SEXP>::r_vector(std::initializer_list<named_arg> il)
     : cpp11::r_vector<SEXP>(safe[Rf_allocVector](VECSXP, il.size())),
diff --git a/inst/include/cpp11/logicals.hpp b/inst/include/cpp11/logicals.hpp
index 7fe94bf5..99f5204c 100644
--- a/inst/include/cpp11/logicals.hpp
+++ b/inst/include/cpp11/logicals.hpp
@@ -58,6 +58,12 @@ inline SEXPTYPE r_vector<r_bool>::get_sexptype() {
   return LGLSXP;
 }
 
+template <>
+inline void r_vector<r_bool>::set_elt(
+    SEXP x, R_xlen_t i, typename traits::get_underlying_type<r_bool>::type value) {
+  SET_LOGICAL_ELT(x, i, value);
+}
+
 template <>
 inline typename r_vector<r_bool>::proxy& r_vector<r_bool>::proxy::operator=(
     const r_bool& rhs) {
@@ -82,15 +88,6 @@ inline bool operator==(const r_vector<r_bool>::proxy& lhs, r_bool rhs) {
   return static_cast<r_bool>(lhs).operator==(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()) {
-  auto it = il.begin();
-  for (R_xlen_t i = 0; i < capacity_; ++i, ++it) {
-    SET_LOGICAL_ELT(data_, i, *it);
-  }
-}
-
 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())),
diff --git a/inst/include/cpp11/r_vector.hpp b/inst/include/cpp11/r_vector.hpp
index c4b22ee4..44cca1e6 100644
--- a/inst/include/cpp11/r_vector.hpp
+++ b/inst/include/cpp11/r_vector.hpp
@@ -189,7 +189,6 @@ class r_vector : public cpp11::r_vector<T> {
   r_vector(const r_vector& rhs);
   r_vector(r_vector&& rhs);
   r_vector(const cpp11::r_vector<T>& rhs);
-  /// Implemented in specialization
   r_vector(std::initializer_list<T> il);
   /// Implemented in specialization
   r_vector(std::initializer_list<named_arg> il);
@@ -308,6 +307,8 @@ class r_vector : public cpp11::r_vector<T> {
  private:
   /// Implemented in specialization
   static SEXPTYPE get_sexptype();
+  /// Implemented in specialization
+  static void set_elt(SEXP x, R_xlen_t i, underlying_type value);
 
   using cpp11::r_vector<T>::get_p;
 };
@@ -717,6 +718,24 @@ template <typename T>
 inline r_vector<T>::r_vector(const cpp11::r_vector<T>& rhs)
     : cpp11::r_vector<T>(safe[Rf_shallow_duplicate](rhs.data_)), capacity_(rhs.length_) {}
 
+template <typename T>
+inline r_vector<T>::r_vector(std::initializer_list<T> il)
+    : cpp11::r_vector<T>(safe[Rf_allocVector](get_sexptype(), il.size())),
+      capacity_(il.size()) {
+  auto it = il.begin();
+
+  if (data_p_ != nullptr) {
+    for (R_xlen_t i = 0; i < capacity_; ++i, ++it) {
+      data_p_[i] = static_cast<underlying_type>(*it);
+    }
+  } else {
+    // Handles both the ALTREP and VECSXP cases
+    for (R_xlen_t i = 0; i < capacity_; ++i, ++it) {
+      set_elt(data_, i, static_cast<underlying_type>(*it));
+    }
+  }
+}
+
 template <typename T>
 inline r_vector<T>::r_vector(const R_xlen_t size) : r_vector() {
   resize(size);
diff --git a/inst/include/cpp11/raws.hpp b/inst/include/cpp11/raws.hpp
index 531b6d0f..c6705bcb 100644
--- a/inst/include/cpp11/raws.hpp
+++ b/inst/include/cpp11/raws.hpp
@@ -5,6 +5,7 @@
 #include <cstdint>           // for uint8_t
 #include <initializer_list>  // for initializer_list
 
+#include "Rversion.h"
 #include "cpp11/R.hpp"                // for RAW, SEXP, SEXPREC, Rf_allocVector
 #include "cpp11/attribute_proxy.hpp"  // for attribute_proxy
 #include "cpp11/named_arg.hpp"        // for named_arg
@@ -67,6 +68,16 @@ inline SEXPTYPE r_vector<uint8_t>::get_sexptype() {
   return RAWSXP;
 }
 
+template <>
+inline void r_vector<uint8_t>::set_elt(
+    SEXP x, R_xlen_t i, typename traits::get_underlying_type<uint8_t>::type value) {
+#if R_VERSION >= R_Version(4, 2, 0)
+  SET_RAW_ELT(x, i, value);
+#else
+  RAW(x)[i] = value;
+#endif
+}
+
 template <>
 inline typename r_vector<uint8_t>::proxy& r_vector<uint8_t>::proxy::operator=(
     const uint8_t& rhs) {
@@ -89,16 +100,6 @@ inline r_vector<uint8_t>::proxy::operator uint8_t() const {
   }
 }
 
-template <>
-inline r_vector<uint8_t>::r_vector(std::initializer_list<uint8_t> il)
-    : cpp11::r_vector<uint8_t>(safe[Rf_allocVector](RAWSXP, il.size())),
-      capacity_(il.size()) {
-  auto it = il.begin();
-  for (R_xlen_t i = 0; i < capacity_; ++i, ++it) {
-    data_p_[i] = *it;
-  }
-}
-
 template <>
 inline r_vector<uint8_t>::r_vector(std::initializer_list<named_arg> il)
     : cpp11::r_vector<uint8_t>(safe[Rf_allocVector](RAWSXP, il.size())),
diff --git a/inst/include/cpp11/strings.hpp b/inst/include/cpp11/strings.hpp
index c121b039..64cee045 100644
--- a/inst/include/cpp11/strings.hpp
+++ b/inst/include/cpp11/strings.hpp
@@ -58,6 +58,12 @@ inline SEXPTYPE r_vector<r_string>::get_sexptype() {
   return STRSXP;
 }
 
+template <>
+inline void r_vector<r_string>::set_elt(
+    SEXP x, R_xlen_t i, typename traits::get_underlying_type<r_string>::type value) {
+  SET_STRING_ELT(x, i, value);
+}
+
 template <>
 inline typename r_vector<r_string>::proxy& r_vector<r_string>::proxy::operator=(
     const r_string& rhs) {
@@ -113,9 +119,26 @@ inline r_vector<r_string>::r_vector(SEXP&& data)
   }
 }
 
+// Requires specialization to handle `NA_STRING` and UTF-8 translation
 template <>
 inline r_vector<r_string>::r_vector(std::initializer_list<r_string> il)
-    : cpp11::r_vector<r_string>(as_sexp(il)), capacity_(il.size()) {}
+    : cpp11::r_vector<r_string>(safe[Rf_allocVector](STRSXP, il.size())),
+      capacity_(il.size()) {
+  unwind_protect([&] {
+    auto it = il.begin();
+
+    for (R_xlen_t i = 0; i < capacity_; ++i, ++it) {
+      // i.e. to `SEXP`
+      underlying_type elt = static_cast<underlying_type>(*it);
+
+      if (elt == NA_STRING) {
+        set_elt(data_, i, elt);
+      } else {
+        set_elt(data_, i, Rf_mkCharCE(Rf_translateCharUTF8(elt), CE_UTF8));
+      }
+    }
+  });
+}
 
 template <>
 inline r_vector<r_string>::r_vector(std::initializer_list<named_arg> il)