Bring back dummy HAS_UNWIND_PROTECT and remove all usage of it

r-lib · Dec 3, 2024 · 3270efe · 3270efe
1 parent bd297ff
commit 3270efe
Show file tree

Hide file tree

Showing 7 changed files with 13 additions and 21 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -1,7 +1,12 @@
 # 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"` as we'd like to remove this file in the future (#411).
+  `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).

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-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/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/protect.hpp b/inst/include/cpp11/protect.hpp
@@ -14,6 +14,12 @@
 #include "R_ext/Print.h"    // for REprintf
 #include "R_ext/Utils.h"    // for R_CheckUserInterrupt
 
+// 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
+
 #ifdef CPP11_USE_FMT
 #define FMT_HEADER_ONLY
 #include "fmt/core.h"