diff --git a/NEWS.md b/NEWS.md index 90148254..810a4795 100644 --- a/NEWS.md +++ b/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). diff --git a/R/register.R b/R/register.R index 3ea10d52..c35cc71a 100644 --- a/R/register.R +++ b/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 ", "#include ", "using namespace Rcpp;") + extra_includes <- c( + extra_includes, + "#include ", + "#define RCPP_NO_R_HEADERS_CHECK", + "#include ", + "using namespace Rcpp;" + ) } pkg_types <- c( diff --git a/cpp11test/src/cpp11.cpp b/cpp11test/src/cpp11.cpp index 421de637..0eb9809c 100644 --- a/cpp11test/src/cpp11.cpp +++ b/cpp11test/src/cpp11.cpp @@ -2,6 +2,7 @@ // clang-format off #include +#define RCPP_NO_R_HEADERS_CHECK #include using namespace Rcpp; #include "cpp11/declarations.hpp" diff --git a/cpp11test/src/find-intervals.cpp b/cpp11test/src/find-intervals.cpp index 0e7933e2..0c9ebcaa 100644 --- a/cpp11test/src/find-intervals.cpp +++ b/cpp11test/src/find-intervals.cpp @@ -2,6 +2,7 @@ #include "cpp11.hpp" using namespace cpp11; +#define RCPP_NO_R_HEADERS_CHECK #include #include using namespace Rcpp; diff --git a/cpp11test/src/matrix.cpp b/cpp11test/src/matrix.cpp index 10348945..82583cda 100644 --- a/cpp11test/src/matrix.cpp +++ b/cpp11test/src/matrix.cpp @@ -32,6 +32,7 @@ using namespace cpp11; return mat; } +#define RCPP_NO_R_HEADERS_CHECK #include using namespace Rcpp; diff --git a/cpp11test/src/protect.cpp b/cpp11test/src/protect.cpp index bb4ef934..f1ece491 100644 --- a/cpp11test/src/protect.cpp +++ b/cpp11test/src/protect.cpp @@ -1,7 +1,8 @@ #include #include -#include "Rcpp.h" +#define RCPP_NO_R_HEADERS_CHECK +#include [[cpp11::register]] void protect_one_(SEXP x, int n) { for (R_xlen_t i = 0; i < n; ++i) { diff --git a/cpp11test/src/release.cpp b/cpp11test/src/release.cpp index 7b321e45..03d158de 100644 --- a/cpp11test/src/release.cpp +++ b/cpp11test/src/release.cpp @@ -1,7 +1,8 @@ #include #include "cpp11/sexp.hpp" -#include "Rcpp.h" +#define RCPP_NO_R_HEADERS_CHECK +#include [[cpp11::register]] void cpp11_release_(int n) { std::vector x; diff --git a/cpp11test/src/test-as.cpp b/cpp11test/src/test-as.cpp index 798596a2..6fef6473 100644 --- a/cpp11test/src/test-as.cpp +++ b/cpp11test/src/test-as.cpp @@ -7,7 +7,8 @@ #include -#include "Rcpp.h" +#define RCPP_NO_R_HEADERS_CHECK +#include context("as_cpp-C++") { test_that("as_cpp(INTSEXP)") { diff --git a/inst/include/cpp11/R.hpp b/inst/include/cpp11/R.hpp index c10c7630..f6a398d3 100644 --- a/inst/include/cpp11/R.hpp +++ b/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 diff --git a/inst/include/cpp11/function.hpp b/inst/include/cpp11/function.hpp index cc5af071..23820ea2 100644 --- a/inst/include/cpp11/function.hpp +++ b/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 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]; diff --git a/inst/include/cpp11/protect.hpp b/inst/include/cpp11/protect.hpp index 9cb4876a..3058ecba 100644 --- a/inst/include/cpp11/protect.hpp +++ b/inst/include/cpp11/protect.hpp @@ -191,25 +191,25 @@ inline void check_user_interrupt() { safe[R_CheckUserInterrupt](); } #ifdef CPP11_USE_FMT template void stop [[noreturn]] (const char* fmt_arg, Args&&... args) { - std::string msg = fmt::format(fmt_arg, std::forward(args)...); + std::string msg = fmt::format(fmt::runtime(fmt_arg), std::forward(args)...); safe.noreturn(Rf_errorcall)(R_NilValue, "%s", msg.c_str()); } template void stop [[noreturn]] (const std::string& fmt_arg, Args&&... args) { - std::string msg = fmt::format(fmt_arg, std::forward(args)...); + std::string msg = fmt::format(fmt::runtime(fmt_arg), std::forward(args)...); safe.noreturn(Rf_errorcall)(R_NilValue, "%s", msg.c_str()); } template void warning(const char* fmt_arg, Args&&... args) { - std::string msg = fmt::format(fmt_arg, std::forward(args)...); + std::string msg = fmt::format(fmt::runtime(fmt_arg), std::forward(args)...); safe[Rf_warningcall](R_NilValue, "%s", msg.c_str()); } template void warning(const std::string& fmt_arg, Args&&... args) { - std::string msg = fmt::format(fmt_arg, std::forward(args)...); + std::string msg = fmt::format(fmt::runtime(fmt_arg), std::forward(args)...); safe[Rf_warningcall](R_NilValue, "%s", msg.c_str()); } #else diff --git a/tests/testthat/_snaps/source.md b/tests/testthat/_snaps/source.md index ad9152aa..3435da17 100644 --- a/tests/testthat/_snaps/source.md +++ b/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'. + diff --git a/tests/testthat/test-source.R b/tests/testthat/test-source.R index 2945d51d..0c7fd0fa 100644 --- a/tests/testthat/test-source.R +++ b/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() + }) +})