jonkeane commented on a change in pull request #11225:
URL: https://github.com/apache/arrow/pull/11225#discussion_r717541592



##########
File path: r/src/altrep.cpp
##########
@@ -70,135 +69,125 @@ R_xlen_t Standard_Get_region<int>(SEXP data2, R_xlen_t i, 
R_xlen_t n, int* buf)
   return INTEGER_GET_REGION(data2, i, n, buf);
 }
 
-// altrep R vector shadowing an Array.
+void DeleteArray(std::shared_ptr<Array>* ptr) { delete ptr; }
+using Pointer = cpp11::external_pointer<std::shared_ptr<Array>, DeleteArray>;
+
+// base class for all altrep vectors
+//
+// The altrep vector stores the Array as an external pointer in data1
+// Implementation classes AltrepVectorPrimitive<> and AltrepVectorString
+// also use data2
+struct AltrepVectorBase {
+  // store the Array as an external pointer in data1, mark as immutable
+  static SEXP Make(R_altrep_class_t class_t, const std::shared_ptr<Array>& 
array) {
+    SEXP alt_ =
+        R_new_altrep(class_t, Pointer(new std::shared_ptr<Array>(array)), 
R_NilValue);
+    MARK_NOT_MUTABLE(alt_);
+
+    return alt_;
+  }
+
+  // the Array that is being wrapped by the altrep object
+  static const std::shared_ptr<Array>& array(SEXP alt_) {
+    return *Pointer(R_altrep_data1(alt_));
+  }
+
+  // Is the vector materialized, i.e. does the data2 slot contain a
+  // standard R vector with the same data as the array.
+  static bool IsMaterialized(SEXP alt_) { return 
!Rf_isNull(R_altrep_data2(alt_)); }
+
+  static R_xlen_t Length(SEXP alt_) { return array(alt_)->length(); }
+
+  static int No_NA(SEXP alt_) { return array(alt_)->null_count() == 0; }
+
+  static int Is_sorted(SEXP alt_) { return UNKNOWN_SORTEDNESS; }
+
+  // What gets printed on .Internal(inspect(<the altrep object>))
+  static Rboolean Inspect(SEXP alt_, int pre, int deep, int pvec,
+                          void (*inspect_subtree)(SEXP, int, int, int)) {
+    const auto& array_ = array(alt_);
+    Rprintf("arrow::Array<%s, %d nulls> len=%d, Array=<%p>\n",

Review comment:
       Oh, even better! Thanks for these. I'll move to using `.Internal()` that 
way we don't need to worry about exporting `arrow:::is_altrep()` (and the 
backwards compatibility of that in {arrowbench}




-- 
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.

To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org

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


Reply via email to