Re: %active-restart
Gary Byers <[email protected]> Thu, 8 Sep 2005 03:11:10 -0600 (MDT)
| Newsgroups | gmane.lisp.openmcl.bugs |
|---|---|
| Message-ID | <[email protected]> |
This seems to have been introduced about a year and a half ago, in an
attempt to fix:
(deftest restart-case.19
(restart-case
(invoke-restart 'foo)
(foo () :test (lambda (c) (declare (ignore c)) nil) 'bad)
(foo () 'good))
good)
To think out loud:
The only callers of CCL::%ACTIVE-RESTART that I can find are INVOKE-RESTART
and INVOKE-RESTART-INTERACTIVELY. What CCL::%ACTIVE-RESTART does is pretty
similar to what FIND-RESTART does (similar enough that those two calls to
CCL::%ACTIVE-RESTART could be replaced with calls to FIND-RESTART; the
description of RESTART-BIND says that FIND-RESTART is used to handle
cases like the test case above.)
FIND-RESTART is used in other contexts as well; it takes an optional
CONDITION argument, which might be null. Let's forget that
CCL::%ACTIVE-RESTART even exists and pretend that INVOKE-RESTART calls
FIND-RESTART with a restart-or-name and a null CONDITION arg.
The spec says that when its CONDITION arg is null, FIND-RESTART
"considers all restarts"; "considering a restart" would seem to mean
"calling its test function with a NULL condition argument". As it's
implemented, FIND-RESTART finds the first matching restart when it
gets a null CONDITION argument, and this seems to be one of the major
differences between FIND-RESTART and Bryan's fix to CCL::%ACTIVE-RESTART.
(I don't read the phrase "considers all restarts" to mean "accepts all
restarts", and the glossary entry for the term "applicable restart"
describes the possibility that a :TEST function could be called with
NIL; CCL::%ACTIVE-RESTART is doing the right thing, and the language
that says that a :TEST function "is a function of a single arg, a
condition ..." should not be read to mean that that function would only
be called with objects of type CONDITION.) See also CLHS 9.1.4.2.3
So, it looks like FIND-RESTART is wrong: when it's called with a null
condition argument, it needs to call the :TEST function with NIL as
an argument for each candidate restart until one of them returns true.
(A missing/unspecified :TEST function always returns true.)
On Wed, 31 Aug 2005, Alan Ruttenberg wrote:
> (restart-bind ((handler (lambda())
> :test-function (lambda(condition) <decide if you want to handle
> this condition based on the type of condition>)
> :interactive-function (lambda() <do something>)
> ))
>
> I think this needs to be called when deciding which restarts to offer, which
> it is, and the restart is offered. But then when the restart is invoked
> %active-restart is called, it calls this again, this time not passing the
> condition. The test fails and you get the inactive-restart error, even though
> the test passed the first time and it is in fact an active restart.
COMPUTE-RESTARTS &OPTIONAL CONDITION returns a list of all restarts if
CONDITION is null and a list of those that're either "associated with
the specified condition" or "not associated with any condition", otherwise.
Being "associated with a condition" doesn't have anything to do with
the restart's :TEST function (it has to do with the context in which
the restart was established by RESTART-CASE or WITH-CONDITION-RESTARTS.)
COMPUTE-RESTARTS is constrained to return "applicable restarts", so
the restart's :TEST functions -do- determine visibility.
The bugs that I see here are that:
a) FIND-RESTART doesn't call :TEST functions when its CONDITION arg is null
b) COMPUTE-RESTARTS may erroneously return restarts whose test-functions
return false (I suspect that it does, but haven't checked.)
c) It's probably more painful than it should be to use :TEST/:TEST-FUNCTION
functions with restarts, since each such function may have to worry
about what it means to be called with NIL.
If (c) is a bug, it's a bug in CL design.
>
> The relevant piece of documentation from the hyperspec entry on restart-bind
> which specifies the test function
I suppose that we can add
d) Some CLHS dictionary entries are a little misleading and have to
be read in the context of other expository material.
I think that you're right that there's a problem here, and I don't
think that COMPUTE-RESTARTS should be offering something that
INVOKE-RESTART will complain about, but I don't think that
%ACTIVE-RESTART is the culprit.