Drop support for R 3.6 and remove dead code (#411)

* Drop support for R 3.6 * NEWS bullet * Remove dead code from tests * Remove dead code from `environment.hpp` * Remove dead code from `protect.hpp` * Simplify `altrep.hpp` as much as possible Likely still need to keep it around for compatibility reasons * NEWS bullet about `altrep.hpp` * Bring back dummy `HAS_UNWIND_PROTECT` and remove all usage of it
r-lib · Dec 3, 2024 · 4b4d780 · 4b4d780
1 parent dc93fb8
commit 4b4d780
Show file tree

Hide file tree

Showing 15 changed files with 22 additions and 103 deletions.
diff --git a/.github/workflows/R-CMD-check.yaml b/.github/workflows/R-CMD-check.yaml
@@ -27,8 +27,6 @@ jobs:
           - {os: macos-latest,   r: 'release'}
 
           - {os: windows-latest, r: 'release'}
-          # Use 3.6 to trigger usage of RTools35
-          - {os: windows-latest, r: '3.6'}
           # use 4.1 to check with rtools40's older compiler
           - {os: windows-latest, r: '4.1'}
 

diff --git a/DESCRIPTION b/DESCRIPTION
@@ -18,7 +18,7 @@ License: MIT + file LICENSE
 URL: https://cpp11.r-lib.org, https://github.com/r-lib/cpp11
 BugReports: https://github.com/r-lib/cpp11/issues
 Depends:
-    R (>= 3.6.0)
+    R (>= 4.0.0)
 Suggests:
     bench,
     brio,

diff --git a/NEWS.md b/NEWS.md
@@ -1,5 +1,16 @@
 # cpp11 (development version)
 
+* Because cpp11 now requires R >=4.0.0 and `R_UnwindProtect()` is always
+  available, `HAS_UNWIND_PROTECT` is no longer useful. Please avoid using it,
+  as we'd like to remove it in the future (#411).
+
+* Because cpp11 now requires R >=4.0.0 and ALTREP is always available, the
+  `cpp11/altrep.hpp` file is no longer useful. Please avoid using `#include "cpp11/altrep.hpp"` and `HAS_ALTREP` as we'd like to remove them in the
+  future (#411).
+
+* cpp11 now requires R >=4.0.0, in line with the
+  [tidyverse version policy](https://www.tidyverse.org/blog/2019/04/r-version-support/) (#411).
+
 # cpp11 0.5.0
 
 ## R non-API related changes

diff --git a/cpp11test/src/safe.cpp b/cpp11test/src/safe.cpp
@@ -26,9 +26,7 @@
   if (buf[0] != '\0') {
     Rf_error("%s", buf);
   } else if (err != R_NilValue) {
-#ifdef HAS_UNWIND_PROTECT
     R_ContinueUnwind(err);
-#endif
   }
 
   return R_NilValue;

diff --git a/cpp11test/src/test-as.cpp b/cpp11test/src/test-as.cpp
@@ -49,11 +49,9 @@ context("as_cpp-C++") {
     auto x5 = cpp11::as_cpp<unsigned long>(r);
     expect_true(x5 == 42UL);
 
-#ifdef HAS_UNWIND_PROTECT
     /* throws a runtime exception if the value is not a integerish one */
     REAL(r)[0] = 42.5;
     expect_error(cpp11::as_cpp<int>(r));
-#endif
 
     UNPROTECT(1);
   }

diff --git a/cpp11test/src/test-doubles.cpp b/cpp11test/src/test-doubles.cpp
@@ -233,7 +233,7 @@ context("doubles-C++") {
     UNPROTECT(1);
   }
 
-#if defined(__APPLE__) && defined(R_VERSION) && R_VERSION >= R_Version(3, 5, 0)
+#if defined(__APPLE__)
   test_that("writable::doubles(ALTREP_SEXP)") {
     // ALTREP compact-seq
     auto seq = cpp11::package("base")["seq"];

diff --git a/cpp11test/src/test-integers.cpp b/cpp11test/src/test-integers.cpp
@@ -1,4 +1,3 @@
-#include "Rversion.h"
 #include "cpp11/R.hpp"
 #include "cpp11/doubles.hpp"
 #include "cpp11/function.hpp"
@@ -218,7 +217,7 @@ context("integers-C++") {
     expect_true(x[2] == 3);
   }
 
-#if defined(__APPLE__) && defined(R_VERSION) && R_VERSION >= R_Version(3, 5, 0)
+#if defined(__APPLE__)
   test_that("writable::integers(ALTREP_SEXP)") {
     // ALTREP compact-seq
     auto seq = cpp11::package("base")["seq"];

diff --git a/cpp11test/src/test-protect-nested.cpp b/cpp11test/src/test-protect-nested.cpp
@@ -2,8 +2,6 @@
 #include "cpp11/protect.hpp"
 #include "testthat.h"
 
-#ifdef HAS_UNWIND_PROTECT
-
 /*
  * See https://github.com/r-lib/cpp11/pull/327 for full details.
  *
@@ -77,5 +75,3 @@ context("unwind_protect-nested-C++") {
     destructed = false;
   }
 }
-
-#endif
diff --git a/cpp11test/src/test-protect.cpp b/cpp11test/src/test-protect.cpp
@@ -2,7 +2,6 @@
 #include "cpp11/protect.hpp"
 #include "testthat.h"
 
-#ifdef HAS_UNWIND_PROTECT
 context("unwind_protect-C++") {
   test_that("unwind_protect works if there is no error") {
     SEXP out = PROTECT(cpp11::unwind_protect([&] {
@@ -49,5 +48,3 @@ context("unwind_protect-C++") {
     expect_error_as(cpp11::safe[Rf_allocVector](REALSXP, -1), cpp11::unwind_exception);
   }
 }
-
-#endif
diff --git a/cpp11test/src/test-r_vector.cpp b/cpp11test/src/test-r_vector.cpp
@@ -7,14 +7,6 @@
 
 #include <algorithm>  // for max_element
 
-#ifdef _WIN32
-#include "Rversion.h"
-#define CPP11_HAS_IS_UTILITIES R_VERSION >= R_Version(4, 0, 0)
-#else
-#define CPP11_HAS_IS_UTILITIES 1
-#endif
-
-#if CPP11_HAS_IS_UTILITIES
 context("r_vector-capabilities-C++") {
   test_that("read only vector capabilities") {
     using cpp11::integers;
@@ -77,7 +69,6 @@ context("r_vector-capabilities-C++") {
     expect_true(std::is_move_assignable<integers::proxy>::value);
   }
 }
-#endif
 
 context("r_vector-C++") {
   test_that("writable vector temporary isn't leaked (integer) (#338)") {

diff --git a/inst/include/cpp11/R.hpp b/inst/include/cpp11/R.hpp
@@ -27,7 +27,6 @@
 // clang-format on
 
 #include <type_traits>
-#include "cpp11/altrep.hpp"
 
 #if defined(R_VERSION) && R_VERSION >= R_Version(4, 4, 0)
 // Use R's new macro

diff --git a/inst/include/cpp11/altrep.hpp b/inst/include/cpp11/altrep.hpp
@@ -1,42 +1,6 @@
 #pragma once
 
-#include "Rversion.h"
-
-#if defined(R_VERSION) && R_VERSION >= R_Version(3, 5, 0)
+// It would be nice to remove this since all supported versions of R have ALTREP, but
+// some groups rely on both this `#define` and `altrep.hpp` itself existing, like arrow:
+// https://github.com/apache/arrow/blob/50f2d6e04e8323119d4dd31506827ee398d6b8e4/r/src/altrep.cpp#L27-L29
 #define HAS_ALTREP
-#endif
-
-#ifndef HAS_ALTREP
-
-#define ALTREP(x) false
-
-#define REAL_ELT(x, i) REAL(x)[i]
-#define INTEGER_ELT(x, i) INTEGER(x)[i]
-#define LOGICAL_ELT(x, i) LOGICAL(x)[i]
-#define RAW_ELT(x, i) RAW(x)[i]
-
-#define SET_REAL_ELT(x, i, val) REAL(x)[i] = val
-#define SET_INTEGER_ELT(x, i, val) INTEGER(x)[i] = val
-#define SET_LOGICAL_ELT(x, i, val) LOGICAL(x)[i] = val
-#define SET_RAW_ELT(x, i, val) RAW(x)[i] = val
-
-#define REAL_GET_REGION(...) \
-  do {                       \
-  } while (false)
-
-#define INTEGER_GET_REGION(...) \
-  do {                          \
-  } while (false)
-#endif
-
-#if !defined HAS_ALTREP || (defined(R_VERSION) && R_VERSION < R_Version(3, 6, 0))
-
-#define LOGICAL_GET_REGION(...) \
-  do {                          \
-  } while (false)
-
-#define RAW_GET_REGION(...) \
-  do {                      \
-  } while (false)
-
-#endif
diff --git a/inst/include/cpp11/declarations.hpp b/inst/include/cpp11/declarations.hpp
@@ -30,14 +30,6 @@ T& unmove(T&& t) {
 }
 }  // namespace cpp11
 
-#ifdef HAS_UNWIND_PROTECT
-#define CPP11_UNWIND R_ContinueUnwind(err);
-#else
-#define CPP11_UNWIND \
-  do {               \
-  } while (false);
-#endif
-
 #define CPP11_ERROR_BUFSIZE 8192
 
 #define BEGIN_CPP11                   \
@@ -58,6 +50,6 @@ T& unmove(T&& t) {
   if (buf[0] != '\0') {                                         \
     Rf_errorcall(R_NilValue, "%s", buf);                        \
   } else if (err != R_NilValue) {                               \
-    CPP11_UNWIND                                                \
+    R_ContinueUnwind(err);                                      \
   }                                                             \
   return R_NilValue;
diff --git a/inst/include/cpp11/environment.hpp b/inst/include/cpp11/environment.hpp
@@ -2,20 +2,11 @@
 
 #include <string>  // for string, basic_string
 
-#include "Rversion.h"         // for R_VERSION, R_Version
 #include "cpp11/R.hpp"        // for SEXP, SEXPREC, Rf_install, r_env_get...
 #include "cpp11/as.hpp"       // for as_sexp
 #include "cpp11/protect.hpp"  // for protect, protect::function, safe, unwin...
 #include "cpp11/sexp.hpp"     // for sexp
 
-#if R_VERSION >= R_Version(4, 0, 0)
-#define HAS_REMOVE_VAR_FROM_FRAME
-#endif
-
-#ifndef HAS_REMOVE_VAR_FROM_FRAME
-#include "cpp11/function.hpp"
-#endif
-
 namespace cpp11 {
 
 class environment {
@@ -51,12 +42,7 @@ class environment {
 
   void remove(SEXP name) {
     PROTECT(name);
-#ifdef HAS_REMOVE_VAR_FROM_FRAME
     R_removeVarFromFrame(name, env_);
-#else
-    auto remove = package("base")["remove"];
-    remove(name, "envir"_nm = env_);
-#endif
     UNPROTECT(1);
   }
 

diff --git a/inst/include/cpp11/protect.hpp b/inst/include/cpp11/protect.hpp
@@ -13,11 +13,12 @@
 #include "R_ext/Error.h"    // for Rf_error, Rf_warning
 #include "R_ext/Print.h"    // for REprintf
 #include "R_ext/Utils.h"    // for R_CheckUserInterrupt
-#include "Rversion.h"       // for R_VERSION, R_Version
 
-#if defined(R_VERSION) && R_VERSION >= R_Version(3, 5, 0)
+// We would like to remove this, since all supported versions of R now support proper
+// unwind protect, but some groups rely on it existing, like arrow and systemfonts
+// https://github.com/r-lib/systemfonts/blob/02b567086379edaca1a9b3620ad6776e6bb876a7/src/utils.h#L11
+// https://github.com/apache/arrow/blob/50f2d6e04e8323119d4dd31506827ee398d6b8e4/r/src/safe-call-into-r-impl.cpp#L49
 #define HAS_UNWIND_PROTECT
-#endif
 
 #ifdef CPP11_USE_FMT
 #define FMT_HEADER_ONLY
@@ -31,8 +32,6 @@ class unwind_exception : public std::exception {
   unwind_exception(SEXP token_) : token(token_) {}
 };
 
-#ifdef HAS_UNWIND_PROTECT
-
 /// Unwind Protection from C longjmp's, like those used in R error handling
 ///
 /// @param code The code to which needs to be protected, as a nullary callable
@@ -95,15 +94,6 @@ unwind_protect(Fun&& code) {
   return out;
 }
 
-#else
-// Don't do anything if we don't have unwind protect. This will leak C++ resources,
-// including those held by cpp11 objects, but the other alternatives are also not great.
-template <typename Fun>
-decltype(std::declval<Fun&&>()()) unwind_protect(Fun&& code) {
-  return std::forward<Fun>(code)();
-}
-#endif
-
 namespace detail {
 
 template <size_t...>