Rcpp: Moving towards defining STRICT_R_HEADERS in Rcpp.h

Created on 25 Apr 2021  路  24Comments  路  Source: RcppCore/Rcpp

As previously discussed in the (preliminary, unmerged) pull requests #613 and #615 as well as in issues #612 and (at much greater length) in issue #898, it is desirable for compilations of R extensions to define STRICT_R_HEADERS so that terms like ERROR or MESSAGE do not clash due to a redefinition, or that PI remains undefined, and of course so that Calloc(), Realloc() or Free() are not defined.

However, as the use of the definition has always been an opt-in, some packages have been making use of the definitions that would otherwise have been defined away -- so that we now cannot simply define it as they would then error. The issue ticket #898 discusses two attempts at quantifying just how extended the damage at CRAN would be: about 58 packages in late Sep 2018, about 62 packages in Jul 2019, and and around 51 earlier this month. Based on the earlier discussions some packages did make changes, and that is greatly appreciated.

And after some more discussion in #898 today, the time seems right to try to address this, one-by-one, in the affected packages. There is no rush: if this takes us six or eight or ten months, so be it. It would be nice to get it done, so let's try to make a start.

What follows is a list by package. The list may be wrong, and if a package is included by accident or error in our process we will remove it. The list may be incomplete: if we find other packages we will add them. Otherwise it will list the names of packages that did come up as failing to pass R CMD check when STRICT_R_HEADERS was defined by us, and the error is almost always one of an immediate failure to compile. A simple re-definition of the affected symbols will be the solution, and we will aim to provide pull requests as we go along. Help in that process would be appreciated.

