memoise icon indicating copy to clipboard operation
memoise copied to clipboard

Infinite Improbability default_args

Open MikeBadescu opened this issue 6 years ago • 2 comments

Jim,

A possible bug:

Line 123: lapply(default_args, eval, envir = environment()))

If default_args contains one of the already defined symbols in memo_f or its enclosed environment (e.g., _f), it will use this value instead of the default argument specified in the function definition. Improbable example:

# based on test: "argument names don't clash with names in memoised function body"
library(memoise)

f <- function(
  # note that `_f` is not included as argument
  `_cache`, `_additional`,
  mc, encl, called_args, default_args, args, hash, res, xtra = `_f`
) list(`_f`, `_cache`, `_additional`, mc, encl, called_args, default_args, args, hash, res, xtra)
f_mem <- memoise(f)

`_f` <- 100
(unlist(f(1, 2, 3, 4, 5, 6, 7, 8, 9)))
# [1] 100   1   2   3   4   5   6   7   8   9 100
(unlist(f_mem(1, 2, 3, 4, 5, 6, 7, 8, 9)))
# [1] 100   1   2   3   4   5   6   7   8   9 100
# looks good

`_f` <- 200
(unlist(f(1, 2, 3, 4, 5, 6, 7, 8, 9)))
# [1] 200   1   2   3   4   5   6   7   8   9 200
(unlist(f_mem(1, 2, 3, 4, 5, 6, 7, 8, 9)))
# [1] 100   1   2   3   4   5   6   7   8   9 100
# does not match

I believe using

envir = environment(encl$`_f`)

would solve this problem as there is no other eval that takes place inside the memo_f frame / enclosing environment. (All the other tests pass.)

Q: We still have eval of _additional on Line 127 which takes place in encl --> there might be a possible (but improbable) conflict of ... formula with names in encl: _f, _additional and _cache. Is this correct?

MikeBadescu avatar Mar 17 '18 03:03 MikeBadescu

evaluating default arguments in environment(encl$`_f`) would have problem with arguments whose default values are other arguments, e.g.

f <- function(x = y, y) x + y
fm <- memoise(f)
fm(y = 1)

bluaze avatar May 08 '19 15:05 bluaze

if we could find a way to "snapshot" the environment at the beginning, then the snapshot could be used to evaluate the default arguments

bluaze avatar May 08 '19 16:05 bluaze