diff --git a/NEWS.md b/NEWS.md index de5e460b..17a55d2d 100644 --- a/NEWS.md +++ b/NEWS.md @@ -5,6 +5,7 @@ # cpp11 0.2.7 +* Outputting more informative error message when cpp11 decorators are incorrectly formatted (@sbearrows, #127) * Fix spurious diffs from `tools::package_native_routine_registration_skeleton()` by temporarily using C collation (@sbearrows, #171) * Fix a transient memory leak for functions that return values from `cpp11::unwind_protect()` and `cpp11::safe` (#154) * `cpp_source()` now gets an argument `dir` to allow customized temporary directory to store generated source files. diff --git a/R/register.R b/R/register.R index 5c98f37c..f0056af9 100644 --- a/R/register.R +++ b/R/register.R @@ -46,6 +46,8 @@ cpp_register <- function(path = ".", quiet = FALSE) { return(invisible(character())) } + check_valid_attributes(all_decorations) + funs <- get_registered_functions(all_decorations, "cpp11::register", quiet) package <- desc::desc_get("Package", file = file.path(path, "DESCRIPTION")) @@ -274,3 +276,22 @@ get_cpp_register_needs <- function() { res <- read.dcf(system.file("DESCRIPTION", package = "cpp11"))[, "Config/Needs/cpp11/cpp_register"] strsplit(res, "[[:space:]]*,[[:space:]]*")[[1]] } + +check_valid_attributes <- function(decorations) { + + bad_decor <- !decorations$decoration %in% c("cpp11::register", "cpp11::init") + + if(any(bad_decor)) { + lines <- decorations$line[bad_decor] + file <- decorations$file[bad_decor] + names <- decorations$decoration[bad_decor] + bad_lines <- glue::glue_collapse(glue::glue("- Invalid attribute `{names}` on + line {lines} in file '{file}'."), "\n") + + msg <- glue::glue("cpp11 attributes must be either `cpp11::register` or `cpp11::init`: + {bad_lines} + ") + stop(msg, call. = FALSE) + + } +} diff --git a/R/source.R b/R/source.R index f9e456b9..3fefeb44 100644 --- a/R/source.R +++ b/R/source.R @@ -105,6 +105,9 @@ cpp_source <- function(file, code = NULL, env = parent.frame(), clean = TRUE, qu suppressWarnings( all_decorations <- decor::cpp_decorations(dir, is_attribute = TRUE) ) + + check_valid_attributes(all_decorations) + cli_suppress( funs <- get_registered_functions(all_decorations, "cpp11::register") ) diff --git a/tests/testthat/multiple_incorrect.cpp b/tests/testthat/multiple_incorrect.cpp new file mode 100644 index 00000000..caa100d9 --- /dev/null +++ b/tests/testthat/multiple_incorrect.cpp @@ -0,0 +1,5 @@ +[[cpp11:register]] int foo() { return 1; } + +[[cpp11:register]] double bar(bool run) { return 1.0; } + +[[cpp11::register]] bool baz(bool run, int value = 0) { return true; } diff --git a/tests/testthat/single_incorrect.cpp b/tests/testthat/single_incorrect.cpp new file mode 100644 index 00000000..7867d53b --- /dev/null +++ b/tests/testthat/single_incorrect.cpp @@ -0,0 +1 @@ +[[cpp11:register]] int foo() { return 1; } diff --git a/tests/testthat/test-register.R b/tests/testthat/test-register.R index 09cdcb65..582d346a 100644 --- a/tests/testthat/test-register.R +++ b/tests/testthat/test-register.R @@ -703,3 +703,36 @@ describe("generate_init_functions", { expect_equal(generate_init_functions(funs), list(declarations = "\nvoid foo(DllInfo* dll);\nvoid bar(DllInfo* dll);\n", calls = "\n foo(dll);\n bar(dll);")) }) }) + +test_that("check_valid_attributes does not return an error if all registers are correct", { + pkg <- local_package() + p <- pkg_path(pkg) + dir.create(file.path(p, "src")) + file.copy(test_path("single.cpp"), file.path(p, "src", "single.cpp")) + + expect_error_free(cpp_register(p)) + + pkg <- local_package() + p <- pkg_path(pkg) + dir.create(file.path(p, "src")) + file.copy(test_path("multiple.cpp"), file.path(p, "src", "multiple.cpp")) + + expect_error_free(cpp_register(p)) +}) + + +test_that("check_valid_attributes returns an error if one or more registers is incorrect", { + pkg <- local_package() + p <- pkg_path(pkg) + dir.create(file.path(p, "src")) + file.copy(test_path("single_incorrect.cpp"), file.path(p, "src", "single_incorrect.cpp")) + + expect_error(cpp_register(p)) + + pkg <- local_package() + p <- pkg_path(pkg) + dir.create(file.path(p, "src")) + file.copy(test_path("multiple_incorrect.cpp"), file.path(p, "src", "multiple_incorrect.cpp")) + + expect_error(cpp_register(p)) +}) diff --git a/tests/testthat/test-source.R b/tests/testthat/test-source.R index c216373e..8d708be3 100644 --- a/tests/testthat/test-source.R +++ b/tests/testthat/test-source.R @@ -102,3 +102,54 @@ test_that("generate_include_paths handles paths with spaces", { } }) +test_that("check_valid_attributes does not return an error if all registers are correct", { + expect_error_free( + cpp11::cpp_source(code = '#include + using namespace cpp11::literals; + [[cpp11::register]] + cpp11::list fn() { + cpp11::writable::list x; + x.push_back({"foo"_nm = 1}); + return x; + } + [[cpp11::register]] + cpp11::list fn2() { + cpp11::writable::list x; + x.push_back({"foo"_nm = 1}); + return x; + }')) +}) + +test_that("check_valid_attributes returns an error if one or more registers is incorrect", { + expect_error( + cpp11::cpp_source(code = '#include + using namespace cpp11::literals; + [[cpp11:register]] + cpp11::list fn() { + cpp11::writable::list x; + x.push_back({"foo"_nm = 1}); + return x; + } + [[cpp11::register]] + cpp11::list fn2() { + cpp11::writable::list x; + x.push_back({"foo"_nm = 1}); + return x; + }')) + + expect_error( + cpp11::cpp_source(code = '#include + using namespace cpp11::literals; + [[cpp11:register]] + cpp11::list fn() { + cpp11::writable::list x; + x.push_back({"foo"_nm = 1}); + return x; + } + [[cpp11::egister]] + cpp11::list fn2() { + cpp11::writable::list x; + x.push_back({"foo"_nm = 1}); + return x; + }')) +}) \ No newline at end of file