Permit compilation with Nullable::operatorT()

[eddelbuettel]

Dear statgenIBD team,

Your package uses Rcpp::Nullable, a feature we would like to complete a little (see [1] and [2]). However, the change in [1] revealed that six packages (listed in [2]) would not compile, yours is one of them.

The issue is a little "C++-ish" but not that complicated. We would like to add an 'operatorT()' returning the templated class T -- something more idiomatic than to use as(). As it goes with C++, sometimes the compiler is kind to use and allows certain statements, sometimes it is better to be more explicit.

From a first look at your code and the patch, you use & on the object in Nullable<>. This actually does not buy as much (as a SEXP is already a pointer-holding small struct) but makes it a different type: T by reference, not T. The fix is easy and in this PR (with a lot of whitespace changes, you can tell GH not to show those). With it, everything builds and test fine again.

The timeline is not very urgent. A new CRAN version of your package "within a month or two", so before the next planned Rcpp release in July, would be great. Of course, sooner is better as we can then move on to other things.

Let me know if/how Team Rcpp or I can help.

Cheers, Dirk

[1] RcppCore/Rcpp#1471
[2] RcppCore/Rcpp#1472

[BartJanvanRossum]

Dear Dirk,

Thanks for informing us and for the pull request. Since I do my package development on a university gitlab instance and I am just mirroring everything here I am not going to accept your pull request. However I did copy your changes. I will make sure that an update of the package including the changes will be sent to CRAN somewhere this month.

Cheers, Bart-Jan

[eddelbuettel]

That was super-speedy, thanks @BartJanvanRossum! It was mostly a pull request because those are easier than emailing. I will keep an eye on CRAN changes, maybe also ping us at our issue 1472.

[eddelbuettel]

BTW a purely stylistic thing: it looks like you use evalDist on a double, you can actually write Nullable<double> evalDist in the signature and then instantiate an actual double via double{evalDist} with either curly or brace instantiation. Makes intent marginally clearer.

PS And the actual PR gets much much shorter when whitespace is omitted (via the trailing `?w=1). Mostly writing it out so that I remember next time 😜

[BartJanvanRossum]

I will let you know in your issue when the new version is released. Will have a look at your remark about the double as well before that