as_sexp and as_cpp are not easily extensible

[bkietz]

Arrow defines as_cpp and as_sexp for shared_ptr and vectors of shared_ptr using external_pointer, since both of those are used throughout the arrow API. This is currently accomplished by declaring the overloads of as_sexp before inclusion of the cpp11 headers:

namespace cpp11 {

template <typename T>
SEXP as_sexp(const std::shared_ptr<T>& ptr);

template <typename T>
SEXP as_sexp(const std::vector<std::shared_ptr<T>>& vec);

}  // namespace cpp11

#include <cpp11.hpp>

https://github.com/apache/arrow/pull/7819/files#diff-db94c392857c3bad4f5f69e86cde917bR24-R32

Without this pre-declaration, ADL fails (for example) when named_arg::operator= attempts to instantiate as_sexp(const shared_ptr<T>&).

Reproducer:

diff --git a/cpp11test/src/test-as.cpp b/cpp11test/src/test-as.cpp
index 76b4ee3..c8770a8 100644
--- a/cpp11test/src/test-as.cpp
+++ b/cpp11test/src/test-as.cpp
@@ -8,7 +8,39 @@

 #include "Rcpp.h"

+namespace test {
+
+struct triple {
+  std::string arch, vendor, os;
+};
+
+}  // namespace test
+
+namespace cpp11 {
+
+template <typename T>
+cpp11::enable_if_t<std::is_same<T, test::triple>::value, test::triple> as_cpp(SEXP from) {
+  cpp11::strings r{from};
+
+  if (r.size() == 3) {
+    return test::triple{.arch = r[0], .vendor = r[1], .os = r[2]};
+  }
+
+  stop("Expected string vector of length 3");
+}
+
+SEXP as_sexp(const test::triple& from) {
+  return cpp11::writable::strings({from.arch, from.vendor, from.os});
+}
+
+}  // namespace cpp11
+
 context("as_cpp-C++") {
+  test_that("as_cpp<custom type>(SEXP)") {
+    cpp11::writable::list(
+        {"fs"_nm = test::triple{.arch = "seven", .vendor = "ono", .os = "sendai"}});
+  }
+
   test_that("as_cpp<integer>(INTSEXP)") {
     SEXP r = PROTECT(Rf_allocVector(INTSXP, 1));
     INTEGER(r)[0] = 42;

This could be resolved by providing a user-specializable trait for conversion, for example:

template <typename T, typename Enable = void>
struct custom_conversion;

template <typename T>
auto as_cpp(SEXP from) -> decltype(custom_conversion<T>::as_cpp(from)) {
  return custom_conversion<T>::as_cpp(from);
}

template <typename T>
auto as_sexp(const T& from) -> decltype(custom_conversion<T>::as_sexp(from)) {
  return custom_conversion<T>::as_sexp(from);
}

Which in the case of the reproducer above would be used like so:

namespace cpp11 {
template <>
struct custom_conversion<test::triple> {
  static test::triple as_cpp(SEXP from) {
    cpp11::strings r{from};

    if (r.size() == 3) {
      return test::triple{.arch = r[0], .vendor = r[1], .os = r[2]};
    }

    stop("Expected string vector of length 3");
  }

  static SEXP as_sexp(const test::triple& from) {
    return cpp11::writable::strings({from.arch, from.vendor, from.os});
  }
};
}

[bkietz]

Alternatively, we might prefer to make the conversion trait struct the single point of truth for conversion

[jimhester]

I think this makes sense as a way for packages to extend the conversions as needed.

If you and @romainfrancois think using the conversion trait struct is easier to understand and is more maintainable than the enable_if code we have now we can definitely go that route.

[bkietz]

I'll write a draft PR using a conversion trait for conversion