Merge branch 'main' into testthat3e

.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'}
 

DESCRIPTION

@@ -1,6 +1,6 @@
 Package: cpp11
 Title: A C++11 Interface for R's C Interface
-Version: 0.5.1
+Version: 0.5.1.9000
 Authors@R:
     c(
       person("Davis", "Vaughan", email = "[email protected]", role = c("aut", "cre"), comment = c(ORCID = "0000-0003-4777-038X")),
@@ -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,
@@ -54,4 +54,4 @@ Config/Needs/cpp11/cpp_register:
     vctrs
 Encoding: UTF-8
 Roxygen: list(markdown = TRUE)
-RoxygenNote: 7.3.1
+RoxygenNote: 7.3.2

MAINTENANCE.md

@@ -7,7 +7,16 @@ The state of cpp11 is pretty stable, it seems to have the features we need for m
 ### Running the cpp11test tests
 
 Most of the test suite is in a sub-package, cpp11test.
-Probably the best way to run these tests is to install the development version of cpp11 and then run `devtools::test()` to run the cpp11test test suite.
+The best way to run these tests is to install the development version of cpp11 after any change, and then run `devtools::test("./cpp11test")`.
+Precisely, this looks like:
+
+```r
+# Install dev cpp11, clean the cpp11test dll manually since it thinks nothing
+# has changed, then recompile and run its tests.
+devtools::install()
+devtools::clean_dll("./cpp11test")
+devtools::test("./cpp11test")
+```
 
 If tests failures occur the output from Catch isn't always easy to interpret.
 I have a branch of testthat https://github.com/jimhester/testthat/tree/catch-detailed-output that should make things easier to understand.
@@ -17,25 +26,6 @@ In addition getting a debugger to catch when errors happen can be fiddly when ru
 
 The GitHub Actions workflow has some additional logic to handle running the cpp11 tests https://github.com/r-lib/cpp11/blob/fd8ef97d006db847f7f17166cf52e1e0383b2d35/.github/workflows/R-CMD-check.yaml#L95-L102, https://github.com/r-lib/cpp11/blob/fd8ef97d006db847f7f17166cf52e1e0383b2d35/.github/workflows/R-CMD-check.yaml#L117-L124.
 
-### False positive URL checks for git repositories in the vignettes
-
-If you run `urlchecker::url_check()` on the repo you will see the following false positives.
-
-```
-! Warning: vignettes/motivations.Rmd:363:11 Moved
-git clone https://github.com/r-lib/cpp11.git
-          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-          https://github.com/r-lib/cpp11
-! Warning: vignettes/motivations.Rmd:354:11 Moved
-git clone https://github.com/RcppCore/Rcpp.git
-          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-          https://github.com/RcppCore/Rcpp
->
-```
-
-These only happen with the urlchecker package, they can be safely ignored and the real CRAN checks will not show them.
-
-
 ## Ensure you use `Sys.setenv("CPP11_EVAL" = "true"); devtools::submit_cran()` when submitting.
 
 If you forget to set `CPP_EVAL = "true"` then the vignette chunks will not run properly and the vignettes will not be rendered properly.

NEWS.md

@@ -1,10 +1,34 @@
 # cpp11 (development version)
 
-# cpp11 0.5.1
+# cpp11 0.5.1.9000
 
 * Uses testthat 3e style tests (#402).
 * Removes the mockery dependence.
 * The vendoring function accepts a custom path (i.e., to use the GitHub version of the package)
+* 
+# cpp11 0.5.1
+
+* 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).
+
+* Because cpp11 now requires R >=4.0.0, a number of previously optional tools
+  are now always available, allowing us to remove some dead code. In
+  particular:
+
+  * `R_UnwindProtect()` is always available, so the defines `HAS_UNWIND_PROTECT`
+    and `CPP11_UNWIND` are no longer useful.
+
+  * ALTREP is always available, so the file `cpp11/altrep.hpp` and the define
+    `HAS_ALTREP` are no longer useful.
+
+  We would like to remove the dead code regarding these tools in the future, so
+  we ask that you please remove usage of them from your own packages (#411).
+
+* `R_NO_REMAP` and `STRICT_R_HEADERS` are now conditionally defined only if they
+  have not already been defined elsewhere. This is motivated by the fact that
+  `R_NO_REMAP` is becoming the default for C++ code in R 4.5.0 (#410).
+
+* Fixed a small protection issue flagged by rchk (#408).
 
 # cpp11 0.5.0
 

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;

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);
   }

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"];

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"];

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

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

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)") {

inst/include/cpp11/R.hpp

@@ -8,8 +8,14 @@
 #endif
 #endif
 
+#ifndef R_NO_REMAP
 #define R_NO_REMAP
+#endif
+
+#ifndef STRICT_R_HEADERS
 #define STRICT_R_HEADERS
+#endif
+
 #include "R_ext/Boolean.h"
 #include "Rinternals.h"
 #include "Rversion.h"
@@ -27,7 +33,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

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/r-lib/cpp11/issues/413
 #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

inst/include/cpp11/declarations.hpp

@@ -30,13 +30,10 @@ T& unmove(T&& t) {
 }
 }  // namespace cpp11
 
-#ifdef HAS_UNWIND_PROTECT
+// 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 textshaping:
+// https://github.com/r-lib/cpp11/issues/414
 #define CPP11_UNWIND R_ContinueUnwind(err);
-#else
-#define CPP11_UNWIND \
-  do {               \
-  } while (false);
-#endif
 
 #define CPP11_ERROR_BUFSIZE 8192
 
@@ -58,6 +55,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;

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);
   }
 

inst/include/cpp11/protect.hpp

@@ -13,11 +13,11 @@
 #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/cpp11/issues/412
 #define HAS_UNWIND_PROTECT
-#endif
 
 #ifdef CPP11_USE_FMT
 #define FMT_HEADER_ONLY
@@ -31,8 +31,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 +93,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...>

inst/include/cpp11/r_vector.hpp

@@ -1323,7 +1323,8 @@ inline SEXP r_vector<T>::reserve_data(SEXP x, bool is_altrep, R_xlen_t size) {
   SEXP out = PROTECT(resize_data(x, is_altrep, size));
 
   // Resize names, if required
-  SEXP names = Rf_getAttrib(x, R_NamesSymbol);
+  // Protection seems needed to make rchk happy
+  SEXP names = PROTECT(Rf_getAttrib(x, R_NamesSymbol));
   if (names != R_NilValue) {
     if (Rf_xlength(names) != size) {
       names = resize_names(names, size);
@@ -1338,7 +1339,7 @@ inline SEXP r_vector<T>::reserve_data(SEXP x, bool is_altrep, R_xlen_t size) {
   // Does not look like it would ever error in our use cases, so no `safe[]`.
   Rf_copyMostAttrib(x, out);
 
-  UNPROTECT(1);
+  UNPROTECT(2);
   return out;
 }