Bug 16111 - withCallingHandlers() failiing inside S4 dispatch
Summary: withCallingHandlers() failiing inside S4 dispatch
Status: CLOSED FIXED
Alias: None
Product: R
Classification: Unclassified
Component: Low-level (show other bugs)
Version: R-devel (trunk)
Hardware: All All
: P5 normal
Assignee: R-core
URL:
Depends on:
Blocks:
 
Reported: 2014-12-16 10:00 UTC by Martin Maechler
Modified: 2016-01-11 10:40 UTC (History)
2 users (show)

See Also:


Attachments
Short reproducible R example code (544 bytes, text/x-matlab)
2014-12-16 10:00 UTC, Martin Maechler
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Maechler 2014-12-16 10:00:48 UTC
Created attachment 1700 [details]
Short reproducible R example code

This started as problem report about knits not showing warning messages of lme4..
Yihui Xie found that it was related to package 'Matrix' being attached, but of course it is a more general bug where the  calling handlers interfere wrongly with S4 method dispatch   [or S4 method dispatch invalidates warning handling ..].

I've interval searched and the bug is *NOT* in R versions <= 2.11.1
but is there in R version 2.12.0.
I.e. the bug was introduced between April and October 2010 :

R version 2.11.0 (2010-04-22)
R version 2.11.1 (2010-05-31)
R version 2.12.0 (2010-10-15)
Comment 1 Martin Maechler 2014-12-16 10:06:00 UTC
The attachment is nice for testing but may look a bit obfuscated.
Here's more readable code for reading :

withCallingHandlers(.t <- summary(sqrt(-1)), warning=function(w) cat("*** See ?\n"))

## MM:  summary(.) is the crucial part, not Matrix:
setClass("Foo")
setMethod("summary", "Foo", function(object, ...) "summmary(<Foo>)")

withCallingHandlers(.t <- summary(sqrt(-1)), warning=function(w) cat("*** See ?\n"))
## ==> The '*** See ?'  is no longer shown :  The handler failed!


## Indeed -- If I remove the S4 method(s) for "summary" it works again:
removeMethods("summary") ## [1] TRUE
withCallingHandlers(.t <- summary(sqrt(-1)), warning=function(w) cat("*** See ?\n"))
Comment 2 Martin Morgan 2014-12-16 13:43:06 UTC
Setting a generic causes the problem

    f <- function() {
        signal <- FALSE
        withCallingHandlers({
            g(sqrt(-1))
        }, warning=function(w) {
            signal <<- TRUE
            invokeRestart("muffleWarning")
        })
        signal
    }

followed by

    > g <- function(x) x
    > f()
    [1] TRUE
    > setGeneric("g")
    [1] "g"
    > f()
    [1] FALSE
    Warning message:
    In sqrt(-1) : NaNs produced

Setting a breakpoint on Rf_warningcall, the problem is during argument evaluation for method dispatch, when the argument is evaluated at /src/library/methods/src/methods_list_dispatch.c:994

		PROTECT(arg = R_tryEvalSilent(arg_sym, ev, &check_err));

Replacing this with a simple eval(arg_sym, ev) recovers calling handlers; the surrounding code seems mostly designed to provide a more friendly error message when the evaluation fails.
Comment 3 Martin Maechler 2016-01-04 16:15:14 UTC
(In reply to Martin Morgan from comment #2)

 [.......]
> 
> Setting a breakpoint on Rf_warningcall, the problem is during argument
> evaluation for method dispatch, when the argument is evaluated at
> /src/library/methods/src/methods_list_dispatch.c:994

is line 1010 now :
> 
> 		PROTECT(arg = R_tryEvalSilent(arg_sym, ev, &check_err));
> 
> Replacing this with a simple eval(arg_sym, ev) recovers calling handlers;
> the surrounding code seems mostly designed to provide a more friendly error
> message when the evaluation fails.

We should try to fix this. ... but I don't feel competent enough.
R_tryEvalSilent() is said to be a temporary hack suppressing error message printing, and then uses  R_tryEval()  -- both are in src/main/context.c
which is

SEXP
R_tryEval(SEXP e, SEXP env, int *ErrorOccurred) {

    Rboolean ok;
    ProtectedEvalData data;

    data.expression = e;
    data.val = NULL;
    data.env = env;

    ok = R_ToplevelExec(protectedEval, &data);

    .... {ok --> ErrorOccured }

    return data.val;
}

and R_TopLevelExec()  (also in .../context.c ) puts the current handler on a stack and operates in toplevel context so it cannot be left by a long jump
((do not assume that I *really* know what I am talking about :-)) and after evaluation puts the original handler back.... and so the warning happens in the wrong context which is "not handled".

I really have no idea how to do all this properly... and I don't understand the issue well enough to dare just replacing R_tryEvalSilent() here by a simple eval().

If Luke or someone else who understands more about this than me, could have a look...
Comment 4 Martin Maechler 2016-01-05 08:07:38 UTC
Martin's "remedy" seems good enough. Actually even an improvement in such a case:

g <- function(x) x; setGeneric("g")

g(sqrt(""))## in R 2.3.3, gives

Error in g(sqrt("")) :
error in evaluating the argument 'x' in selecting a method for function 'g': Error in sqrt("") : non-numeric argument to mathematical function

## whereas in an R-devel with eval(.) instead of R_tryEvalSilent(.) it simply gives

Error in sqrt("") : non-numeric argument to mathematical function


## and I find the latter rather more helpful than the former...  

--
Also such a change does not break any of our own regression tests.
If nobody takes this thread up within a day or so, I'll commit such a change (leaving the old commented there)