Remove non-API usage of R_NamespaceRegistry

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

inst/include/cpp11/protect.hpp

@@ -191,25 +191,25 @@ inline void check_user_interrupt() { safe[R_CheckUserInterrupt](); }
 #ifdef CPP11_USE_FMT
 template <typename... Args>
 void stop [[noreturn]] (const char* fmt_arg, Args&&... args) {
-  std::string msg = fmt::format(fmt_arg, std::forward<Args>(args)...);
+  std::string msg = fmt::format(fmt::runtime(fmt_arg), std::forward<Args>(args)...);
   safe.noreturn(Rf_errorcall)(R_NilValue, "%s", msg.c_str());
 }
 
 template <typename... Args>
 void stop [[noreturn]] (const std::string& fmt_arg, Args&&... args) {
-  std::string msg = fmt::format(fmt_arg, std::forward<Args>(args)...);
+  std::string msg = fmt::format(fmt::runtime(fmt_arg), std::forward<Args>(args)...);
   safe.noreturn(Rf_errorcall)(R_NilValue, "%s", msg.c_str());
 }
 
 template <typename... Args>
 void warning(const char* fmt_arg, Args&&... args) {
-  std::string msg = fmt::format(fmt_arg, std::forward<Args>(args)...);
+  std::string msg = fmt::format(fmt::runtime(fmt_arg), std::forward<Args>(args)...);
   safe[Rf_warningcall](R_NilValue, "%s", msg.c_str());
 }
 
 template <typename... Args>
 void warning(const std::string& fmt_arg, Args&&... args) {
-  std::string msg = fmt::format(fmt_arg, std::forward<Args>(args)...);
+  std::string msg = fmt::format(fmt::runtime(fmt_arg), std::forward<Args>(args)...);
   safe[Rf_warningcall](R_NilValue, "%s", msg.c_str());
 }
 #else

tests/testthat/_snaps/source.md

@@ -7,3 +7,11 @@
       ! Can't find `file` at this path:
       {NON_EXISTENT_FILEPATH}
 
+# `cpp11::package` throws expected error on unknown packages
+
+    Code
+      test()
+    Condition
+      Error:
+      ! Can't find namespace: 'definitely_not_a_package'.
+

tests/testthat/test-source.R

@@ -227,3 +227,22 @@ test_that("cpp_source fails informatively for nonexistent file", {
     transform = ~ sub("^.+[.]cpp$", "{NON_EXISTENT_FILEPATH}", .x)
   )
 })
+
+test_that("`cpp11::package` throws expected error on unknown packages", {
+  skip_on_os("solaris")
+  dll_info <- cpp_source(
+    code = '
+    #include "cpp11/function.hpp"
+
+    [[cpp11::register]]
+    SEXP test() {
+      auto pkg = cpp11::package("definitely_not_a_package");
+      return R_NilValue;
+    }
+    ', clean = TRUE)
+  on.exit(dyn.unload(dll_info[["path"]]))
+
+  expect_snapshot(error = TRUE, {
+    test()
+  })
+})