Sahil Kang
2017-Jul-06 14:40 UTC
[Rd] [Bug Fix] Default values not applied to ... arguments
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 -------------- next part -------------- A non-text attachment was scrubbed... Name: patch.diff Type: text/x-patch Size: 531 bytes Desc: not available URL: <https://stat.ethz.ch/pipermail/r-devel/attachments/20170706/9c1a55db/attachment.bin>
Tomas Kalibera
2017-Jul-06 16:29 UTC
[Rd] [Bug Fix] Default values not applied to ... arguments
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 > > > ______________________________________________ > R-devel at r-project.org mailing list > https://stat.ethz.ch/mailman/listinfo/r-devel[[alternative HTML version deleted]]
Sahil Kang
2017-Jul-07 09:42 UTC
[Rd] [Bug Fix] Default values not applied to ... arguments
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 >> >> >> ______________________________________________ >> R-devel at r-project.org mailing list >> https://stat.ethz.ch/mailman/listinfo/r-devel > >[[alternative HTML version deleted]]