nimble-dev / nimble

The base NIMBLE package for R
http://R-nimble.org
BSD 3-Clause "New" or "Revised" License
158 stars 24 forks source link

scoping/exporting issue with nested nimbleFunctions #1354

Open paciorek opened 1 year ago

paciorek commented 1 year ago

Consider a package with these nimbleFunctions:

nimNothing <- nimbleFunction(
  run = function(){
    returnType(double(0))
    return(0)
  })

#' @export
nestedNim <- nimbleFunction(
  run = function(){
    returnType(double(0))
    return(nimNothing())
  }
)

A user reported that if nimNothing is not exported, one gets this error when compiling nestedNim:

Error in if (typeEnv$return$type == "nimbleList" || code$args[[1]]$type == : missing value where TRUE/FALSE needed

The issue arises in the call to sizeReturn, in particular the recursive call asserts <- recurseSetSizes(code, symTab, typeEnv).

If nimNothing is exported, we see:

Browse[2]> code$args[[1]]
rcFun_testscope5
Browse[2]> code
return
  rcFun_testscope5

if not exported, we see:

Browse[2]> code$args[[1]]
nimNothing
Browse[2]> code
return
  nimNothing

So apparently because of the exporting, some additional processing of the nimNothing RCfun has happened. I haven't dug deeper but I will do so when I have time.

It's a bit surprising this hasn't come up before.

paciorek commented 11 months ago

So the issue here seems to be that in exprClasses_setSizes we invoke if (exists(code$name)) {. If nimNothing is exported it is found in the package namespace based on the nested enclosing environments. If it is not exported it is not found.

Not seeing how to deal with this at the moment...

paciorek commented 11 months ago

So if we have a private nimbleFunction in nimble then it can be found when inside the compilation process because we are in the package namespace and private members are available. If it is in another package, I don't see a way to get around exporting all the nfs.

As far as the inscrutable error, that seems to be a separate issue not related to scoping. I will file a separate issue about that. Once that is error trapped at least it will be more obvious that nimNothing is not being found.

paciorek commented 9 months ago

Just a note that as of 1.1.0 we will now report that nimNothing is not being found, so at least the source of the issue should be clearer.