[Bug Fix] Default values not applied to ... arguments

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[Bug Fix] Default values not applied to ... arguments

Sahil Kang
Hi Duncan, Martin

Here's a small patch that fixes bug 15199 reported at:
https://bugs.r-project.org/bugzilla3/show_bug.cgi?id=15199

I was able to reproduce the bug as Duncan had outlined just fine, but I
did notice that when we debug(f), the problem went away.
I later realized that f(1,,3) behaved correctly the first time it was
executed, but misbehaved as documented on subsequent calls.
This narrowed the problem down to the byte-compilation that occurs on
subsequent function calls.

This patch prevents byte-compilation on closure objects.
Although it's a less than ideal solution, this patch fixes the bug at
least until the underlying byte-compilation issue can be found (I'm
currently scrutinizing the promiseArgs function at eval.c:2771).

Thanks,
Sahil

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel

patch.diff (546 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Bug Fix] Default values not applied to ... arguments

Tomas Kalibera

Thanks for the report, I've fixed 15199 in the AST interpreter in 72664,
I will fix it in the byte-code interpreter as well.

If you ever needed to disable the JIT, there is API for that, see
?compiler. Note though that by disabling the JIT you won't disable the
byte-code interpreter, because code also gets compiled when packages are
installed or when the compiler is invoked explicitly.

Best,
Tomas

On 07/06/2017 04:40 PM, Sahil Kang wrote:

> Hi Duncan, Martin
>
> Here's a small patch that fixes bug 15199 reported at:
> https://bugs.r-project.org/bugzilla3/show_bug.cgi?id=15199
>
> I was able to reproduce the bug as Duncan had outlined just fine, but
> I did notice that when we debug(f), the problem went away.
> I later realized that f(1,,3) behaved correctly the first time it was
> executed, but misbehaved as documented on subsequent calls.
> This narrowed the problem down to the byte-compilation that occurs on
> subsequent function calls.
>
> This patch prevents byte-compilation on closure objects.
> Although it's a less than ideal solution, this patch fixes the bug at
> least until the underlying byte-compilation issue can be found (I'm
> currently scrutinizing the promiseArgs function at eval.c:2771).
>
> Thanks,
> Sahil
>
>
> ______________________________________________
> [hidden email] mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel



        [[alternative HTML version deleted]]

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: [Bug Fix] Default values not applied to ... arguments

Sahil Kang
Yes, I see what you mean. My patch only disables JIT compilation for
closures. If a user manually compiles a closure, however, the bug pops
up again.

I think the bug may either lie in how the byte-compiler compiles
closures, or how closures with compiled body's are executed by the AST
interpreter (without my patch applied):

compiler::enableJIT(0) # turn off JIT compilation
f <- function(...) { g(...) }
g <- function(a=4, b=5, c=6) {
   print(c(missing(a), missing(b), missing(c)))
   b
}

f(1,,3) # works fine
# [1] FALSE TRUE FALSE
# [1] 5

f(1,,3) # works fine
# [1] FALSE TRUE FALSE
# [1] 5

ff_1 <- compiler::compile(f) # compile f
ff_2 <- compiler::cmpfun(f) # compile body of closure

eval(ff_1)(1,,3) # works fine
# [1] FALSE TRUE FALSE
# [1] 5

eval(ff_2)(1,,3) # bug shows up again
# [1] FALSE TRUE FALSE
# Error in g(...) : argument is missing, with no default

Thanks,
Sahil

On 07/06/2017 09:29 AM, Tomas Kalibera wrote:

>
> Thanks for the report, I've fixed 15199 in the AST interpreter in
> 72664, I will fix it in the byte-code interpreter as well.
>
> If you ever needed to disable the JIT, there is API for that, see
> ?compiler. Note though that by disabling the JIT you won't disable the
> byte-code interpreter, because code also gets compiled when packages
> are installed or when the compiler is invoked explicitly.
>
> Best,
> Tomas
>
> On 07/06/2017 04:40 PM, Sahil Kang wrote:
>> Hi Duncan, Martin
>>
>> Here's a small patch that fixes bug 15199 reported at:
>> https://bugs.r-project.org/bugzilla3/show_bug.cgi?id=15199
>>
>> I was able to reproduce the bug as Duncan had outlined just fine, but
>> I did notice that when we debug(f), the problem went away.
>> I later realized that f(1,,3) behaved correctly the first time it was
>> executed, but misbehaved as documented on subsequent calls.
>> This narrowed the problem down to the byte-compilation that occurs on
>> subsequent function calls.
>>
>> This patch prevents byte-compilation on closure objects.
>> Although it's a less than ideal solution, this patch fixes the bug at
>> least until the underlying byte-compilation issue can be found (I'm
>> currently scrutinizing the promiseArgs function at eval.c:2771).
>>
>> Thanks,
>> Sahil
>>
>>
>> ______________________________________________
>> [hidden email]  mailing list
>> https://stat.ethz.ch/mailman/listinfo/r-devel
>
>


        [[alternative HTML version deleted]]

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: [Bug Fix] Default values not applied to ... arguments

