trace examples broken in master

trace examples broken in master

[Julian Graham]

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

Hi Guilers,

I went to try out some of the traps / tracing features in the debugger
last night, and I ran into some compilation issues with `(ice-9
debugging ice-9-debugger-extensions)'.  A patch that resolves them is
attached -- I haven't pushed because I'm not sure I fully understand
the broader context of the code (e.g., why are we checking the version
when the debugger is now part of Guile itself?), and because I'm still
a little git-shy.

More importantly, though, tracing (as per the examples in section
5.21.3.5 of the manual) doesn't seem to produce any output.  Not sure
where to look to figure that out.


Regards,
Julian

[-- Attachment #2: 0001-Fix-compilation-issue-in-ice-9-debugger-extensions.patch --]
[-- Type: text/x-patch, Size: 1893 bytes --]

From 8751fedfe28eb94cb2974aeb5a2a08310c03d942 Mon Sep 17 00:00:00 2001
Message-Id: <8751fedfe28eb94cb2974aeb5a2a08310c03d942.1247165262.git.julian.graham@aya.yale.edu>
From: Julian Graham <julian.graham@aya.yale.edu>
Date: Thu, 9 Jul 2009 14:44:53 -0400
Subject: [PATCH] Fix compilation issue in ice-9-debugger-extensions

* module/ice-9/debugging/ice-9-debugger-extensions.scm: Replace `define' in cond expressions.
---
 .../ice-9/debugging/ice-9-debugger-extensions.scm  |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/module/ice-9/debugging/ice-9-debugger-extensions.scm b/module/ice-9/debugging/ice-9-debugger-extensions.scm
index a8b8c97..df4f4a6 100644
--- a/module/ice-9/debugging/ice-9-debugger-extensions.scm
+++ b/module/ice-9/debugging/ice-9-debugger-extensions.scm
@@ -32,10 +32,10 @@
 	 #:use-module (ice-9 debugger)
 	 #:use-module (ice-9 debugger state)
 	 #:use-module (ice-9 debugging traps))
-       (define new-define-command define-command)
-       (set! define-command
-	     (lambda (name argument-template documentation procedure)
-	       (new-define-command name argument-template procedure))))
+       (let ((new-define-command define-command))
+	 (set! define-command
+	       (lambda (name argument-template documentation procedure)
+		 (new-define-command name argument-template procedure)))))
       (else
        (define-module (ice-9 debugger))))
 
