Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cpp source include fixes #251

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# cpp11 (development version)

* Type files `<basename>_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

* Fix crash related to unwind protect optimization (#244)
Expand Down
42 changes: 35 additions & 7 deletions R/source.R
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<basename>_types.h` or
#' `<basename>_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.
Expand Down Expand Up @@ -84,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 = "/")
Expand All @@ -111,12 +123,20 @@ 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")

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)
Expand All @@ -134,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())
Expand Down
6 changes: 1 addition & 5 deletions man/cpp11-package.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions man/cpp_source.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions tests/testthat/test-source.R
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down