[R-pkg-devel] use of assert in C++

[Duncan Murdoch]

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 using r-project.org mailing list
> || https://stat.ethz.ch/mailman/listinfo/r-package-devel
> |
> |--
> |dirk.eddelbuettel.com | @eddelbuettel | edd using debian.org
> |
> 
> 
> ______________________________________________
> R-package-devel using r-project.org mailing list
> https://stat.ethz.ch/mailman/listinfo/r-package-devel