Repository navigation
Description
Build-time feature detection does not seem to be an option here, but I'd like to suggest using a whitelist for backtrace() functionality rather than an ever-growing stoplist (e.g., #1338, #796):
Rcpp/inst/include/Rcpp/exceptions_impl.h
Lines 24 to 44 in 6570751
| // disable demangler on platforms where we have no support | |
| #ifndef RCPP_DEMANGLER_ENABLED | |
| # if defined(_WIN32) || \ | |
| defined(__FreeBSD__) || \ | |
| defined(__NetBSD__) || \ | |
| defined(__OpenBSD__) || \ | |
| defined(__DragonFly__) || \ | |
| defined(__CYGWIN__) || \ | |
| defined(__sun) || \ | |
| defined(_AIX) || \ | |
| defined(__MUSL__) || \ | |
| defined(__HAIKU__) || \ | |
| defined(__ANDROID__) | |
| # define RCPP_DEMANGLER_ENABLED 0 | |
| # elif defined(__GNUC__) || defined(__clang__) | |
| # include <execinfo.h> | |
| # define RCPP_DEMANGLER_ENABLED 1 | |
| # else | |
| # define RCPP_DEMANGLER_ENABLED 0 | |
| # endif | |
| #endif |
What brought me here is that the __MUSL__ exception (added long ago in #449) is an unreliable kludge as musl itself doesn't define such a macro (by design).
AFAICS, a suitable whitelist should be as simple as
defined(__GLIBC__) || defined(__APPLE__)where execinfo is built-in. The additional compiler check (gcc|clang) seems to be redundant (probably a relict). The suggested change is similar in spirit to the original commit in #449, which was discarded simply because it forgot to cover macOS. BSDs have long provided execinfo, too, but a web search suggests they require separate linking with -lexecinfo.
Activity
eddelbuettel commentedon Oct 20, 2025
Philosophically speaking, with default behaviour working as is, I am finding that the specific exclusion list is working well. When someone on an exotic system has a need, they usually send a PR. That worked well so far.
So with that I would prefer it you could devise a way to detect your musl environment. We could also wrap an outer control variable around so that deficient systems that cannot have exceptions can opt to skip them using such an outer variable (off by default, of course). How exactly do things work out here for you? Do you fall into the wrong bucket because you still have
gcc?But it is all complicated and we probably have whitelists in other places. So open for discussion. We have time til the next release in January anyway .
eddelbuettel commentedon Oct 20, 2025
I see this has come up recently-ish?
eddelbuettel commentedon Oct 20, 2025
Could your
__MUSL__be replaced bygcc and not glibc?bastistician commentedon Oct 20, 2025
I learned the hard way (already a few years ago) that a huge fraction of available R packages fail their installation with something like:
(Yes, gcc.)
Of course, I can work around that by setting, e.g.,
CXXFLAGS="${CXXFLAGS} -D__MUSL__"in my R'sconfig.siteas expected by current Rcpp. But I think it would be safer and simpler to test for the (developer) platforms where execinfo is known to work rather than try to exclude all others.__GLIBC__is certainly not defined by musl.eddelbuettel commentedon Oct 20, 2025
Yes so with the initial comment I made above, could I ask you to maybe devise and test a strategy to detect your build environment reliably and that would free you from have to add the
-D__MUSL__workaround?Maybe
(defined(__GNUC__) && !defined(__GLIBC__))does that if musl refuses to supply something better?eddelbuettel commentedon Oct 20, 2025
I will also try to do a code search here at GitHub to see some other best practices for shielding
#include <execinfo.h>.Enchufa2 commentedon Oct 20, 2025
What about
__has_include?10 years ago it was a new-ish preprocessor feature, but nowadays it should be everywhere.
eddelbuettel commentedon Oct 20, 2025
Excellent. I was bemoaning all morning that we'd need a test for existence of an include file and forgot we had one! (Just back in from a bike ride.)
eddelbuettel commentedon Oct 20, 2025
Technically C++17 per cpp reference but the guard you suggest should do.
bastistician commentedon Oct 20, 2025
The
__has_includetest has the nice little side effect of also succeeding automatically under__ANDROID_API__ >= 33, which introduced<execinfo.h>.I guess there is no guarantee that
<execinfo.h>, if it exists, will always provide the anticipatedbacktrace()andbacktrace_symbols(), butdefined(__GLIBC__) || defined(__APPLE__)should be OK even without a__has_include()check.Thanks for the quick responses!
eddelbuettel commentedon Oct 20, 2025
I think we prefer switching to that over switching to the whitelist you advocate for.
The net effect should be the same, and for you and the at most three other musl users 😜 life will be so much easier.
bastistician commentedon Oct 22, 2025
Thanks! I can confirm that the patch works for me.