This is an automated email from the ASF dual-hosted git repository.
paleolimbot pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-nanoarrow.git
The following commit(s) were added to refs/heads/main by this push:
new 4241f5f0 chore(r): Remove conditional ALTREP usage and remove call to
DATAPTR (#741)
4241f5f0 is described below
commit 4241f5f07a642a615912f339a5c089c1fe597ed0
Author: Dewey Dunnington <de...@dunnington.ca>
AuthorDate: Mon Apr 14 21:25:33 2025 -0500
chore(r): Remove conditional ALTREP usage and remove call to DATAPTR (#741)
From the CRAN check page:
```
File ‘nanoarrow/libs/nanoarrow.so’:
Found non-API call to R: ‘DATAPTR’
Compiled code should not call non-API entry points in R.
See ‘Writing portable packages’ in the ‘Writing R Extensions’ manual,
and section ‘Moving into C API compliance’ for issues with the use of
non-API entry points.
```
When fixing this I also saw that we have conditional ALTREP usage.
ALTREP has been available since R 3.5.0, which is two versions before
what our CI matrix checks. I removed the conditional bits that were no
longer needed (i.e., we now always assume that `R_ext/altrep.h` is
available).
---
r/src/altrep.c | 15 ++++-----------
r/src/altrep.h | 19 ++++++-------------
2 files changed, 10 insertions(+), 24 deletions(-)
diff --git a/r/src/altrep.c b/r/src/altrep.c
index 94fd7203..847951b1 100644
--- a/r/src/altrep.c
+++ b/r/src/altrep.c
@@ -28,8 +28,6 @@
#include "nanoarrow.h"
#include "util.h"
-#ifdef HAS_ALTREP
-
// This file defines all ALTREP classes used to speed up conversion
// from an arrow_array to an R vector. Currently only string and
// large string arrays are converted to ALTREP.
@@ -102,7 +100,10 @@ static SEXP nanoarrow_altstring_materialize(SEXP
altrep_sexp) {
}
static void* nanoarrow_altrep_dataptr(SEXP altrep_sexp, Rboolean writable) {
- return DATAPTR(nanoarrow_altstring_materialize(altrep_sexp));
+ // DATAPTR() can't be called in R >= 4.5.0 without a check NOTE, but
+ // there doesn't appear to be an alternative to support an ALTREP string
+ // class that can materialize.
+ return (void*)DATAPTR_RO(nanoarrow_altstring_materialize(altrep_sexp));
}
static const void* nanoarrow_altrep_dataptr_or_null(SEXP altrep_sexp) {
@@ -116,10 +117,7 @@ static const void* nanoarrow_altrep_dataptr_or_null(SEXP
altrep_sexp) {
static R_altrep_class_t nanoarrow_altrep_chr_cls;
-#endif
-
static void register_nanoarrow_altstring(DllInfo* info) {
-#ifdef HAS_ALTREP
nanoarrow_altrep_chr_cls =
R_make_altstring_class("nanoarrow::altrep_chr", "nanoarrow", info);
R_set_altrep_Length_method(nanoarrow_altrep_chr_cls,
&nanoarrow_altrep_length);
@@ -143,13 +141,11 @@ static void register_nanoarrow_altstring(DllInfo* info) {
// indices.
// - The duplicate method may be useful because it's used when setting
attributes
// or unclassing the vector.
-#endif
}
void register_nanoarrow_altrep(DllInfo* info) {
register_nanoarrow_altstring(info); }
SEXP nanoarrow_c_make_altrep_chr(SEXP array_xptr) {
-#ifdef HAS_ALTREP
SEXP schema_xptr = array_xptr_get_schema(array_xptr);
// Create the converter
@@ -184,9 +180,6 @@ SEXP nanoarrow_c_make_altrep_chr(SEXP array_xptr) {
MARK_NOT_MUTABLE(out);
UNPROTECT(3);
return out;
-#else
- return R_NilValue;
-#endif
}
SEXP nanoarrow_c_is_altrep(SEXP x_sexp) {
diff --git a/r/src/altrep.h b/r/src/altrep.h
index 98be7e2f..1e2ce2a5 100644
--- a/r/src/altrep.h
+++ b/r/src/altrep.h
@@ -18,16 +18,15 @@
#ifndef R_ALTREP_H_INCLUDED
#define R_ALTREP_H_INCLUDED
-#include "Rversion.h"
+#include <R.h>
+#include <Rinternals.h>
+#include <Rversion.h>
-#include <string.h>
-
-// ALTREP available in R >= 3.5
-#if defined(R_VERSION) && R_VERSION >= R_Version(3, 5, 0)
-
-#define HAS_ALTREP
+// Needs to be at the end of the R include list
#include <R_ext/Altrep.h>
+#include <string.h>
+
// Returns the ALTREP class name or NULL if x is not an altrep
// object.
static inline const char* nanoarrow_altrep_class(SEXP x) {
@@ -39,12 +38,6 @@ static inline const char* nanoarrow_altrep_class(SEXP x) {
}
}
-#else
-
-static inline const char* nanoarrow_altrep_class(SEXP x) { return NULL; }
-
-#endif
-
// Performs the ALTREP type registration and should be called on package load
void register_nanoarrow_altrep(DllInfo* info);
