force 'finalize' method to be materialized on Modules

[kevinushey]

Closes #1230.

Checklist

• Code compiles correctly
• R CMD check still passes all tests
• Preferebly, new tests were added which fail without the change
• Document the changes by file in ChangeLog

[eddelbuettel]

Old Emacs roots here: 1 tab or eight spaces, please.

(Not sure editorconfig has a mode for ChangeLog...)

[kevinushey]

Sorry, will fix that up! RStudio wasn't smart enough to infer the indentation here...

[eddelbuettel]

If only we knew someone working on the IDE 😹

[eddelbuettel]

Wow. A one-liner to fix a woe. I presume you played with some Rprintf or REprintf here to see it hits?

[eddelbuettel]

I'll turn the rev.dep machine on. It'll take a few moments as usual.

[kevinushey]

Wow. A one-liner to fix a woe. I presume you played with some Rprintf or REprintf here to see it hits?

Right -- just to confirm, this is the script I was running to exercise the issue.

trace(Rcpp:::cpp_object_initializer, quote({
    writeLines("Hello from C++Object initializer!")
}))

library("terra")
library("raster")
#terra version 1.2.13
fnz <- "nz_elev.rda"
if (!file.exists(fnz)) {
 download.file("https://github.com/Nowosad/spDataLarge/raw/master/data/nz_elev.rda", fnz, mode = "wb")
}
load("nz_elev.rda")
nz <- rast(nz_elev)
plot(nz)

The tracer indeed fires, and the above code reliably prints some errors with the CRAN release of Rcpp, but not with the bandaid from this PR.

Also emphasizing; it's not clear whether this is an issue in Rcpp Modules or something else but this is still the simplest way for us to paper over the issue -- evidently it's been reported in a number of places...

sneumann/xcms#288
https://stackoverflow.com/questions/65556253/r-raster-selffinalize-error-causing-failure
https://discourse.mc-stan.org/t/very-mysterious-debug-error-when-running-rstanarm-rstan-chains-error-in-x-self-finalize-attempt-to-apply-non-function/4746

[eddelbuettel]

Yes. I encountered similar mysterious issues with XPtr once and in one use case ensure I always explicitly set one. It's weird because it generally works. But corner cases exist, so I think this goes the right way.

The rev.dep tests being CI we may not see real side effects. But it being September maybe we can get some user segments (geospatial, rstan, ...) to pretty-please try a rc release from the drat before 1.0.10 gets cases in January (under normal timelines).

[eddelbuettel]

So big thank you to you for digging in here!

[eddelbuettel]

Ran the (slow, long) battery of reverse depends checks (resuls as usual in the rcpp-logs sibbling repo) and "no change to worse" so merging this now.

[eddelbuettel]

I also marked it as 1.0.9.3 and pushed a build into the rcpp-drat so we could reach out to the affected packages and suggest they try this version.

[kevinushey]

Thanks! I'll send a ping on Twitter and see if there's anyone interested in testing.

[rhijmans]

Halleluja. I can confirm that this fixes the issue in "terra" --- at least for the cases that I could reproduce this with