Skip to content

Fix ISNAN macro in bessel.hpp for GCC C++17 compliance#414

Open
samueldnj wants to merge 1 commit intokaskr:masterfrom
samueldnj:fix/isnan-bessel
Open

Fix ISNAN macro in bessel.hpp for GCC C++17 compliance#414
samueldnj wants to merge 1 commit intokaskr:masterfrom
samueldnj:fix/isnan-bessel

Conversation

@samueldnj
Copy link

The ISNAN macro expands to bare isnan() which GCC rejects in C++17 mode ('isnan' was not declared in this scope). Changed to std::isnan(asDouble(x)) to match the existing R_finite pattern at line 32. Reproduces on Ubuntu with GCC + R >= 4.5; macOS clang is unaffected.

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>
@kaskr
Copy link
Owner

kaskr commented Mar 20, 2026

OK, but

  • Why do I not see this issue on any CRAN checks? Neither for TMB itself or dependencies?
  • How sure are you, that this change would not break anything on CRAN?
  • What's your motivation for the change (package/reproducible example) on which platform?

@samueldnj
Copy link
Author

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
line 33 of the same file.

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:

  • CRAN TMB: all 8 GCC cells fail with 'isnan' was not declared in this scope at bessel.hpp:60 when TMB.hpp is included before Rcpp.hpp (tmb-first); all macOS and rcpp-first (reverse inclusion order) cells pass. This suggests that this is probably a GCC compiler issue
  • Fixed TMB: all 24 cells pass when I use the suggested fix in the PR
  • R CMD check --as-cran on the suggested fix passes on both Ubuntu and Windows — TMB's own examples and tests are unaffected

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants