From murdoch.duncan at gmail.com  Wed Nov 25 00:58:45 2015
From: murdoch.duncan at gmail.com (Duncan Murdoch)
Date: Tue, 24 Nov 2015 18:58:45 -0500
Subject: [Rd] Custom C finalizers for .Call
In-Reply-To: <20151124232023.GF64589@piskorski.com>
References: <CABFfbXtMz3s5MeQ-5=iaJA=5Kr_dhk9RLB82qeXRz1oa4qOpsw@mail.gmail.com>
	<20151124232023.GF64589@piskorski.com>
Message-ID: <5654F9B5.2080407@gmail.com>

On 24/11/2015 6:20 PM, Andrew Piskorski wrote:
> On Tue, Nov 24, 2015 at 12:10:12AM +0100, Jeroen Ooms wrote:
>
>> Currently it is all to easy for package authors to introduce a memory
>> leak or stack imbalance by calling Rf_error() or
>> R_CheckUserInterrupt() in a way that skips over the usual cleanup
>> steps.
>
> I have a more modest request:  Please improve the documentation of
> exactly what Rf_error() does and how it should be used!  E.g., does
> calling it terminate execution of the C function from which it is
> called, or not?  The docs below don't actually say:
>
>    https://cran.r-project.org/doc/manuals/r-devel/R-exts.html#Error-handling
>    https://cran.r-project.org/doc/manuals/r-devel/R-ints.html#Warnings-and-errors
>
> The docs imply that these are "C-level equivalents" to the R stop()
> function, which of course DOES terminate execution.  I believe
> Rf_error() does also, but I really wish it was explicitly stated one
> way or the other.

I'd imagine the author of that section thought it *was* explicit.  The 
documentation can't answer every question, or clarify every 
misunderstanding.  That's why R-devel exists.  fortune("source")
is also relevant.  But now that you know the truth, you could suggest 
rewording, as an svn patch against the trunk version of R-exts.texi.

> Grepping the R source, I never did find where Rf_error() is actually
> implemented.  What am I missing?

You probably forgot to grep the .h files:

include\R_ext\Error.h(48): #define error Rf_error

The implementation of the function calls it error(), and uses the define 
to remap that to Rf_error().  You can find the implementation in (where 
else?) src/main/errors.c.

>
> In src/include/Rinternals.h, the implementation of error_return()
> first calls Rf_error(msg) and then does a separate "return R_NilValue"
> in the next statement.  So you might think that Rf_error() must NOT
> terminate execution, otherwise why the extra return statement?  But it
> also has a comment:
>
>    /* return(.) NOT reached : for -Wall */
>
> Meaning that that return statement is just to silence compiler
> warnings, and is not supposed to be reached because Rf_error()
> terminates execution.  But this is a ridiculously roundabout way to
> infer what the behavior of Rf_error() is supposed to be...

I won't disagree that you took a roundabout route to the right conclusion.

Duncan Murdoch



[Rd] Custom C finalizers for .Call
Duncan Murdoch