-
-
Notifications
You must be signed in to change notification settings - Fork 211
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
env argument not used to actually source the R code #851
Comments
@jjallaire Thoughts? Potential solution could work. Not sure how many people pass an arg there and have a local R snippet... |
Seems like this is an oversight. @filipsch Could you give us a PR for this? |
I have opened a PR, but honestly have no clue on how to include a unit test for this. The structure of the Rcpp package is too unfamiliar. Let's continue the discussion there. |
@filipsch I am seeing side effects. Ubuntu 17.10; g++ 7.2; R 3.4.4. On the console: * checking tests ...
Running ‘doRUnit.R’ [142s/109s]
[143s/109s] ERROR
Running the tests in ‘tests/doRUnit.R’ failed.
Last 13 lines of output:
13: try(eval(expr, envir = parent.frame()), silent = silent)
14: inherits(try(eval(expr, envir = parent.frame()), silent = silent), "try-error")
15: checkException(Rcpp::sourceCpp(file.path(path, "embeddedR2.cpp"), env = newEnv2), " not available in other env")
16: func()
17: system.time(func(), gcFirst = RUnitEnv$.gcBeforeTest)
18: doTryCatch(return(expr), name, parentenv, handler)
19: tryCatchOne(expr, names, parentenv, handlers[[1L]])
20: tryCatchList(expr, classes, parentenv, handlers)
21: tryCatch(expr, error = function(e) { call <- conditionCall(e) if (!is.null(call)) { if (identical(call[[1L]], quote(doTryCatch))) call <- sys.call(-4L) dcall <- deparse(call)[1L] prefix <- paste("Error in", dcall, ": ") LONG <- 75L msg <- conditionMessage(e) sm <- strsplit(msg, "\n")[[1L]] w <- 14L + nchar(dcall, type = "w") + nchar(sm[1L], type = "w") if (is.na(w)) w <- 14L + nchar(dcall, type = "b") + nchar(sm[1L], type = "b") if (w > LONG) prefix <- paste0(prefix, "\n ") } else prefix <- "Error : " msg <- paste0(prefix, conditionMessage(e), "\n") .Internal(seterrmessage(msg[1L])) if (!silent && identical(getOption("show.error.messages"), TRUE)) { cat(msg, file = outFile) .Internal(printDeferredWarnings()) } invisible(structure(msg, class = "try-error", condition = e))})
22: try(system.time(func(), gcFirst = RUnitEnv$.gcBeforeTest))
23: .executeTestCase(funcName, envir = sandbox, setUpFunc = .setUp, tearDownFunc = .tearDown)
24: .sourceTestFile(testFile, testSuite$testFuncRegexp)
25: runTestSuite(testSuite)
An irrecoverable exception occurred. R is aborting now ...
Segmentation fault (core dumped)
* checking for unstated dependencies in vignettes ... OK
* checking package vignettes in ‘inst/doc’ ... OK
* checking re-building of vignette outputs ... [70s/66s] OK
* checking PDF version of manual ... OK
* DONE
Status: 1 ERROR, 3 NOTEs In the log file: Executing test function test.embeddedR ...
> x <- foo()
> x
[1] 42
> x <- foo()
*** caught segfault ***
address 0x55660000000c, cause 'memory not mapped'
Traceback:
1: .External(list(name = "InternalFunction_invoke", address = <pointer: 0x556629108820>, dll = list(name = "Rcpp", path = "/home/edd/git/rcpp/Rcpp.Rcheck/Rcpp/libs/Rcpp.so", dynamicLookup = TRUE, handle = <pointer: 0x556629164270>, info = <pointer: 0x556627cd3930>), numParameters = -1L), <pointer: 0x556627ed7bd0>, ...)
2: foo()
3: eval(ei, envir)
4: eval(ei, envir)
5: withVisible(eval(ei, envir))
6: source(file = srcConn, local = env, echo = TRUE)
7: Rcpp::sourceCpp(file.path(path, "embeddedR2.cpp"), env = newEnv2)
8: eval(expr, envir = parent.frame())
9: doTryCatch(return(expr), name, parentenv, handler)
10: tryCatchOne(expr, names, parentenv, handlers[[1L]])
11: tryCatchList(expr, classes, parentenv, handlers)
12: tryCatch(expr, error = function(e) { call <- conditionCall(e) if (!is.null(call)) { if (identical(call[[1L]], quote(doTryCatch))) call <- sys.call(-4L) dcall <- deparse(call)[1L] prefix <- paste("Error in", dcall, ": ") LONG <- 75L msg <- conditionMessage(e) sm <- strsplit(msg, "\n")[[1L]] w <- 14L + nchar(dcall, type = "w") + nchar(sm[1L], type = "w") if (is.na(w)) w <- 14L + nchar(dcall, type = "b") + nchar(sm[1L], type = "b") if (w > LONG) prefix <- paste0(prefix, "\n ") } else prefix <- "Error : " msg <- paste0(prefix, conditionMessage(e), "\n") .Internal(seterrmessage(msg[1L])) if (!silent && identical(getOption("show.error.messages"), TRUE)) { cat(msg, file = outFile) .Internal(printDeferredWarnings()) } invisible(structure(msg, class = "try-error", condition = e))})
13: try(eval(expr, envir = parent.frame()), silent = silent)
14: inherits(try(eval(expr, envir = parent.frame()), silent = silent), "try-error")
15: checkException(Rcpp::sourceCpp(file.path(path, "embeddedR2.cpp"), env = newEnv2), " not available in other env")
16: func()
17: system.time(func(), gcFirst = RUnitEnv$.gcBeforeTest)
18: doTryCatch(return(expr), name, parentenv, handler)
19: tryCatchOne(expr, names, parentenv, handlers[[1L]])
20: tryCatchList(expr, classes, parentenv, handlers)
21: tryCatch(expr, error = function(e) { call <- conditionCall(e) if (!is.null(call)) { if (identical(call[[1L]], quote(doTryCatch))) call <- sys.call(-4L) dcall <- deparse(call)[1L] prefix <- paste("Error in", dcall, ": ") LONG <- 75L msg <- conditionMessage(e) sm <- strsplit(msg, "\n")[[1L]] w <- 14L + nchar(dcall, type = "w") + nchar(sm[1L], type = "w") if (is.na(w)) w <- 14L + nchar(dcall, type = "b") + nchar(sm[1L], type = "b") if (w > LONG) prefix <- paste0(prefix, "\n ") } else prefix <- "Error : " msg <- paste0(prefix, conditionMessage(e), "\n") .Internal(seterrmessage(msg[1L])) if (!silent && identical(getOption("show.error.messages"), TRUE)) { cat(msg, file = outFile) .Internal(printDeferredWarnings()) } invisible(structure(msg, class = "try-error", condition = e))})
22: try(system.time(func(), gcFirst = RUnitEnv$.gcBeforeTest))
23: .executeTestCase(funcName, envir = sandbox, setUpFunc = .setUp, tearDownFunc = .tearDown)
24: .sourceTestFile(testFile, testSuite$testFuncRegexp)
25: runTestSuite(testSuite)
An irrecoverable exception occurred. R is aborting now ... Any idea? Do you have a similar setup? |
Same result on macOS R 3.5.0:
I also get:
Not sure where that is coming from... |
@eddelbuettel cc @coatless I will look into this asap, hope before the end of today. |
@filipsch Thanks, appreciate it. @lionel- hinted that maybe it is the (larger, overall) test setup (as well as possibly something with your tests, we don't know). He also left one idea there somewhere as I recall (distinct envs to run in). We have had on occassion weird behaviour come from test setups so thst we ended up not running certain tests regularly. It would be a shame. At the end of the day your code is not that large; maybe we find another to trigger it. First check, I guess, would be for you to see if |
For reference, conversation with @lionel- happened here: #807 (comment). I'll try to reproduce and resolve this. |
I cannot reproduce. Trying on MacOS, R 3.5.0, I instead get another error message:
In the log file:
I can try a PR that implements @lionel- 's suggestion of using |
Hm, |
I managed to reproduce the side effects. Switching to |
If you set the
env
variable insourceCpp
,Rcpp
correctly makes the R functions and modules available in the specified environment. However, the R snippet in the/*** R
block is not executed in this same environment. This causes unexpected errors.Reproducible example
test.rcpp
Next, in R:
Expected output:
Actual output:
Context
I work at DataCamp, where we're building a course on Rcpp together with Romain Francois. Our R execution backend heavily relies on the concept of environments: we run the solution code in a solution environment and the student code in a student environment, so that we can check the equality of objects. Because of the above, it is currently very hard for Rcpp exercise to do this. For even more context, you can check out this issue.
Potential solution
I think that passing
local = env
to thesource()
call here fixes this, as done here. Before doing a PR, I wanted to know whether the current behavior is intended and what the authors' take on it is.cc @sumedh10
The text was updated successfully, but these errors were encountered: