[Rd] Usage of PROTECT_WITH_INDEX in R-exts

[Kirill Müller]

On 06.06.2017 22:14, Kirill Müller wrote:
>
>
> 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

I forgot to mention that this patch applies cleanly to r72768.


-Kirill

>
>
>>
>> Martin
>>
>>      > Best regards
>>      > Kirill
>
> ______________________________________________
> R-devel at r-project.org mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel