[Rd] C/C++ 'assert' should not be used in R packages

[Duncan Murdoch]

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