>>>>> Martin Maechler
>>>>> on Fri, 27 Sep 2024 18:47:06 +0200 writes:
>>>>> Ivan Krylov via R-devel
>>>>> on Fri, 27 Sep 2024 13:32:27 +0300 writes:
>> Hello,
>> This problem originally surfaced as an interaction between 'brms',
>> 'rstan' and 'Rcpp' [1]: a call to dimnames() from the 'brms' package on
>> an object of an S4 class owned by the 'rstan' package tried to load its
>> namespace. rstan:::.onLoad needs to load Rcpp modules, which uses load
>> actions and reference classes. Since methods:::.findInheritedMethods
>> temporarily disables primitive S4 dispatch [2], reference classes break
>> and the namespace fails to load. I have prepared a small reproduction
>> package [3], which will need to be installed to show the problem:
>> R -q -s -e "saveRDS(repro::mk_external(), 'foo.rds')"
>> R -q -s -e "readRDS('foo.rds')"
>> # Loading required package: repro
>> # Error: package or namespace load failed for ‘repro’ in
>> # .doLoadActions(where, attach):
>> # error in load action .__A__.1 for package repro: bar$foo(): attempt
>> # to apply non-function
>> # Error in .requirePackage(package) : unable to find required package
>> # ‘repro’
>> # Calls: <Anonymous> ... .findInheritedMethods -> getClass ->
>> # getClassDef -> .requirePackage
>> # Execution halted
>> (Here it has to be a show() call to trigger the package load, not just
>> dimnames().)
>> I have verified that the following patch prevents the failure in
>> loading the namespace, but which other problems could it introduce?
>> Index: src/library/methods/R/RClassUtils.R
>> ===================================================================
>> --- src/library/methods/R/RClassUtils.R (revision 87194)
>> +++ src/library/methods/R/RClassUtils.R (working copy)
>> @@ -1812,6 +1812,9 @@
>> ## real version of .requirePackage
>> ..requirePackage <- function(package, mustFind = TRUE) {
>> + # we may be called from .findInheritedMethods, which disables S4
primitive dispatch
>> + primMethods <- .allowPrimitiveMethods(TRUE)
>> + on.exit(.allowPrimitiveMethods(primMethods))
>> value <- package
>> if(nzchar(package)) {
>> ## lookup as lightning fast as possible:
>> The original change to disable S4 primitive dispatch during method
>> resolution was done in r50609 (2009); this may be the first documented
>> instance of it causing a problem. The comment says "At the moment, this
>> is just for efficiency, but in principle it could be needed to avoid
>> recursive calls to findInheritedMethods."
>> --
>> Best regards,
>> Ivan
> Thank you, Ivan.
> Your patch makes sense indeed, given your previous findings on
> the R-package-devel list ([1]). It is short and looks ok.
> As you mention, speed loss may still be an issue, for S4/methods
> (including reference classes) - using R code.
> I was additionally interested to see your 'repro' package ([3]) and
> try if it does reproduce the problem ... and then if the patch
> resolves it.... and confirm that it does this nicely.
> I think we will get some speed if your new code is replaced by
> if(!.allowPrimitiveMethods(TRUE))
> on.exit(.allowPrimitiveMethods(FALSE))
> which is correct as we know that the argument and return value
> are both either TRUE or FALSE.
> Martin
A version of that has now been committed to "R-devel" (the
development version of R, in svn rev 87202 .
... with thanks to Ivan.
... I expect that the CRAN checks for broom.mixed (e.g.) will no
longer give 'ERROR' once the checks will use svn rev >= 87202.
Martin
______________________________________________
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
