[Openmcl-devel] INVOKE-RESTART bug

[Madhu]

To suggest another idiom to address the problem:

* Gary Byers <alpine.BSF.2.00.1006030359060.26934 at abq.clozure.com> :
Wrote on Thu, 3 Jun 2010 05:24:52 -0600 (MDT):

| The restart isn't applicable: when called with NIL as an argument, its
| test function returns NIL.
|
| INVOKE-RESTART is supposed to signal a CONTROL-ERROR if the restart
| isn't valid.  Some implementations (LispWorks and SBCL) that I looked
| at appear to consider inapplicable restarts to be valid; others
| (Allegro, CLISP, and CCL) don't.  I lean towards believing that the
| latter interpretation is correct (both because it's what I'm used to
| and because I find the concept of "valid, but inapplicable restarts" a
| little hard to believe in), but the language that the spec uses seems
| unfortunately vague.  (The term "valid restart" doesn't seem to be
| defined anywhere; interpreting "valid restart" to mean "active, though
| not necessarily applicable restart" isn't totally unreasonable,
| either.
|
| The problem with that latter interpretation is that a test function
| has to return T if called with NIL as an argument in order to be
| considered applicable.  (It's not clear that restart test functions
| that don't accept NIL are particularly useful in that interpretation,
| but the somewhat awkward idiom:
|
|   :test (lambda (c) (or (null c) (typep c 'some-condition)))
|
| is likely necessary in practice, given that implementations behave
| differently when the (NULL C) case returns false.

As parallel subthreads show, this idiom is not desirable and wrong for
several reasons.  However Stelian's concern as a library writer is
valid: one wishes to use the restart TEST mechanism to ensure that the
IGNORE-FOO restart will be called as only as a consequence of
FOO-ERRORs.  It may be possible to do this portably, by adding to CL the
concept of a "Current Condition Being Handled".  Now the test would look
like

      :test (lambda (c) (typep (or c *last-condition*) 'foo-error))


First let me sketch how I would modify Stelian's example:

(defvar *last-condition* nil)

(define-condition foo-error (error) ())

(defun signal-foo ()
  (restart-case (error 'foo-error)
    (ignore-foo ()
      :report "Ignore FOO error"
      :test (lambda (c) (typep (or c *last-condition*) 'foo-error))
      'yay)))

(defun trigger-bug ()
  (handler-bind ((foo-error
		  (lambda (e &aux (*last-condition* e))
		    (invoke-restart (find-restart 'ignore-foo e)))))
    (signal-foo)))


This idiom works when the library writer explicitly controls the call to
INVOKE-RESTART.  Of course this idiom will NOT WORK when INVOKE-RESTART
is called via an implementation's interactive debugger, unless
*LAST-CONDITION* is suitably bound.

I suggested this, the last time something related came up in this
comp.lang.lisp thread in Sep 08: `Subject: Re: [Q] passing
non-interactive arguments to a restart from the debugger'

(setq *debugger-hook*
	     ;; Date: Tue, 16 Sep 2008 23:50:48 +0530
	     ;; Message-ID: <m3iqswc5q7.fsf at moon.robolove.meer.net>
       (let ((orig-debugger-hook *debugger-hook*))
	  (lambda (condition debugger-hook)
	      (declare (ignore debugger-hook))
	      (setq *last-condition* condition)
	      (when orig-debugger-hook
		 (funcall orig-debugger-hook condition orig-debugger-hook)))))


As noted in that cll thread, some lisps already stash the "current
condition", what I called *LAST-CONDITION*.  In CMUCL It is bound to
DEBUG::*DEBUG-CONDITION*.  Note CMUCL behaviour in this context differs
from CCL behaviour: INVOKE-RESTART in the interactive debugger skips the
"active" check, but even CMUCL's behaviour is not reliable if one were
using CMUCL with SLIME, with SLIME's debugger you would face the
"Non-active restart" problem all over again.  When using SLIME with this
idiom, I would have to do:

(define-symbol-macro *last-condition* swank::*swank-debugger-condition*)

Who would have thought CL would need "design patterns"

|> When a restart FOO has a non-trivial test, (invoke-restart
|> (find-restart foo) won't work because invoke-restart calls the
|> restart's test which will return NIL, and invoke-restart signals a
|> "Restart ... is not active"

--
Madhu