GeezerVison

So, as discussed, and the revdep machine made it clear, we cannot generate a note. Pragma messages are considered statements by gcc (see e.g. here and bugs linked there). Options are: be silent, or generate a warning without replacing.

In order to ask CRAN about what they think, we need to calculate how many packages would generate this warning. An upper bound was provided here by just grepping the sources of all reverse dependencies: 263 packages. From these, only 24 would also fail compilation or checks as documented in #1406. To further refine this list, we could collect the install logs and grep for the pragma (are they in the revdep machine from the previous run?).

This is a version that emits a message instead of a warning, and avoids the problem of inserting two statements. Could we run a revdep with this?

First package to break compilation is BayesProject:

/usr/local/lib/R/site-library/Rcpp/include/Rcpp/macros/mask.h:42:23: error: invalid use of ‘::’ [-Wtemplate-body]
   42 | #define Rf_error(...) Rcpp::internal::Rf_error(__VA_ARGS__)
      |                       ^~~~
/usr/share/R/include/R_ext/Error.h:84:15: note: in expansion of macro ‘Rf_error’
   84 | #define error Rf_error
      |               ^~~~~~~~
/usr/local/lib/R/site-library/RcppEigen/include/Eigen/src/IterativeLinearSolvers/IterativeSolverBase.h:305:14: note: in expansion of macro ‘error’
  305 |   RealScalar error() const
      |              ^~~~~

It's an interaction with Boost Eigen and R 😿 This one also triggers the 'R.h included' message so a cleanup may fix both.

Ah, of course, group 1 in #1406 is expected to fail due to the inclusion of <R.h>. But we want to fix these as soon as possible.

Apologies for the back-and-forth, but the internal template thing didn't work. I thought that the message would be shown only if the template was instantiated, but that's not the case. I thought I tested it correctly, but obviously I did something wrong, because now I see messages all over the place when Rf_error is not there.

So we are back at square 1, meaning two options:

1. Prepare things, no warning yet, but at least compileAttributes will start protecting Rf_error calls. I.e. the PR as it is right know, because I've commented out the warning.
2. Uncomment the warning, so we will start generating warnings in hundreds of packages, although none of them would fail, it's just the warnings. CRAN knows and could "activate" this warning when they see fit.

Let's take (1) and merge this then?

Yes. Let me catch my breath after all the excitement in #1430 and get back to you.

Should we maybe not comment it out (as right now) but make it active behind an 'opt-in' environment variable? Then we could merge now, and we (and Kurt) can run with the variable set. Better?

Sounds good to me. Here it is with an on switch.

Looks like a double env var :) That'll do.

Yes, it's a great idea. CRAN may inject that flag when they're ready to process such warnings. In June the warning may be unconditional (unless disabled explicitly), then next year this may be an error. Thanks, merging here!

So for completeness I ran a reverse-dependency check over what is now in the main branch as 1.1.0.13, and a single package came up with an issue: a std::min( an_int, new_nrow()) balked because we now return R_xlen_t. I made a quick PR for the affected package (see here, I also slipped in a 'vistor' to replace one ATTRIB()) so we should be good to go.