possible bug in utils::removeSource - NULL argument is silently dropped

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

possible bug in utils::removeSource - NULL argument is silently dropped

Dénes Tóth-2
Dear R-Core Team,

I found an unexpected behaviour in utils::removeSource (also present in
r-devel as of today).

---

# create a function which accepts NULL argument
foo <- function(x, y) {
   if (is.null(y)) y <- "default foo"
   attr(x, "foo") <- y
   x
}

# create a function which utilizes 'foo'
testSrc <- function() {
   x <- 1:3
   x <- foo(x, NULL)
   x
}

# this works fine
testSrc()

# this fails
testNoSrc <- utils::removeSource(testSrc)
testNoSrc()

# removeSource removes NULL from the 'foo' call
print(testNoSrc)

---

I traced back the bug to this row in removeSource:

(line 33 in sourceutils.R)
part[[i]] <- recurse(part[[i]])

it should be (IMHO):
part[i] <- list(recurse(part[[i]]))

---

# create a function with the above patch
rmSource <- function (fn) {
   stopifnot(is.function(fn))
   if (is.primitive(fn))
     return(fn)
   attr(fn, "srcref") <- NULL
   attr(body(fn), "wholeSrcref") <- NULL
   attr(body(fn), "srcfile") <- NULL
   recurse <- function(part) {
     if (is.name(part))
       return(part)
     attr(part, "srcref") <- NULL
     attr(part, "wholeSrcref") <- NULL
     attr(part, "srcfile") <- NULL
     if (is.language(part) && is.recursive(part)) {
       for (i in seq_along(part))
         part[i] <- list(recurse(part[[i]]))
     }
     part
   }
   body(fn) <- recurse(body(fn))
   fn
}

# test
( testNoSrc2 <- rmSource(testSrc) )
testNoSrc2()


Regards,
Denes

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

Re: possible bug in utils::removeSource - NULL argument is silently dropped

Martin Maechler
>>>>> Dénes Tóth <[hidden email]>
>>>>>     on Mon, 11 Dec 2017 11:39:38 +0100 writes:

    > Dear R-Core Team,

    > I found an unexpected behaviour in utils::removeSource (also present in
    > r-devel as of today).

> ---
>
> # create a function which accepts NULL argument
> foo <- function(x, y) {
>    if (is.null(y)) y <- "default foo"
>    attr(x, "foo") <- y
>    x
> }
>
> # create a function which utilizes 'foo'
> testSrc <- function() {
>    x <- 1:3
>    x <- foo(x, NULL)
>    x
> }
>
> # this works fine
> testSrc()
>
> # this fails
> testNoSrc <- utils::removeSource(testSrc)
> testNoSrc()
>
> # removeSource removes NULL from the 'foo' call
> print(testNoSrc)
>
> ---
>
> I traced back the bug to this row in removeSource:
>
> (line 33 in sourceutils.R)
> part[[i]] <- recurse(part[[i]])
>
> it should be (IMHO):
> part[i] <- list(recurse(part[[i]]))
>
> ---
>
> # create a function with the above patch
> rmSource <- function (fn) {
>    stopifnot(is.function(fn))
>    if (is.primitive(fn))
>      return(fn)
>    attr(fn, "srcref") <- NULL
>    attr(body(fn), "wholeSrcref") <- NULL
>    attr(body(fn), "srcfile") <- NULL
>    recurse <- function(part) {
>      if (is.name(part))
>        return(part)
>      attr(part, "srcref") <- NULL
>      attr(part, "wholeSrcref") <- NULL
>      attr(part, "srcfile") <- NULL
>      if (is.language(part) && is.recursive(part)) {
>        for (i in seq_along(part))
>          part[i] <- list(recurse(part[[i]]))
>      }
>      part
>    }
>    body(fn) <- recurse(body(fn))
>    fn
> }
>
> # test
> ( testNoSrc2 <- rmSource(testSrc) )
> testNoSrc2()
>
>
> Regards,
> Denes

Thank you very much, Denes. This is indeed a bug

As it happens I had wanted to improve the removeSource()
function many months ago [to make it work so it removes source attributes also
from non-function expressions] and have had a changed version of
it in my version of R-devel.

Your proposed change looks quite reasonable, and I'll try adding
it (and look at the consequences if it is ok in all use cases).

Thank you once more!

Martin Maechler
ETH Zurich

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