@@ -126,8 +126,9 @@ print the result obtained."
 
 (cond ((string>=? (version) "1.7"))
       (else
-       (define (debugger-command-loop state)
-	 (read-and-dispatch-commands state (current-input-port)))))
+       (set! debugger-command-loop 
+	     (lambda (state)
+	       (read-and-dispatch-commands state (current-input-port))))))
 
 (define-public (debug-trap trap-context)
   "Invoke the Guile debugger to explore the stack at the specified @var{trap}."
-- 
1.6.0.4

Re: trace examples broken in master

[Neil Jerram]

Julian Graham <joolean@gmail.com> writes:

> Hi Guilers,

Hi Julian,

I'm afraid this is a case of `snap!'...  I've been looking into these
things too and have patches that I hope to commit shortly.

> I went to try out some of the traps / tracing features in the debugger
> last night, and I ran into some compilation issues with `(ice-9
> debugging ice-9-debugger-extensions)'.  A patch that resolves them is
> attached -- I haven't pushed because I'm not sure I fully understand
> the broader context of the code (e.g., why are we checking the version
> when the debugger is now part of Guile itself?), and because I'm still
> a little git-shy.

You're completely right.  The version stuff is historical and no
longer needed in master, so we should definitely blow it away.  In
fact the same applies to the whole existence of
ice-9-debugger-extensions.  (It was all about wanting to support 1.6
and 1.8 from a single set of guile-debugging sources.)

> More importantly, though, tracing (as per the examples in section
> 5.21.3.5 of the manual) doesn't seem to produce any output.  Not sure
> where to look to figure that out.

This is to do with decorate-source in psyntax.scm using
set-source-properties!, and with the scm_set_source_properties_x ()
code being completely wrong.

Here's a rewriting that works:

(define (decorate-source e s)
  (if (and (pair? e) s)
      (let loop ((specials '(breakpoint line column copy filename)) (s s))
	(or (null? specials)
	    (let ((special (assq (car specials) s)))
	      (if special
		  (begin
		    (set-source-property! e (car special) (cdr special))
		    (loop (cdr specials) (delq! special s)))
		  (loop (cdr specials) s))))))
  e)

but the right thing to do is to fix the scm_set_source_properties_x ()
code.

Regards,
        Neil

Re: trace examples broken in master

[Andy Wingo]

On Tue 14 Jul 2009 10:07, Neil Jerram <neil@ossau.uklinux.net> writes:

> but the right thing to do is to fix the scm_set_source_properties_x ()
> code.

Yes, this would be better.

Andy
-- 
http://wingolog.org/

Re: trace examples broken in master

[Neil Jerram]

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

Andy Wingo <wingo@pobox.com> writes:

> On Tue 14 Jul 2009 10:07, Neil Jerram <neil@ossau.uklinux.net> writes:
>
>> but the right thing to do is to fix the scm_set_source_properties_x ()
>> code.
>
> Yes, this would be better.

Here are my proposed changes for that, for master.  Please let me know
of any comments.

Unfortunately too late now for 1.9.2, but I'll commit (subject to
comments) after that release.

    Neil


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Minor-improvements-to-doc-on-source-properties.patch --]
[-- Type: text/x-diff, Size: 2416 bytes --]

From 4448d1b02ab6724d05a95ba3e15ca662fce43d50 Mon Sep 17 00:00:00 2001
From: Neil Jerram <neil@ossau.uklinux.net>
Date: Tue, 4 Aug 2009 15:40:06 +0100
Subject: [PATCH] Minor improvements to doc on source properties

In particular avoid any suggestion that the API uses the
property list format, i.e. (key1 value1 key2 value2 ...),
as opposed to the alist format that it actually does use.
---
 doc/ref/api-debug.texi |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/doc/ref/api-debug.texi b/doc/ref/api-debug.texi
index 7886366..dbf3c00 100644
--- a/doc/ref/api-debug.texi
+++ b/doc/ref/api-debug.texi
@@ -283,9 +283,9 @@ runs a script non-interactively.
 The following procedures can be used to access and set the source
 properties of read expressions.
 
-@deffn {Scheme Procedure} set-source-properties! obj plist
-@deffnx {C Function} scm_set_source_properties_x (obj, plist)
-Install the association list @var{plist} as the source property
+@deffn {Scheme Procedure} set-source-properties! obj alist
+@deffnx {C Function} scm_set_source_properties_x (obj, alist)
+Install the association list @var{alist} as the source property
 list for @var{obj}.
 @end deffn
 
@@ -302,12 +302,12 @@ Return the source property association list of @var{obj}.
 
 @deffn {Scheme Procedure} source-property obj key
 @deffnx {C Function} scm_source_property (obj, key)
-Return the source property specified by @var{key} from
-@var{obj}'s source property list.
+Return the property specified by @var{key} from @var{obj}'s source
+properties.
 @end deffn
 
 In practice there are only two ways that you should use the ability to
-set an expression's source breakpoints.
+set an expression's source properties.
 
 @itemize
 @item
@@ -330,9 +330,9 @@ involved in a backtrace or error report.
 
 If you are looking for a way to attach arbitrary information to an
 expression other than these properties, you should use
-@code{make-object-property} instead (@pxref{Object Properties}), because
-that will avoid bloating the source property hash table, which is really
-only intended for the specific purposes described in this section.
+@code{make-object-property} instead (@pxref{Object Properties}).  That
+will avoid bloating the source property hash table, which is really
+only intended for the debugging purposes just described.
 
 
 @node Decoding Memoized Source Expressions
-- 
1.5.6.5


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-In-srcprop.c-change-all-occurrences-of-plist-to-a.patch --]
[-- Type: text/x-diff, Size: 8583 bytes --]

From 2e2fdcdef595bb7c4969783390399ae3ddbe6bab Mon Sep 17 00:00:00 2001
From: Neil Jerram <neil@ossau.uklinux.net>
Date: Tue, 4 Aug 2009 18:54:50 +0100
Subject: [PATCH] In srcprop.c change all occurrences of "plist" to "alist"

As with the previous commit, this is to avoid any suggestion that
the source properties API uses the property list format, i.e.
(key1 value1 key2 value2 ...).

Also remove scm_srcprops_to_plist () from the API.  It doesn't have any
external usefulness and has never documented.
---
 libguile/srcprop.c |   76 ++++++++++++++++++++++++++-------------------------
 libguile/srcprop.h |    2 -
 2 files changed, 39 insertions(+), 39 deletions(-)

diff --git a/libguile/srcprop.c b/libguile/srcprop.c
index efa0b7f..a43f4ce 100644
--- a/libguile/srcprop.c
+++ b/libguile/srcprop.c
@@ -68,7 +68,7 @@ SCM_GLOBAL_SYMBOL (scm_sym_breakpoint, "breakpoint");
  * car = tag
  * cbr = pos
  * ccr = copy
- * cdr = plist 
+ * cdr = alist 
  */
 
 #define SRCPROPSP(p) (SCM_SMOB_PREDICATE (scm_tc16_srcprops, (p)))
@@ -77,7 +77,7 @@ SCM_GLOBAL_SYMBOL (scm_sym_breakpoint, "breakpoint");
 #define SRCPROPLINE(p) (SRCPROPPOS(p) >> 12)
 #define SRCPROPCOL(p) (SRCPROPPOS(p) & 0x0fffL)
 #define SRCPROPCOPY(p) (SCM_CELL_OBJECT(p,2))
-#define SRCPROPPLIST(p) (SCM_CELL_OBJECT_3(p))
+#define SRCPROPALIST(p) (SCM_CELL_OBJECT_3(p))
 #define SETSRCPROPBRK(p) \
  (SCM_SET_SMOB_FLAGS ((p), \
                       SCM_SMOB_FLAGS (p) | SCM_SOURCE_PROPERTY_FLAG_BREAK))
@@ -89,9 +89,11 @@ SCM_GLOBAL_SYMBOL (scm_sym_breakpoint, "breakpoint");
 #define SETSRCPROPLINE(p, l) SETSRCPROPPOS (p, l, SRCPROPCOL (p))
 #define SETSRCPROPCOL(p, c) SETSRCPROPPOS (p, SRCPROPLINE (p), c)
 #define SETSRCPROPCOPY(p, c) (SCM_SET_CELL_WORD(p, 2, c))
-#define SETSRCPROPPLIST(p, l) (SCM_SET_CELL_WORD(p, 3, l))
+#define SETSRCPROPALIST(p, l) (SCM_SET_CELL_WORD(p, 3, l))
 
 
+static SCM scm_srcprops_to_alist (SCM obj);
+
 
 scm_t_bits scm_tc16_srcprops;
 
@@ -99,7 +101,7 @@ static SCM
 srcprops_mark (SCM obj)
 {
   scm_gc_mark (SRCPROPCOPY (obj));
-  return SRCPROPPLIST (obj);
+  return SRCPROPALIST (obj);
 }
 
 static int
@@ -108,7 +110,7 @@ srcprops_print (SCM obj, SCM port, scm_print_state *pstate)
   int writingp = SCM_WRITINGP (pstate);
   scm_puts ("#<srcprops ", port);
   SCM_SET_WRITINGP (pstate, 1);
-  scm_iprin1 (scm_srcprops_to_plist (obj), port, pstate);
+  scm_iprin1 (scm_srcprops_to_alist (obj), port, pstate);
   SCM_SET_WRITINGP (pstate, writingp);
   scm_putc ('>', port);
   return 1;
@@ -124,57 +126,57 @@ scm_c_source_property_breakpoint_p (SCM form)
 
 
 /*
- * We remember the last file name settings, so we can share that plist
+ * We remember the last file name settings, so we can share that alist
  * entry.  This works because scm_set_source_property_x does not use
- * assoc-set! for modifying the plist.
+ * assoc-set! for modifying the alist.
  *
  * This variable contains a protected cons, whose cdr is the cached
- * plist
+ * alist
  */
-static SCM scm_last_plist_filename;
+static SCM scm_last_alist_filename;
 
 SCM
-scm_make_srcprops (long line, int col, SCM filename, SCM copy, SCM plist)
+scm_make_srcprops (long line, int col, SCM filename, SCM copy, SCM alist)
 {
   if (!SCM_UNBNDP (filename))
     {
-      SCM old_plist = plist;
+      SCM old_alist = alist;
 
       /*
 	have to extract the acons, and operate on that, for
 	thread safety.
        */
-      SCM last_acons = SCM_CDR (scm_last_plist_filename);
-      if (old_plist == SCM_EOL
+      SCM last_acons = SCM_CDR (scm_last_alist_filename);
+      if (old_alist == SCM_EOL
 	  && SCM_CDAR (last_acons) == filename)
 	{
-	  plist = last_acons;
+	  alist = last_acons;
 	}
       else
 	{
-	  plist = scm_acons (scm_sym_filename, filename, plist);
-	  if (old_plist == SCM_EOL)
-	    SCM_SETCDR (scm_last_plist_filename, plist);
+	  alist = scm_acons (scm_sym_filename, filename, alist);
+	  if (old_alist == SCM_EOL)
+	    SCM_SETCDR (scm_last_alist_filename, alist);
 	}
     }
   
   SCM_RETURN_NEWSMOB3 (scm_tc16_srcprops,
 		       SRCPROPMAKPOS (line, col),
 		       copy,
-		       plist);
+		       alist);
 }
 
 
-SCM
-scm_srcprops_to_plist (SCM obj)
+static SCM
+scm_srcprops_to_alist (SCM obj)
 {
-  SCM plist = SRCPROPPLIST (obj);
+  SCM alist = SRCPROPALIST (obj);
   if (!SCM_UNBNDP (SRCPROPCOPY (obj)))
-    plist = scm_acons (scm_sym_copy, SRCPROPCOPY (obj), plist);
-  plist = scm_acons (scm_sym_column, scm_from_int (SRCPROPCOL (obj)), plist);
-  plist = scm_acons (scm_sym_line, scm_from_int (SRCPROPLINE (obj)), plist);
-  plist = scm_acons (scm_sym_breakpoint, scm_from_bool (SRCPROPBRK (obj)), plist);
-  return plist;
+    alist = scm_acons (scm_sym_copy, SRCPROPCOPY (obj), alist);
+  alist = scm_acons (scm_sym_column, scm_from_int (SRCPROPCOL (obj)), alist);
+  alist = scm_acons (scm_sym_line, scm_from_int (SRCPROPLINE (obj)), alist);
+  alist = scm_acons (scm_sym_breakpoint, scm_from_bool (SRCPROPBRK (obj)), alist);
+  return alist;
 }
 
 SCM_DEFINE (scm_source_properties, "source-properties", 1, 0, 0, 
@@ -190,7 +192,7 @@ SCM_DEFINE (scm_source_properties, "source-properties", 1, 0, 0,
     SCM_WRONG_TYPE_ARG (1, obj);
   p = scm_hashq_ref (scm_source_whash, obj, SCM_EOL);
   if (SRCPROPSP (p))
-    return scm_srcprops_to_plist (p);
+    return scm_srcprops_to_alist (p);
   else
     /* list from set-source-properties!, or SCM_EOL for not found */
     return p;
@@ -200,8 +202,8 @@ SCM_DEFINE (scm_source_properties, "source-properties", 1, 0, 0,
 /* Perhaps this procedure should look through an alist
    and try to make a srcprops-object...? */
 SCM_DEFINE (scm_set_source_properties_x, "set-source-properties!", 2, 0, 0,
-            (SCM obj, SCM plist),
-	    "Install the association list @var{plist} as the source property\n"
+            (SCM obj, SCM alist),
+	    "Install the association list @var{alist} as the source property\n"
 	    "list for @var{obj}.")
 #define FUNC_NAME s_scm_set_source_properties_x
 {
@@ -211,9 +213,9 @@ SCM_DEFINE (scm_set_source_properties_x, "set-source-properties!", 2, 0, 0,
     obj = SCM_MEMOIZED_EXP (obj);
   else if (!scm_is_pair (obj))
     SCM_WRONG_TYPE_ARG(1, obj);
-  handle = scm_hashq_create_handle_x (scm_source_whash, obj, plist);
-  SCM_SETCDR (handle, plist);
-  return plist;
+  handle = scm_hashq_create_handle_x (scm_source_whash, obj, alist);
+  SCM_SETCDR (handle, alist);
+  return alist;
 }
 #undef FUNC_NAME
 
@@ -231,15 +233,15 @@ SCM_DEFINE (scm_source_property, "source-property", 2, 0, 0,
     SCM_WRONG_TYPE_ARG (1, obj);
   p = scm_hashq_ref (scm_source_whash, obj, SCM_EOL);
   if (!SRCPROPSP (p))
-    goto plist;
+    goto alist;
   if      (scm_is_eq (scm_sym_breakpoint, key)) p = scm_from_bool (SRCPROPBRK (p));
   else if (scm_is_eq (scm_sym_line,       key)) p = scm_from_int (SRCPROPLINE (p));
   else if (scm_is_eq (scm_sym_column,     key)) p = scm_from_int (SRCPROPCOL (p));
   else if (scm_is_eq (scm_sym_copy,       key)) p = SRCPROPCOPY (p);
   else
     {
-      p = SRCPROPPLIST (p);
-    plist:
+      p = SRCPROPALIST (p);
+    alist:
       p = scm_assoc (key, p);
       return (SCM_NIMP (p) ? SCM_CDR (p) : SCM_BOOL_F);
     }
@@ -315,7 +317,7 @@ SCM_DEFINE (scm_set_source_property_x, "set-source-property!", 3, 0, 0,
   else
     {
       if (SRCPROPSP (p))
-	SETSRCPROPPLIST (p, scm_acons (key, datum, SRCPROPPLIST (p)));
+	SETSRCPROPALIST (p, scm_acons (key, datum, SRCPROPALIST (p)));
       else
 	SCM_WHASHSET (scm_source_whash, h, scm_acons (key, datum, p));
     }
@@ -334,7 +336,7 @@ scm_init_srcprop ()
   scm_source_whash = scm_make_weak_key_hash_table (scm_from_int (2047));
   scm_c_define ("source-whash", scm_source_whash);
 
-  scm_last_plist_filename
+  scm_last_alist_filename
     = scm_permanent_object (scm_cons (SCM_EOL,
 				      scm_acons (SCM_EOL, SCM_EOL, SCM_EOL)));
 
diff --git a/libguile/srcprop.h b/libguile/srcprop.h
index 2a27e04..89063be 100644
--- a/libguile/srcprop.h
+++ b/libguile/srcprop.h
@@ -64,13 +64,11 @@ SCM_API SCM scm_sym_breakpoint;
 \f
 
 SCM_API int scm_c_source_property_breakpoint_p (SCM form);
-SCM_API SCM scm_srcprops_to_plist (SCM obj);
 SCM_API SCM scm_make_srcprops (long line, int col, SCM fname, SCM copy, SCM plist);
 SCM_API SCM scm_source_property (SCM obj, SCM key);
 SCM_API SCM scm_set_source_property_x (SCM obj, SCM key, SCM datum);
 SCM_API SCM scm_source_properties (SCM obj);
 SCM_API SCM scm_set_source_properties_x (SCM obj, SCM props);
-SCM_API void scm_finish_srcprop (void);
 SCM_INTERNAL void scm_init_srcprop (void);
 
 #if SCM_ENABLE_DEPRECATED == 1
-- 
1.5.6.5


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-Fix-set-source-properties-so-that-the-special-source.patch --]
[-- Type: text/x-diff, Size: 4834 bytes --]

From bcc4242438744e9357fff90da249e6b5fe99bb6b Mon Sep 17 00:00:00 2001
From: Neil Jerram <neil@ossau.uklinux.net>
Date: Tue, 4 Aug 2009 18:57:18 +0100
Subject: [PATCH] Fix set-source-properties so that the special source properties work

* libguile/srcprop.c (scm_set_source_properties_x): Look for the special
  source properties, save them off, and then construct a srcprops object
  using them.
---
 libguile/srcprop.c            |   63 +++++++++++++++++++++++++++++++++++++++++
 test-suite/tests/srcprop.test |   42 ++++++++++++++++++++++++++-
 2 files changed, 104 insertions(+), 1 deletions(-)

diff --git a/libguile/srcprop.c b/libguile/srcprop.c
index a43f4ce..c182702 100644
--- a/libguile/srcprop.c
+++ b/libguile/srcprop.c
@@ -208,11 +208,74 @@ SCM_DEFINE (scm_set_source_properties_x, "set-source-properties!", 2, 0, 0,
 #define FUNC_NAME s_scm_set_source_properties_x
 {
   SCM handle;
+  long line = 0, col = 0;
+  SCM fname = SCM_UNDEFINED, copy = SCM_UNDEFINED, breakpoint = SCM_BOOL_F;
+  SCM others = SCM_EOL;
+  SCM *others_cdrloc = &others;
+  int need_srcprops = 0;
+  SCM tail, key;
+
   SCM_VALIDATE_NIM (1, obj);
   if (SCM_MEMOIZEDP (obj))
     obj = SCM_MEMOIZED_EXP (obj);
   else if (!scm_is_pair (obj))
     SCM_WRONG_TYPE_ARG(1, obj);
+
+  tail = alist;
+  while (!scm_is_null (tail))
+    {
+      key = SCM_CAAR (tail);
+      if (scm_is_eq (key, scm_sym_line))
+	{
+	  line = scm_to_long (SCM_CDAR (tail));
+	  need_srcprops = 1;
+	}
+      else if (scm_is_eq (key, scm_sym_column))
+	{
+	  col = scm_to_long (SCM_CDAR (tail));
+	  need_srcprops = 1;
+	}
+      else if (scm_is_eq (key, scm_sym_filename))
+	{
+	  fname = SCM_CDAR (tail);
+	  need_srcprops = 1;
+	}
+      else if (scm_is_eq (key, scm_sym_copy))
+	{
+	  copy = SCM_CDAR (tail);
+	  need_srcprops = 1;
+	}
+      else if (scm_is_eq (key, scm_sym_breakpoint))
+	{
+	  breakpoint = SCM_CDAR (tail);
+	  need_srcprops = 1;
+	}
+      else
+	{
+	  /* Do we allocate here, or clobber the caller's alist?
+
+	     Source properties aren't supposed to be used for anything
+	     except the special properties above, so the mainline case
+	     is that we never execute this else branch, and hence it
+	     doesn't matter much.
+
+	     We choose allocation here, as that seems safer.
+	  */
+	  *others_cdrloc = scm_cons (scm_cons (key, SCM_CDAR (tail)),
+				     SCM_EOL);
+	  others_cdrloc = SCM_CDRLOC (*others_cdrloc);
+	}
+      tail = SCM_CDR (tail);
+    }
+  if (need_srcprops)
+    {
+      alist = scm_make_srcprops (line, col, fname, copy, others);
+      if (scm_is_true (breakpoint))
+	SETSRCPROPBRK (alist);
+    }
+  else
+    alist = others;
+
   handle = scm_hashq_create_handle_x (scm_source_whash, obj, alist);
   SCM_SETCDR (handle, alist);
   return alist;
diff --git a/test-suite/tests/srcprop.test b/test-suite/tests/srcprop.test
index 8ec2989..17d8ae2 100644
--- a/test-suite/tests/srcprop.test
+++ b/test-suite/tests/srcprop.test
@@ -36,11 +36,51 @@
       (not (null? (source-properties s))))))
 
 ;;;
+;;; set-source-property!
+;;;
+
+(with-test-prefix "set-source-property!"
+  (read-enable 'positions)
+
+  (pass-if "setting the breakpoint property works"
+    (let ((s (read (open-input-string "(+ 3 4)"))))
+      (set-source-property! s 'breakpoint #t)
+      (let ((current-trap-opts (evaluator-traps-interface))
+	    (current-debug-opts (debug-options-interface))
+	    (trap-called #f))
+	(trap-set! enter-frame-handler (lambda _ (set! trap-called #t)))
+	(trap-enable 'traps)
+	(debug-enable 'debug)
+	(debug-enable 'breakpoints)
+	(with-traps (lambda ()
+		      (primitive-eval s)))
+	(evaluator-traps-interface current-trap-opts)
+	(debug-options-interface current-debug-opts)
+	trap-called))))
+
+;;;
 ;;; set-source-properties!
 ;;;
 
 (with-test-prefix "set-source-properties!"
   (read-enable 'positions)
+
+  (pass-if "setting the breakpoint property works"
+    (let ((s (read (open-input-string "(+ 3 4)"))))
+      (set-source-properties! s '((breakpoint #t)))
+      (let ((current-trap-opts (evaluator-traps-interface))
+	    (current-debug-opts (debug-options-interface))
+	    (trap-called #f))
+	(trap-set! enter-frame-handler (lambda _ (set! trap-called #t)))
+	(trap-enable 'traps)
+	(debug-enable 'debug)
+	(debug-enable 'breakpoints)
+	(with-traps (lambda ()
+		      (primitive-eval s)))
+	(evaluator-traps-interface current-trap-opts)
+	(debug-options-interface current-debug-opts)
+	trap-called)))
+
   (let ((s (read (open-input-string "(1 . 2)"))))
     
     (with-test-prefix "copied props"
@@ -48,7 +88,7 @@
 	(let ((t (cons 3 4)))
 	  (set-source-properties! t (source-properties s))
 	  (number? (source-property t 'line))))
-      
+
       (pass-if "visible to source-properties"
 	(let ((t (cons 3 4)))
 	  (set-source-properties! t (source-properties s))
-- 
1.5.6.5


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: 0004-Remove-trailing-whitespace.patch --]
[-- Type: text/x-diff, Size: 650 bytes --]

From 3027b104541ccc65dace5c100efcd041a1e5ffc1 Mon Sep 17 00:00:00 2001
From: Neil Jerram <neil@ossau.uklinux.net>
Date: Tue, 4 Aug 2009 19:11:21 +0100
Subject: [PATCH] Remove trailing whitespace

---
 libguile/srcprop.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/libguile/srcprop.c b/libguile/srcprop.c
index c182702..8fa0393 100644
--- a/libguile/srcprop.c
+++ b/libguile/srcprop.c
@@ -68,7 +68,7 @@ SCM_GLOBAL_SYMBOL (scm_sym_breakpoint, "breakpoint");
  * car = tag
  * cbr = pos
  * ccr = copy
- * cdr = alist 
+ * cdr = alist
  */
 
 #define SRCPROPSP(p) (SCM_SMOB_PREDICATE (scm_tc16_srcprops, (p)))
-- 
1.5.6.5

Re: trace examples broken in master

[Julian Graham]

Hey Neil,

Thanks for addressing this.  Looking forward to trying it out -- it'll
be a great help.


Regards,
Julian


On Fri, Aug 14, 2009 at 1:00 PM, Neil Jerram<neil@ossau.uklinux.net> wrote:
> Andy Wingo <wingo@pobox.com> writes:
>
>> On Tue 14 Jul 2009 10:07, Neil Jerram <neil@ossau.uklinux.net> writes:
>>
>>> but the right thing to do is to fix the scm_set_source_properties_x ()
>>> code.
>>
>> Yes, this would be better.
>
> Here are my proposed changes for that, for master.  Please let me know
> of any comments.
>
> Unfortunately too late now for 1.9.2, but I'll commit (subject to
> comments) after that release.
>
>    Neil
>
>

Re: trace examples broken in master

[Neil Jerram]

Hi Julian!

Neil Jerram <neil@ossau.uklinux.net> writes:

> You're completely right.  The version stuff is historical and no
> longer needed in master, so we should definitely blow it away.  In
> fact the same applies to the whole existence of
> ice-9-debugger-extensions.  (It was all about wanting to support 1.6
> and 1.8 from a single set of guile-debugging sources.)

[...]

> but the right thing to do is to fix the scm_set_source_properties_x ()
> code.

This should all be fixed in master now.  Can you have a go and let me
know if you still see any problems?

Thanks!
        Neil

Re: trace examples broken in master

[Julian Graham]

Hi Neil,

Ack, sorry for not replying sooner -- I've been occupied with
switching jobs and moving apartments.


> This should all be fixed in master now.  Can you have a go and let me
> know if you still see any problems?

Just built from HEAD.  The errors I reported earlier are gone, but I'm
still not getting any trace output from the `rev' example in the
manual.


Regards,
Julian

Re: trace examples broken in master

[Neil Jerram]

Julian Graham <joolean@gmail.com> writes:

>> This should all be fixed in master now.  Can you have a go and let me
>> know if you still see any problems?
>
> Just built from HEAD.  The errors I reported earlier are gone, but I'm
> still not getting any trace output from the `rev' example in the
> manual.

I'm sorry, I didn't actually try running those examples myself, and I
still haven't done that yet now - but just a thought: did you try with 

  ,o interp #t

so as to use the evaluator instead of the compiler + VM ?  It's
absolutely expected that all the trap stuff doesn't work yet with the
VM.  (It's on my list!)

     Neil

Re: trace examples broken in master

[Neil Jerram]

Neil Jerram <neil@ossau.uklinux.net> writes:

> Julian Graham <joolean@gmail.com> writes:
>
>>> This should all be fixed in master now.  Can you have a go and let me
>>> know if you still see any problems?
>>
>> Just built from HEAD.  The errors I reported earlier are gone, but I'm
>> still not getting any trace output from the `rev' example in the
>> manual.
>
> I'm sorry, I didn't actually try running those examples myself, and I
> still haven't done that yet now - but just a thought: did you try with 
>
>   ,o interp #t
>
> so as to use the evaluator instead of the compiler + VM ?

It works for me with current Git and `,o interp #t'; full transcript
below.  Note that this is even when all of the debugging infrastructure
modules are auto-compiled; the only code that must _not_ be compiled,
for that example to work, is the `rev' procedure.

There is an unexpected "warning: stack count incorrect!", though.  I'll
look into that.

    Neil


Guile Scheme interpreter 0.5 on Guile 1.9.2
Copyright (C) 2001-2008 Free Software Foundation, Inc.

Enter `,help' for help.
scheme@(guile-user)> (version)
"1.9.2"
scheme@(guile-user)> ,o interp #t
scheme@(guile-user)> (version)
"1.9.2"
scheme@(guile-user)> (define (rev ls)
         (if (null? ls)
             ls
             (append (rev (cdr ls))
                     (list (car ls)))))
scheme@(guile-user)> (use-modules (ice-9 debugging traps) (ice-9 debugging trace))
;;; note: autocompilation is enabled, set GUILE_AUTO_COMPILE=0
;;;       or pass the --no-autocompile argument to disable.
;;; compiling /usr/local/share/guile/1.9/ice-9/debugging/traps.scm
;;; compiling /usr/local/share/guile/1.9/ice-9/debugging/trc.scm
;;; compiled /home/neil/.cache/guile/ccache/1.9-0.D-LE-4/usr/local/share/guile/1.9/ice-9/debugging/trc.scm.go
;;; compiled /home/neil/.cache/guile/ccache/1.9-0.D-LE-4/usr/local/share/guile/1.9/ice-9/debugging/traps.scm.go
;;; compiling /usr/local/share/guile/1.9/ice-9/debugging/trace.scm
;;; compiling /usr/local/share/guile/1.9/ice-9/debugger/command-loop.scm
;;; compiling /usr/local/share/guile/1.9/ice-9/debugger/commands.scm
;;; compiling /usr/local/share/guile/1.9/ice-9/debugger/state.scm
;;; compiled /home/neil/.cache/guile/ccache/1.9-0.D-LE-4/usr/local/share/guile/1.9/ice-9/debugger/state.scm.go
;;; compiling /usr/local/share/guile/1.9/ice-9/debugger/utils.scm
;;; compiled /home/neil/.cache/guile/ccache/1.9-0.D-LE-4/usr/local/share/guile/1.9/ice-9/debugger/utils.scm.go
;;; compiling /usr/local/share/guile/1.9/ice-9/debugging/steps.scm
;;; compiled /home/neil/.cache/guile/ccache/1.9-0.D-LE-4/usr/local/share/guile/1.9/ice-9/debugging/steps.scm.go
;;; compiled /home/neil/.cache/guile/ccache/1.9-0.D-LE-4/usr/local/share/guile/1.9/ice-9/debugger/commands.scm.go
;;; compiled /home/neil/.cache/guile/ccache/1.9-0.D-LE-4/usr/local/share/guile/1.9/ice-9/debugger/command-loop.scm.go
;;; compiled /home/neil/.cache/guile/ccache/1.9-0.D-LE-4/usr/local/share/guile/1.9/ice-9/debugging/trace.scm.go
scheme@(guile-user)> (define t1 (make <procedure-trap>
                    #:procedure rev
                    #:behaviour (list trace-trap
                                      trace-at-exit)))
scheme@(guile-user)> rev
#<procedure rev (#{ls\ 37}#)>
scheme@(guile-user)> (install-trap t1)
scheme@(guile-user)> (rev '(a b c))
|  2: [rev (a b c)]
warning: stack count incorrect!
|  2: =>(#@if (null? #{ls\ 37}#) #{ls\ 37}# (append (rev (cdr #{ls\ 37}#)) (list (car #{ls\ 37}#))))
|  3: [rev (b c)]
|  4: [rev (c)]
|  5: [rev ()]
|  5: =>()
|  4: =>(c)
|  3: =>(c b)
(c b a)
scheme@(guile-user)> 

Re: trace examples broken in master

[Neil Jerram]

Neil Jerram <neil@ossau.uklinux.net> writes:

> There is an unexpected "warning: stack count incorrect!", though.  I'll
> look into that.

It is fixed (i.e. doesn't occur) with the following changes.

diff --git a/libguile/stacks.c b/libguile/stacks.c
index 45566ca..849a9c7 100644
--- a/libguile/stacks.c
+++ b/libguile/stacks.c
@@ -143,6 +143,11 @@ stack_depth (scm_t_debug_frame *dframe, scm_t_ptrdiff offset, SCM vmframe,
 	{
 	  scm_t_debug_info *info = RELOC_INFO (dframe->info, offset);
 	  scm_t_debug_info *vect = RELOC_INFO (dframe->vect, offset);
+	  /* If current frame is a macro during expansion, we should
+	     skip the previously recorded macro transformer
+	     application frame.  */
+	  if (SCM_MACROEXPP (*dframe) && n > 0)
+	    --n;
 	  n += (info - vect) / 2 + 1;
 	  /* Data in the apply part of an eval info frame comes from previous
 	     stack frame if the scm_t_debug_info vector is overflowed. */

Here n is the running stack depth.  Missing this decrement will cause
the "stack count incorrect" warning but is actually benign, as it only
means that scm_make_stack allocates a bit more space for the stack than
it really needs.

@@ -178,7 +183,10 @@ stack_depth (scm_t_debug_frame *dframe, scm_t_ptrdiff offset, SCM vmframe,
             ++n; /* increment for non-program apply frame */
         }
       else
-	++n;
+	{
+	  abort ();
+	  ++n;
+	}
     }
   if (dframe && SCM_VOIDFRAMEP (*dframe))
     *id = RELOC_INFO(dframe->vect, offset)[0].id;

I'm pretty sure this last else branch is unhittable, hence the added
abort ().

@@ -282,6 +290,7 @@ read_frames (scm_t_debug_frame *dframe, scm_t_ptrdiff offset,
 	    {
 	      *(iframe - 1) = *iframe;
 	      --iframe;
+	      ++n;
 	    }
 	  info =  RELOC_INFO (dframe->info, offset);
 	  vect =  RELOC_INFO (dframe->vect, offset);

Here n is the number of frames still unused in the allocated stack
object.  Missing this increment will cause the "stack count incorrect"
warning, and will make read_frames think it's filled up all the
available stack frames when there's actually still one frame unused;
it's not benign, because it can cause information to be missing from a
stack trace.  (But unfortunately it doesn't appear to be the fix for the
missing VM frames problem.)

     Neil

Re: trace examples broken in master

[Julian Graham]

Hi Neil,

> It works for me with current Git and `,o interp #t'; full transcript
> below.  Note that this is even when all of the debugging infrastructure
> modules are auto-compiled; the only code that must _not_ be compiled,
> for that example to work, is the `rev' procedure.

I've cleared my .cache directory and just finished doing a build from
master.  My transcript now looks exactly like yours, except that, as
before, there's no trace output.  Anything else I need to set up
locally?

Re: trace examples broken in master

[Neil Jerram]

Julian Graham <joolean@gmail.com> writes:

> I've cleared my .cache directory and just finished doing a build from
> master.  My transcript now looks exactly like yours, except that, as
> before, there's no trace output.  Anything else I need to set up
> locally?

Not that I'm aware of!  I just tried the transcript again, with current
master; no change from what I posted before, except that the version is
now 1.9.3, and the stack count incorrect warning has gone.

One detail that I didn't say before: I'm starting Guile by running
`meta/guile -q' from the root directory of my working tree.  It's
possible, if you are starting Guile differently, that you could be
picking up either libraries or Scheme modules from outside the tree, and
hence which might not be up to date.

       Neil

Re: trace examples broken in master

[Julian Graham]

On Tue, Sep 22, 2009 at 3:57 PM, Neil Jerram <neil@ossau.uklinux.net> wrote:
> Not that I'm aware of!  I just tried the transcript again, with current
> master; no change from what I posted before, except that the version is
> now 1.9.3, and the stack count incorrect warning has gone.
>
> One detail that I didn't say before: I'm starting Guile by running
> `meta/guile -q' from the root directory of my working tree.  It's
> possible, if you are starting Guile differently, that you could be
> picking up either libraries or Scheme modules from outside the tree, and
> hence which might not be up to date.


Argh, yes, that was it.  Thanks!  I'll clean up my environment.


Regards,
Julian

Re: trace examples broken in master

[Neil Jerram]

Julian Graham <joolean@gmail.com> writes:

> Argh, yes, that was it.  Thanks!  I'll clean up my environment.

Phew.  Otherwise I was out of ideas...

    Neil