Skip to content

Commit 8955bbf

Browse files
authored
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: - Rdatatable/data.table#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 <thisisnic@gmail.com> Signed-off-by: Nic Crane <thisisnic@gmail.com>
1 parent b90a2b8 commit 8955bbf

File tree

2 files changed

+20
-18
lines changed

2 files changed

+20
-18
lines changed

r/src/altrep.cpp

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -149,8 +149,8 @@ struct AltrepVectorBase {
149149
// What gets printed on .Internal(inspect(<the altrep object>))
150150
static Rboolean Inspect(SEXP alt, int pre, int deep, int pvec,
151151
void (*inspect_subtree)(SEXP, int, int, int)) {
152-
SEXP data_class_sym = CAR(ATTRIB(ALTREP_CLASS(alt)));
153-
const char* class_name = CHAR(PRINTNAME(data_class_sym));
152+
SEXP class_sym = R_altrep_class_name(alt);
153+
const char* class_name = CHAR(PRINTNAME(class_sym));
154154

155155
if (IsMaterialized(alt)) {
156156
Rprintf("materialized %s len=%ld\n", class_name,
@@ -574,11 +574,10 @@ struct AltrepFactor : public AltrepVectorBase<AltrepFactor> {
574574
// the representation integer vector
575575
SEXP dup = PROTECT(Rf_shallow_duplicate(Materialize(alt)));
576576

577-
// additional attributes from the altrep
578-
SEXP atts = PROTECT(Rf_duplicate(ATTRIB(alt)));
579-
SET_ATTRIB(dup, atts);
577+
// copy attributes from the altrep object
578+
DUPLICATE_ATTRIB(dup, alt);
580579

581-
UNPROTECT(2);
580+
UNPROTECT(1);
582581
return dup;
583582
}
584583

@@ -1094,9 +1093,7 @@ SEXP MakeAltrepVector(const std::shared_ptr<ChunkedArray>& chunked_array) {
10941093

10951094
bool is_arrow_altrep(SEXP x) {
10961095
if (ALTREP(x)) {
1097-
SEXP info = ALTREP_CLASS_SERIALIZED_CLASS(ALTREP_CLASS(x));
1098-
SEXP pkg = ALTREP_SERIALIZED_CLASS_PKGSYM(info);
1099-
1096+
SEXP pkg = R_altrep_class_package(x);
11001097
if (pkg == symbols::arrow) return true;
11011098
}
11021099

@@ -1160,8 +1157,8 @@ sexp test_arrow_altrep_is_materialized(sexp x) {
11601157
return Rf_ScalarLogical(NA_LOGICAL);
11611158
}
11621159

1163-
sexp data_class_sym = CAR(ATTRIB(ALTREP_CLASS(x)));
1164-
std::string class_name(CHAR(PRINTNAME(data_class_sym)));
1160+
SEXP class_sym = R_altrep_class_name(x);
1161+
std::string class_name(CHAR(PRINTNAME(class_sym)));
11651162

11661163
int result = NA_LOGICAL;
11671164
if (class_name == "arrow::array_dbl_vector") {
@@ -1191,8 +1188,8 @@ bool test_arrow_altrep_force_materialize(sexp x) {
11911188
stop("x is already materialized");
11921189
}
11931190

1194-
sexp data_class_sym = CAR(ATTRIB(ALTREP_CLASS(x)));
1195-
std::string class_name(CHAR(PRINTNAME(data_class_sym)));
1191+
SEXP class_sym = R_altrep_class_name(x);
1192+
std::string class_name(CHAR(PRINTNAME(class_sym)));
11961193

11971194
if (class_name == "arrow::array_dbl_vector") {
11981195
arrow::r::altrep::AltrepVectorPrimitive<REALSXP>::Materialize(x);

r/src/arrow_cpp11.h

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,6 @@
3939
#define ARROW_R_DCHECK(EXPR)
4040
#endif
4141

42-
// For context, see:
43-
// https://github.com/r-devel/r-svn/blob/6418faeb6f5d87d3d9b92b8978773bc3856b4b6f/src/main/altrep.c#L37
44-
#define ALTREP_CLASS_SERIALIZED_CLASS(x) ATTRIB(x)
45-
#define ALTREP_SERIALIZED_CLASS_PKGSYM(x) CADR(x)
46-
4742
#if (R_VERSION < R_Version(3, 5, 0))
4843
#define LOGICAL_RO(x) ((const int*)LOGICAL(x))
4944
#define INTEGER_RO(x) ((const int*)INTEGER(x))
@@ -55,6 +50,16 @@
5550
#define DATAPTR(x) (void*)STRING_PTR(x)
5651
#endif
5752

53+
// R_altrep_class_name and R_altrep_class_package don't exist before R 4.6
54+
#if R_VERSION < R_Version(4, 6, 0)
55+
inline SEXP R_altrep_class_name(SEXP x) {
56+
return ALTREP(x) ? CAR(ATTRIB(ALTREP_CLASS(x))) : R_NilValue;
57+
}
58+
inline SEXP R_altrep_class_package(SEXP x) {
59+
return ALTREP(x) ? CADR(ATTRIB(ALTREP_CLASS(x))) : R_NilValue;
60+
}
61+
#endif
62+
5863
namespace arrow {
5964
namespace r {
6065

0 commit comments

Comments
 (0)