[GitHub] [arrow] bkietz commented on a change in pull request #8256: ARROW-9001: [R] Box outputs as correct type in call_function

[GitBox <gi...@apache.org>]

bkietz commented on a change in pull request #8256:
URL: https://github.com/apache/arrow/pull/8256#discussion_r494467875



##########
File path: r/src/compute.cpp
##########
@@ -123,19 +123,19 @@ arrow::Datum as_cpp<arrow::Datum>(SEXP x) {
 SEXP from_datum(arrow::Datum datum) {

Review comment:
       Someday this should probably be
   ```c++
   SEXP as_sexp(arrow::Datum datum)
   ```
   so that we don't need to wrap datums in `from_datum`

##########
File path: r/src/arrow_cpp11.h
##########
@@ -310,4 +312,32 @@ enable_if_enum<E, SEXP> as_sexp(E e) {
   return as_sexp(static_cast<int>(e));
 }
 
+template <typename T>
+SEXP R6_make(SEXP symbol, SEXP fun_symbol, const std::shared_ptr<T>& x) {
+  if (x == nullptr) {
+    return R_NilValue;
+  }
+  cpp11::external_pointer<std::shared_ptr<T>> xp(new std::shared_ptr<T>(x));
+
+  // make call:  <symbol>$new(<x>)

Review comment:
       ```suggestion
     // make call:  <symbol>$<fun_symbol>(<x>)
   ```

##########
File path: r/src/arrow_cpp11.h
##########
@@ -310,4 +312,32 @@ enable_if_enum<E, SEXP> as_sexp(E e) {
   return as_sexp(static_cast<int>(e));
 }
 
+template <typename T>
+SEXP R6_make(SEXP symbol, SEXP fun_symbol, const std::shared_ptr<T>& x) {
+  if (x == nullptr) {
+    return R_NilValue;
+  }
+  cpp11::external_pointer<std::shared_ptr<T>> xp(new std::shared_ptr<T>(x));
+
+  // make call:  <symbol>$new(<x>)
+  SEXP call = PROTECT(Rf_lang3(R_DollarSymbol, symbol, fun_symbol));
+  SEXP call2 = PROTECT(Rf_lang2(call, xp));
+
+  // and then eval:
+  SEXP r6 = PROTECT(Rf_eval(call2, arrow::r::ns::arrow));
+
+  UNPROTECT(3);
+  return r6;

Review comment:
       This seems like something we'd like to use cpp11::function for, but among other things that always calls the function in R_GlobalEnv

##########
File path: r/src/arrow_cpp11.h
##########
@@ -310,4 +312,32 @@ enable_if_enum<E, SEXP> as_sexp(E e) {
   return as_sexp(static_cast<int>(e));
 }
 
+template <typename T>
+SEXP R6_make(SEXP symbol, SEXP fun_symbol, const std::shared_ptr<T>& x) {
+  if (x == nullptr) {
+    return R_NilValue;
+  }
+  cpp11::external_pointer<std::shared_ptr<T>> xp(new std::shared_ptr<T>(x));
+
+  // make call:  <symbol>$new(<x>)
+  SEXP call = PROTECT(Rf_lang3(R_DollarSymbol, symbol, fun_symbol));
+  SEXP call2 = PROTECT(Rf_lang2(call, xp));
+
+  // and then eval:
+  SEXP r6 = PROTECT(Rf_eval(call2, arrow::r::ns::arrow));
+
+  UNPROTECT(3);
+  return r6;
+}
+
+template <typename T>
+SEXP R6_new(SEXP symbol, const std::shared_ptr<T>& x) {
+  return R6_make(symbol, arrow::r::symbols::new_, x);
+}
+
+template <typename T>
+SEXP R6_create(SEXP symbol, const std::shared_ptr<T>& x) {
+  return R6_make(symbol, arrow::r::symbols::create, x);
+}

Review comment:
       This isn't shared_ptr specific; we could just write
   ```suggestion
   template <typename T>
   SEXP R6_new(SEXP symbol, T x) {
     return R6_make(symbol, arrow::r::symbols::new_, std::move(x));
   }
   
   template <typename T>
   SEXP R6_create(SEXP symbol, T x) {
     return R6_make(symbol, arrow::r::symbols::create, std::move(x));
   }
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org