Fix ISNAN macro in bessel.hpp for GCC C++17 compliance#414
Fix ISNAN macro in bessel.hpp for GCC C++17 compliance#414samueldnj wants to merge 1 commit intokaskr:masterfrom
Conversation
The ISNAN macro expanded to bare isnan() which GCC rejects in C++17 mode because isnan is in std::, not the global namespace. Changed to std::isnan(asDouble(x)) to match the existing R_finite pattern at line 32. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
OK, but
|
|
Hi Dr Kristensen, Thanks for your quick response. I guess I was a bit hasty/incomplete in my enthusiasm. Here are responses to your questions (not in the same order, sorry, as I think Motivation sets the scene a little bit). Motivation: We're developing a package that refactors shared C++ kernel functions (Baranov solvers, composition likelihood functions, etc.) from multiple fisheries stock assessment models in our internal library into a single header-only library. These are standalone template functions that use TMB types (tmbutils::array, vector) but have no objective_function object. They are building blocks for our models, and we want to centralise them to improve testing and limit drift among projects. Since we can't test them through MakeADFun(), we expose them to R via Rcpp exports and test directly with testthat. This requires both #include <TMB.hpp> and #include <Rcpp.h> in the same translation unit. There are workarounds, but we prefer this. Apparently we're the first to try it or at least raise it. Why we don't see it on CRAN: TMB's own CRAN checks compile TMB in isolation without Rcpp headers. We checked glmmTMB (arguably the most popular downstream package) and it never #includes Rcpp.h in any source file. They test entirely through R via MakeADFun(), validating numerical output from the objective function and fitted model components (fixed effects, variance parameters, etc.). The macro conflict only surfaces when both TMB and Rcpp headers are in the same translation unit. How we can be sure the fix doesn't break downstream packages: Short answer, we can't, but we did some testing. The change is one line: (isnan(x)!=0) to (std::isnan(asDouble(x))!=0), matching the R_finite pattern already at We built a reprex package and ran a full diagnostic matrix (3 OS x 4 R versions x 2 include orders = 24 cells) with both CRAN TMB and the fixed TMB from our fork. The CI results show:
Let me know if you think this is enough, or otherwise. Happy to just lodge it as an issue, I have a workaround that works but it felt like an easy fix. Sam |
The
ISNANmacro expands to bareisnan()which GCC rejects in C++17 mode ('isnan' was not declared in this scope). Changed tostd::isnan(asDouble(x))to match the existingR_finitepattern at line 32. Reproduces on Ubuntu with GCC + R >= 4.5; macOS clang is unaffected.