This is an automated email from the ASF dual-hosted git repository.
thisisnic pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new 8955bbf7b2 GH-48631: [R] Non-API calls: 'ATTRIB', 'SET_ATTRIB' (#48634)
8955bbf7b2 is described below
commit 8955bbf7b266d7e293f5037bd20c3e2e983e004c
Author: Nic Crane <[email protected]>
AuthorDate: Mon Jan 12 09:14:33 2026 +0000
GH-48631: [R] Non-API calls: 'ATTRIB', 'SET_ATTRIB' (#48634)
### Rationale for this change
CRAN complains about non-API calls
### What changes are included in this PR?
Update those items to use alternative approaches
### Are these changes tested?
Well, if the tests pass I think we're good?
### Are there any user-facing changes?
Nope
### Summary of AI changes
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Have added comments inline regarding assumptions behind changes, and where
I questioned those to try to verify it.
Sources referenced in approach:
- https://github.com/Rdatatable/data.table/pull/7487 - similar fixes
- https://cran.r-project.org/doc/manuals/r-release/R-ints.html on
`DUPLICATE_ATTRIB`
* GitHub Issue: #48631
Authored-by: Nic Crane <[email protected]>
Signed-off-by: Nic Crane <[email protected]>
---
r/src/altrep.cpp | 23 ++++++++++-------------
r/src/arrow_cpp11.h | 15 ++++++++++-----
2 files changed, 20 insertions(+), 18 deletions(-)
diff --git a/r/src/altrep.cpp b/r/src/altrep.cpp
index bb91ca03c4..c9ed6b59e8 100644
--- a/r/src/altrep.cpp
+++ b/r/src/altrep.cpp
@@ -149,8 +149,8 @@ struct AltrepVectorBase {
// 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)) {
- SEXP data_class_sym = CAR(ATTRIB(ALTREP_CLASS(alt)));
- const char* class_name = CHAR(PRINTNAME(data_class_sym));
+ SEXP class_sym = R_altrep_class_name(alt);
+ const char* class_name = CHAR(PRINTNAME(class_sym));
if (IsMaterialized(alt)) {
Rprintf("materialized %s len=%ld\n", class_name,
@@ -574,11 +574,10 @@ struct AltrepFactor : public
AltrepVectorBase<AltrepFactor> {
// the representation integer vector
SEXP dup = PROTECT(Rf_shallow_duplicate(Materialize(alt)));
- // additional attributes from the altrep
- SEXP atts = PROTECT(Rf_duplicate(ATTRIB(alt)));
- SET_ATTRIB(dup, atts);
+ // copy attributes from the altrep object
+ DUPLICATE_ATTRIB(dup, alt);
- UNPROTECT(2);
+ UNPROTECT(1);
return dup;
}
@@ -1094,9 +1093,7 @@ SEXP MakeAltrepVector(const
std::shared_ptr<ChunkedArray>& chunked_array) {
bool is_arrow_altrep(SEXP x) {
if (ALTREP(x)) {
- SEXP info = ALTREP_CLASS_SERIALIZED_CLASS(ALTREP_CLASS(x));
- SEXP pkg = ALTREP_SERIALIZED_CLASS_PKGSYM(info);
-
+ SEXP pkg = R_altrep_class_package(x);
if (pkg == symbols::arrow) return true;
}
@@ -1160,8 +1157,8 @@ sexp test_arrow_altrep_is_materialized(sexp x) {
return Rf_ScalarLogical(NA_LOGICAL);
}
- sexp data_class_sym = CAR(ATTRIB(ALTREP_CLASS(x)));
- std::string class_name(CHAR(PRINTNAME(data_class_sym)));
+ SEXP class_sym = R_altrep_class_name(x);
+ std::string class_name(CHAR(PRINTNAME(class_sym)));
int result = NA_LOGICAL;
if (class_name == "arrow::array_dbl_vector") {
@@ -1191,8 +1188,8 @@ bool test_arrow_altrep_force_materialize(sexp x) {
stop("x is already materialized");
}
- sexp data_class_sym = CAR(ATTRIB(ALTREP_CLASS(x)));
- std::string class_name(CHAR(PRINTNAME(data_class_sym)));
+ SEXP class_sym = R_altrep_class_name(x);
+ std::string class_name(CHAR(PRINTNAME(class_sym)));
if (class_name == "arrow::array_dbl_vector") {
arrow::r::altrep::AltrepVectorPrimitive<REALSXP>::Materialize(x);
diff --git a/r/src/arrow_cpp11.h b/r/src/arrow_cpp11.h
index c4483ce531..f44fd635fd 100644
--- a/r/src/arrow_cpp11.h
+++ b/r/src/arrow_cpp11.h
@@ -39,11 +39,6 @@
#define ARROW_R_DCHECK(EXPR)
#endif
-// For context, see:
-//
https://github.com/r-devel/r-svn/blob/6418faeb6f5d87d3d9b92b8978773bc3856b4b6f/src/main/altrep.c#L37
-#define ALTREP_CLASS_SERIALIZED_CLASS(x) ATTRIB(x)
-#define ALTREP_SERIALIZED_CLASS_PKGSYM(x) CADR(x)
-
#if (R_VERSION < R_Version(3, 5, 0))
#define LOGICAL_RO(x) ((const int*)LOGICAL(x))
#define INTEGER_RO(x) ((const int*)INTEGER(x))
@@ -55,6 +50,16 @@
#define DATAPTR(x) (void*)STRING_PTR(x)
#endif
+// R_altrep_class_name and R_altrep_class_package don't exist before R 4.6
+#if R_VERSION < R_Version(4, 6, 0)
+inline SEXP R_altrep_class_name(SEXP x) {
+ return ALTREP(x) ? CAR(ATTRIB(ALTREP_CLASS(x))) : R_NilValue;
+}
+inline SEXP R_altrep_class_package(SEXP x) {
+ return ALTREP(x) ? CADR(ATTRIB(ALTREP_CLASS(x))) : R_NilValue;
+}
+#endif
+
namespace arrow {
namespace r {