So give this list, the suggested plan is to submit PRs. And then to follow-up as PRs are folded in addressing the issue, and to 'tick off' each package and maybe mark the version of the package containing the fix and PR. Let's see how it goes, and discuss below what we learn.

  • [x] activegp (1.0.5) -- not on GitHub, patch emailed 2021-04-25, ack from maintainer 2021-04-25, 'real soon'
  • [x] atakrig (0.9.7) -- ~not on GitHub~ at maoguihu/atakrig, patch emailed 2021-04-25, PR #1 send 2021-04-26 too, ack from maintainer 2021-04-26, 'soon', 0.9.8 on CRAN 2021-04-30
  • [ ] basad (0.2.0) -- not on GitHub, patch emailed 2021-04-25
  • [x] Benchmarking (0.29) -- not on GitHub, patch emailed 2021-04-26, ack from maintainer 2021-04-26, 'done locally', 'CRAN later...'
  • [x] BGVAR (2.1.5) -- at mboeck11/BGVAR (2.2.0), PR #4 sent 2021-04-26, merged 2021-04-26
  • [x] bigmemory (4.5.36) -- at kaneplusplus/bigmemory, PR #104 sent 2021-04-26, merged 2021-06-14
  • [ ] binnednp (0.4.0) -- not on GH, patch emailed 2021-04-27
  • [x] biwavelet (0.20.19) -- at tgouhier/biwavelet, PR #96 sent 2021-04-27; heard back via email 2021-05-24, merged, "upload soon", on CRAN 2021-05-26
  • [x] blatent (0.1.1) -- at jonathantemplin/blatent, PR #1 sent 2021-04-27, heard via email 2021-05-24, "soon"
  • [x] cbinom (1.5) -- not on GH, patch emailed 2021-04-27, heard back 2021-04-27 "soon", on CRAN 2021-04-28
  • [x] circumplex (0.3.6) -- at jmgirard/circumplex, PR #13 sent on 2021-04-28, merged 2021-04-28, on CRAN 2021-05-28
  • [x] collapse (1.5.3) -- at SebKrantz/collapse, PR #149 sent on 2021-04-29; merged 2021-05-01
  • [ ] compas (0.1) -- at wongswk/compass, PR #1 sent on 2021-04-29
  • [ ] dfphase1 (1.1.3) -- not on GH, patch emailed 2021-04-29
  • [x] emIRT (0.0.11) -- at kosukeimai/emIRT, PR #6 sent 2021-04-30. merged 2021-05-12
  • [x] facilitation (0.5.2) -- at Lobz/facilitation and already fixed since 2018 (!!) via https://github.com/Lobz/facilitation/commit/130ac76e733176c4dd314fcb98b144aada7d28af
  • [ ] fad (0.3-3) -- at somakd/fad, PR #4 filed 2021-04-30
  • [ ] fasteraster (1.1.1) -- not on GH, patch emailed 2021-05-01
  • [x] fixest (0.8.4) -- at lrberge/fixest, PR #138 filed 2021-05-01, merged 2021-05-01
  • [x] FORTLS (1.0.2) -- at Molina-Valero/FORTLS, PR #2 filed 2021-05-01, applied to branch 2021-05-24
  • [x] fracture (0.1.2) -- at rossellhayes/fracture, PR #1 filed 2021-05-01, merged 2021-05-24
  • [x] frailtySurv (1.3.6) -- at vmonaco/frailtySurv, PR #8 filed 2021-05-01, merged 2021-05-04
  • [x] Gmisc (2.0.1) -- at gforge/Gmisc, PR #55 filed 2021-05-02, merged 2021-05-03, on CRAN 2021-05-04
  • [x] huge (1.3.4.1) -- at HMJiangGatech/huge, PR #14 filed 2021-05-02, merged 2021-05-24, CRAN "soon"
  • [x] IFC (0.1.1) -- at gitdemont/IFC, PR #1 filed 2021-05-02, merged 2021-05-06
  • [x] lavacreg (0.1-1) -- at chkiefer/lavacreg, PR #1 filed 2021-05-02, merged 2021-05-02
  • [x] matrixprofiler (0.1.4) -- at matrix-profile-foundation/matrixprofiler, PR #15 filed 2021-05-03; email back 2021-05-23 'merge and CRAN soon', on CRAN 2021-05-26
  • [x] mcmcsae (0.6.0) -- not on GH, patch emailed 2021-05-03, heard back 2021-05-05, merged
  • [x] medfate (1.1.0) -- ~not on GH~ at vegmod/medfate, patch emailed 2021-05-03, heard back 2021-05-04, merged
  • [ ] multivariance (2.4.0) -- not on GH, patch emailed 2021-05-03
  • [x] mwaved (1.1.7) -- at jrwishart/mwaved, PR #2 filed 2021-05-03, merged 2021-05-07
  • [x] nnlib2Rcpp (0.1.6) -- at VNNikolaidis/nnlib2Rcpp, PR #11 file 2021-05-04, merged 2021-05-05, on CRAN 2021-05-05
  • [x] npsf (0.8.0) -- not on GH, patch emailed 2021-05-10, heard back in email 2021-05-24: 'soon'
  • [x] oce (1.4-0) -- at dankelley/oce, PR #1822 filed 2021-05-04, merged 2021-05-05
  • [x] OpenMx (2.19.5) at OpenMx/OpenMx, PR #319 filed 2021-05-04, merged 2021-05-04
  • [x] PAFit (1.0.1.8) at thongphamthe/PAFit, PR #3 filed 2021-05-05, merged 2021-05-11, on CRAN 2021-05-12
  • [ ] partialCI (1.2.0) not on GH, patch emailed 2021-05-05
  • [x] PoissonBinomial (1.2.3) not on GH, patch emailed 2021-05-05, heard back 2021-05-05, "soon"
  • [x] precrec (0.12.5) at evalclass/precrec, PR #17 filed 2021-05-05, merged 2021-05-25
  • [x] rayrender (0.21.2) at tylermorganwall/rayrender, PR #25 filed 2021-05-06, merged 2021-06-25
  • [x] rcosmo (1.1.2) at frycast/rcosmo, PR #3 filed 2021-05-06, merged 2021-05-10
  • [ ] read.gt3x (1.0.2) at THLfi/read.gt3x, PR #39 filed 2021-05-06
  • [x] recmap (1.0.7) at cpanse/recmap, PR #20 filed 2021-05-06, merged 2021-05-07, on CRAN 2021-05-10
  • [x] robustlmm (2.4-3) at kollerma/robustlmm, PR #7 filed 2021-05-07, heard back via GH on 2021-05-24, merged, "soon to CRAN"
  • [x] rpf (1.0.5) at jpritikin/rpf, PR #15 filed 2021-05-07, merged 2021-05-07
  • [x] rstpm2 (1.5.2) at mclements/rstpm2, PR #20 filed 2021-05-07, merged 2021-05-07
  • [x] rtdists (0.11-2) at rtdists/rtdists, PR #14 filed 2021-05-07, merged 2021-05-12
  • [x] shrinkTVP (2.0.1) not on GH, patch emailed 2021-05-08, on CRAN 2021-05-13
  • [x] smam (0.5.4) at ChaoranHu/smam, PR #1 filed 2021-05-08, merged 2021-05-24
  • [ ] supc (0.2.2) at wush978/supc, PR #33 filed 2021-05-08
  • [x] unmarked (1.0.1) -- at rbchan/unmarked, fixed by maintainer in PR #202
  • [x] s2 (1.0.5) -- at r-spatial/s2, PR #118 filed 2021-06-02, merged 2021-06-02
  • [ ] bcf, not on GH, patch emailed 2021-06-10
  • [ ] biglasso, at YaohuiZeng/biglasso, PR #40 filed 2021-06-10
  • [ ] blockmodels, not on GH, patch emailed 2021-06-10
  • [ ] DGM, at schw4b/DGM, PR #14 filed 2021-06-10
  • [ ] EMVS, not on GH, patch emailed 2021-06-10
  • [x] eulerrr, at jolars/eulerr, PR #82 filed 2021-06-10, merged 2021-06-11
  • [ ] fctbases, at naolsen/fctbases, PR #1 files 2021-06-10
  • [ ] GAS, at LeopoldoCatania/GAS PR #14 filed 2021-06-11
  • [x] gcKrig, not on GH, patch emailed 2021-06-11, heard back 2021-06-14: "soon"
  • [x] gfilmm, at stla/gfilmm, PR #1 filed 2021-06-12, merged 2021-06-25
  • [x] goffda, at egarpor/goffda, PR #5 filed 2021-06-12, merged 2021-06-26
  • [x] GWmodel, not on GH, patch emailed 2021-06-13, heard back 2021-06-15: "asap"
  • [x] HDTSA, at Linc2021/HDTSA, PR #1 filed 2021-06-13, merged 2021-06-28
  • [ ] ideq, at eastonhuch/ideq, PR #1 filed 2021-06-13
  • [x] jackalope, at lucasnell/jackalope, PR #15 filed 2021-06-13, merged 2021-06-14
  • [x] ldsep, at dcgerard/ldsep, PR #5 filed 2021-06-13, merged 2021-06-14
  • [ ] lpme, not on GH, patch emailed 2021-06-14
  • [ ] mcse, not on GH, patch emailed 2021-06-14
  • [x] metapack at daeyounglim/metapack, PR #1 files 2021-06-14, merged 2021-06-16
  • [ ] minerva at rsamantha/minerva, PR #11 filed 2021-06-15
  • [ ] MultiFit, not on GH, patch emailed 2021-06-15
  • [ ] parglm, at boennecd/parglm, PR #11 filed 2021-06-15
  • [ ] RobustGaSP, not on GH, patch emailed 2021-06-15
  • [ ] partialAR, not on GH, patch emailed 2021-06-17
  • [x] sdetorus, at egarpor/sdetorus, PR #5 filed 2021-06-17, merged 2021-06-26
  • [x] serrsBayes, at mooresm/serrsBayes, PR #1 files 2021-06-15, merged 2021-06-18
  • [x] lme4, at lme4/lme4, PR #633 filed 2021-06-16, merged 2021-06-16
  • [x] RxODE, at nlmixrdevelopment/RxODE, PR #437 files 2021-06-20, merged 2021-06-21
  • [ ] ClusPred, not at GH, patch emailed 2021-06-27

