Use original path names in compiler outputs

NEWS.md

@@ -2,6 +2,8 @@
 * added `as_double()` and `as_integer()` method to coerce integers to doubles and doubles to integers to doubles (@sbearrows, #46)
 * Removed redundant `.Call calls` in cpp11.cpp file (@sbearrows, #170)
 * Allow cpp11 decorators of the form `cpp11::linking_to` (@sbearrows, #193)
+* Error messages now output original file name rather than the temporary file name (@sbearrows, #194)
+* Fixed bug when running `cpp_source()` on the same file more than once (@sbearrows, #202)
 * Removed internal instances of `cpp11::stop()` and replaced with C++ exceptions (@sbearrows, #203)
 
 # cpp11 0.3.1

R/register.R

@@ -297,14 +297,13 @@ get_cpp_register_needs <- function() {
  strsplit(res, "[[:space:]]*,[[:space:]]*")[[1]]
 }
 
-check_valid_attributes <- function(decorations) {
+check_valid_attributes <- function(decorations, file = decorations$file) {
 
  bad_decor <- startsWith(decorations$decoration, "cpp11::") &
  (!decorations$decoration %in% c("cpp11::register", "cpp11::init", "cpp11::linking_to"))
 
  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")

R/source.R

@@ -84,36 +84,34 @@ cpp_source <- function(file, code = NULL, env = parent.frame(), clean = TRUE, qu
  stop("`file` must have a `.cpp` or `.cc` extension")
  }
 
- base_cpp <- (basename(file) %in% "cpp11.cpp")
+ name <- generate_cpp_name(file)
+ package <- tools::file_path_sans_ext(name)
 
- if (base_cpp) {
- name <- set_cpp_name(file)
- package <- tools::file_path_sans_ext(name)
- }
- else {
- # name and package might be different if cpp_source was called multiple times
- name <- basename(file)
- package <- tools::file_path_sans_ext(generate_cpp_name(file))
- }
+ orig_dir <- normalizePath(dirname(file), winslash = "/")
+ new_dir <- normalizePath(file.path(dir, "src"), winslash = "/")
+
+ # file now points to another location
+ file.copy(file, file.path(new_dir, name))
 
- # file not points to another location
- file.copy(file, file.path(dir, "src", name))
  #change variable name to reflect this
- file <- file.path(dir, "src", name)
+ new_file_path <- file.path(new_dir, name)
+ new_file_name <- basename(new_file_path)
 
+ orig_file_path <- file.path(orig_dir, new_file_name)
 
  suppressWarnings(
  all_decorations <- decor::cpp_decorations(dir, is_attribute = TRUE)
  )
 
- check_valid_attributes(all_decorations)
+ #provide original path for error messages
+ check_valid_attributes(all_decorations, file = orig_file_path)
 
  cli_suppress(
  funs <- get_registered_functions(all_decorations, "cpp11::register")
  )
  cpp_functions_definitions <- generate_cpp_functions(funs, package = package)
 
- cpp_path <- file.path(dirname(file), "cpp11.cpp")
+ 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)
 
  linking_to <- union(get_linking_to(all_decorations), "cpp11")
@@ -128,17 +126,20 @@ cpp_source <- function(file, code = NULL, env = parent.frame(), clean = TRUE, qu
 
  makevars_content <- generate_makevars(includes, cxx_std)
 
- brio::write_lines(makevars_content, file.path(dir, "src", "Makevars"))
+ brio::write_lines(makevars_content, file.path(new_dir, "Makevars"))
 
- source_files <- normalizePath(c(file, cpp_path), winslash = "/")
- res <- callr::rcmd("SHLIB", source_files, user_profile = TRUE, show = !quiet, wd = file.path(dir, "src"))
+ source_files <- normalizePath(c(new_file_path, cpp_path), winslash = "/")
+ res <- callr::rcmd("SHLIB", source_files, user_profile = TRUE, show = !quiet, wd = new_dir)
  if (res$status != 0) {
- cat(res$stderr)
+ 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)
+ cat(error_messages)
  stop("Compilation failed.", call. = FALSE)
  }
 
-
- shared_lib <- file.path(dir, "src", paste0(tools::file_path_sans_ext(basename(file)), .Platform$dynlib.ext))
+ shared_lib <- file.path(dir, "src", paste0(tools::file_path_sans_ext(new_file_name), .Platform$dynlib.ext))
  r_path <- file.path(dir, "R", "cpp11.R")
  brio::write_lines(r_functions, r_path)
  source(r_path, local = env)
@@ -149,14 +150,6 @@ cpp_source <- function(file, code = NULL, env = parent.frame(), clean = TRUE, qu
 the <- new.env(parent = emptyenv())
 the$count <- 0L
 
-set_cpp_name <- function(name) {
- ext <- tools::file_ext(name)
- root <- tools::file_path_sans_ext(basename(name))
- count <- 2
- new_name <- sprintf("%s_%i", root, count)
- sprintf("%s.%s", new_name, ext)
-}
-
 generate_cpp_name <- function(name, loaded_dlls = c("cpp11", names(getLoadedDLLs()))) {
  ext <- tools::file_ext(name)
  root <- tools::file_path_sans_ext(basename(name))

tests/testthat/single_error.cpp

@@ -0,0 +1 @@
+[[cpp11::register]] int foo() { return 1 }

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 returns original file name on error", {
+
+ expect_output(try(cpp_source(test_path("single_error.cpp"), clean = TRUE), silent = TRUE),
+ normalizePath(test_path("single_error.cpp"), winslash = "/"), fixed = TRUE)
+
+ #error generated for incorrect attributes is separate from compilation errors
+ expect_error(cpp_source(test_path("single_incorrect.cpp"), clean = TRUE),
+ normalizePath(test_path("single_incorrect.cpp"), winslash = "/"), fixed = TRUE)
+
+})
+
 test_that("cpp_source lets you set the C++ standard", {
  skip_on_os("solaris")
  skip_on_os("windows") # Older windows toolchains do not support C++14
@@ -104,7 +115,7 @@ 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 <cpp11.hpp>
+ cpp11::cpp_source(clean = TRUE, code = '#include <cpp11.hpp>
  using namespace cpp11::literals;
  [[cpp11::register]]
  cpp11::list fn() {
@@ -119,7 +130,7 @@ test_that("check_valid_attributes does not return an error if all registers are
  return x;
  }'))
  expect_error_free(
- cpp11::cpp_source(
+ cpp11::cpp_source(clean = TRUE,
  code = '#include <cpp11/R.hpp>
  #include <RProgress.h>
 
@@ -181,6 +192,8 @@ test_that("check_valid_attributes returns an error if one or more registers is i
  return x;
  }'))
 
+
+
  expect_error(
  cpp11::cpp_source(
  code = '
@@ -195,6 +208,12 @@ test_that("check_valid_attributes returns an error if one or more registers is i
  pb.tick();
  }
  }
-')
- )
+'))
+})
+
+test_that("cpp_source(d) functions work after sourcing file more than once", {
+ cpp11::cpp_source(test_path("single.cpp"), clean = TRUE)
+ expect_equal(foo(), 1)
+ cpp11::cpp_source(test_path("single.cpp"), clean = TRUE)
+ expect_equal(foo(), 1)
 })