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

[Mark H Weaver]

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


>From 7914328a91d15639399f7c2926da4ae2fd793e84 Mon Sep 17 00:00:00 2001
From: Mark H Weaver <address@hidden>
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)))))
+
 
 
 ;;; {Time Structures}
-- 
1.7.5.4