From kirill.mueller at ivt.baug.ethz.ch  Tue Jun  6 22:14:03 2017
From: kirill.mueller at ivt.baug.ethz.ch (=?UTF-8?Q?Kirill_M=c3=bcller?=)
Date: Tue, 6 Jun 2017 22:14:03 +0200
Subject: [Rd] Usage of PROTECT_WITH_INDEX in R-exts
In-Reply-To: <22838.25257.371534.516276@stat.math.ethz.ch>
References: <7f60a4f4-3d29-1e2b-04e1-4bfd1d2bc4d0@ivt.baug.ethz.ch>
 <22838.25257.371534.516276@stat.math.ethz.ch>
Message-ID: <8a9a6084-2e39-50f1-d043-fa8c5ebdbd66@ivt.baug.ethz.ch>



On 06.06.2017 10:07, Martin Maechler wrote:
>>>>>> Kirill M?ller <kirill.mueller at ivt.baug.ethz.ch>
>>>>>>      on Mon, 5 Jun 2017 17:30:20 +0200 writes:
>      > Hi I've noted a minor inconsistency in the documentation:
>      > Current R-exts reads
>
>      > s = PROTECT_WITH_INDEX(eval(OS->R_fcall, OS->R_env), &ipx);
>
>      > but I believe it has to be
>
>      > PROTECT_WITH_INDEX(s = eval(OS->R_fcall, OS->R_env), &ipx);
>
>      > because PROTECT_WITH_INDEX() returns void.
>
> Yes indeed, thank you Kirill!
>
> note that the same is true for its partner function|macro REPROTECT()
>
> However, as  PROTECT() is used a gazillion times  and
> PROTECT_WITH_INDEX() is used about 100 x less, and PROTECT()
> *does* return the SEXP,
> I do wonder why PROTECT_WITH_INDEX() and REPROTECT() could not
> behave the same as PROTECT()
> (a view at the source code seems to suggest a change to be trivial).
> I assume usual compiler optimization would not create less
> efficient code in case the idiom   PROTECT_WITH_INDEX(s = ...)
> is used, i.e., in case the return value is not used ?
>
> Maybe this is mainly a matter of taste,  but I find the use of
>
>     SEXP s = PROTECT(........);
>
> quite nice in typical cases where this appears early in a function.
> Also for that reason -- but even more for consistency -- it
> would also be nice if  PROTECT_WITH_INDEX()  behaved the same.
Thanks, Martin, this sounds reasonable. I've put together a patch for 
review [1], a diff for applying to SVN (via `cat | patch -p1`) would be 
[2]. The code compiles on my system.


-Kirill


[1] https://github.com/krlmlr/r-source/pull/5/files

[2] https://patch-diff.githubusercontent.com/raw/krlmlr/r-source/pull/5.diff


>
> Martin
>
>      > Best regards
>      > Kirill



[Rd] Usage of PROTECT_WITH_INDEX in R-exts
=?UTF-8?Q?Kirill_M=c3=bcller?=