-
-
Notifications
You must be signed in to change notification settings - Fork 218
Mask definition of Rf_error to avoid longjmp issues #1402
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
base: master
Are you sure you want to change the base?
Conversation
|
Thanks for putting this together! I can (and will) turn on the reverse-depends machinery but as that does not generally involve |
eddelbuettel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks pretty good. I only have two "style" questions / comments. Maybe @kevinushey can be the referee :)
|
Ok, new solution with a bit of macro trickery. |
|
Wheeee ==:-) Almost worse 😜 That should work. 🤞 |
|
Mmmh, maybe I should put that into an ifdef... |
|
Ok, it should be ready now. |
eddelbuettel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me so far.
|
Thanks, let's wait for another pair of eyes and the revdep machine then. Meanwhile, I'll investigate how many packages need to be adapted to avoid this warning. |
|
Should we include a reference to |
|
Thumbs up on more informative message. 'That file' is technically a policy violation anyway (via a trick borrowed, if memory serves, from another posit package) and not something I get paid to care about anymore. |
|
What about something like... "Use of Rf_error() replaced with Rcpp::stop(). Calls to Rf_error() in C++ contexts are unsafe: consider using Rcpp::stop() instead, or define RCPP_NO_MASK_RF_ERROR if this is a false positive." A bit long, but... |
eddelbuettel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thank you both
|
Ok, some first results are in this log file. It looks worse than it likely is as a) there are the usual 'new' packages for which I have to add their dependencies in a follow-up run and b) the packages already on 'deadline' with known-to-CRAN issues not from us. And then there is the remainder. I will take a closer look at those but it may take me another day or so. I have logs here to follow-up with. |
|
Hm. And I mean Hmm here. An incremental run (after adding all the packages listed as missing, and running against everything not passing in first one, i.e. those who were skipped, who failed, or are new) still has a lot of failures: I looked at two: dqrngRcppSimdJsonResults table log is here. |
|
Mind you maybe it is just like @Enchufa2 hinted: a simple edd@paul:~/git/rcppsimdjson(master)$ tt.r -f inst/tinytest/test_fparse_fload.R
test_fparse_fload.R........... 8 tests OK terminate called after throwing an instance of 'Rcpp::exception'
what(): TAPE_ERROR: The JSON document has an improper structure: missing or superfluous commas, braces, missing keys, etc. This is a fatal and unrecoverable error.
Aborted (core dumped)
edd@paul:~/git/rcppsimdjson(master)$ compAttr.r
edd@paul:~/git/rcppsimdjson(master)$ install.r
* installing *source* package found in current working directory ...
* installing *source* package ‘RcppSimdJson’ ...
** this is package ‘RcppSimdJson’ version ‘0.1.14’
** using staged installation
** libs
using C++ compiler: ‘g++-15 (Ubuntu 15-20250404-0ubuntu1) 15.0.1 20250404 (experimental) [master r15-9193-g08e803aa9be]’
using C++17
ccache g++-15 -std=gnu++17 -I"/usr/share/R/include" -DNDEBUG -I'/usr/local/lib/R/site-library/Rcpp/include' -DSIMDJSON_NO_COMPUTED_GOTO -I../inst/include -fopenmp -fpic -O3 -Wall -pipe -pedantic -Wno-parentheses -Wno-ignored-attributes -Wno-unused-function -c RcppExports.cpp -o RcppExports.o
ccache g++-15 -std=gnu++17 -Wl,-S -shared -L/usr/lib/R/lib -Wl,-Bsymbolic-functions -flto=auto -ffat-lto-objects -Wl,-z,relro -o RcppSimdJson.so RcppExports.o deserialize.o exported-utils.o internal-utils.o rcppsimdjson_utils_check.o simdjson_example.o -fopenmp -L/usr/lib/R/lib -lR
installing to /usr/local/lib/R/site-library/00LOCK-rcppsimdjson/00new/RcppSimdJson/libs
** R
** inst
** byte-compile and prepare package for lazy loading
** help
*** installing help indices
** building package indices
** testing if installed package can be loaded from temporary location
** checking absolute paths in shared objects and dynamic libraries
** testing if installed package can be loaded from final location
** testing if installed package keeps a record of temporary installation path
* DONE (RcppSimdJson)
edd@paul:~/git/rcppsimdjson(master)$ tt.r -f inst/tinytest/test_fparse_fload.R
test_fparse_fload.R........... 2666 tests OK 4.2s
All ok, 2666 results (4.2s)
edd@paul:~/git/rcppsimdjson(master)$ PS And no side effects. After re-installing CRAN Rcpp, and reinstalling RcppSimdJson under it (and with its updated |
9fee4c9 to
a941564
Compare
a941564 to
40f2fca
Compare
|
Rebased, and ran another incremental run. Results summary in this commit: RcppCore/rcpp-logs@ef3a42f containing the summary file. This is a bit more involved than usual:
We probably need a new issue to triage. May revisit tomorrow. |
eddelbuettel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch on the newline
|
Ok -- we are in fact having issues here. The Edit: Oh boy that one was work. I got #include <RcppEigen.h>
using namespace Rcpp;
using namespace RcppEigen;
using namespace Eigen;and re-running See the next paragraph inside the 'details' though as running Edit 2: For completeness the very short diff for This is an updated diff. As @Enchufa2 pointed out we really only need to remove the root@c95b60d71856:/tmp/checks/BayesProject# diff -ru
data/ DESCRIPTION man/ MD5 NAMESPACE R/ src/
root@c95b60d71856:/tmp/checks/BayesProject# cd ..
root@c95b60d71856:/tmp/checks# diff -ru BayesProject.orig/ BayesProject
diff -ru BayesProject.orig/src/bayes.cpp BayesProject/src/bayes.cpp
--- BayesProject.orig/src/bayes.cpp 2020-06-01 00:45:46.000000000 +0000
+++ BayesProject/src/bayes.cpp 2025-11-05 18:58:16.780202588 +0000
@@ -1,4 +1,3 @@
-#include <R.h>
#include <Rcpp.h>
using namespace Rcpp;
#include <RcppEigen.h>
root@c95b60d71856:/tmp/checks# The older / longer diff follows. root@c95b60d71856:/tmp/checks# diff -Nru BayesProject.orig/ BayesProject
diff -Nru BayesProject.orig/inst/include/BayesProject_types.h BayesProject/inst/include/BayesProject_types.h
--- BayesProject.orig/inst/include/BayesProject_types.h 1970-01-01 00:00:00.000000000 +0000
+++ BayesProject/inst/include/BayesProject_types.h 2025-11-05 12:51:23.874653154 +0000
@@ -0,0 +1,6 @@
+
+#include <RcppEigen.h>
+using namespace Rcpp;
+using namespace RcppEigen;
+using namespace Eigen;
+
diff -Nru BayesProject.orig/src/bayes.cpp BayesProject/src/bayes.cpp
--- BayesProject.orig/src/bayes.cpp 2020-06-01 00:45:46.000000000 +0000
+++ BayesProject/src/bayes.cpp 2025-11-05 12:40:49.344415413 +0000
@@ -1,4 +1,4 @@
-#include <R.h>
+/*#include <R.h>*/
#include <Rcpp.h>
using namespace Rcpp;
#include <RcppEigen.h>
diff -Nru BayesProject.orig/src/Makevars BayesProject/src/Makevars
--- BayesProject.orig/src/Makevars 1970-01-01 00:00:00.000000000 +0000
+++ BayesProject/src/Makevars 2025-11-05 12:42:28.491159649 +0000
@@ -0,0 +1 @@
+PKG_CXXFLAGS += -Wno-ignored-attributes
diff -Nru BayesProject.orig/src/RcppExports.cpp BayesProject/src/RcppExports.cpp
--- BayesProject.orig/src/RcppExports.cpp 2020-09-27 00:05:44.000000000 +0000
+++ BayesProject/src/RcppExports.cpp 2025-11-05 12:45:33.389215971 +0000
@@ -1,11 +1,16 @@
// Generated by using Rcpp::compileAttributes() -> do not edit by hand
// Generator token: 10BE3573-1514-4C36-9D1C-5A225CD40393
+#include "../inst/include/BayesProject_types.h"
#include <RcppEigen.h>
#include <Rcpp.h>
using namespace Rcpp;
-using namespace Eigen;
+
+#ifdef RCPP_USE_GLOBAL_ROSTREAM
+Rcpp::Rostream<true>& Rcpp::Rcout = Rcpp::Rcpp_cout_get();
+Rcpp::Rostream<false>& Rcpp::Rcerr = Rcpp::Rcpp_cerr_get();
+#endif
// bayes_vhat
MatrixXd bayes_vhat(MatrixXd x, VectorXd timePoints, double K);
root@c95b60d71856:/tmp/checks#The Edit 3: Similar for root@c95b60d71856:/tmp/checks# diff -Nru GeneralizedWendland.orig/ GeneralizedWendland
diff -Nru GeneralizedWendland.orig/inst/doc/GeneralizedWendland.pdf.asis GeneralizedWendland/inst/doc/GeneralizedWendland.pdf.asis
--- GeneralizedWendland.orig/inst/doc/GeneralizedWendland.pdf.asis 2022-06-16 21:46:11.000000000 +0000
+++ GeneralizedWendland/inst/doc/GeneralizedWendland.pdf.asis 1970-01-01 00:00:00.000000000 +0000
@@ -1,2 +0,0 @@
-%\VignetteIndexEntry{Generalized Wendland function}
-%\VignetteEngine{R.rsp::asis}
\ No newline at end of file
diff -Nru GeneralizedWendland.orig/src/Wendland.h GeneralizedWendland/src/Wendland.h
--- GeneralizedWendland.orig/src/Wendland.h 2025-10-15 19:02:57.000000000 +0000
+++ GeneralizedWendland/src/Wendland.h 2025-11-05 13:14:59.923085742 +0000
@@ -2,7 +2,7 @@
#pragma once
#include <limits>
-#include <R.h>
+//#include <R.h>
#include <Rcpp.h>
#include <RcppEigen.h>
#include <boost/math/special_functions/beta.hpp>
root@c95b60d71856:/tmp/checks#
The vignette business can probably be ignored, I am working in a container without texlive and just skip vignettes. Edit 4: Same thing in A more minimal diff is root@c95b60d71856:/tmp/checks# diff -ru locStra.orig/ locStra
diff -ru locStra.orig/src/auxcode.cpp locStra/src/auxcode.cpp
--- locStra.orig/src/auxcode.cpp 2021-01-27 02:29:32.000000000 +0000
+++ locStra/src/auxcode.cpp 2025-11-05 19:04:15.956493072 +0000
@@ -1,4 +1,3 @@
-#include <R.h>
#include <Rcpp.h>
using namespace Rcpp;
#include <RcppEigen.h>
root@c95b60d71856:/tmp/checks# The initial, longer one follows. root@c95b60d71856:/tmp/checks# diff -Nru locStra.orig/ locStra
diff -Nru locStra.orig/inst/include/locStra_types.h locStra/inst/include/locStra_types.h
--- locStra.orig/inst/include/locStra_types.h 1970-01-01 00:00:00.000000000 +0000
+++ locStra/inst/include/locStra_types.h 2025-11-05 13:45:56.648854713 +0000
@@ -0,0 +1,3 @@
+
+#include <RcppEigen.h>
+using namespace Eigen;
diff -Nru locStra.orig/src/auxcode.cpp locStra/src/auxcode.cpp
--- locStra.orig/src/auxcode.cpp 2021-01-27 02:29:32.000000000 +0000
+++ locStra/src/auxcode.cpp 2025-11-05 13:44:20.429326159 +0000
@@ -1,4 +1,4 @@
-#include <R.h>
+//#include <R.h>
#include <Rcpp.h>
using namespace Rcpp;
#include <RcppEigen.h>
diff -Nru locStra.orig/src/RcppExports.cpp locStra/src/RcppExports.cpp
--- locStra.orig/src/RcppExports.cpp 2021-01-27 02:46:27.000000000 +0000
+++ locStra/src/RcppExports.cpp 2025-11-05 13:43:34.431117382 +0000
@@ -1,11 +1,16 @@
// Generated by using Rcpp::compileAttributes() -> do not edit by hand
// Generator token: 10BE3573-1514-4C36-9D1C-5A225CD40393
+#include "../inst/include/locStra_types.h"
#include <RcppEigen.h>
#include <Rcpp.h>
using namespace Rcpp;
-using namespace Eigen;
+
+#ifdef RCPP_USE_GLOBAL_ROSTREAM
+Rcpp::Rostream<true>& Rcpp::Rcout = Rcpp::Rcpp_cout_get();
+Rcpp::Rostream<false>& Rcpp::Rcerr = Rcpp::Rcpp_cerr_get();
+#endif
// powerMethodCpp
VectorXd powerMethodCpp(MatrixXd& X, VectorXd& v, double eps, int maxiter);
root@c95b60d71856:/tmp/checks# Edit 5: Same for Edit 6: Same for root@c95b60d71856:/tmp/checks# diff -ru TDA.orig/ TDA
Only in TDA.orig/build: vignette.rds
Only in TDA.orig/inst: doc
diff -ru TDA.orig/src/diag.cpp TDA/src/diag.cpp
--- TDA.orig/src/diag.cpp 2024-01-23 16:07:14.000000000 +0000
+++ TDA/src/diag.cpp 2025-11-05 19:16:24.525589991 +0000
@@ -1,6 +1,6 @@
// for R
-#include <R.h>
-#include <R_ext/Print.h>
+//#include <R.h>
+//#include <R_ext/Print.h>
// for Rcpp
#include <Rcpp.h>
root@c95b60d71856:/tmp/checks# Edit 7: Similar for LOMAR root@b7af4e9981a0:/work# diff -ru LOMAR.orig/ LOMAR
diff -ru LOMAR.orig/src/diag.cpp LOMAR/src/diag.cpp
--- LOMAR.orig/src/diag.cpp 2022-10-25 09:44:07.000000000 +0000
+++ LOMAR/src/diag.cpp 2025-11-07 22:49:48.638040407 +0000
@@ -1,6 +1,6 @@
// for R
-#include <R.h>
-#include <R_ext/Print.h>
+//#include <R.h>
+//#include <R_ext/Print.h>
// for Rcpp
#include <Rcpp.h>
root@b7af4e9981a0:/work# |
|
I believe this is a bug in RcppEigen. I see: If you protect that |
Yes/no/maybe/unsure/I don't care: It goes away if you do not include A related issue is that we should probably warn when users include |
|
Ah, ok, I thought it was RcppEigen who was including |
|
I am very confused as to why I now need the It's the same as always: Also, to be more precise: |
Ah, because your are running My point was that the package, as is, can be fixed for this PR just by removing the include: diff --git a/src/bayes.cpp b/src/bayes.cpp
index c856b42..b9e5e65 100644
--- a/src/bayes.cpp
+++ b/src/bayes.cpp
@@ -1,4 +1,3 @@
-#include <R.h>
#include <Rcpp.h>
using namespace Rcpp;
#include <RcppEigen.h>How the client package deals with flatten namespaces or not it's their problem.
I think we should at least warn now and at some point throw an error. |
|
Thumbs up on more minimal diffs. That may affect more than one package. Needing an Agreed on that adding a warning is probably a good. |
|
|
It's been a while. We needed it then for finetune wrappers and forwarding, it's been like this ever since. I think I should add a warning to RcppEigen. Edit Issue opened at its repo. |
|
I was trying to look at the other packages that are neither a) a simple case of
On second thought, maybe these package need the |
|
Sure, I'll take a look tomorrow. |
diff --git a/src/bsplines.cpp b/src/bsplines.cpp
index b523ebf..dc4b2c1 100644
--- a/src/bsplines.cpp
+++ b/src/bsplines.cpp
@@ -23,7 +23,7 @@ Rcpp::NumericMatrix cpp_bsplines(arma::vec x, arma::vec iknots, arma::vec bknots
Rcpp::NumericMatrix cpp_bsplinesD1(arma::vec x, arma::vec iknots, arma::vec bknots, unsigned int order) {
if ((order - 1) <= 0) {
- Rf_error("(order - 1) <= 0");
+ Rcpp::stop("(order - 1) <= 0");
}
bbasis B0(x, iknots, bknots, order);
@@ -74,7 +74,7 @@ Rcpp::NumericMatrix cpp_bsplinesD1(arma::vec x, arma::vec iknots, arma::vec bkno
Rcpp::NumericMatrix cpp_bsplinesD2(arma::vec x, arma::vec iknots, arma::vec bknots, unsigned int order) {
if ((order - 2) <= 0) {
- Rf_error("(order - 2) <= 0");
+ Rcpp::stop("(order - 2) <= 0");
}
bbasis B0(x, iknots, bknots, order);
diff --git a/src/cpr.cpp b/src/cpr.cpp
index 7f0caa9..c7c13e3 100644
--- a/src/cpr.cpp
+++ b/src/cpr.cpp
@@ -32,7 +32,7 @@ bbasis::bbasis(arma::vec& x_, arma::vec & iknots_, arma::vec & bknots_, unsigned
}
if (!xi.is_sorted()) {
- Rf_error("Knots are not sorted.");
+ Rcpp::stop("Knots are not sorted.");
}
// define xi_star
diff --git a/tests/test-bsplineD.R b/tests/test-bsplineD.R
index 83f84ad..5d1204c 100644
--- a/tests/test-bsplineD.R
+++ b/tests/test-bsplineD.R
@@ -75,15 +75,15 @@ with(e, {
stopifnot(isTRUE( all.equal(cprD2[-100, ], baseD2[-100, ])))
x <- tryCatch(bsplineD(xvec, derivative = 1.5), error = function(e) {e})
- stopifnot(identical(class(x), c("simpleError", "error", "condition")))
+ stopifnot(inherits(x, "error"))
stopifnot(identical(x$message, "Only first and second derivatives are supported"))
x <- tryCatch(bsplineD(xvec, derivative = 3), error = function(e) {e})
- stopifnot(identical(class(x), c("simpleError", "error", "condition")))
+ stopifnot(inherits(x, "error"))
stopifnot(identical(x$message, "Only first and second derivatives are supported"))
x <- tryCatch(bsplineD(xvec, order = 2, derivative = 2), error = function(e) {e})
- stopifnot(identical(class(x), c("simpleError", "error", "condition")))
+ stopifnot(inherits(x, "error"))
stopifnot(identical(x$message, "(order - 2) <= 0"))
})
@@ -97,16 +97,16 @@ with(e, {
bmatD1 <- tryCatch(bsplineD(xvec, bknots = bknots, order = 1, derivative = 1L), error = function(e) e)
bmatD2 <- tryCatch(bsplineD(xvec, bknots = bknots, order = 1, derivative = 2L), error = function(e) e)
- stopifnot(inherits(bmat, "simpleError"))
- stopifnot(inherits(bmatD1, "simpleError"))
- stopifnot(inherits(bmatD2, "simpleError"))
+ stopifnot(inherits(bmat, "error"))
+ stopifnot(inherits(bmatD1, "error"))
+ stopifnot(inherits(bmatD2, "error"))
stopifnot(identical(bmat$message, "order needs to be an integer value >= 2."))
stopifnot(identical(bmatD1$message, "order needs to be an integer value >= 2."))
stopifnot(identical(bmatD2$message, "order needs to be an integer value >= 2."))
bmat <- tryCatch(bsplines(xvec, bknots = bknots, order = 1.9), error = function(e) e)
- stopifnot(inherits(bmat, "simpleError"))
+ stopifnot(inherits(bmat, "error"))
stopifnot(identical(bmat$message, "order needs to be an integer value >= 2."))
bmat <- tryCatch(bsplines(xvec, bknots = bknots, order = 2.9), error = function(e) e)
diff --git a/tests/test-bsplines.R b/tests/test-bsplines.R
index b24a993..bb8f2c1 100644
--- a/tests/test-bsplines.R
+++ b/tests/test-bsplines.R
@@ -68,7 +68,7 @@ with(e, {
bknots = c(-1, 1)
x <- tryCatch(bsplines(xvec, iknots = iknots, bknots = bknots), error = function(e) {e})
- stopifnot(inherits(x, "simpleError"))
+ stopifnot(inherits(x, "error"))
stopifnot(identical(x$message, "Knots are not sorted."))
x <- tryCatch(bsplines(xvec, iknots = sort(iknots), bknots = bknots), error = function(e) {e})
@@ -92,7 +92,7 @@ with(e, {
e <- new.env()
with(e, {
x <- tryCatch(bsplines(list(1:10)), error = function(e) { e })
- stopifnot(inherits(x, "simpleError"))
+ stopifnot(inherits(x, "error"))
stopifnot("error if list passed to bsplines" = identical(x$message, "x is a list. Use btensor instead of bsplines."))
})
@@ -104,7 +104,7 @@ with(e, {
bknots <- c(-1, 1)
x <- tryCatch(bsplines(xvec, df = 2, bknots = bknots), warning = function(w) {w})
- stopifnot(identical(class(x), c("simpleWarning", "warning", "condition")))
+ stopifnot(inherits(x, "warning"))
stopifnot(identical(x$message, "df being set to order"))
x <- suppressWarnings(bsplines(xvec, df = 2, bknots = bknots))
@@ -118,7 +118,7 @@ with(e, {
stopifnot(isTRUE(all.equal(x, z, check.attributes = FALSE)))
x <- tryCatch(bsplines(xvec, iknots = 0.2, df = 6, bknots = bknots), warning = function(w) {w})
- stopifnot(identical(class(x), c("simpleWarning", "warning", "condition")))
+ stopifnot(inherits(x, "warning"))
stopifnot(identical(x$message, "Both iknots and df defined, using iknots"))
x <- suppressWarnings(bsplines(xvec, iknots = 0.2, df = 6, bknots = bknots))
@@ -210,16 +210,16 @@ with(e, {
stopifnot(isTRUE( all.equal(cprD2[-100, ], baseD2[-100, ])))
x <- tryCatch(bsplineD(xvec, derivative = 1.5), error = function(e) {e})
- stopifnot(identical(class(x), c("simpleError", "error", "condition")))
+ stopifnot(inherits(x, "error"))
stopifnot(identical(x$message, "Only first and second derivatives are supported"))
rm(x)
x <- tryCatch(bsplineD(xvec, derivative = 3), error = function(e) {e})
- stopifnot(identical(class(x), c("simpleError", "error", "condition")))
+ stopifnot(inherits(x, "error"))
stopifnot(identical(x$message, "Only first and second derivatives are supported"))
x <- tryCatch(bsplineD(xvec, order = 2, derivative = 2), error = function(e) {e})
- stopifnot(identical(class(x), c("simpleError", "error", "condition")))
+ stopifnot(inherits(x, "error"))
stopifnot(identical(x$message, "(order - 2) <= 0"))
})
@@ -233,16 +233,16 @@ with(e, {
bmatD1 <- tryCatch(bsplineD(xvec, bknots = bknots, order = 1, derivative = 1L), error = function(e) e)
bmatD2 <- tryCatch(bsplineD(xvec, bknots = bknots, order = 1, derivative = 2L), error = function(e) e)
- stopifnot(inherits(bmat, "simpleError"))
- stopifnot(inherits(bmatD1, "simpleError"))
- stopifnot(inherits(bmatD2, "simpleError"))
+ stopifnot(inherits(bmat, "error"))
+ stopifnot(inherits(bmatD1, "error"))
+ stopifnot(inherits(bmatD2, "error"))
stopifnot(identical(bmat$message, "order needs to be an integer value >= 2."))
stopifnot(identical(bmatD1$message, "order needs to be an integer value >= 2."))
stopifnot(identical(bmatD2$message, "order needs to be an integer value >= 2."))
bmat <- tryCatch(bsplines(xvec, bknots = bknots, order = 1.9), error = function(e) e)
- stopifnot(inherits(bmat, "simpleError"))
+ stopifnot(inherits(bmat, "error"))
stopifnot(identical(bmat$message, "order needs to be an integer value >= 2."))
bmat <- tryCatch(bsplines(xvec, bknots = bknots, order = 2.9), error = function(e) e)
diff --git a/tests/test-order_statistics.R b/tests/test-order_statistics.R
index ecd9c3f..58b4e84 100644
--- a/tests/test-order_statistics.R
+++ b/tests/test-order_statistics.R
@@ -76,11 +76,11 @@ e <- new.env()
with(e, {
d <- tryCatch(d_order_statistic(x = x, n = 4:6, j = 2, distribution = "norm")
, error = function(e) e)
- stopifnot(inherits(d, "simpleError"))
+ stopifnot(inherits(d, "error"))
stopifnot(d$message == "length(n) == 1 is not TRUE")
p <- tryCatch(p_order_statistic(q = x, n = 4:6, j = 2, distribution = "norm")
, error = function(e) e)
- stopifnot(inherits(p, "simpleError"))
+ stopifnot(inherits(p, "error"))
stopifnot(p$message == "length(n) == 1 is not TRUE")
})
rm(e)
@@ -89,12 +89,12 @@ e <- new.env()
with(e, {
d <- tryCatch(d_order_statistic(x = x, n = numeric(0), j = 2, distribution = "norm")
, error = function(e) e)
- stopifnot(inherits(d, "simpleError"))
+ stopifnot(inherits(d, "error"))
stopifnot(d$message == "length(n) == 1 is not TRUE")
p <- tryCatch(p_order_statistic(q = x, n = numeric(0), j = 2, distribution = "norm")
, error = function(e) e)
- stopifnot(inherits(p, "simpleError"))
+ stopifnot(inherits(p, "error"))
stopifnot(p$message == "length(n) == 1 is not TRUE")
})
rm(e)
@@ -103,12 +103,12 @@ e <- new.env()
with(e, {
d <- tryCatch(d_order_statistic(x = 0, n = NA_real_, j = 2, distribution = "norm")
, error = function(e) e)
- stopifnot(inherits(d, "simpleError"))
+ stopifnot(inherits(d, "error"))
stopifnot(d$message == "!is.na(n) is not TRUE")
p <- tryCatch(p_order_statistic(q = 0, n = NA_real_, j = 2, distribution = "norm")
, error = function(e) e)
- stopifnot(inherits(p, "simpleError"))
+ stopifnot(inherits(p, "error"))
stopifnot(p$message == "!is.na(n) is not TRUE")
})
rm(e)
@@ -117,12 +117,12 @@ e <- new.env()
with(e, {
d <- tryCatch(d_order_statistic(x = 0, n = 10, j = 11, distribution = "norm")
, error = function(e) e)
- stopifnot(inherits(d, "simpleError"))
+ stopifnot(inherits(d, "error"))
stopifnot(d$message == "n >= stats::na.omit(j) is not TRUE")
p <- tryCatch(p_order_statistic(q = 0, n = 10, j = 11, distribution = "norm")
, error = function(e) e)
- stopifnot(inherits(p, "simpleError"))
+ stopifnot(inherits(p, "error"))
stopifnot(p$message == "n >= stats::na.omit(j) is not TRUE")
})
rm(e)
@@ -131,12 +131,12 @@ e <- new.env()
with(e, {
d <- tryCatch(d_order_statistic(x = 0, n = 10, j = -1, distribution = "norm")
, error = function(e) e)
- stopifnot(inherits(d, "simpleError"))
+ stopifnot(inherits(d, "error"))
stopifnot(d$message == "stats::na.omit(j) >= 1 is not TRUE")
p <- tryCatch(p_order_statistic(q = 0, n = 10, j = -1, distribution = "norm")
, error = function(e) e)
- stopifnot(inherits(p, "simpleError"))
+ stopifnot(inherits(p, "error"))
stopifnot(p$message == "stats::na.omit(j) >= 1 is not TRUE")
})
rm(e)
diff --git a/src/vic/vic_run/include/vic_log.h b/src/vic/vic_run/include/vic_log.h
index e64ed44..381c861 100644
--- a/src/vic/vic_run/include/vic_log.h
+++ b/src/vic/vic_run/include/vic_log.h
@@ -66,8 +66,8 @@ void setup_logging(int id, char log_path[], FILE **logfile);
// Print and error functions of R
void Rprintf(const char *, ...);
-void Rf_error(const char *, ...);
-void Rf_warning(const char *, ...);
+void rcpp_error(const char *, ...);
+void rcpp_warning(const char *, ...);
// Debug Level
#if LOG_LVL < 10
@@ -87,14 +87,14 @@ void Rf_warning(const char *, ...);
// Warn Level
#if LOG_LVL < 30
-#define log_warn Rf_warning
+#define log_warn rcpp_warning
#else
#define log_warn(M, ...)
#endif
-#define log_err Rf_error
+#define log_err rcpp_error
-#define check_alloc_status(A, M) if (A == NULL) {Rf_error(M "%s\n", "");}
+#define check_alloc_status(A, M) if (A == NULL) {rcpp_error(M "%s\n", "");}
#endif
diff --git a/src/vic_R.h b/src/vic_R.h
index 00f0236..ff60123 100644
--- a/src/vic_R.h
+++ b/src/vic_R.h
@@ -3,7 +3,6 @@
#include <Rcpp.h>
using namespace Rcpp;
-#undef ERROR /*replace C error with Rcpp*/
extern "C" {
#include <vic_driver_shared_all.h>
diff --git a/src/vic_run_cell.cpp b/src/vic_run_cell.cpp
index a2e4b2d..002c942 100644
--- a/src/vic_run_cell.cpp
+++ b/src/vic_run_cell.cpp
@@ -1,5 +1,22 @@
#include <vic_R.h>
+void rcpp_error(const char* format, ...) {
+ char buf[256];
+ va_list ap;
+ va_start(ap, format);
+ vsnprintf(buf, (size_t) 256, format, ap);
+ va_end(ap);
+ Rcpp::stop("%s", buf);
+}
+void rcpp_warning(const char* format, ...) {
+ char buf[256];
+ va_list ap;
+ va_start(ap, format);
+ vsnprintf(buf, (size_t) 256, format, ap);
+ va_end(ap);
+ Rcpp::warning("%s", buf);
+}
+
// [[Rcpp::export]]
List vic_run_cell(List vic_options, NumericMatrix forcing,
NumericVector soil_par, |
|
Nice work. I clearly wasn' thinking straight in the case of I think I will set up a new issue with a clean table and also calmly sit down with rev.dep machine and try the 'suspected |
Closes #1247. It turns out we are generating a call to
Rf_errorinRcppExports.cppfor C++ interfaces. I think that code path is obsolete, and it should never run with unwind-protect, but I prefer to keep this PR safe and simple. So to be able to undef, callRf_errorthen define again, I put the definition into a separate file without guards, so that it can be included more than once. Please let me know if there's a better way and/or you'd like this definition in another place.Checklist
R CMD checkstill passes all tests