Enhance is check for module object

[tqchen]

fix to #380

[eddelbuettel]

That probably requires a reverse-dependency check but I am traveling right now and may not get to it for some time.

[tqchen]

please take your time. we have applied a local specialization patch to our package for now. Thanks

[thirdwing]

BTW, maybe you can add a simple test in https://github.com/RcppCore/Rcpp/tree/master/inst/unitTests/testRcppModule/src

[eddelbuettel]

+1 for adding unit test support to this (or any, really) pull request.

[kevinushey]

This does not seem like the right change. I think the only reason you're getting punted into the module method is because you haven't let Rcpp understand how to handle as / wrap.

Please read https://cran.r-project.org/web/packages/Rcpp/vignettes/Rcpp-extending.pdf for more information on how you can let Rcpp::as<T> and Rcpp::wrap understand how to interact with custom classes; I am guessing your problem will go away if you do this.

(I could be wrong -- but my understanding is that if you have the appropriate architecture set up, the as / wrap will dispatch / use your methods, and you're trying to use that plainly on your own user-defined class. The module methods there are basically fallback methods.)

[thirdwing]

A very simple example to make things clear. Just use our RcppModuleWorld.

check_module is nothing but Rcpp::is<RcppModuleWorld>(x).

> m <- new(RcppModuleWorld)
> m
C++ object <0x2858180> of class 'RcppModuleWorld' <0x306a480>
> check_module(m)
[1] TRUE
> check_module(1)
Error: expecting an external pointer

If other things pass into this function, we will get a warning, since env.get(".cppclass") return a NULL.

@tqchen thought we need to check whether it is NULL. If it is NULL, we should return FALSE, not a warning.

[tqchen]

I think @thirdwing explains my point. Ideally it would be great Rcpp::is<RcppModuleWorld> only return true or false. So we can use it to check parameters.

For example, we can use this to check if input parameter is a certain type, and give more friendly error message to the user.

void dosomething(Rcpp::List param_list) {
  for (size_t i = 0; i < param_list.size(); ++i) {
    if (!Rcpp::is<RcppModuleClassA>(param_list[i]))
        throw Rcpp::exception("The input  is expected to be list of RcppModuleClassA")
  }
}

[kevinushey]

Thanks, it indeed looks like the RCPP_EXPOSED_CLASS is responsible for exposing as / wrap methods, hence where the dispatch is coming from. The PR looks fine to me, then.

[eddelbuettel]

Agreed -- will merge now and launch a full reverse-dependency test. We can always revert should something bubble up (which we have no reason to expect).