Remove non-API usage of R_NamespaceRegistry (#488)

* Don't rely on transitive include of `"fmt/core.h"`

We got it through `cpp11/protect.hpp`, but we should not rely on that

* Correctly use `fmt::runtime()` on runtime strings passed to `fmt::format()`

In fmt, there is a `FMT_CONSTEVAL` macro that resolves to `consteval` on "new enough" C++ (otherwise it doesn't do anything). For R 4.6+, the default C++ used is finally "new enough" (`__cplusplus > 201703L`).

This causes all `fmt::format()` calls to require a constant expression for `const char*` and `std::string&` input, which we are not currently doing via `fmt_arg`.

We have a runtime provided string, which must now be wrapped in `fmt::runtime()`, which is what we should have been doing all along.

* Add `r_ns_env()` and use in `get_namespace()`

Throwing an informative (and tested!) error when we can't find the package namespace

* Define `RCPP_NO_R_HEADERS_CHECK` before all `#include <Rcpp.h>` usage

`#include <cpp11/R.hpp>` sets everything up the right way, and otherwise we get a warning from Rcpp which doesn't seem to end up being relevant for this use case

* Use `r_env_has()` + `r_env_get()`

To avoid triggering a NOTE about usage of `Rf_findVarInFrame3()` on R 4.5, where technically we had the tools to avoid that

Author: DavisVaughan

NEWS.md

@@ -1,5 +1,9 @@
 # cpp11 (development version)
 
+* Removed non-API usage of `R_NamespaceRegistry`.
+
+* Fixed a bug with `CPP11_USE_FMT` where the input was not being correctly wrapped in `fmt::runtime()`.
+
 # cpp11 0.5.3
 
 * Removed non-API usage of `ATTRIB()` (#481).

R/register.R

@@ -89,7 +89,13 @@ cpp_register <- function(path = ".", quiet = !is_interactive(), extension = c(".
 
   extra_includes <-  character()
   if (pkg_links_to_rcpp(path)) {
-    extra_includes <- c(extra_includes, "#include <cpp11/R.hpp>", "#include <Rcpp.h>", "using namespace Rcpp;")
+    extra_includes <- c(
+      extra_includes,
+      "#include <cpp11/R.hpp>",
+      "#define RCPP_NO_R_HEADERS_CHECK",
+      "#include <Rcpp.h>",
+      "using namespace Rcpp;"
+    )
   }
 
   pkg_types <- c(

cpp11test/src/cpp11.cpp

@@ -2,6 +2,7 @@
 // clang-format off
 
 #include <cpp11/R.hpp>
+#define RCPP_NO_R_HEADERS_CHECK
 #include <Rcpp.h>
 using namespace Rcpp;
 #include "cpp11/declarations.hpp"

cpp11test/src/find-intervals.cpp

@@ -2,6 +2,7 @@
 #include "cpp11.hpp"
 using namespace cpp11;
 
+#define RCPP_NO_R_HEADERS_CHECK
 #include <Rcpp.h>
 #include <algorithm>
 using namespace Rcpp;

cpp11test/src/matrix.cpp

@@ -32,6 +32,7 @@ using namespace cpp11;
   return mat;
 }
 
+#define RCPP_NO_R_HEADERS_CHECK
 #include <Rcpp.h>
 using namespace Rcpp;
 

cpp11test/src/protect.cpp

@@ -1,7 +1,8 @@
 #include <cpp11/sexp.hpp>
 #include <vector>
 
-#include "Rcpp.h"
+#define RCPP_NO_R_HEADERS_CHECK
+#include <Rcpp.h>
 
 [[cpp11::register]] void protect_one_(SEXP x, int n) {
   for (R_xlen_t i = 0; i < n; ++i) {

cpp11test/src/release.cpp

@@ -1,7 +1,8 @@
 #include <vector>
 #include "cpp11/sexp.hpp"
 
-#include "Rcpp.h"
+#define RCPP_NO_R_HEADERS_CHECK
+#include <Rcpp.h>
 
 [[cpp11::register]] void cpp11_release_(int n) {
   std::vector<cpp11::sexp> x;

cpp11test/src/test-as.cpp

@@ -7,7 +7,8 @@
 
 #include <testthat.h>
 
-#include "Rcpp.h"
+#define RCPP_NO_R_HEADERS_CHECK
+#include <Rcpp.h>
 
 context("as_cpp-C++") {
   test_that("as_cpp<integer>(INTSEXP)") {

inst/include/cpp11/R.hpp

@@ -113,6 +113,29 @@ inline bool r_env_has(SEXP env, SEXP sym) {
 #endif
 }
 
+/// Get a namespace from the namespace registry
+///
+/// Returns `R_NilValue` if the namespace is not in the registry, i.e. if the package has
+/// not been loaded yet.
+///
+/// Unlike `R_FindNamespace()`, does not attempt to load the package, does not error when
+/// the namespace can't be found, and does not go through R.
+///
+/// SAFETY: Keep as a pure C function. Call like an R API function, i.e. wrap in `safe[]`
+/// as required.
+inline SEXP r_ns_env(const char* name) {
+#if R_VERSION >= R_Version(4, 6, 0)
+  return R_getRegisteredNamespace(name);
+#else
+  SEXP sym = Rf_install(name);
+  if (r_env_has(R_NamespaceRegistry, sym)) {
+    return r_env_get(R_NamespaceRegistry, sym);
+  } else {
+    return R_NilValue;
+  }
+#endif
+}
+
 }  // namespace detail
 
 template <typename T>

inst/include/cpp11/function.hpp

@@ -8,9 +8,14 @@
 #include "cpp11/R.hpp"          // for SEXP, SEXPREC, CDR, Rf_install, SETCAR
 #include "cpp11/as.hpp"         // for as_sexp
 #include "cpp11/named_arg.hpp"  // for named_arg
-#include "cpp11/protect.hpp"    // for protect, protect::function, safe
+#include "cpp11/protect.hpp"    // for protect, protect::function, safe, stop
 #include "cpp11/sexp.hpp"       // for sexp
 
+#ifdef CPP11_USE_FMT
+#define FMT_HEADER_ONLY
+#include "fmt/core.h"
+#endif
+
 namespace cpp11 {
 
 class function {
@@ -66,8 +71,11 @@ class package {
     if (strcmp(name, "base") == 0) {
       return R_BaseEnv;
     }
-    sexp name_sexp = safe[Rf_install](name);
-    return safe[detail::r_env_get](R_NamespaceRegistry, name_sexp);
+    SEXP env = safe[detail::r_ns_env](name);
+    if (env == R_NilValue) {
+      stop("Can't find namespace: '%s'.", name);
+    }
+    return env;
   }
 
   // Either base env or in namespace registry, so no protection needed
@@ -106,7 +114,7 @@ inline void r_message(const char* x) {
 
 inline void message(const char* fmt_arg) {
 #ifdef CPP11_USE_FMT
-  std::string msg = fmt::format(fmt_arg);
+  std::string msg = fmt::format(fmt::runtime(fmt_arg));
   safe[detail::r_message](msg.c_str());
 #else
   char buff[1024];
@@ -121,7 +129,7 @@ inline void message(const char* fmt_arg) {
 template <typename... Args>
 void message(const char* fmt_arg, Args... args) {
 #ifdef CPP11_USE_FMT
-  std::string msg = fmt::format(fmt_arg, args...);
+  std::string msg = fmt::format(fmt::runtime(fmt_arg), args...);
   safe[detail::r_message](msg.c_str());
 #else
   char buff[1024];