Most helpful comment

Thank you, fracture 0.1.3 is on its way to CRAN!

All 24 comments

As a preliminary task, I could adapt the code in https://github.com/RcppCore/Rcpp/commit/222a17bd1303e3fbf6736da6e13998d32c2473f8 to download an analyze all these packages to produce a comprehensive list of files and line numbers containing any of these macros.

You could, but it is a royal pain to set a box up with all the build dependencies (see my comments to Duncan on r-package-devel today) and I do have one set up... I can also build on that box, so I am three patch sets in so it is fairly quick (on simple packages). But should we discuss that elsewhere to not dilute this issue ticket?

I merged your pull request Dirk, although I'm not sure why you chose those 2 files, from your comment I presume because I was using DBL_MIN?

Great, thank you!

And yes it breaks without the PR I sent as you can check easily if you want: use e.g. the CRAN sources, add #define STRICT_R_HEADERS and it will fail because switching to STRICT_R_HEADERS no longer includes float.h (or, if one prefers C++ headers) cmath. Which ... strikes me as a bug in R for no longer including it but anyway...

The modern higher-end variant (if one is willing to use C++11 or newer) would be to use std::numeric_limits<double>::min(), maybe even in a constexpr just as we all could just 'compute' the value of \pi at compile-time with constexpr double pi = std::acos(-1);.

The changes I have sent to maintainers so far really were almost always one of these two:

  • PI changes to M_PI (as it has been in C headers since possibly Kernighan and Ritchie themselves, or so it feels to me)
  • including float.h or cfloat so that DBL_MIN and DBL_EPSILON are known; sometime also conversion from DOUBLE_EPS
    so it really is not earth-shattering, and we'll all be better off as ERROR and MESSAGE and other definitions will be gone.

I merged the pull request. Thanks for all your work!

