gbm-developers / gbm

Gradient boosted models (the old gbm package)
Other
51 stars 27 forks source link

Consider supporting -DR_NO_REMAP #77

Closed michaelquinn32 closed 7 months ago

michaelquinn32 commented 7 months ago

Changes to the LLVM compiler introduce errors when compiling gbm. The transitive dependency between and is being removed, which leads to errors like this:

include/c++/v1/locale:3226:31: error: no member named 'Rf_error' in 'std::codecvt_base'
 3226 |           __r = codecvt_base::error;
      |                 ~~~~~~~~~~~~~~^
R/src/include/R_ext/Error.h:52:15: note: expanded from macro 'error'
   52 | #define error Rf_error

One way to fix this is to use R_NO_REMAP. https://rstudio.github.io/r-manuals/r-exts/The-R-API.html

But that will require calling error and warning with their Rf_ prefixed versions instead. https://github.com/gbm-developers/gbm/blob/20a31ff2e9dcc27572c249869e7c5b2ec64cdb8a/src/node_search.cpp#L149

Thanks!

gregridgeway commented 7 months ago

Thanks for finding the error and pointing me to a solution. Much easier to fix than I originally thought.

michaelquinn32 commented 7 months ago

Awesome! Thanks for the quick fix.

I can confirm that this works against the new LLVM version coming up.

FYI, if you don't want to inline the definitions in the future, you can use a Makevars file to handle you compilation flags.

gregridgeway commented 7 months ago

Glad it worked. Thanks for the Makevars tip too.