Development version broken for STRICT_R_HEADERS

[jeroen]

For some packages we need to #define STRICT_R_HEADERS because macros like Calloc, Free and ERROR conflict with identically named macros in other libraries.

This is working fine in Rcpp from CRAN, however in the development version of Rcpp I now get errors on Windows. It looks like a recent change in sugar/functions/sample.h has started using S-style Calloc and Free instead of stdlib calloc and free.

Is this intentional? It will break many packages that use <Rcpp.h> in conjunction with STRICT_R_HEADERS.

devtools::install_github("RcppCore/Rcpp")
devtools::install_github("ropensci/tesseract")

[eddelbuettel]

It is an oversight by our newest team member. We'll fix. Thanks for the heads up.

[nathan-russell]

@jeroenooms Apologies, I was trying to be consistent with the implementation in R's random.c file, but was unaware of this. Wrapping those sections with the appropriate macro guards in sample.h, e.g.

#ifdef STRICT_R_HEADERS
        HL = static_cast<int*>(R_Calloc(n, int));
        q = static_cast<double*>(R_Calloc(n, double));
#else
        HL = static_cast<int*>(Calloc(n, int));
        q = static_cast<double*>(Calloc(n, double));
#endif

seemed to work, as I was able to compile tesseract successfully. Will this be sufficient to address the problem?

[jeroen]

Why can't you use stdlib malloc?

[eddelbuettel]

Because it is data that may go back to R.

But do a recursive grep (or my fav, ag from the silversearcher) for alloc. We do use Rf_allocVector a lot. Why not use that? The #ifdef above is ... weird. We're not forced to translate the code from R base line by line.

[eddelbuettel]

My code reviews skills clearly need honing too.

[jeroen]

I have no idea what you're trying to do in sample.h but it looks like this code could benefit from some rcpp-ification. Much code in base-r is old messy C code that nobody wants to touch because it will break stuff. The nice thing about Rcpp code is that it is more elegant and maintainable.

[nathan-russell]

@jeroenooms Again, trying to follow the conventions used in the R source, but I also got the impression from Writing R Extensions that these function were preferred to their stdlib counterparts:

[...] providing analogues of calloc, realloc and free. If there is an error during allocation it is handled by R, so if these routines return the memory has been successfully allocated or freed.

However, if my interpretation of that passage is incorrect and you are confident that calloc, free are sufficient then I will use those instead.

[eddelbuettel]

Hm, I could swear I had uses of Rf_Calloc in code of mine but I don't find it now.

To me either explicit prefix with Rf_ offered by R works, or what we did in the sugar random functions and do it ourselves via R::....

[eddelbuettel]

Of course if it is really just temp storage then either malloc() / free() or some C++ containers are preferable.

[nathan-russell]

@eddelbuettel Understood; I'll make these changes.

[eddelbuettel]

@jeroenooms Just FYI this all goes back to the earlier and useful variant in RcppArmadillo which also stayed "close" to how R does things to offer a replacement.