[R-pkg-devel] use of assert in C++
On 2024-12-19 5:14 a.m., Bielow, Chris wrote:
|| hoping this is the right place for this: || I stumbled upon documentation regarding the use of `assert` in C++ || code, in particular, || https://cran.r-project.org/web/packages/policies.html states that || || ``` || Thus C/C++ calls to assert/abort/exit/std::terminate, Fortran calls to || STOP and so on must be avoided. || ``` || || , which goes against all best practice guidelines I'm aware of. | |Yes and no. | |Defensize programming is good, of course, but `assert()` leads to an `abort()` and immediate abort which for an _environment_ such as R is really not appropriate. You want a proper error return (i.e. Rf_error with a message or alike) to the R prompt. | |Note that you can always test something like this; there are several packages allowing ad-hoc compilation from `inline` to `Rcpp` and others. So here: | | > Rcpp::cppFunction(r"(void ohnoes() { assert(false); })", includes=c("#undef NDEBUG", "#include <assert.h>")) | > ohnoes() | R: file2cd1353560062b.cpp:8: void ohnoes(): Assertion `false' failed. | | Process R:3 aborted (core dumped) at Wed Dec 18 16:22:19 2024 | |... and that took my R session down just like `assert()` is expected. Also note the trickery I had to do to re-enable `assert()` by undefining `NDEBUG` and including the header after that. Exactly! You had to jump through a lot of hoops to even make assert() crash your R session. This would _not_ happen in production code. So a developer of an R extension could happily use asserts, then use your method to test his/her code. No need to remove the asserts, since NDEBUG is defined in production.
I think it would be really misleading to have code that routinely ignored the assert() calls. In a year would you remember that those asserts were effectively just comments, not being acted on without some trickery to enable them? You'd be much safer if you used a different function specific to R that triggered an R error if the assertion was false. Duncan Murdoch
| |So viewed from that angle, it really is not against _best practices for R extensions_ which is what the CRAN Policy is about. I'm not following this argument, since you (in my eyes) actually made a point for the opposite (see above). Keeping in mind that R is used mostly for scientific purposes I'd favor correctness over inconenviences like a crashing R session (keeping in mind that it would not even crash as per the arguments above). Encouraging asserts() would be a more C++-like experience for the developer, resulting in safer, more robust code, which is a good thing. Just my 50ct. cheers Chris | |Cheers, Dirk | || Therefore I would propose to remove 'assert' from the above statement. || In support, || https://cran.r-project.org/doc/manuals/r-release/R-exts.html || states that || || ``` || One usage that could call abort is the assert macro in C or C++ || functions, which should never be active in production code. The normal || way to ensure that is to define the macro NDEBUG, and R CMD INSTALL || does so as part of the compilation flags. || ``` || which seems a reasonable thing to do. || || As a sidenote, the guide could be extended to encourage new mechanisms || available in modern C++ like `static_assert`. || || || Cheers || Chris || || ______________________________________________ || R-package-devel at r-project.org mailing list || https://stat.ethz.ch/mailman/listinfo/r-package-devel | |-- |dirk.eddelbuettel.com | @eddelbuettel | edd at debian.org |
______________________________________________ R-package-devel at r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-package-devel