Get rid of STDVEC_DATAPTR() and DATAPTR() usage (#562)

* Get rid of STDVEC_DATAPTR() usage

* Remove usage of DATAPTR (well, use the recommended backport solution)

* Try to force a current version of ggplot2

* How about not building vignettes on really old R?

* Go back to original workflow config

* Try pinning the version of patchwork, instead of ggplot2

* Add NEWS bullet

* Use DATAPTR_RW() in all the Dataptr() methods

* Flesh out force_materialization() and use it in more places

* Use read-only access when de-altrep-ing prior to write

Author: jennybc

.github/workflows/R-CMD-check.yaml

@@ -62,7 +62,7 @@ jobs:
 
       - uses: r-lib/actions/setup-r-dependencies@v2
         with:
-          extra-packages: any::rcmdcheck
+          extra-packages: any::rcmdcheck, [email protected]
           needs: check
 
       - uses: r-lib/actions/check-r-package@v2

NEWS.md

@@ -1,5 +1,7 @@
 # vroom (development version)
 
+* vroom no longer uses `STDVEC_DATAPTR()` and takes the recommended approach for phasing out usage of `DATAPTR()` (#561).
+
 # vroom 1.6.6
 
 * Fixed a bad URL in the README at CRAN's request.

src/altrep.cc

@@ -16,7 +16,26 @@
 
 [[cpp11::register]] void force_materialization(SEXP x) {
 #ifdef HAS_ALTREP
-  DATAPTR(x);
+  // Note: vroom_lgl has no ALTREP implementation, so not included
+  if (R_altrep_inherits(x, vroom_chr::class_t)) {
+    vroom_chr::Materialize(x);
+  } else if (R_altrep_inherits(x, vroom_date::class_t)) {
+    vroom_date::Materialize(x);
+  } else if (R_altrep_inherits(x, vroom_dbl::class_t)) {
+    vroom_dbl::Materialize(x);
+  } else if (R_altrep_inherits(x, vroom_dttm::class_t)) {
+    vroom_dttm::Materialize(x);
+  } else if (R_altrep_inherits(x, vroom_fct::class_t)) {
+    vroom_fct::Materialize(x);
+  } else if (R_altrep_inherits(x, vroom_int::class_t)) {
+    vroom_int::Materialize(x);
+  } else if (R_altrep_inherits(x, vroom_num::class_t)) {
+    vroom_num::Materialize(x);
+  } else if (R_altrep_inherits(x, vroom_time::class_t)) {
+    vroom_time::Materialize(x);
+  } else if (R_altrep_inherits(x, vroom_big_int::class_t)) {
+    vroom_big_int::Materialize(x);
+  }
 #endif
 }
 
@@ -40,15 +59,13 @@ bool vroom_altrep(SEXP x) {
 [[cpp11::register]] SEXP vroom_materialize(SEXP x, bool replace) {
 #ifdef HAS_ALTREP
   for (R_xlen_t col = 0; col < Rf_xlength(x); ++col) {
-
     SEXP elt = VECTOR_ELT(x, col);
-    // First materialize all of the non-character vectors
     if (vroom_altrep(elt)) {
-      DATAPTR(elt);
+      force_materialization(elt);
     }
   }
 
-  // If replace replace the altrep vectors with their materialized
+  // If replace, replace the altrep vectors with their materialized
   // vectors
   if (replace) {
     for (R_xlen_t col = 0; col < Rf_xlength(x); ++col) {
@@ -81,7 +98,7 @@ bool vroom_altrep(SEXP x) {
       case LGLSXP: {
         SET_VECTOR_ELT(out, col, Rf_allocVector(LGLSXP, nrow));
         int* out_p = LOGICAL(VECTOR_ELT(out, col));
-        int* in_p = LOGICAL(elt);
+        const int* in_p = LOGICAL_RO(elt);
         for (R_xlen_t row = 0; row < nrow; ++row) {
           out_p[row] = in_p[row];
         }
@@ -90,7 +107,7 @@ bool vroom_altrep(SEXP x) {
       case INTSXP: {
         SET_VECTOR_ELT(out, col, Rf_allocVector(INTSXP, nrow));
         int* out_p = INTEGER(VECTOR_ELT(out, col));
-        int* in_p = INTEGER(elt);
+        const int* in_p = INTEGER_RO(elt);
         for (R_xlen_t row = 0; row < nrow; ++row) {
           out_p[row] = in_p[row];
         }
@@ -99,7 +116,7 @@ bool vroom_altrep(SEXP x) {
       case REALSXP: {
         SET_VECTOR_ELT(out, col, Rf_allocVector(REALSXP, nrow));
         double* out_p = REAL(VECTOR_ELT(out, col));
-        double* in_p = REAL(elt);
+        const double* in_p = REAL_RO(elt);
         for (R_xlen_t row = 0; row < nrow; ++row) {
           out_p[row] = in_p[row];
         }
@@ -108,9 +125,9 @@ bool vroom_altrep(SEXP x) {
       case STRSXP: {
         SET_VECTOR_ELT(out, col, Rf_allocVector(STRSXP, nrow));
         SEXP out_elt = VECTOR_ELT(out, col);
-        DATAPTR(elt);
+        const SEXP* in_p = STRING_PTR_RO(elt);
         for (R_xlen_t row = 0; row < nrow; ++row) {
-          SET_STRING_ELT(out_elt, row, STRING_ELT(elt, row));
+          SET_STRING_ELT(out_elt, row, in_p[row]);
         }
         break;
       }

src/altrep.h

@@ -39,4 +39,10 @@ extern "C" {
 #include <R_ext/Altrep.h>
 }
 #endif
+
+// Backport DATAPTR_RW for R < 4.6.0 (as recommended in Writing R Extensions)
+#if R_VERSION < R_Version(4, 6, 0)
+# define DATAPTR_RW(x) DATAPTR(x)
+#endif
+
 #endif

src/vroom_big_int.h

@@ -99,7 +99,7 @@ class vroom_big_int : public vroom_vec {
   }
 
   static void* Dataptr(SEXP vec, Rboolean) {
-    return STDVEC_DATAPTR(Materialize(vec));
+    return DATAPTR_RW(Materialize(vec));
   }
 
   // -------- initialize the altrep class with the methods above

src/vroom_chr.h

@@ -103,7 +103,7 @@ struct vroom_chr : vroom_vec {
   }
 
   static void* Dataptr(SEXP vec, Rboolean) {
-    return STDVEC_DATAPTR(Materialize(vec));
+    return DATAPTR_RW(Materialize(vec));
   }
 
   // -------- initialize the altrep class with the methods above

src/vroom_date.h

@@ -102,7 +102,7 @@ class vroom_date : public vroom_dttm {
   }
 
   static void* Dataptr(SEXP vec, Rboolean) {
-    return STDVEC_DATAPTR(Materialize(vec));
+    return DATAPTR_RW(Materialize(vec));
   }
 
   // -------- initialize the altrep class with the methods above

src/vroom_dbl.h

@@ -84,7 +84,7 @@ class vroom_dbl : public vroom_vec {
   }
 
   static void* Dataptr(SEXP vec, Rboolean) {
-    return STDVEC_DATAPTR(Materialize(vec));
+    return DATAPTR_RW(Materialize(vec));
   }
 
   // -------- initialize the altrep class with the methods above

src/vroom_dttm.h

@@ -210,7 +210,7 @@ class vroom_dttm : public vroom_vec {
   }
 
   static void* Dataptr(SEXP vec, Rboolean) {
-    return STDVEC_DATAPTR(Materialize(vec));
+    return DATAPTR_RW(Materialize(vec));
   }
 
   // -------- initialize the altrep class with the methods above

src/vroom_fct.h

@@ -215,7 +215,7 @@ struct vroom_fct : vroom_vec {
   }
 
   static void* Dataptr(SEXP vec, Rboolean) {
-    return STDVEC_DATAPTR(Materialize(vec));
+    return DATAPTR_RW(Materialize(vec));
   }
 
   static SEXP Extract_subset(SEXP x, SEXP indx, SEXP) {