C/C++ 'assert' should not be used in R packages
On 10/11/2007 4:27 PM, Simon Urbanek wrote:
On Nov 10, 2007, at 1:05 PM, Duncan Murdoch wrote:
On 10/11/2007 1:00 PM, Duncan Temple Lang wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Duncan Murdoch wrote:
Prof Brian Ripley wrote:
Please don't use 'assert' in R packages. If called, this means that an error in your code aborts the whole R process, including your user's work. I see several R packages doing this, and one of them called 'assert' on me earlier in the week.
I partly disagree about this. If assert() is triggered, it clearly indicates a bug in the package. If it just generated an R error, most users would ignore it, and not report it to the package maintainer. It may well be that when an assertion fails, none of the subsequent calculations are reliable, in which case returning control to the user could result in data corruption. That's worse than losing a session, because at least when you lose a session, you know it. Could we write our own implementation of assert() that displays an R error and unloads the package? I think I could do something like that in Windows by calling FreeLibrary to unload the DLL, but I'd prefer a cross-platform solution.
I am not sure why you think we need to discard the DLL.
The package author has asserted that it is no longer safe to run. They are shutting down R, and Brian finds that too extreme.
Me too. A package should never intentionally close the R session. If it does, it is a very severe design bug in the package (IMHO). Anything you use assert for can be gracefully handled (as opposed to let's say a segfault), so there is no excuse for killing the whole process at all (deliberately wiping all user's work is rude to put it mildly). Even if it might be unsafe to run anything, I'd still want to attempt to save my data.
Perhaps I use assert() differently than you, but the way I use it is to assert assumptions that I believe will always be true. If one of those assertions fails, it means that my program is operating in a way that I did not foresee. I can't recover gracefully from that, because at that point my basic assumptions about the program have been proven to be incorrect. I have no basis for reasoning. It is by definition a bug. Now, it's possible that this bug has corrupted R and it really would be best for the user to discard all his work, but I agree that's unlikely. That's why I proposed that R should offer a way to handle an assertion failure that limits the shutdown to just my package. I don't offer a guarantee that my work won't corrupt a user's data, but I do work hard to avoid that. Allowing my package to tell the user that it is now unsafe to use is one way to do so. That's what assert() is for. In a standalone program, an (uncaught) assertion failure will cause the RTL to shut down the process. I think R should support a version of that, but limit the shutdown to the package that has failed. The version of assert() that I put together for rgl this morning doesn't attempt to shut down rgl, because that's really something that R should be responsible for. All it does is issue a dire warning to the user. Offline, Brian mentioned that some packages use assert() for input validation. That's an incorrect use of it, and those packages should use error(), as Brian suggested. Duncan Murdoch