Sahil Kang
I'm glad I could help, and I look forward to reading your patch so that I can learn more about the R internals. It'd be nice to close this 4 year old bug.

Sahil



-------- Original Message --------
From: Tomas Kalibera <[hidden email]>
Sent: July 7, 2017 11:10:05 AM PDT
To: Sahil Kang <[hidden email]>
Subject: Re: [Rd] [Bug Fix] Default values not applied to ... arguments

As I said the old behavior had been for several years both in AST and BC
interpreter. I fixed it in AST and I will fix it in BC. I know exactly
where it is and how to fix it, there is no more help I could use on this
issue and I don't think there is any need to discuss this further, and
particularly so on R-devel. Please do not send speculative emails to
R-devel, in the interest of saving time of people following the list.

Thanks
Tomas

On 07/07/2017 11:42 AM, Sahil Kang wrote:
> Yes, I see what you mean. My patch only disables JIT compilation for
> closures. If a user manually compiles a closure, however, the bug pops
> up again.
>

> I think the bug may either lie in how the byte-compiler compiles
> closures, or how closures with compiled body's are executed by the AST
> interpreter (without my patch applied):
>
> compiler::enableJIT(0) # turn off JIT compilation
> f <- function(...) { g(...) }
> g <- function(a=4, b=5, c=6) {
>   print(c(missing(a), missing(b), missing(c)))
>   b
> }
>
> f(1,,3) # works fine
> # [1] FALSE TRUE FALSE
> # [1] 5
>
> f(1,,3) # works fine
> # [1] FALSE TRUE FALSE
> # [1] 5
>
> ff_1 <- compiler::compile(f) # compile f
> ff_2 <- compiler::cmpfun(f) # compile body of closure
>
> eval(ff_1)(1,,3) # works fine
> # [1] FALSE TRUE FALSE
> # [1] 5
>
> eval(ff_2)(1,,3) # bug shows up again
> # [1] FALSE TRUE FALSE
> # Error in g(...) : argument is missing, with no default
>
> Thanks,
> Sahil
>
> On 07/06/2017 09:29 AM, Tomas Kalibera wrote:
>>
>> Thanks for the report, I've fixed 15199 in the AST interpreter in
>> 72664, I will fix it in the byte-code interpreter as well.
>>
>> If you ever needed to disable the JIT, there is API for that, see
>> ?compiler. Note though that by disabling the JIT you won't disable
>> the byte-code interpreter, because code also gets compiled when
>> packages are installed or when the compiler is invoked explicitly.
>>
>> Best,
>> Tomas
>>
>> On 07/06/2017 04:40 PM, Sahil Kang wrote:
>>> Hi Duncan, Martin
>>>
>>> Here's a small patch that fixes bug 15199 reported at:
>>> https://bugs.r-project.org/bugzilla3/show_bug.cgi?id=15199
>>>
>>> I was able to reproduce the bug as Duncan had outlined just fine,
>>> but I did notice that when we debug(f), the problem went away.
>>> I later realized that f(1,,3) behaved correctly the first time it
>>> was executed, but misbehaved as documented on subsequent calls.
>>> This narrowed the problem down to the byte-compilation that occurs
>>> on subsequent function calls.
>>>
>>> This patch prevents byte-compilation on closure objects.
>>> Although it's a less than ideal solution, this patch fixes the bug
>>> at least until the underlying byte-compilation issue can be found
>>> (I'm currently scrutinizing the promiseArgs function at eval.c:2771).
>>>
>>> Thanks,
>>> Sahil
>>>
>>>
>>> ______________________________________________
>>> [hidden email]  mailing list
>>> https://stat.ethz.ch/mailman/listinfo/r-devel
>>
>>
>

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel