From 36e7a98493b485630c6bf20b29b57d0537d577b6 Mon Sep 17 00:00:00 2001 From: Vitalie Spinu Date: Tue, 23 Nov 2021 16:50:25 +0100 Subject: [PATCH 1/3] [Fix #216] Include _types.h into cpp11.cpp on `source_cpp()` --- NEWS.md | 2 ++ R/source.R | 13 ++++++++++++- man/cpp11-package.Rd | 6 +----- man/cpp_source.Rd | 4 ++++ 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/NEWS.md b/NEWS.md index 9083e169..1d2b1740 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,7 @@ # cpp11 (development version) +* Type files `_type.h` is no honored by `source_cpp()` (#216). + # cpp11 0.4.1 * Fix crash related to unwind protect optimization (#244) diff --git a/R/source.R b/R/source.R index 0632228d..e8bf30d6 100644 --- a/R/source.R +++ b/R/source.R @@ -8,6 +8,10 @@ #' external packages. This is equivalent to putting those packages in the #' `LinkingTo` field in a package DESCRIPTION. #' +#' Custom types, if any, should be declared in `_types.h` or +#' `_types.hpp`. If such file exists it will be automatically included +#' into the routine registration file (`cpp11.cpp`) during compilation. +#' #' @param file A file containing C++ code to compile #' @param code If non-null, the C++ code to compile #' @param env The R environment where the R wrapping functions should be defined. @@ -111,8 +115,15 @@ cpp_source <- function(file, code = NULL, env = parent.frame(), clean = TRUE, qu ) cpp_functions_definitions <- generate_cpp_functions(funs, package = package) + basename <- tools::file_path_sans_ext(basename(file)) + type_paths <- file.path(orig_dir, paste0(basename, "_", c("types.h", "types.hpp"))) + type_paths <- type_paths[file.exists(type_paths)] cpp_path <- file.path(dirname(new_file_path), "cpp11.cpp") - brio::write_lines(c('#include "cpp11/declarations.hpp"', "using namespace ::cpp11;", cpp_functions_definitions), cpp_path) + brio::write_lines(c(sprintf('#include "%s"', type_paths), + '#include "cpp11/declarations.hpp"', + "using namespace ::cpp11;", + cpp_functions_definitions), + cpp_path) linking_to <- union(get_linking_to(all_decorations), "cpp11") diff --git a/man/cpp11-package.Rd b/man/cpp11-package.Rd index 4a4f147d..cb196da0 100644 --- a/man/cpp11-package.Rd +++ b/man/cpp11-package.Rd @@ -6,11 +6,7 @@ \alias{cpp11-package} \title{cpp11: A C++11 Interface for R's C Interface} \description{ -Provides a header only, C++11 interface to R's C - interface. Compared to other approaches 'cpp11' strives to be safe - against long jumps from the C API as well as C++ exceptions, conform - to normal R function semantics and supports interaction with 'ALTREP' - vectors. +Provides a header only, C++11 interface to R's C interface. Compared to other approaches 'cpp11' strives to be safe against long jumps from the C API as well as C++ exceptions, conform to normal R function semantics and supports interaction with 'ALTREP' vectors. } \seealso{ Useful links: diff --git a/man/cpp_source.Rd b/man/cpp_source.Rd index 87164ef5..2f57675d 100644 --- a/man/cpp_source.Rd +++ b/man/cpp_source.Rd @@ -64,6 +64,10 @@ expression. Within C++ code you can use \verb{[[cpp11::linking_to("pkgxyz")]]} to link to external packages. This is equivalent to putting those packages in the \code{LinkingTo} field in a package DESCRIPTION. + +Custom types, if any, should be declared in \verb{_types.h} or +\verb{_types.hpp}. If such file exists it will be automatically included +into the routine registration file (\code{cpp11.cpp}) during compilation. } \examples{ From c3be07241e5e7b33675bd56a89cb4da0004ce7a4 Mon Sep 17 00:00:00 2001 From: Vitalie Spinu Date: Tue, 23 Nov 2021 17:10:18 +0100 Subject: [PATCH 2/3] Include file's directory into include path in `source_cpp()` compilation --- NEWS.md | 1 + R/source.R | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 1d2b1740..c0c4a200 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,7 @@ # cpp11 (development version) * Type files `_type.h` is no honored by `source_cpp()` (#216). +* The directory of the sourced file is included into the include path during `source_cpp()` compilation. # cpp11 0.4.1 diff --git a/R/source.R b/R/source.R index e8bf30d6..3bbaecfe 100644 --- a/R/source.R +++ b/R/source.R @@ -127,7 +127,8 @@ cpp_source <- function(file, code = NULL, env = parent.frame(), clean = TRUE, qu linking_to <- union(get_linking_to(all_decorations), "cpp11") - includes <- generate_include_paths(linking_to) + includes <- c(generate_include_paths(linking_to), + sprintf("-I'%s'", orig_dir)) if (isTRUE(clean)) { on.exit(unlink(dir, recursive = TRUE), add = TRUE) From 37fd5c85215477983d993b8f84b695f37523158c Mon Sep 17 00:00:00 2001 From: Vitalie Spinu Date: Sun, 28 Nov 2021 20:56:25 +0100 Subject: [PATCH 3/3] cpp_source: Don't generate _N.cpp temporary suffixes in error locs --- R/source.R | 26 +++++++++++++++++++++----- tests/testthat/test-source.R | 11 +++++++++++ 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/R/source.R b/R/source.R index 3bbaecfe..8f6c7b24 100644 --- a/R/source.R +++ b/R/source.R @@ -88,8 +88,16 @@ cpp_source <- function(file, code = NULL, env = parent.frame(), clean = TRUE, qu stop("`file` must have a `.cpp` or `.cc` extension") } - name <- generate_cpp_name(file) - package <- tools::file_path_sans_ext(name) + # In order to preserve error links to the original location we do the + # following: + # 1) We compile with the original base name in a tmp location + # 2) In case of an error replace all tmp locations with original location () + # 3) Load schlib with a package name suffixed with _N where N is incremented + # on each compilation + name <- basename(file) + if (identical(name, "cpp11.cpp")) + name <- "cpp11x.cpp" # corner case + package <- tools::file_path_sans_ext(generate_cpp_name(file)) orig_dir <- normalizePath(dirname(file), winslash = "/") new_dir <- normalizePath(file.path(dir, "src"), winslash = "/") @@ -146,17 +154,25 @@ cpp_source <- function(file, code = NULL, env = parent.frame(), clean = TRUE, qu error_messages <- res$stderr # Substitute temporary file path with original file path - error_messages <- gsub(tools::file_path_sans_ext(new_file_path), tools::file_path_sans_ext(orig_file_path), error_messages, fixed = TRUE) + error_messages <- gsub(tools::file_path_sans_ext(new_file_path), + tools::file_path_sans_ext(orig_file_path), + error_messages, fixed = TRUE) cat(error_messages) stop("Compilation failed.", call. = FALSE) } - shared_lib <- file.path(dir, "src", paste0(tools::file_path_sans_ext(new_file_name), .Platform$dynlib.ext)) + # During compilation we keep original file name, but the name of the package + # and shared lib is suffixed with _N where N is incremented on every + # cpp_source. + slib_orig <- file.path(dir, "src", paste0(tools::file_path_sans_ext(name), .Platform$dynlib.ext)) + slib_new <- file.path(dir, "src", paste0(package, .Platform$dynlib.ext)) + file.rename(slib_orig, slib_new) + r_path <- file.path(dir, "R", "cpp11.R") brio::write_lines(r_functions, r_path) source(r_path, local = env) - dyn.load(shared_lib, local = TRUE, now = TRUE) + dyn.load(slib_new, local = TRUE, now = TRUE) } the <- new.env(parent = emptyenv()) diff --git a/tests/testthat/test-source.R b/tests/testthat/test-source.R index 8f813d0e..dafd13ed 100644 --- a/tests/testthat/test-source.R +++ b/tests/testthat/test-source.R @@ -54,6 +54,17 @@ test_that("cpp_source works with files called `cpp11.cpp`", { expect_true(always_true()) }) +test_that("cpp_source does not generate _N.cpp suffixes in error links", { + skip_on_os("solaris") + tf <- file.path(tempdir(), "dummy.cpp") + on.exit(unlink(tf)) + file.copy(test_path("single.cpp"), tf) + cpp_source(tf, clean = TRUE, quiet = TRUE) + file.copy(test_path("single_error.cpp"), tf) + output <- paste(capture.output(cpp_source(tf, clean = TRUE, quiet = FALSE)), collapse = "\n") + expect_false(grepl("_[0-9]+.cpp:[0-9]+:[0-9]+", output)) +}) + test_that("cpp_source returns original file name on error", { expect_output(try(cpp_source(test_path("single_error.cpp"), clean = TRUE), silent = TRUE),