[Rcpp-devel] assert() for Rcpp?
Dirk Eddelbuettel and I were discussing how people do not seem to be putting assert() statements in their C code in Rcpp packages. I expect the reason is because assert() prints to cerr, which is a violation of the CRAN policies of proper packages. However, the assert() command is a simple macro, and we could tweak it so it is compliant with CRAN standards. Below, I have an example of what might be included. Dirk suggested that this idea be posted to this list to gather peoples insights and thoughts (and to catch any memory management issues, for example, that might exist with the code below). The traditional assert() is a macro that calls a function __assert() which in turn seems fairly simple, but the C program for Rcpp should not just abort but instead throw an error, I think. (The macro allows for detection of line numbers in the code, and also allows for a NDEBUG flag to omit all asserts for efficient code.) The proposed code: #include #ifdef NDEBUG # define assert(EX) #else # define assert(EX) (void)((EX) || (__assert (#EX, __FILE__, __LINE__),0)) #endif void __assert (const char *msg, const char *file, int line) { char buffer [100]; snprintf( buffer, 100, "Assert Failure: %s at %s line #%d", msg, file, line ); ::Rf_error( buffer ); } Anyway, I would love to hear people¹s thoughts on this. I found assert() useful in wrapping an existing spaghetti code base with an R package; it seems like a nice tool to provide enhanced ability to track down bugs. I am currently using the above in my package Œtextreg¹ and it appears to work great. Sincerely, Luke Miratrix Assistant Professor of Statistics Note: Due to my RSI (wrist trouble), e-mail often abrupt. -- Department of Statistics Science Center Harvard University 1 Oxford Street Cambridge MA 02138-2901 lmirat...@stat.harvard.edu 510-735-7635 ___ Rcpp-devel mailing list Rcpp-devel@lists.r-forge.r-project.org https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel
Re: [Rcpp-devel] assert() for Rcpp?
I agree that having an assert which complies with CRAN standards would be valuable. One piece of immediate feedback on your initial implementation: you can't call Rf_error from C++ code (as it will bypass C++ destructors on the stack). Rather, you should throw Rcpp::exception. Whether assert should throw is another issue entirely. The traditional semantics of assert don't give it control flow behavior (either do nothing in release mode or blow the process up in debug mode) so I'm not sure whether we'd want to introduce a variation of it that does have control flow. A couple of options: (1) Create another function e.g. "verify" with the semantics you suggest (with the different name not fooling people into thinking it has traditional assert semantics) (2) Call it assert but have it call Rcpp::warning rather than yielding an error. On Tue, Feb 17, 2015 at 5:41 PM, Miratrix, Luke wrote: > > Dirk Eddelbuettel and I were discussing how people do not seem to be > putting assert() statements in their C code in Rcpp packages. I expect > the reason is because assert() prints to cerr, which is a violation of the > CRAN policies of proper packages. However, the assert() command is a > simple macro, and we could tweak it so it is compliant with CRAN > standards. Below, I have an example of what might be included. Dirk > suggested that this idea be posted to this list to gather peoples insights > and thoughts (and to catch any memory management issues, for example, that > might exist with the code below). > > The traditional assert() is a macro that calls a function __assert() which > in turn seems fairly simple, but the C program for Rcpp should not just > abort but instead throw an error, I think. (The macro allows for > detection of line numbers in the code, and also allows for a NDEBUG flag > to omit all asserts for efficient code.) > > The proposed code: > > #include > > #ifdef NDEBUG > # define assert(EX) > #else > # define assert(EX) (void)((EX) || (__assert (#EX, __FILE__, __LINE__),0)) > #endif > > void __assert (const char *msg, const char *file, int line) { > char buffer [100]; > snprintf( buffer, 100, "Assert Failure: %s at %s line #%d", msg, file, > line ); > ::Rf_error( buffer ); > } > > > > Anyway, I would love to hear people¹s thoughts on this. I found assert() > useful in wrapping an existing spaghetti code base with an R package; it > seems like a nice tool to provide enhanced ability to track down bugs. I > am currently using the above in my package Œtextreg¹ and it appears to > work great. > > > > Sincerely, > > Luke Miratrix > Assistant Professor of Statistics > > Note: Due to my RSI (wrist trouble), e-mail often abrupt. > > > -- > > Department of Statistics > Science Center > > Harvard University > 1 Oxford Street > Cambridge MA 02138-2901 > > > lmirat...@stat.harvard.edu > 510-735-7635 > > > > > ___ > Rcpp-devel mailing list > Rcpp-devel@lists.r-forge.r-project.org > https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel ___ Rcpp-devel mailing list Rcpp-devel@lists.r-forge.r-project.org https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel
Re: [Rcpp-devel] assert() for Rcpp?
On Tue, 2015-02-17 at 17:53 -0700, JJ Allaire wrote: > One piece of immediate feedback on your initial implementation: you > can't call Rf_error from C++ code (as it will bypass C++ destructors > on the stack). Rather, you should throw Rcpp::exception. Hi JJ: Ok, this puzzles me. We can't call Rf_error from C++ code, but we can call it from within the exception handler, right? It would seem so since Dirk has written a nice gallery post that does it that way... http://gallery.rcpp.org/articles/intro-to-exceptions Just my $0.02, but I would stick to exceptions and avoid assert in C++. Rodney ___ Rcpp-devel mailing list Rcpp-devel@lists.r-forge.r-project.org https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel
Re: [Rcpp-devel] assert() for Rcpp?
We *can* call Rf_error from C++ code, but when we do it bypasses all C++ destructors on the stack, so we don't do it so as not leak memory and otherwise have deterministic behavior around construction/destruction. You'll notice in Dirk's example that there are no C++ objects on the stack when he calls Rf_error. The core idea in Rcpp is to use exceptions to get high enough in the stack that there are no more destructors. On Wed, Feb 18, 2015 at 9:08 AM, Sparapani, Rodney wrote: > On Tue, 2015-02-17 at 17:53 -0700, JJ Allaire wrote: >> One piece of immediate feedback on your initial implementation: you >> can't call Rf_error from C++ code (as it will bypass C++ destructors >> on the stack). Rather, you should throw Rcpp::exception. > > Hi JJ: > > Ok, this puzzles me. We can't call Rf_error from C++ code, but we can > call it from within the exception handler, right? It would seem so > since Dirk has written a nice gallery post that does it that way... > http://gallery.rcpp.org/articles/intro-to-exceptions > > Just my $0.02, but I would stick to exceptions and avoid assert in C++. > > Rodney > > > > ___ > Rcpp-devel mailing list > Rcpp-devel@lists.r-forge.r-project.org > https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel ___ Rcpp-devel mailing list Rcpp-devel@lists.r-forge.r-project.org https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel
Re: [Rcpp-devel] assert() for Rcpp?
Isn't there a potential issue if a package depends on multiple header-only C++ libraries, each of which may include ? You'd have multiple definitions of the macro floating around, and the compiler will barf. --Kevi > On Feb 17, 2015, at 4:41 PM, Miratrix, Luke wrote: > > > Dirk Eddelbuettel and I were discussing how people do not seem to be > putting assert() statements in their C code in Rcpp packages. I expect > the reason is because assert() prints to cerr, which is a violation of the > CRAN policies of proper packages. However, the assert() command is a > simple macro, and we could tweak it so it is compliant with CRAN > standards. Below, I have an example of what might be included. Dirk > suggested that this idea be posted to this list to gather peoples insights > and thoughts (and to catch any memory management issues, for example, that > might exist with the code below). > > The traditional assert() is a macro that calls a function __assert() which > in turn seems fairly simple, but the C program for Rcpp should not just > abort but instead throw an error, I think. (The macro allows for > detection of line numbers in the code, and also allows for a NDEBUG flag > to omit all asserts for efficient code.) > > The proposed code: > > #include > > #ifdef NDEBUG > # define assert(EX) > #else > # define assert(EX) (void)((EX) || (__assert (#EX, __FILE__, __LINE__),0)) > #endif > > void __assert (const char *msg, const char *file, int line) { >char buffer [100]; >snprintf( buffer, 100, "Assert Failure: %s at %s line #%d", msg, file, > line ); >::Rf_error( buffer ); > } > > > > Anyway, I would love to hear people¹s thoughts on this. I found assert() > useful in wrapping an existing spaghetti code base with an R package; it > seems like a nice tool to provide enhanced ability to track down bugs. I > am currently using the above in my package Œtextreg¹ and it appears to > work great. > > > > Sincerely, > > Luke Miratrix > Assistant Professor of Statistics > > Note: Due to my RSI (wrist trouble), e-mail often abrupt. > > > -- > > Department of Statistics > Science Center > > Harvard University > 1 Oxford Street > Cambridge MA 02138-2901 > > > lmirat...@stat.harvard.edu > 510-735-7635 > > > > > ___ > Rcpp-devel mailing list > Rcpp-devel@lists.r-forge.r-project.org > https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel ___ Kevin Thornton Associate Professor Ecology and Evolutionary Biology University of California, Irvine http://www.molpopgen.org http://github.com/molpopgen http://github.com/ThorntonLab ___ Rcpp-devel mailing list Rcpp-devel@lists.r-forge.r-project.org https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel
Re: [Rcpp-devel] assert() for Rcpp?
On 18 February 2015 at 08:34, Kevin Thornton wrote: | Isn't there a potential issue if a package depends on multiple header-only C++ libraries, each of which may include ? You'd have multiple definitions of the macro floating around, and the compiler will barf. Been there, done that -- this whole discussion started because the most recent Rcpp release had accidentally borked Luke's use of assert (as we didn't prevent the newly added and hany 'tinyformat' one-header-library from re-defining assert). (And Kevin also added some other push/pop logic to only define some other macros when we need them and to generally avoid spillage.) So now that this has been taken care of, we are discussing how to provide a more useful feature "not unlike assert" with proper Rcpp semantics. Dirk -- http://dirk.eddelbuettel.com | @eddelbuettel | e...@debian.org ___ Rcpp-devel mailing list Rcpp-devel@lists.r-forge.r-project.org https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel
Re: [Rcpp-devel] assert() for Rcpp?
Hi Dirk, Sorry, but your response is unclear to me. What was "taken care of"? What is 'tinyformat'? Are you trying to say that this method of defining assert will work with any other C++ library that includes ? --Kevin > On Feb 18, 2015, at 8:53 AM, Dirk Eddelbuettel wrote: > > > On 18 February 2015 at 08:34, Kevin Thornton wrote: > | Isn't there a potential issue if a package depends on multiple header-only > C++ libraries, each of which may include ? You'd have multiple > definitions of the macro floating around, and the compiler will barf. > > Been there, done that -- this whole discussion started because the most > recent Rcpp release had accidentally borked Luke's use of assert (as we > didn't prevent the newly added and hany 'tinyformat' one-header-library from > re-defining assert). (And Kevin also added some other push/pop logic to only > define some other macros when we need them and to generally avoid spillage.) > > So now that this has been taken care of, we are discussing how to provide a > more useful feature "not unlike assert" with proper Rcpp semantics. > > Dirk > > -- > http://dirk.eddelbuettel.com | @eddelbuettel | e...@debian.org ___ Kevin Thornton Associate Professor Ecology and Evolutionary Biology University of California, Irvine http://www.molpopgen.org http://github.com/molpopgen http://github.com/ThorntonLab ___ Rcpp-devel mailing list Rcpp-devel@lists.r-forge.r-project.org https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel
Re: [Rcpp-devel] assert() for Rcpp?
Hi Kevin, On 18 February 2015 at 09:02, Kevin Thornton wrote: | Hi Dirk, | | Sorry, but your response is unclear to me. What was "taken care of"? What is 'tinyformat'? Are you trying to say that this method of defining assert will work with any other C++ library that includes ? May I suggest you look at the ChangeLog and NEWS files in the git repo, or your installed version? They should answer both your questions. You are claiming breakage with cassert, while not providing a reproducible example. See an existence proof below -- hope this helps. It works for me at home (Rcpp recent from git) and work (Rcpp current with CRAN). Dirk ## Running example R> library(Rcpp) R> sourceCpp("/tmp/kt.cpp") R> happyNow() [1] TRUE R> ## Code below #include #include // [[Rcpp::export]] bool happyNow() { assert(1 > 0); return true; } /*** R happyNow() */ | | --Kevin | | > On Feb 18, 2015, at 8:53 AM, Dirk Eddelbuettel wrote: | > | > | > On 18 February 2015 at 08:34, Kevin Thornton wrote: | > | Isn't there a potential issue if a package depends on multiple header-only C++ libraries, each of which may include ? You'd have multiple definitions of the macro floating around, and the compiler will barf. | > | > Been there, done that -- this whole discussion started because the most | > recent Rcpp release had accidentally borked Luke's use of assert (as we | > didn't prevent the newly added and hany 'tinyformat' one-header-library from | > re-defining assert). (And Kevin also added some other push/pop logic to only | > define some other macros when we need them and to generally avoid spillage.) | > | > So now that this has been taken care of, we are discussing how to provide a | > more useful feature "not unlike assert" with proper Rcpp semantics. | > | > Dirk | > | > -- | > http://dirk.eddelbuettel.com | @eddelbuettel | e...@debian.org | | ___ | Kevin Thornton | Associate Professor | Ecology and Evolutionary Biology | University of California, Irvine | http://www.molpopgen.org | http://github.com/molpopgen | http://github.com/ThorntonLab | | | | | | -- http://dirk.eddelbuettel.com | @eddelbuettel | e...@debian.org ___ Rcpp-devel mailing list Rcpp-devel@lists.r-forge.r-project.org https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel
Re: [Rcpp-devel] assert() for Rcpp?
Hi Dirk, Thanks. I did check the recent Rcpp announcement, which corresponds to my installed version, and mentions nothing re: assert (https://www.mail-archive.com/rcpp-devel@lists.r-forge.r-project.org/msg07977.html). --Kevin > On Feb 18, 2015, at 9:22 AM, Dirk Eddelbuettel wrote: > > > Hi Kevin, > > On 18 February 2015 at 09:02, Kevin Thornton wrote: > | Hi Dirk, > | > | Sorry, but your response is unclear to me. What was "taken care of"? What > is 'tinyformat'? Are you trying to say that this method of defining assert > will work with any other C++ library that includes ? > > May I suggest you look at the ChangeLog and NEWS files in the git repo, or > your installed version? They should answer both your questions. > > You are claiming breakage with cassert, while not providing a reproducible > example. See an existence proof below -- hope this helps. It works for me at > home (Rcpp recent from git) and work (Rcpp current with CRAN). > > Dirk > > ## Running example > > R> library(Rcpp) > R> sourceCpp("/tmp/kt.cpp") > > R> happyNow() > [1] TRUE > R> > > ## Code below > > > #include > #include > > // [[Rcpp::export]] > bool happyNow() { > assert(1 > 0); > return true; > } > > /*** R > happyNow() > */ > > > > | > | --Kevin > | > | > On Feb 18, 2015, at 8:53 AM, Dirk Eddelbuettel wrote: > | > > | > > | > On 18 February 2015 at 08:34, Kevin Thornton wrote: > | > | Isn't there a potential issue if a package depends on multiple > header-only C++ libraries, each of which may include ? You'd have > multiple definitions of the macro floating around, and the compiler will barf. > | > > | > Been there, done that -- this whole discussion started because the most > | > recent Rcpp release had accidentally borked Luke's use of assert (as we > | > didn't prevent the newly added and hany 'tinyformat' one-header-library > from > | > re-defining assert). (And Kevin also added some other push/pop logic to > only > | > define some other macros when we need them and to generally avoid > spillage.) > | > > | > So now that this has been taken care of, we are discussing how to provide > a > | > more useful feature "not unlike assert" with proper Rcpp semantics. > | > > | > Dirk > | > > | > -- > | > http://dirk.eddelbuettel.com | @eddelbuettel | e...@debian.org > | > | ___ > | Kevin Thornton > | Associate Professor > | Ecology and Evolutionary Biology > | University of California, Irvine > | http://www.molpopgen.org > | http://github.com/molpopgen > | http://github.com/ThorntonLab > | > | > | > | > | > | > > -- > http://dirk.eddelbuettel.com | @eddelbuettel | e...@debian.org ___ Kevin Thornton Associate Professor Ecology and Evolutionary Biology University of California, Irvine http://www.molpopgen.org http://github.com/molpopgen http://github.com/ThorntonLab ___ Rcpp-devel mailing list Rcpp-devel@lists.r-forge.r-project.org https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel
Re: [Rcpp-devel] assert() for Rcpp?
On 18 February 2015 at 11:22, Dirk Eddelbuettel wrote: | | Hi Kevin, | | On 18 February 2015 at 09:02, Kevin Thornton wrote: | | Hi Dirk, | | | | Sorry, but your response is unclear to me. What was "taken care of"? What is 'tinyformat'? Are you trying to say that this method of defining assert will work with any other C++ library that includes ? | | May I suggest you look at the ChangeLog and NEWS files in the git repo, or | your installed version? They should answer both your questions. | | You are claiming breakage with cassert, while not providing a reproducible | example. See an existence proof below -- hope this helps. It works for me at | home (Rcpp recent from git) and work (Rcpp current with CRAN). | | Dirk | | ## Running example | | R> library(Rcpp) | R> sourceCpp("/tmp/kt.cpp") | | R> happyNow() | [1] TRUE | R> | | ## Code below | And you also need #undef NDEBUG here Dirk | #include | #include | | // [[Rcpp::export]] | bool happyNow() { | assert(1 > 0); | return true; | } | | /*** R | happyNow() | */ | | | | | | | --Kevin | | | | > On Feb 18, 2015, at 8:53 AM, Dirk Eddelbuettel wrote: | | > | | > | | > On 18 February 2015 at 08:34, Kevin Thornton wrote: | | > | Isn't there a potential issue if a package depends on multiple header-only C++ libraries, each of which may include ? You'd have multiple definitions of the macro floating around, and the compiler will barf. | | > | | > Been there, done that -- this whole discussion started because the most | | > recent Rcpp release had accidentally borked Luke's use of assert (as we | | > didn't prevent the newly added and hany 'tinyformat' one-header-library from | | > re-defining assert). (And Kevin also added some other push/pop logic to only | | > define some other macros when we need them and to generally avoid spillage.) | | > | | > So now that this has been taken care of, we are discussing how to provide a | | > more useful feature "not unlike assert" with proper Rcpp semantics. | | > | | > Dirk | | > | | > -- | | > http://dirk.eddelbuettel.com | @eddelbuettel | e...@debian.org | | | | ___ | | Kevin Thornton | | Associate Professor | | Ecology and Evolutionary Biology | | University of California, Irvine | | http://www.molpopgen.org | | http://github.com/molpopgen | | http://github.com/ThorntonLab | | | | | | | | | | | | | | -- | http://dirk.eddelbuettel.com | @eddelbuettel | e...@debian.org -- http://dirk.eddelbuettel.com | @eddelbuettel | e...@debian.org ___ Rcpp-devel mailing list Rcpp-devel@lists.r-forge.r-project.org https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel
Re: [Rcpp-devel] assert() for Rcpp?
On Wed, Feb 18, 2015 at 10:53 AM, Dirk Eddelbuettel wrote: > > So now that this has been taken care of, we are discussing how to provide a > more useful feature "not unlike assert" with proper Rcpp semantics. > > Something like: #define stopifnot(a) if (!(a)) stop(#a) ? THK -- http://www.keittlab.org/ ___ Rcpp-devel mailing list Rcpp-devel@lists.r-forge.r-project.org https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel
Re: [Rcpp-devel] assert() for Rcpp?
On 18 February 2015 at 09:31, Kevin Thornton wrote: | Hi Dirk, | | Thanks. I did check the recent Rcpp announcement, which corresponds to my installed version, and mentions nothing re: assert (https://www.mail-archive.com/rcpp-devel@lists.r-forge.r-project.org/msg07977.html). See eg https://github.com/RcppCore/Rcpp/issues/237 https://github.com/RcppCore/Rcpp/pull/238 Dirk -- http://dirk.eddelbuettel.com | @eddelbuettel | e...@debian.org ___ Rcpp-devel mailing list Rcpp-devel@lists.r-forge.r-project.org https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel
Re: [Rcpp-devel] assert() for Rcpp?
Thanks--this helps. I have a project based on Rcpp and another header-only library where this could have been an issue. > On Feb 18, 2015, at 10:53 AM, Dirk Eddelbuettel wrote: > > > On 18 February 2015 at 09:31, Kevin Thornton wrote: > | Hi Dirk, > | > | Thanks. I did check the recent Rcpp announcement, which corresponds to my > installed version, and mentions nothing re: assert > (https://www.mail-archive.com/rcpp-devel@lists.r-forge.r-project.org/msg07977.html). > > See eg > >https://github.com/RcppCore/Rcpp/issues/237 >https://github.com/RcppCore/Rcpp/pull/238 > > Dirk > > -- > http://dirk.eddelbuettel.com | @eddelbuettel | e...@debian.org ___ Kevin Thornton Associate Professor Ecology and Evolutionary Biology University of California, Irvine http://www.molpopgen.org http://github.com/molpopgen http://github.com/ThorntonLab ___ Rcpp-devel mailing list Rcpp-devel@lists.r-forge.r-project.org https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel
Re: [Rcpp-devel] assert() for Rcpp?
On Tue, Feb 17, 2015 at 4:41 PM, Miratrix, Luke wrote: > The proposed code: > > #include > > #ifdef NDEBUG > # define assert(EX) > #else > # define assert(EX) (void)((EX) || (__assert (#EX, __FILE__, __LINE__),0)) > #endif > > void __assert (const char *msg, const char *file, int line) { > char buffer [100]; > snprintf( buffer, 100, "Assert Failure: %s at %s line #%d", msg, file, > line ); > ::Rf_error( buffer ); > } Getting more people using assert-like macros seems like a great idea. Weighing in as a C programmer with limited knowledge of Rcpp, I'd suggest: 1) Don't redefine assert() or __assert(). You'll confuse people and it will somehow manage to break things. Instead, define your own macro with a different name, likely one in all caps that starts with "R_" or "RCPP_". 2) Only keep the name 'assert' in the macro if you are keeping the semantics of assert(), that is, it dies on failure if NDEBUG not defined. If it prints a warning and recovers, or defaults to off, use a different word. I personally don't like the inverted NDEBUG approach, so would suggest a different semantics and different word. 3) It's debatable if you want to allow it to be turned off or not. If it can be turned off, people will misuse it to guard against errors and be surprised when it doesn't. I often use a noisy warning that only runs when DEBUG is positively defined (DEBUG_WARN_IF) and an abort that cannot be turned off (ERROR_ABORT_IF). 4) Don't try to snprintf() to a local buffer and then return. I don't know C++ semantics for exceptions, but at least in C, combining local stack variables with stack unwinding is asking for trouble. If you are aborting, print to STDERR directly. If you are continuing, use a normal heap allocation. Nathan Kurz n...@verse.com ___ Rcpp-devel mailing list Rcpp-devel@lists.r-forge.r-project.org https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel
Re: [Rcpp-devel] assert() for Rcpp?
I haven't been very active lately with Rcpp, but I like Nathan's advice below. Don't redefine assert(), I think that's not friendly to inexperienced people and will just generate more questions. Dale Smith, Ph.D. | Data Scientist | nexidia | office: +1 404 495 7220 ext 4008 | fax: +1 404 495 7221| nexidia.com -Original Message- From: rcpp-devel-boun...@lists.r-forge.r-project.org [mailto:rcpp-devel-boun...@lists.r-forge.r-project.org] On Behalf Of Nathan Kurz Sent: Wednesday, February 18, 2015 3:19 PM To: Miratrix, Luke Cc: rcpp-devel@lists.r-forge.r-project.org Subject: Re: [Rcpp-devel] assert() for Rcpp? On Tue, Feb 17, 2015 at 4:41 PM, Miratrix, Luke wrote: > The proposed code: > > #include > > #ifdef NDEBUG > # define assert(EX) > #else > # define assert(EX) (void)((EX) || (__assert (#EX, __FILE__, > __LINE__),0)) #endif > > void __assert (const char *msg, const char *file, int line) { > char buffer [100]; > snprintf( buffer, 100, "Assert Failure: %s at %s line #%d", msg, > file, line ); > ::Rf_error( buffer ); > } Getting more people using assert-like macros seems like a great idea. Weighing in as a C programmer with limited knowledge of Rcpp, I'd suggest: 1) Don't redefine assert() or __assert(). You'll confuse people and it will somehow manage to break things. Instead, define your own macro with a different name, likely one in all caps that starts with "R_" or "RCPP_". 2) Only keep the name 'assert' in the macro if you are keeping the semantics of assert(), that is, it dies on failure if NDEBUG not defined. If it prints a warning and recovers, or defaults to off, use a different word. I personally don't like the inverted NDEBUG approach, so would suggest a different semantics and different word. 3) It's debatable if you want to allow it to be turned off or not. If it can be turned off, people will misuse it to guard against errors and be surprised when it doesn't. I often use a noisy warning that only runs when DEBUG is positively defined (DEBUG_WARN_IF) and an abort that cannot be turned off (ERROR_ABORT_IF). 4) Don't try to snprintf() to a local buffer and then return. I don't know C++ semantics for exceptions, but at least in C, combining local stack variables with stack unwinding is asking for trouble. If you are aborting, print to STDERR directly. If you are continuing, use a normal heap allocation. Nathan Kurz n...@verse.com ___ Rcpp-devel mailing list Rcpp-devel@lists.r-forge.r-project.org https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel ___ Rcpp-devel mailing list Rcpp-devel@lists.r-forge.r-project.org https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel
Re: [Rcpp-devel] assert() for Rcpp?
Envoyé de mon iPhone > Le 18 févr. 2015 à 17:31, JJ Allaire a écrit : > > We *can* call Rf_error from C++ code, but when we do it bypasses all > C++ destructors on the stack, so we don't do it so as not leak memory > and otherwise have deterministic behavior around > construction/destruction. > > You'll notice in Dirk's example that there are no C++ objects on the > stack when he calls Rf_error. This would still bypass the dror for the RNGscope object automatically added by Rcpp::export right ? > The core idea in Rcpp is to use > exceptions to get high enough in the stack that there are no more > destructors. > >> On Wed, Feb 18, 2015 at 9:08 AM, Sparapani, Rodney wrote: >>> On Tue, 2015-02-17 at 17:53 -0700, JJ Allaire wrote: >>> One piece of immediate feedback on your initial implementation: you >>> can't call Rf_error from C++ code (as it will bypass C++ destructors >>> on the stack). Rather, you should throw Rcpp::exception. >> >> Hi JJ: >> >> Ok, this puzzles me. We can't call Rf_error from C++ code, but we can >> call it from within the exception handler, right? It would seem so >> since Dirk has written a nice gallery post that does it that way... >> http://gallery.rcpp.org/articles/intro-to-exceptions >> >> Just my $0.02, but I would stick to exceptions and avoid assert in C++. >> >> Rodney >> >> >> >> ___ >> Rcpp-devel mailing list >> Rcpp-devel@lists.r-forge.r-project.org >> https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel > ___ > Rcpp-devel mailing list > Rcpp-devel@lists.r-forge.r-project.org > https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel ___ Rcpp-devel mailing list Rcpp-devel@lists.r-forge.r-project.org https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel
Re: [Rcpp-devel] assert() for Rcpp?
> > This would still bypass the dror for the RNGscope object automatically added > by Rcpp::export right ? > Indeed it would. I think Dirk's example is really just an illustration of how we handle exceptions internally via the BEGIN/END macros and not something that user code should ever emulate (unless it's outside of Rcpp::export and doesn't use the BEGIN/END macros. ___ Rcpp-devel mailing list Rcpp-devel@lists.r-forge.r-project.org https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel
Re: [Rcpp-devel] assert() for Rcpp?
As an inexperienced person myself, I was trying to wrap a C++ stand-alone package so it could be called from R and was trying to preserve some of the safety features and error-checking. My understanding of asserts is they are to catch disasters that indicate bugs in the code itself, and are thus distinct from normal exceptions. They therefore print out the failed check and a line-number in the source file and bail. When using Rcpp, I came up against CRAN¹s admonition to not print to stderr and so couldn¹t use assert() and stay ³legal.² But I found this out only after annoying people by not doing what I was supposed to do. Given that, having assert() redefined so it is compliant with CRAN and plays nice with R, but also maintains its behavior would be pretty cool, I think. All this being said, Nathan Kurz¹s comments also seem good except I think it is ³illegal² to print to STDERR directly before aborting, since it is only aborting the C++ part, and not the entire R session. Hence my hack of printing to a string buffer and then handing it to the Rf_error call. I don¹t know the guts of Rf_error, but I had assumed it would copy the string to its own world before unwinding the stack. If it doesn¹t then I agree this is going to cause problems. What is the correct way to pass a message up, then? (Sorry for delay in posting this: the rcpp devel list does not get along with my mail client.) Sincerely, Luke Miratrix Assistant Professor of Statistics Note: Due to my RSI (wrist trouble), e-mail often abrupt. -- Department of Statistics Science Center Harvard University 1 Oxford Street Cambridge MA 02138-2901 lmirat...@stat.harvard.edu 510-735-7635 On 2/18/15, 3:25 PM, "Dale Smith" wrote: >I haven't been very active lately with Rcpp, but I like Nathan's advice >below. Don't redefine assert(), I think that's not friendly to >inexperienced people and will just generate more questions. > >Dale Smith, Ph.D. | Data Scientist | nexidia | office: +1 404 495 7220 >ext 4008 | fax: +1 404 495 7221| nexidia.com > >-Original Message- >From: rcpp-devel-boun...@lists.r-forge.r-project.org >[mailto:rcpp-devel-boun...@lists.r-forge.r-project.org] On Behalf Of >Nathan Kurz >Sent: Wednesday, February 18, 2015 3:19 PM >To: Miratrix, Luke >Cc: rcpp-devel@lists.r-forge.r-project.org >Subject: Re: [Rcpp-devel] assert() for Rcpp? > >On Tue, Feb 17, 2015 at 4:41 PM, Miratrix, Luke > wrote: >> The proposed code: >> >> #include >> >> #ifdef NDEBUG >> # define assert(EX) >> #else >> # define assert(EX) (void)((EX) || (__assert (#EX, __FILE__, >> __LINE__),0)) #endif >> >> void __assert (const char *msg, const char *file, int line) { >> char buffer [100]; >> snprintf( buffer, 100, "Assert Failure: %s at %s line #%d", msg, >> file, line ); >> ::Rf_error( buffer ); >> } > >Getting more people using assert-like macros seems like a great idea. >Weighing in as a C programmer with limited knowledge of Rcpp, I'd >suggest: > >1) Don't redefine assert() or __assert(). You'll confuse people and it >will somehow manage to break things. Instead, define your own macro with >a different name, likely one in all caps that starts with "R_" or "RCPP_". > >2) Only keep the name 'assert' in the macro if you are keeping the >semantics of assert(), that is, it dies on failure if NDEBUG not defined. > If it prints a warning and recovers, or defaults to off, use >a different word. I personally don't like the inverted NDEBUG >approach, so would suggest a different semantics and different word. > >3) It's debatable if you want to allow it to be turned off or not. >If it can be turned off, people will misuse it to guard against errors >and be surprised when it doesn't. I often use a noisy warning that >only runs when DEBUG is positively defined (DEBUG_WARN_IF) and an abort >that cannot be turned off (ERROR_ABORT_IF). > >4) Don't try to snprintf() to a local buffer and then return. I >don't know C++ semantics for exceptions, but at least in C, combining >local stack variables with stack unwinding is asking for trouble. If you >are aborting, print to STDERR directly. If you are continuing, use a >normal heap allocation. > >Nathan Kurz >n...@verse.com >___ >Rcpp-devel mailing list >Rcpp-devel@lists.r-forge.r-project.org >https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel ___ Rcpp-devel mailing list Rcpp-devel@lists.r-forge.r-project.org https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel
Re: [Rcpp-devel] assert() for Rcpp?
On Tue, Feb 24, 2015 at 9:45 AM, Miratrix, Luke wrote: > > As an inexperienced person myself, I was trying to wrap a C++ stand-alone > package so it could be called from R and was trying to preserve some of > the safety features and error-checking. My understanding of asserts is > they are to catch disasters that indicate bugs in the code itself, and are > thus distinct from normal exceptions. They therefore print out the failed > check and a line-number in the source file and bail. > > When using Rcpp, I came up against CRAN¹s admonition to not print to > stderr and so couldn¹t use assert() and stay ³legal.² But I found this > out only after annoying people by not doing what I was supposed to do. > Given that, having assert() redefined so it is compliant with CRAN and > plays nice with R, but also maintains its behavior would be pretty cool, I > think. > > All this being said, Nathan Kurz¹s comments also seem good except I think > it is ³illegal² to print to STDERR directly before aborting, since it is > only aborting the C++ part, and not the entire R session. Hence my hack > of printing to a string buffer and then handing it to the Rf_error call. > I don¹t know the guts of Rf_error, but I had assumed it would copy the > string to its own world before unwinding the stack. If it doesn¹t then I > agree this is going to cause problems. What is the correct way to pass a > message up, then? > If I recall, the other prohibition of CRAN is to never ever call abort (R owns the event loop) so assert-like behavior is not gonna fly. THK > > > (Sorry for delay in posting this: the rcpp devel list does not get along > with my mail client.) > > > > Sincerely, > > Luke Miratrix > Assistant Professor of Statistics > > Note: Due to my RSI (wrist trouble), e-mail often abrupt. > > > -- > > Department of Statistics > Science Center > > Harvard University > 1 Oxford Street > Cambridge MA 02138-2901 > > > lmirat...@stat.harvard.edu > 510-735-7635 > > > > > > > On 2/18/15, 3:25 PM, "Dale Smith" wrote: > > >I haven't been very active lately with Rcpp, but I like Nathan's advice > >below. Don't redefine assert(), I think that's not friendly to > >inexperienced people and will just generate more questions. > > > >Dale Smith, Ph.D. | Data Scientist | nexidia | office: +1 404 495 7220 > >ext 4008 | fax: +1 404 495 7221| nexidia.com > > > >-Original Message- > >From: rcpp-devel-boun...@lists.r-forge.r-project.org > >[mailto:rcpp-devel-boun...@lists.r-forge.r-project.org] On Behalf Of > >Nathan Kurz > >Sent: Wednesday, February 18, 2015 3:19 PM > >To: Miratrix, Luke > >Cc: rcpp-devel@lists.r-forge.r-project.org > >Subject: Re: [Rcpp-devel] assert() for Rcpp? > > > >On Tue, Feb 17, 2015 at 4:41 PM, Miratrix, Luke > > wrote: > >> The proposed code: > >> > >> #include > >> > >> #ifdef NDEBUG > >> # define assert(EX) > >> #else > >> # define assert(EX) (void)((EX) || (__assert (#EX, __FILE__, > >> __LINE__),0)) #endif > >> > >> void __assert (const char *msg, const char *file, int line) { > >> char buffer [100]; > >> snprintf( buffer, 100, "Assert Failure: %s at %s line #%d", msg, > >> file, line ); > >> ::Rf_error( buffer ); > >> } > > > >Getting more people using assert-like macros seems like a great idea. > >Weighing in as a C programmer with limited knowledge of Rcpp, I'd > >suggest: > > > >1) Don't redefine assert() or __assert(). You'll confuse people and it > >will somehow manage to break things. Instead, define your own macro with > >a different name, likely one in all caps that starts with "R_" or "RCPP_". > > > >2) Only keep the name 'assert' in the macro if you are keeping the > >semantics of assert(), that is, it dies on failure if NDEBUG not defined. > > If it prints a warning and recovers, or defaults to off, use > >a different word. I personally don't like the inverted NDEBUG > >approach, so would suggest a different semantics and different word. > > > >3) It's debatable if you want to allow it to be turned off or not. > >If it can be turned off, people will misuse it to guard against errors > >and be surprised when it doesn't. I often use a noisy warning that > >only runs when DEBUG is positively defined (DEBUG_WARN_IF) and an abort > >that cannot be turned off (ERROR_ABORT_IF). > > > >4) Don't try to snprintf()
Re: [Rcpp-devel] assert() for Rcpp?
On Tue, Feb 24, 2015 at 10:59 AM, Tim Keitt wrote: > > > On Tue, Feb 24, 2015 at 9:45 AM, Miratrix, Luke > wrote: > >> >> As an inexperienced person myself, I was trying to wrap a C++ stand-alone >> package so it could be called from R and was trying to preserve some of >> the safety features and error-checking. My understanding of asserts is >> they are to catch disasters that indicate bugs in the code itself, and are >> thus distinct from normal exceptions. They therefore print out the failed >> check and a line-number in the source file and bail. >> >> When using Rcpp, I came up against CRAN¹s admonition to not print to >> stderr and so couldn¹t use assert() and stay ³legal.² But I found this >> out only after annoying people by not doing what I was supposed to do. >> Given that, having assert() redefined so it is compliant with CRAN and >> plays nice with R, but also maintains its behavior would be pretty cool, I >> think. >> >> All this being said, Nathan Kurz¹s comments also seem good except I think >> it is ³illegal² to print to STDERR directly before aborting, since it is >> only aborting the C++ part, and not the entire R session. Hence my hack >> of printing to a string buffer and then handing it to the Rf_error call. >> I don¹t know the guts of Rf_error, but I had assumed it would copy the >> string to its own world before unwinding the stack. If it doesn¹t then I >> agree this is going to cause problems. What is the correct way to pass a >> message up, then? >> > > If I recall, the other prohibition of CRAN is to never ever call abort (R > owns the event loop) so assert-like behavior is not gonna fly. > Double negative, but you get the idea. > > THK > > >> >> >> (Sorry for delay in posting this: the rcpp devel list does not get along >> with my mail client.) >> >> >> >> Sincerely, >> >> Luke Miratrix >> Assistant Professor of Statistics >> >> Note: Due to my RSI (wrist trouble), e-mail often abrupt. >> >> >> -- >> >> Department of Statistics >> Science Center >> >> Harvard University >> 1 Oxford Street >> Cambridge MA 02138-2901 >> >> >> lmirat...@stat.harvard.edu >> 510-735-7635 >> >> >> >> >> >> >> On 2/18/15, 3:25 PM, "Dale Smith" wrote: >> >> >I haven't been very active lately with Rcpp, but I like Nathan's advice >> >below. Don't redefine assert(), I think that's not friendly to >> >inexperienced people and will just generate more questions. >> > >> >Dale Smith, Ph.D. | Data Scientist | nexidia | office: +1 404 495 7220 >> >ext 4008 | fax: +1 404 495 7221| nexidia.com >> > >> >-Original Message- >> >From: rcpp-devel-boun...@lists.r-forge.r-project.org >> >[mailto:rcpp-devel-boun...@lists.r-forge.r-project.org] On Behalf Of >> >Nathan Kurz >> >Sent: Wednesday, February 18, 2015 3:19 PM >> >To: Miratrix, Luke >> >Cc: rcpp-devel@lists.r-forge.r-project.org >> >Subject: Re: [Rcpp-devel] assert() for Rcpp? >> > >> >On Tue, Feb 17, 2015 at 4:41 PM, Miratrix, Luke >> > wrote: >> >> The proposed code: >> >> >> >> #include >> >> >> >> #ifdef NDEBUG >> >> # define assert(EX) >> >> #else >> >> # define assert(EX) (void)((EX) || (__assert (#EX, __FILE__, >> >> __LINE__),0)) #endif >> >> >> >> void __assert (const char *msg, const char *file, int line) { >> >> char buffer [100]; >> >> snprintf( buffer, 100, "Assert Failure: %s at %s line #%d", msg, >> >> file, line ); >> >> ::Rf_error( buffer ); >> >> } >> > >> >Getting more people using assert-like macros seems like a great idea. >> >Weighing in as a C programmer with limited knowledge of Rcpp, I'd >> >suggest: >> > >> >1) Don't redefine assert() or __assert(). You'll confuse people and it >> >will somehow manage to break things. Instead, define your own macro with >> >a different name, likely one in all caps that starts with "R_" or >> "RCPP_". >> > >> >2) Only keep the name 'assert' in the macro if you are keeping the >> >semantics of assert(), that is, it dies on failure if NDEBUG not defined. >> > If it prints a warning and recovers, or defaults to off, use >> >a
Re: [Rcpp-devel] assert() for Rcpp?
On 24 February 2015 at 15:45, Miratrix, Luke wrote: | As an inexperienced person myself, I was trying to wrap a C++ stand-alone | package so it could be called from R and was trying to preserve some of | the safety features and error-checking. My understanding of asserts is | they are to catch disasters that indicate bugs in the code itself, and are | thus distinct from normal exceptions. They therefore print out the failed | check and a line-number in the source file and bail. | | When using Rcpp, I came up against CRAN¹s admonition to not print to | stderr and so couldn¹t use assert() and stay ³legal.² But I found this | out only after annoying people by not doing what I was supposed to do. | Given that, having assert() redefined so it is compliant with CRAN and | plays nice with R, but also maintains its behavior would be pretty cool, I | think. | | All this being said, Nathan Kurz¹s comments also seem good except I think | it is ³illegal² to print to STDERR directly before aborting, since it is You are allowed to use Rcpp::Cerr which redirects into R's error stream. | only aborting the C++ part, and not the entire R session. Hence my hack | of printing to a string buffer and then handing it to the Rf_error call. | I don¹t know the guts of Rf_error, but I had assumed it would copy the | string to its own world before unwinding the stack. If it doesn¹t then I | agree this is going to cause problems. What is the correct way to pass a | message up, then? You are close. We are doing similar things with exceptions we catch; we create a string and pass it to Rf_errror. Dirk -- http://dirk.eddelbuettel.com | @eddelbuettel | e...@debian.org ___ Rcpp-devel mailing list Rcpp-devel@lists.r-forge.r-project.org https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel