From: Peter Seibel
Subject: condtion handling question
Date: 
Message-ID: <m3el918231.fsf@localhost.localdomain>
Below is a function, safe-invoke, I wrote to wrap invocation of an
arbitrary function in appropriate condition handling to allow me to
return as multiple values the primary value of the invoked function or
nil if it signals an error and any condition it may signal. (This is
for use in a unit testing framework.) Given that fn can return a value
(call it 'x') or nothing (if it signals an error) and can signal a
warning, an error, or nothing, the possible results for safe-invoke
are shown in this table:

fn returns | fn signals | safe-invoke returns
-----------|------------|--------------------
    x      | warning    | (values x <the warning>)
  nothing  | error      | (values nil <the error>)
    x      | nothing    | (values x nil)

I realize that x could itself be nil. That's okay. 

Anyway, here's the code:

(defun safe-invoke (fn)
  "Invoke fn and return its primary value (or nil) and any error or
warning it signaled. Additionally, if fn signals a warning and the
muffle-warning restart is established, we invoke it."
  (let ((warning nil))
    (handler-bind
     ((warning
       #'(lambda (w)
	   (setq warning w)
	   (let ((muffle (find-restart 'muffle-warning)))
	     (when muffle (invoke-restart muffle)))))
      (error
       #'(lambda (e)
	   (return-from safe-invoke (values nil e)))))
     (values (funcall fn) warning))))

I have a couple questions that came up as I was writing this:

0. Does this code look reasonable--is there some other better, more
idiomatic way to do this? I feel okay about this code--but I tried
several other ways before settling on this, such as using restart-case
to set up a restart that I used to return (values nil e) instead of
using RETURN-FROM.

1. Both the ability to muffle warnings and the fact that they are
printed to the error output are consequences of the way WARN is
implemented, not any magic associated with the WARNING condition type,
correct? After thinking about it a bit I deduced that WARN must be
written something like this (though with better error checking on the
arguments):

(defun warn (datum &rest arguments)
  (restart-case
   (let ((c
          (cond ((symbolp datum)
                 (apply #'make-conditio n datum arguments))
                ((stringp datum)        
                 (make-condition 'simple-warning
                                 :format-control datum
                                 :format-arguments arguments))
                ((typep datum 'condition) datum))))
     (signal c)
     (format *error-output* "Warning: ~A" c))
   (muffle-warning ())))

Does that look about right?

2. Suppose I wanted to change safe-invoke to not discard the
non-primary values of fn. Assuming I wanted to return whatever
multiple values fn returned plus the warning, is there a better way
than:

  (values-list (multiple-value-call #'list (funcall fn) warning))

Thanks.

-Peter

-- 
Peter Seibel
·····@javamonkey.com

From: Kalle Olavi Niemitalo
Subject: Re: condtion handling question
Date: 
Message-ID: <878yz8u9vd.fsf@Astalo.y2000.kon.iki.fi>
Peter Seibel <·····@javamonkey.com> writes:

>   (values-list (multiple-value-call #'list (funcall fn) warning))

  (multiple-value-call #'values (funcall fn) warning)
From: Peter Seibel
Subject: Re: condtion handling question
Date: 
Message-ID: <m3znro74m1.fsf@localhost.localdomain>
>>>>> "Kalle" == Kalle Olavi Niemitalo <···@iki.fi> writes:

    Kalle> Peter Seibel <·····@javamonkey.com> writes:
    >> (values-list (multiple-value-call #'list (funcall fn) warning))

    Kalle>   (multiple-value-call #'values (funcall fn) warning)

Duh. Thanks.

-Peter

-- 
Peter Seibel
·····@javamonkey.com
From: Kent M Pitman
Subject: Re: condtion handling question
Date: 
Message-ID: <sfw65ucnwa5.fsf@shell01.TheWorld.com>
Peter Seibel <·····@javamonkey.com> writes:

> >>>>> "Kalle" == Kalle Olavi Niemitalo <···@iki.fi> writes:
> 
>     Kalle> Peter Seibel <·····@javamonkey.com> writes:
>     >> (values-list (multiple-value-call #'list (funcall fn) warning))
> 
>     Kalle>   (multiple-value-call #'values (funcall fn) warning)
> 
> Duh. Thanks.

I recommend instead:

 (values (multiple-value-list (funcall fn)) warning)

The problem is otherwise that if fn has a variable number of return values,
you'll confuse it.  e.g., a lot of things that mean to return NIL, NIL
just return NIL figuring the second value will default to NIL anyway, and
only return NIL, true in some unusual case.  So if you just get back NIL
you don't know if the second NIL was implied, and by injecting your own
NIL, true you are going to fake something out because they might already have
done

 (multiple-value-bind (result ok-p)
    (f)
   ...)

and they might rewrite this as

 (multiple-value-bind (result ok-p warning)
    (safe-invoke #'f)
   ...)

and it won't give you the expected effect.  You're better off giving them
back a list of the values so that things can be reliably mechanically
rewritten.

 (multiple-value-bind (f-results warning) (safe-invoke #'f)
   (destructuring-bind (&optional result ok-p) f-results
     ...))

It's a little more consing, but it's reliable.

Disclaimer: None of the above is a statement of endorsement of this
whole idea of a safe-invoke function.  I'll have to think more on that
and post something if I have a comment one way or another. I meant
here only to comment on the argument conventions.

Btw, I'm iffy on the question of blindly invoking the muffle-warning
restart without at least a conditional controlling that.  (This gets into
the question of offering keyword args to safe-invoke separate from args
to the function it calls.  But I'd still like a :no-muffle keyword, at least.)
Also, you're definitely invoking FIND-RESTART with no condition argument
and that's VERY VERY bad, since you're risking that in a recursive error 
situation, the MULFFLE-WARNING restart has been set up for a reason other
than the warning that comes out, and that you'll be invoking the wrong
restart.

Further disclaimer: I didn't do a thorough lookthrough of the code. This
just glaringly caught my eye.  There may be other problems.
From: Rob Warnock
Subject: Re: condtion handling question
Date: 
Message-ID: <jXOdneL_LeK8CXGgXTWc3A@giganews.com>
Kent M Pitman <······@world.std.com> wrote:
+---------------
| Peter Seibel <·····@javamonkey.com> writes:
| > >>>>> "Kalle" == Kalle Olavi Niemitalo <···@iki.fi> writes:
| > Kalle>   (multiple-value-call #'values (funcall fn) warning)
| > 
| > Duh. Thanks.
| 
| I recommend instead:
| 
|  (values (multiple-value-list (funcall fn)) warning)
| 
| The problem is otherwise that if fn has a variable number of
| return values, you'll confuse it.
+---------------

One thing that would help with Kalle's version would be to change
the protocol for SAFE-FUNCALL (or whatever the name was) to swap
the position of the warning/error return value with the function's
value(s):

	(multiple-value-call #'values warning (funcall fn))

thus removing the ambiguity. The first value is then *always*
an error-p, and the remaining however-many values (if any) come
from the called function.


-Rob

-----
Rob Warnock, PP-ASEL-IA		<····@rpw3.org>
627 26th Avenue			<URL:http://www.rpw3.org/>
San Mateo, CA 94403		(650)572-2607
From: Peter Seibel
Subject: Re: condtion handling question
Date: 
Message-ID: <m3vg2b7epn.fsf@localhost.localdomain>
>>>>> "Kent" == Kent M Pitman <Kent> writes:

[snip various bits of good advice]

    Kent> It's a little more consing, but it's reliable.

Yeah, I thought briefly about those kinds of issues and decided not to
deal with them for now--thus only returning the primary value. Thanks
for the discussion though.

    Kent> Disclaimer: None of the above is a statement of endorsement
    Kent> of this whole idea of a safe-invoke function. I'll have to
    Kent> think more on that and post something if I have a comment
    Kent> one way or another. I meant here only to comment on the
    Kent> argument conventions.

    Kent> Btw, I'm iffy on the question of blindly invoking the
    Kent> muffle-warning restart without at least a conditional
    Kent> controlling that. (This gets into the question of offering
    Kent> keyword args to safe-invoke separate from args to the
    Kent> function it calls. But I'd still like a :no-muffle keyword,
    Kent> at least.) 

Fair enough. Keep in mind that the only use of safe-invoke is in a
unit testing framework where the function that it's going to invoke is
a test that's going to return true or false to indicate pass or
failure. Or possibly signal an error which will also be treated as a
test failure. If it signals a warning I want to capture and muffle it
because I'm going to take responsibility for reporting it in my test
output. (Though the way the framework works currently, it doesn't
handle multiple warnings well--only the last is reported. Fixing that
is on my TODO list.)

    Kent> Also, you're definitely invoking FIND-RESTART with no
    Kent> condition argument and that's VERY VERY bad, since you're
    Kent> risking that in a recursive error situation, the
    Kent> MULFFLE-WARNING restart has been set up for a reason other
    Kent> than the warning that comes out, and that you'll be invoking
    Kent> the wrong restart.

Is that general advice--i.e. (almost) always use the condition
argument to FIND-RESTART. In what circumstances might one not?

    Kent> Further disclaimer: I didn't do a thorough lookthrough of
    Kent> the code. This just glaringly caught my eye. There may be
    Kent> other problems.

FWIW, here's the complete code of my framework as it stands now. It's
a work in progress and I'm just getting serious about learning Lisp
(quit my job to do it too -- wheee!) so I'd certainly be interested in
any feedback anyone has the time and inclination to give me. (BTW,
those of you who have been paying close attention and have good
memories may recall I posted a *different* test framework a few months
back. This time around I wanted to try a different approach though I
did try to incorporate into this version several bits of feedback
folks were kind enough to give me back then.)

(defpackage "TEST" (:use "COMMON-LISP")
  (:export "DEFTEST"))

(in-package "TEST")

(defvar *test-results* nil)
(defvar *result-stream* t)

(defmacro deftest (name lambda-list &body body)
  "Define a test function that reports pass or fail to *result-stream*"
  (let ((result (gensym))
	(condition (gensym)))
    `(defun ,name ,lambda-list
      (multiple-value-bind (,result ,condition)
	  (safe-invoke #'(lambda () ,@body))
	(format-result
	 *result-stream*
	 ,result
	 (list ',name ,@lambda-list)
	 ,condition)))))

(defun safe-invoke (fn)
  "Invoke fn and return its primary value (or nil) and any error or
warning it signaled. Additionally, if fn signals a warning and the
muffle-warning restart is established, we invoke it."
  (let ((warning nil))
    (handler-bind
     ((warning
       #'(lambda (w)
	   (setq warning w)
	   (let ((muffle (find-restart 'muffle-warning w)))
	     (when muffle (invoke-restart muffle)))))
      (error
       #'(lambda (e)
	   (return-from safe-invoke (values nil e)))))
     (values (funcall fn) warning))))

(defun format-result (stream result form condition)
  "Format the results of a single test in a form like:

  PASS ... (the form)
  PASS ... (the form) (warning: warning message)
  FAIL ... (the form)
  FAIL ... (the form) (warning: warning message)
  FAIL ... (the form) (error: error message)
"
  (format stream
	  "~&~:[FAIL~;PASS~] ... ···@[ (~1*~:[error~;warning~]: ~-2*~A)~]"
	  result form condition (typep condition 'warning)))



-Peter

-- 
Peter Seibel
·····@javamonkey.com
From: Kent M Pitman
Subject: Re: condtion handling question
Date: 
Message-ID: <sfwu1hvj7z4.fsf@shell01.TheWorld.com>
Peter Seibel <·····@javamonkey.com> writes:

>     Kent> Also, you're definitely invoking FIND-RESTART with no
>     Kent> condition argument and that's VERY VERY bad, since you're
>     Kent> risking that in a recursive error situation, the
>     Kent> MULFFLE-WARNING restart has been set up for a reason other
>     Kent> than the warning that comes out, and that you'll be invoking
>     Kent> the wrong restart.
> 
> Is that general advice--i.e. (almost) always use the condition
> argument to FIND-RESTART. In what circumstances might one not?

None that I know of.

The condition argument was added later to fix some bugs.  For backward
compatibility, we didn't require the argument.  That was probably wrong.
I recommend always supplying it.  I know of no case where this provides
superior results.

I do think it's sometimes reasonable to do COMPUTE-RESTARTS with no
condition argument, but the usual case there is for interactive use,
where the interactive user probably knows if there are several pending
conditions and is capable of making an informed decision to restart an
outer situation, bypassing the need to debug an inner one.

But ordinarily, for programmatic work, a handler sought on behalf of a
particular condition should act only on behalf of that condition, or so
I claim -- if there are counterpoint opinions, I'd love to hear them. I
doubt there are, though.