[PATCH] Prevent 'error' from being tail-called, for better diagnostics

[PATCH] Prevent 'error' from being tail-called, for better diagnostics

[Mark H Weaver]

[-- Attachment #1: Type: text/plain, Size: 1424 bytes --]

Hello all,

Paul Smith got a rather unhelpful error message from Guile when he
called 'error' in tail position from one of his procedures, and asked
for input on how to improve error reporting, so I've been looking into
it.

In this case, the error came from a procedure like this:

  (define (to-string-maybe x)
    (cond
     ;; Many clauses here
     (else (error "Unknown object:" x))))

Since 'error' is called in tail position, 'to-string-maybe' is missing
from the backtrace, making the error message rather mysterious.  Of
course a simple fix would be to include the name of the procedure within
the string passed to 'error', but this really shouldn't be necessary.  I
would be rather embarrassed to suggest such a thing.  We should be able
to do much better than this.

When 'error' is called, the user should _automatically_ be told the
exact source location where it was called, and the procedure that called
it should be present in the backtrace.  Therefore, we should somehow
prevent it from being tail-called.

Any ideas on how best to accomplish this?

One idea is to change 'error' into a macro, and use tricks similar to
what 'load' does, to cleverly makes it look like a procedure.  This
macro would, in the common case, force the call to the "real" error
procedure to be done in push context instead of tail context.

Maybe something along the lines of the attached patch.

What do you think?

    Mark



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: [PATCH] Prevent 'error' from being tail-called, for better diagnostics --]
[-- Type: text/x-patch, Size: 1394 bytes --]

From 7914328a91d15639399f7c2926da4ae2fd793e84 Mon Sep 17 00:00:00 2001
From: Mark H Weaver <mhw@netris.org>
Date: Mon, 30 Jan 2012 01:33:08 -0500
Subject: [PATCH] Prevent 'error' from being tail-called, for better
 diagnostics

* module/ice-9/boot-9.scm (error): Rename 'error' to '%%error'.
  'error' is now a variable transformer that calls '%%error' in
  non-tail-position when 'error' is found in operator position.
  When 'error' is found elsewhere, it simply expands to '%%error'.
---
 module/ice-9/boot-9.scm |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/module/ice-9/boot-9.scm b/module/ice-9/boot-9.scm
index c8a56e0..2d3a254 100644
--- a/module/ice-9/boot-9.scm
+++ b/module/ice-9/boot-9.scm
@@ -1187,7 +1187,7 @@ VALUE."
 ;;; {Error Handling}
 ;;;
 
-(define error
+(define %%error
   (case-lambda
     (()
      (scm-error 'misc-error #f "?" #f #f))
@@ -1195,6 +1195,18 @@ VALUE."
      (let ((msg (string-join (cons "~A" (make-list (length args) "~S")))))
        (scm-error 'misc-error #f msg (cons message args) #f)))))
 
+(define-syntax error
+  (make-variable-transformer
+   (lambda (x)
+     (syntax-case x ()
+       ((_ arg ...)
+        #'(call-with-values
+              (lambda () (%%error arg ...))
+            values))
+       (id
+        (identifier? #'id)
+        #'%%error)))))
+
 \f
 
 ;;; {Time Structures}
-- 
1.7.5.4

Re: [PATCH] Prevent 'error' from being tail-called, for better diagnostics

[Andy Wingo]

On Mon 30 Jan 2012 07:39, Mark H Weaver <mhw@netris.org> writes:

> When 'error' is called, the user should _automatically_ be told the
> exact source location where it was called, and the procedure that called
> it should be present in the backtrace.  Therefore, we should somehow
> prevent it from being tail-called.

This is the argument behind R6RS's inclusion of "assert"; see section
11.14.

> One idea is to change 'error' into a macro, and use tricks similar to
> what 'load' does, to cleverly makes it look like a procedure.

Unfortunately this is an incompatible change.  Existing compiled files
which reference the "error" binding expect it to be a procedure, not a
macro.

To provide this source information, a technique like this one might be
applicable: 

  http://funcall.blogspot.com/2009/05/you-knew-id-say-something-part-iv.html

Until we do something to address this issue, we should probably document
this aspect of the behavior of "error".

Andy
-- 
http://wingolog.org/

Re: [PATCH] Prevent 'error' from being tail-called, for better diagnostics

[Mark H Weaver]

Andy Wingo <wingo@pobox.com> writes:

> On Mon 30 Jan 2012 07:39, Mark H Weaver <mhw@netris.org> writes:
>
>> One idea is to change 'error' into a macro, and use tricks similar to
>> what 'load' does, to cleverly makes it look like a procedure.
>
> Unfortunately this is an incompatible change.  Existing compiled files
> which reference the "error" binding expect it to be a procedure, not a
> macro.

Ah, good point.  Damn.

> To provide this source information, a technique like this one might be
> applicable: 
>
>   http://funcall.blogspot.com/2009/05/you-knew-id-say-something-part-iv.html

Thanks for the pointer, that sounds like a great strategy!

     Mark