From 802d4fd1ee1a0fd83a27d3dcb5178be4f6e3f747 Mon Sep 17 00:00:00 2001 From: Nic Crane Date: Sat, 4 Apr 2026 18:35:08 +0100 Subject: [PATCH 1/4] Update based on feedback --- r/src/arrow_cpp11.h | 34 +++++++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/r/src/arrow_cpp11.h b/r/src/arrow_cpp11.h index d28ad0feded..1edbe70d7e2 100644 --- a/r/src/arrow_cpp11.h +++ b/r/src/arrow_cpp11.h @@ -208,14 +208,26 @@ Pointer r6_to_pointer(SEXP self) { cpp11::stop("Invalid R object for %s, must be an ArrowObject", type_name.c_str()); } -#if R_VERSION >= R_Version(4, 5, 0) +// R_UnboundValue and Rf_findVarInFrame are non-API as of R 4.6 +#if R_VERSION >= R_Version(4, 6, 0) + if (!R_existsVarInFrame(self, arrow::r::symbols::xp)) { + cpp11::stop("Invalid: self$`.:xp:.` is NULL"); + } + SEXP xp = R_getVar(arrow::r::symbols::xp, self, FALSE); + if (xp == R_NilValue) { + cpp11::stop("Invalid: self$`.:xp:.` is NULL"); + } +#elif R_VERSION >= R_Version(4, 5, 0) SEXP xp = R_getVarEx(arrow::r::symbols::xp, self, FALSE, R_UnboundValue); + if (xp == R_UnboundValue || xp == R_NilValue) { + cpp11::stop("Invalid: self$`.:xp:.` is NULL"); + } #else SEXP xp = Rf_findVarInFrame(self, arrow::r::symbols::xp); -#endif if (xp == R_UnboundValue || xp == R_NilValue) { cpp11::stop("Invalid: self$`.:xp:.` is NULL"); } +#endif void* p = R_ExternalPtrAddr(xp); if (p == nullptr) { @@ -227,10 +239,22 @@ Pointer r6_to_pointer(SEXP self) { template void r6_reset_pointer(SEXP r6) { -#if R_VERSION >= R_Version(4, 5, 0) +// R_UnboundValue and Rf_findVarInFrame are non-API as of R 4.6 +#if R_VERSION >= R_Version(4, 6, 0) + if (!R_existsVarInFrame(r6, arrow::r::symbols::xp)) { + return; + } + SEXP xp = R_getVar(arrow::r::symbols::xp, r6, FALSE); +#elif R_VERSION >= R_Version(4, 5, 0) SEXP xp = R_getVarEx(arrow::r::symbols::xp, r6, FALSE, R_UnboundValue); + if (xp == R_UnboundValue) { + return; + } #else SEXP xp = Rf_findVarInFrame(r6, arrow::r::symbols::xp); + if (xp == R_UnboundValue) { + return; + } #endif void* p = R_ExternalPtrAddr(xp); if (p != nullptr) { @@ -388,8 +412,8 @@ SEXP to_r6(const std::shared_ptr& ptr, const char* r6_class_name) { cpp11::external_pointer> xp(new std::shared_ptr(ptr)); SEXP r6_class = Rf_install(r6_class_name); -// R_existsVarInFrame doesn't exist before R 4.2, so we need to fall back to -// Rf_findVarInFrame3 if it is not defined. +// Rf_findVarInFrame3 and R_UnboundValue are non-API as of R 4.6. +// R_existsVarInFrame doesn't exist before R 4.2. #if R_VERSION >= R_Version(4, 2, 0) if (!R_existsVarInFrame(arrow::r::ns::arrow, r6_class)) { cpp11::stop("No arrow R6 class named '%s'", r6_class_name); From 93cc361920bf021093c48c41ca131dd97e4d5cc2 Mon Sep 17 00:00:00 2001 From: Nic Crane Date: Sat, 4 Apr 2026 18:50:17 +0100 Subject: [PATCH 2/4] Simplify paths --- r/src/arrow_cpp11.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/r/src/arrow_cpp11.h b/r/src/arrow_cpp11.h index 1edbe70d7e2..66c40728432 100644 --- a/r/src/arrow_cpp11.h +++ b/r/src/arrow_cpp11.h @@ -210,10 +210,10 @@ Pointer r6_to_pointer(SEXP self) { // R_UnboundValue and Rf_findVarInFrame are non-API as of R 4.6 #if R_VERSION >= R_Version(4, 6, 0) - if (!R_existsVarInFrame(self, arrow::r::symbols::xp)) { - cpp11::stop("Invalid: self$`.:xp:.` is NULL"); + SEXP xp = R_NilValue; + if (R_existsVarInFrame(self, arrow::r::symbols::xp)) { + xp = R_getVar(arrow::r::symbols::xp, self, FALSE); } - SEXP xp = R_getVar(arrow::r::symbols::xp, self, FALSE); if (xp == R_NilValue) { cpp11::stop("Invalid: self$`.:xp:.` is NULL"); } @@ -412,8 +412,8 @@ SEXP to_r6(const std::shared_ptr& ptr, const char* r6_class_name) { cpp11::external_pointer> xp(new std::shared_ptr(ptr)); SEXP r6_class = Rf_install(r6_class_name); -// Rf_findVarInFrame3 and R_UnboundValue are non-API as of R 4.6. -// R_existsVarInFrame doesn't exist before R 4.2. +// R_existsVarInFrame doesn't exist before R 4.2, so we need to fall back to +// Rf_findVarInFrame3 if it is not defined. #if R_VERSION >= R_Version(4, 2, 0) if (!R_existsVarInFrame(arrow::r::ns::arrow, r6_class)) { cpp11::stop("No arrow R6 class named '%s'", r6_class_name); From 97a5debd7d3313eaf712665e8b2e90ac868ab27a Mon Sep 17 00:00:00 2001 From: Nic Crane Date: Sat, 4 Apr 2026 18:53:51 +0100 Subject: [PATCH 3/4] preserve original logic --- r/src/arrow_cpp11.h | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/r/src/arrow_cpp11.h b/r/src/arrow_cpp11.h index 66c40728432..122f8d5f194 100644 --- a/r/src/arrow_cpp11.h +++ b/r/src/arrow_cpp11.h @@ -217,13 +217,12 @@ Pointer r6_to_pointer(SEXP self) { if (xp == R_NilValue) { cpp11::stop("Invalid: self$`.:xp:.` is NULL"); } -#elif R_VERSION >= R_Version(4, 5, 0) +#else +#if R_VERSION >= R_Version(4, 5, 0) SEXP xp = R_getVarEx(arrow::r::symbols::xp, self, FALSE, R_UnboundValue); - if (xp == R_UnboundValue || xp == R_NilValue) { - cpp11::stop("Invalid: self$`.:xp:.` is NULL"); - } #else SEXP xp = Rf_findVarInFrame(self, arrow::r::symbols::xp); +#endif if (xp == R_UnboundValue || xp == R_NilValue) { cpp11::stop("Invalid: self$`.:xp:.` is NULL"); } @@ -245,16 +244,12 @@ void r6_reset_pointer(SEXP r6) { return; } SEXP xp = R_getVar(arrow::r::symbols::xp, r6, FALSE); -#elif R_VERSION >= R_Version(4, 5, 0) +#else +#if R_VERSION >= R_Version(4, 5, 0) SEXP xp = R_getVarEx(arrow::r::symbols::xp, r6, FALSE, R_UnboundValue); - if (xp == R_UnboundValue) { - return; - } #else SEXP xp = Rf_findVarInFrame(r6, arrow::r::symbols::xp); - if (xp == R_UnboundValue) { - return; - } +#endif #endif void* p = R_ExternalPtrAddr(xp); if (p != nullptr) { From 815bc35f8695a39666b661832770fa6257a95885 Mon Sep 17 00:00:00 2001 From: Nic Crane Date: Sat, 4 Apr 2026 18:55:52 +0100 Subject: [PATCH 4/4] Sort out elif --- r/src/arrow_cpp11.h | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/r/src/arrow_cpp11.h b/r/src/arrow_cpp11.h index 122f8d5f194..43296238513 100644 --- a/r/src/arrow_cpp11.h +++ b/r/src/arrow_cpp11.h @@ -217,12 +217,13 @@ Pointer r6_to_pointer(SEXP self) { if (xp == R_NilValue) { cpp11::stop("Invalid: self$`.:xp:.` is NULL"); } -#else -#if R_VERSION >= R_Version(4, 5, 0) +#elif R_VERSION >= R_Version(4, 5, 0) SEXP xp = R_getVarEx(arrow::r::symbols::xp, self, FALSE, R_UnboundValue); + if (xp == R_UnboundValue || xp == R_NilValue) { + cpp11::stop("Invalid: self$`.:xp:.` is NULL"); + } #else SEXP xp = Rf_findVarInFrame(self, arrow::r::symbols::xp); -#endif if (xp == R_UnboundValue || xp == R_NilValue) { cpp11::stop("Invalid: self$`.:xp:.` is NULL"); } @@ -244,12 +245,10 @@ void r6_reset_pointer(SEXP r6) { return; } SEXP xp = R_getVar(arrow::r::symbols::xp, r6, FALSE); -#else -#if R_VERSION >= R_Version(4, 5, 0) +#elif R_VERSION >= R_Version(4, 5, 0) SEXP xp = R_getVarEx(arrow::r::symbols::xp, r6, FALSE, R_UnboundValue); #else SEXP xp = Rf_findVarInFrame(r6, arrow::r::symbols::xp); -#endif #endif void* p = R_ExternalPtrAddr(xp); if (p != nullptr) {