-
-
Notifications
You must be signed in to change notification settings - Fork 219
Description
Over the years, programming practice has evolved and improved when it comes to header inclusion and spillover of defintions. For example, as of release 4.5.0 R now makes R_NO_REMAP a default for inclusion from C++ code (WRE, Chapter 6), and Rcpp has long done so too via the file inst/include/Rcpp/r/headers.h. The #define STRICT_R_HEADERS is another example.
To be most effective, the header includes should happen in a controlled order. As Rcpp itself includes the R headers after making certain definition itself it is preferable to not include R headers before Rcpp does. So far, we have not checked for this, and it may be time to change this.
Conceptually, things start for us from RcppCommon.h which first includes Rcpp/r/headers.h. We could precede this with header file checking for symbols i.e. something along the lines of
#if defined(R_R_H) & defined(USING_R)
#pragma message "R.h has been included before any Rcpp headers. This can lead to hard-to-debug errors, \
and is generally not necessary. See https://github.com/RcppCore/Rcpp/issues/1410"
#endifWe could leave this at 'message' level for some time (a year?) and then switch to 'warning'. For comparison, RcppArmadillo relies on a fairly intricate interplay between Rcpp headers and its own and made it an error of Rcpp was included first. This has been in place for years, and works well. We probably also want to check for Rinterface.h, Rdefines.h and Rinternals.h, and possibly more. I have a local test branch I can push is other want to play along.
PS We could and should also over an override to skip the check in case someone bulk compiles or installs and doesn't want to see it, i.e. wrap it all in #if !defined(RCPP_SKIP_CHECK_FOR_R_H) or alike.