Gmisc merged, thanks! This was incredibly smooth, awesome work!

Thanks for your simple pull. Merged OpenMx

This was simple enough. The related changes will be submitted to CRAN as of 'nnlib2Rcpp' v.0.1.7. Thanks for all your work!

All seems to have worked for "oce". Many thanks, Dirk!

I have just merged the PR in IFC package.
Thanks a lot for Rcpp and your commitment.
Yohann

Merged the PR in mwaved. Thanks for the heads up and for making Rcpp too.

Many thanks for your work. Merged in PAFit.

Thanks, we have merged it in rtdists. Will try to schedule our next CRAN update in July so it will be fixed by then on CRAN. Please let me know (e.g., here) if we need to update on CRAN earlier (it did not look like it).

Great, thanks! And earlier "would be nice" in the sense of me continuing to run tests for this, but for this as a whole we are really looking at the Rcpp release after next so early 2022. I will nag more in the late summer and fall.

@eddelbuettel I've merged your PR for emIRT. Rcpp has been a great resource for many of my projects. Thanks for your amazing work.

First reminder email sent to 24 maintainers:

Four weeks ago, we opened Rcpp GitHub issue #1158 [1] to account
for the 51 reverse dependencies of Rcpp which are affected when
STRICT_R_HEADERS is defined. Please see the write-up at the issue
ticket (link is below) for details and context.

Your package is one of these 51 packages.

We helped all 51 packages by either emailing a sets of patches
or, equivalently, making a pull request. We are happy to report
that 27 (by our count) already responded and merged the patch or
pull request. Some even updated the package on CRAN.

This leaves 24 packages for which we have not yet heard from the
maintainer. Yours is one of these 24.

And that is why we are sending this reminder email to you and the
23 other maintainer in BCC. Please let us know if we can help
you in any way -- beside having set up a patch or pull request
addressing the issue. Which we, of course, tested first.

It would be lovely to have this addressed over the summer. There
is no rush. This does not have to be in time for Rcpp 1.0.7 in July
but for Rcpp 1.0.8 planned for January 2022 leaving us the fall to
finalize and test this. Please let us know if we can count on your
help, and/or if we can assist you.

It is possible that my earlier email did not reach you (as email
appears be become less and less reliable) so I am not sending
this one from a different host. Let's hope it reaches most if not
all of you.

Thanks, Dirk (for the Rcpp Core team)

[1] https://github.com/RcppCore/Rcpp/issues/1158

I'm a bit confused. I followed your PR for oce (in the "develop" branch) very soon after I got the initial contact (see https://github.com/dankelley/oce/commit/623c11ea82414da380043b447f4dfd49026fe789) so I'm not too sure why I am still on the list.

I see that oce is ticked as done in the first comment in this thread.

Maybe your tests of packages remaining to be done are against "master" or "main" branches? (I normally only merge "develop" into "master" in the run-up to a CRAN release. And I'm waiting to time a CRAN release until after the sf package again handles the "ortho" projection, which is broken lately.

Yes, I agree (and thank you for the prompt response on oce), and I do not see you in the list of 24 recipients I compiled. I would rather not post the list publically (could upset someone) though it is based on the 24 'Maintainer' fields of the 24 packages so it is somewhat public. Yours is not in them but maybe-just-maybe one of those recipients forwarded?

(And to clarify: my process was to take the 24 package names, create a vector and make it a data.table object that was then join'ed with tools::CRAN_package_db() on Package, extract Maintainer and collate the 24 Maintainer fields. No Dan Kelley in there. )

Oh, sorry -- I see now that it was just an email from github, not from you. So, it went to anyone following the issue. Please ignore my comment. And thanks again for Rcpp, which is a wonderful help to the community.

Yes, correct, just an email echo of the comment 1 hour ago. There are lots of them; my github folder is by far the most voluminous of my email folders :)

Dear eddelbuettel,

I have impplemented your comments in the branch devel (https://github.com/Molina-Valero/FORTLS/tree/devel). Please, let me know if everything is correct. Thank you very much!

Howdy -- what I emailed worked when I tested it agains the (then-CRAN-current) version of FORTLS so I am fairly confident it will be fine. Thanks for applying the patch! And the commit looks good to me too.

Many thanks for your work. Merged in huge. I will update the package on CRAN asap.

Thank you, fracture 0.1.3 is on its way to CRAN!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

zhiruiwang picture zhiruiwang  路  6Comments

jgellar picture jgellar  路  8Comments

nettoyoussef picture nettoyoussef  路  6Comments

iago-pssjd picture iago-pssjd  路  4Comments

eddelbuettel picture eddelbuettel  路  3Comments