r-lib / debugme

Easy and efficient debugging for R packages
https://r-lib.github.io/debugme/
Other
147 stars 10 forks source link

Let instrument recurse into environments #15

Closed mllg closed 6 years ago

mllg commented 7 years ago

Currently the package only instruments functions which are defined at top level of a package namespace. But sometimes additional functions are stored inside environments which then do not get instrumented. This is also the case for R6 classes which are stored as environment in the namespace.

gaborcsardi commented 7 years ago

:+1:

Btw. I need to rethink debugme soon, because it seems to conflict with R-devel (https://cran.rstudio.com/web/checks/check_results_debugme.html), I suspect that this is because of the JIT being enabled in R-devel...

gaborcsardi commented 6 years ago

Recursing into environments needs to be done with care, because as soon as you have reference semantics, you might have circular references and circular data structures. So we need to keep track of which environments we are already instrumenting. Probably by using the address of the environment.

gaborcsardi commented 6 years ago

For R6 to work, I also need to go inside lists. Hopefully this won't cause any problems.

mllg commented 6 years ago

Thanks, this seems to work for batchtools.

However, I noticed that the instrumentation does not work properly with inheritance. Here is a simple example using https://github.com/mllg/bugme:

bar = Bar$new(x = 12)
print(bar$getxsquare)

getxsquare() gets instrumented twice.

gaborcsardi commented 6 years ago

Oh, yeah, of course. I will somehow mark the already instrumented functions to avoid instrumenting them again.

mllg commented 6 years ago

You could port data.table::address() to debugme which gives you addresses for functions and environments.

gaborcsardi commented 6 years ago

That requires C code, unfortunately, so I would avoid it if possible.

gaborcsardi commented 6 years ago

Unless I do some .Internal hack.....

mllg commented 6 years ago

5 lines of C code vs obscuring a call to .Internal() and violating CRAN policies? :confused:

SEXP address(SEXP x) {
    char buffer[32];
    snprintf(buffer, 32, "%p", (void *)x);
    return(mkString(buffer));
}

What's wrong with C?

gaborcsardi commented 6 years ago

It makes debugme heavier, as you either need to rely on CRAN binaries or have a compiler.

gaborcsardi commented 6 years ago

Actually, I can also just change the debug string, so that the second run does not pick it up.

mllg commented 6 years ago

Actually, I can also just change the debug string, so that the second run does not pick it up.

:+1: