unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* debugger-toggle-locals
@ 2013-11-23  8:57 Helmut Eller
  2013-11-24 14:13 ` debugger-toggle-locals Stefan Monnier
  2013-12-01 22:47 ` debugger-toggle-locals Stefan Monnier
  0 siblings, 2 replies; 5+ messages in thread
From: Helmut Eller @ 2013-11-23  8:57 UTC (permalink / raw)
  To: emacs-devel

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

I'd like to add a new debugger command `debugger-toggle-locals', that
displays the names and values of local variables of a stack frame.
This looks like so:

  Debugger entered--Lisp error: (void-variable d)
    foo(10 113)
    foo(9 114)
      x: 9
      y: 114
      z: (9 . 114)
    foo(8 115)

The command inserts the lines with x:, y: and z:. Pressing t a second
time deletes those lines again.

To do this I needed a new primitive `backtrace-locals' which extracts
the names and values from the specpdl, similar to backtrace-eval.  This
works for interpreted functions and compiled non-lexically-scoped
functions.  Compiled lexically scoped functions would need compiler
support, in the form of stackmaps.  Until somebody implements that, the
command only displays "no locals".

Helmut


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: toggle-locals.patch --]
[-- Type: text/x-diff, Size: 6005 bytes --]

diff --git a/lisp/emacs-lisp/debug.el b/lisp/emacs-lisp/debug.el
index 6c7a0d2..d8596aa 100644
--- a/lisp/emacs-lisp/debug.el
+++ b/lisp/emacs-lisp/debug.el
@@ -494,9 +494,13 @@ removes itself from that hook."
       (forward-line 1)
       (while (progn
 	       (forward-char 2)
-	       (if (= (following-char) ?\()
-		   (forward-sexp 1)
-		 (forward-sexp 2))
+	       (cond ((debugger--locals-visible-p)
+		      (goto-char (next-single-char-property-change
+				  (1+ (point)) 'debugger-locals-visible-p)))
+		     ((= (following-char) ?\()
+		      (forward-sexp 1))
+		     (t
+		      (forward-sexp 2)))
 	       (forward-line 1)
 	       (<= (point) opoint))
 	(if (looking-at " *;;;")
@@ -558,6 +562,89 @@ The environment used is the one when entering the activation frame at point."
             (prin1 val t)
           (let ((str (eval-expression-print-format val)))
             (if str (princ str t))))))))
+
+(defun debugger--backtrace-base ()
+  "Return the function name that marks the top of the backtrace.
+See `backtrace-frame'."
+  (cond ((eq 'debug--implement-debug-on-entry
+	     (cadr (backtrace-frame 1 'debug)))
+	 'debug--implement-debug-on-entry)
+	(t 'debug)))
+
+(defun debugger--locals-visible-p ()
+  "Are the local variables of the current stack frame visible?"
+  (save-excursion
+    (move-to-column 2)
+    (get-text-property (point) 'debugger-locals-visible-p)))
+
+(defun debugger--insert-locals (locals)
+  "Insert the local variables LOCALS at point."
+  ;; A bit messy because we need to deal with lexical frames, i.e.
+  ;; the ones with the magic symbol 'internal-interpreter-environment.
+  (when (null locals)
+    (insert "\n    [no locals]"))
+  (dolist (s+v locals)
+    (let ((symbol (car s+v))
+	  (value (cdr s+v))
+	  (print-escape-newlines t)
+	  (insert (lambda (s v)
+		    (insert "\n    ")
+		    (prin1 s (current-buffer))
+		    (insert ": ")
+		    (prin1 v (current-buffer)))))
+      (cond ((string= (symbol-name symbol) "internal-interpreter-environment")
+	     (cond ((or (null value)
+			(and (null (cdr value))
+			     (symbol (car value))))
+		    (insert "\n    [no locals]"))
+		   (t
+		    (dolist (s+v value)
+		      (unless (symbolp s+v)
+			(let ((symbol (car s+v))
+			      (value (cdr s+v)))
+			  (funcall insert symbol value)))))))
+	    (t
+	     (funcall insert symbol value))))))
+
+(defun debugger--show-locals ()
+  "For the frame at point, insert locals and add text properties."
+  (let* ((nframe (debugger-frame-number 'skip-base))
+	 (base (debugger--backtrace-base))
+	 (locals (backtrace-locals nframe base))
+	 (inhibit-read-only t))
+    (save-excursion
+      (let ((start (progn
+		     (beginning-of-line)
+		     (skip-chars-forward " ")
+		     (point))))
+	(end-of-line)
+	(debugger--insert-locals locals)
+	(add-text-properties start (point) '(debugger-locals-visible-p t))))))
+
+(defun debugger--hide-locals ()
+  "Delete local variables and remove the text property."
+  (let* ((col (current-column))
+	 (end (progn
+		(move-to-column 2)
+		(next-single-char-property-change
+		 (point) 'debugger-locals-visible-p)))
+	 (start (previous-single-char-property-change
+		 end 'debugger-locals-visible-p))
+	 (inhibit-read-only t))
+    (goto-char start)
+    (end-of-line)
+    (remove-text-properties start end '(debugger-locals-visible-p))
+    (delete-region (point) end)
+    (move-to-column col)))
+
+(defun debugger-toggle-locals ()
+  "Show or hide local variables of the current stack frame."
+  (interactive)
+  (cond ((debugger--locals-visible-p)
+	 (debugger--hide-locals))
+	(t
+	 (debugger--show-locals))))
+
 \f
 (defvar debugger-mode-map
   (let ((map (make-keymap))
@@ -575,6 +662,7 @@ The environment used is the one when entering the activation frame at point."
     (define-key map "h" 'describe-mode)
     (define-key map "q" 'top-level)
     (define-key map "e" 'debugger-eval-expression)
+    (define-key map "t" 'debugger-toggle-locals)
     (define-key map " " 'next-line)
     (define-key map "R" 'debugger-record-expression)
     (define-key map "\C-m" 'debug-help-follow)
diff --git a/src/eval.c b/src/eval.c
index d3fcec5..a455d95 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -3576,6 +3576,53 @@ NFRAMES and BASE specify the activation frame to use, as in `backtrace-frame'.
      from the debugger.  */
   return unbind_to (count, eval_sub (exp));
 }
+
+DEFUN ("backtrace-locals", Fbacktrace_locals, Sbacktrace_locals, 1, 2, NULL,
+       doc: /* Return names and values of local variables of a stack frame.
+NFRAMES and BASE specify the activation frame to use, as in `backtrace-frame'.
+The result is an alist. */)
+  (Lisp_Object nframes, Lisp_Object base)
+{
+  union specbinding *frame = get_backtrace_frame (nframes, base);
+  union specbinding *olderframe =
+    get_backtrace_frame (make_number (XFASTINT (nframes) + 1), base);
+  ptrdiff_t distance = specpdl_ptr - olderframe;
+  Lisp_Object result = Qnil;
+  eassert (distance >= 0);
+
+  if (!backtrace_p (frame))
+    error ("Activation frame not found!");
+  if (!backtrace_p (olderframe))
+    error ("Activation frame not found!");
+
+  /* move values to specpdl */
+  backtrace_eval_unrewind (distance);
+
+  /* grab values */
+  {
+    union specbinding *tmp = frame;
+    for (; tmp > olderframe; tmp--)
+      {
+	switch (tmp->kind)
+	  {
+	  case SPECPDL_LET:
+	  case SPECPDL_LET_DEFAULT:
+	  case SPECPDL_LET_LOCAL:
+	    {
+	      Lisp_Object sym = specpdl_symbol (tmp);
+	      Lisp_Object val = specpdl_old_value (tmp);
+	      result = Fcons (Fcons (sym, val), result);
+	    }
+	  }
+      }
+  }
+
+  /* restore values from specpdl to orignal place */
+  backtrace_eval_unrewind (-distance);
+
+  return result;
+}
+
 \f
 void
 mark_specpdl (void)
@@ -3824,6 +3871,7 @@ alist of active lexical bindings.  */);
   defsubr (&Sbacktrace);
   defsubr (&Sbacktrace_frame);
   defsubr (&Sbacktrace_eval);
+  defsubr (&Sbacktrace_locals);
   defsubr (&Sspecial_variable_p);
   defsubr (&Sfunctionp);
 }

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: debugger-toggle-locals
  2013-11-23  8:57 debugger-toggle-locals Helmut Eller
@ 2013-11-24 14:13 ` Stefan Monnier
  2013-12-01 22:47 ` debugger-toggle-locals Stefan Monnier
  1 sibling, 0 replies; 5+ messages in thread
From: Stefan Monnier @ 2013-11-24 14:13 UTC (permalink / raw)
  To: Helmut Eller; +Cc: emacs-devel

> I'd like to add a new debugger command `debugger-toggle-locals', that
> displays the names and values of local variables of a stack frame.

Haven't had time to look at the patch yet, but: sounds great!


        Stefan



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: debugger-toggle-locals
  2013-11-23  8:57 debugger-toggle-locals Helmut Eller
  2013-11-24 14:13 ` debugger-toggle-locals Stefan Monnier
@ 2013-12-01 22:47 ` Stefan Monnier
  2013-12-02 11:04   ` debugger-toggle-locals Helmut Eller
  1 sibling, 1 reply; 5+ messages in thread
From: Stefan Monnier @ 2013-12-01 22:47 UTC (permalink / raw)
  To: Helmut Eller; +Cc: emacs-devel

>       x: 9
>       y: 114
>       z: (9 . 114)

Nitpick: I'd rather have "x=" instead of "x:" (for me, ":" means "has type").

> +(defun debugger--backtrace-base ()
> +  "Return the function name that marks the top of the backtrace.
> +See `backtrace-frame'."
> +  (cond ((eq 'debug--implement-debug-on-entry
> +	     (cadr (backtrace-frame 1 'debug)))
> +	 'debug--implement-debug-on-entry)
> +	(t 'debug)))

Please use it in debugger-eval-expression as well.

> +(defun debugger--locals-visible-p ()
> +  "Are the local variables of the current stack frame visible?"
> +  (save-excursion
> +    (move-to-column 2)
> +    (get-text-property (point) 'debugger-locals-visible-p)))

The text property shouldn't have a name that ends in `-p'; these names
are for predicates (i.e. functions returning a boolean), not for
variables or object fields, or symbol/text properties.

> +  (dolist (s+v locals)
> +    (let ((symbol (car s+v))
> +	  (value (cdr s+v))

You can use (pcase-dolist (`(,symbol . ,value) locals) ...).

> +      (cond ((string= (symbol-name symbol) "internal-interpreter-environment")
> +	     (cond ((or (null value)
> +			(and (null (cdr value))
> +			     (symbol (car value))))
                              ^^^^^^
                              should be `symbolp', right?

`value' can also be of the form (a b t).
I.e. (null (cdr value)) is false, but there are no values.

But I tend to think that the handling of
internal-interpreter-environment should be kept in the C code: this
symbol should by and large not be exported to Elisp.

> +  backtrace_eval_unrewind (distance);
[...]
> +  /* restore values from specpdl to orignal place */
> +  backtrace_eval_unrewind (-distance);

It's kind of annoying that we have to modify the backtrace in order to
build the result, since the function is otherwise side-effect-free.
It's admittedly simpler to do it as above rather than "doing it right".


        Stefan



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: debugger-toggle-locals
  2013-12-01 22:47 ` debugger-toggle-locals Stefan Monnier
@ 2013-12-02 11:04   ` Helmut Eller
  2013-12-02 14:46     ` debugger-toggle-locals Stefan Monnier
  0 siblings, 1 reply; 5+ messages in thread
From: Helmut Eller @ 2013-12-02 11:04 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

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

On Sun, Dec 01 2013, Stefan Monnier wrote:

>>       x: 9
>>       y: 114
>>       z: (9 . 114)
>
> Nitpick: I'd rather have "x=" instead of "x:" (for me, ":" means "has type").

Well, it's a debugger for Lisp not for ML.

>> +(defun debugger--backtrace-base ()
>> +  "Return the function name that marks the top of the backtrace.
>> +See `backtrace-frame'."
>> +  (cond ((eq 'debug--implement-debug-on-entry
>> +	     (cadr (backtrace-frame 1 'debug)))
>> +	 'debug--implement-debug-on-entry)
>> +	(t 'debug)))
>
> Please use it in debugger-eval-expression as well.

OK.

>> +(defun debugger--locals-visible-p ()
>> +  "Are the local variables of the current stack frame visible?"
>> +  (save-excursion
>> +    (move-to-column 2)
>> +    (get-text-property (point) 'debugger-locals-visible-p)))
>
> The text property shouldn't have a name that ends in `-p'; these names
> are for predicates (i.e. functions returning a boolean), not for
> variables or object fields, or symbol/text properties.

OK.

>> +  (dolist (s+v locals)
>> +    (let ((symbol (car s+v))
>> +	  (value (cdr s+v))
>
> You can use (pcase-dolist (`(,symbol . ,value) locals) ...).

So many ` , . cause cancer of the semicolon.

>> + (cond ((string= (symbol-name symbol)
>> "internal-interpreter-environment")
>> +	     (cond ((or (null value)
>> +			(and (null (cdr value))
>> +			     (symbol (car value))))
>                               ^^^^^^
>                               should be `symbolp', right?
>
> `value' can also be of the form (a b t).
> I.e. (null (cdr value)) is false, but there are no values.
>
> But I tend to think that the handling of
> internal-interpreter-environment should be kept in the C code: this
> symbol should by and large not be exported to Elisp.

Done. Moved it to C.

Helmut


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: locals.patch --]
[-- Type: text/x-diff, Size: 6511 bytes --]

diff --git a/lisp/emacs-lisp/debug.el b/lisp/emacs-lisp/debug.el
index aa5b25b..ce2616c 100644
--- a/lisp/emacs-lisp/debug.el
+++ b/lisp/emacs-lisp/debug.el
@@ -494,9 +494,13 @@ removes itself from that hook."
       (forward-line 1)
       (while (progn
 	       (forward-char 2)
-	       (if (= (following-char) ?\()
-		   (forward-sexp 1)
-		 (forward-sexp 2))
+	       (cond ((debugger--locals-visible-p)
+		      (goto-char (next-single-char-property-change
+				  (point) 'locals-visible)))
+		     ((= (following-char) ?\()
+		      (forward-sexp 1))
+		     (t
+		      (forward-sexp 2)))
 	       (forward-line 1)
 	       (<= (point) opoint))
 	(if (looking-at " *;;;")
@@ -541,6 +545,14 @@ Applies to the frame whose line point is on in the backtrace."
         (progn ,@body)
       (setq debugger-outer-match-data (match-data)))))
 
+(defun debugger--backtrace-base ()
+  "Return the function name that marks the top of the backtrace.
+See `backtrace-frame'."
+  (cond ((eq 'debug--implement-debug-on-entry
+	     (cadr (backtrace-frame 1 'debug)))
+	 'debug--implement-debug-on-entry)
+	(t 'debug)))
+
 (defun debugger-eval-expression (exp &optional nframe)
   "Eval an expression, in an environment like that outside the debugger.
 The environment used is the one when entering the activation frame at point."
@@ -549,15 +561,70 @@ The environment used is the one when entering the activation frame at point."
   (let ((nframe (or nframe
                     (condition-case nil (1+ (debugger-frame-number 'skip-base))
                       (error 0)))) ;; If on first line.
-         (base (if (eq 'debug--implement-debug-on-entry
-                      (cadr (backtrace-frame 1 'debug)))
-                  'debug--implement-debug-on-entry 'debug)))
+	(base (debugger--backtrace-base)))
     (debugger-env-macro
       (let ((val (backtrace-eval exp nframe base)))
         (prog1
             (prin1 val t)
           (let ((str (eval-expression-print-format val)))
             (if str (princ str t))))))))
+
+(defun debugger--locals-visible-p ()
+  "Are the local variables of the current stack frame visible?"
+  (save-excursion
+    (move-to-column 2)
+    (get-text-property (point) 'locals-visible)))
+
+(defun debugger--insert-locals (locals)
+  "Insert the local variables LOCALS at point."
+  (cond ((null locals)
+	 (insert "\n    [no locals]"))
+	(t
+	 (let ((print-escape-newlines t))
+	   (dolist (s+v locals)
+	     (let ((symbol (car s+v))
+		   (value (cdr s+v)))
+	       (insert "\n    ")
+	       (prin1 symbol (current-buffer))
+	       (insert " = ")
+	       (prin1 value (current-buffer))))))))
+
+(defun debugger--show-locals ()
+  "For the frame at point, insert locals and add text properties."
+  (let* ((nframe (1+ (debugger-frame-number 'skip-base)))
+	 (base (debugger--backtrace-base))
+	 (locals (backtrace-locals nframe base))
+	 (inhibit-read-only t))
+    (save-excursion
+      (let ((start (progn
+		     (move-to-column 2)
+		     (point))))
+	(end-of-line)
+	(debugger--insert-locals locals)
+	(add-text-properties start (point) '(locals-visible t))))))
+
+(defun debugger--hide-locals ()
+  "Delete local variables and remove the text property."
+  (let* ((col (current-column))
+	 (end (progn
+		(move-to-column 2)
+		(next-single-char-property-change (point) 'locals-visible)))
+	 (start (previous-single-char-property-change end 'locals-visible))
+	 (inhibit-read-only t))
+    (remove-text-properties start end '(locals-visible))
+    (goto-char start)
+    (end-of-line)
+    (delete-region (point) end)
+    (move-to-column col)))
+
+(defun debugger-toggle-locals ()
+  "Show or hide local variables of the current stack frame."
+  (interactive)
+  (cond ((debugger--locals-visible-p)
+	 (debugger--hide-locals))
+	(t
+	 (debugger--show-locals))))
+
 \f
 (defvar debugger-mode-map
   (let ((map (make-keymap))
@@ -575,6 +642,7 @@ The environment used is the one when entering the activation frame at point."
     (define-key map "h" 'describe-mode)
     (define-key map "q" 'top-level)
     (define-key map "e" 'debugger-eval-expression)
+    (define-key map "t" 'debugger-toggle-locals)
     (define-key map " " 'next-line)
     (define-key map "R" 'debugger-record-expression)
     (define-key map "\C-m" 'debug-help-follow)
diff --git a/src/eval.c b/src/eval.c
index d3fcec5..b538a40 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -3576,6 +3576,66 @@ NFRAMES and BASE specify the activation frame to use, as in `backtrace-frame'.
      from the debugger.  */
   return unbind_to (count, eval_sub (exp));
 }
+
+DEFUN ("backtrace-locals", Fbacktrace_locals, Sbacktrace_locals, 1, 2, NULL,
+       doc: /* Return names and values of local variables of a stack frame.
+NFRAMES and BASE specify the activation frame to use, as in `backtrace-frame'.
+The result is an alist. */)
+  (Lisp_Object nframes, Lisp_Object base)
+{
+  union specbinding *frame = get_backtrace_frame (nframes, base);
+  union specbinding *prevframe =
+    get_backtrace_frame (make_number (XFASTINT (nframes) - 1), base);
+  ptrdiff_t distance = specpdl_ptr - frame;
+  Lisp_Object result = Qnil;
+  eassert (distance >= 0);
+
+  if (!backtrace_p (prevframe))
+    error ("Activation frame not found!");
+  if (!backtrace_p (frame))
+    error ("Activation frame not found!");
+
+  /* move values to specpdl */
+  backtrace_eval_unrewind (distance);
+
+  /* grab values */
+  {
+    union specbinding *tmp = prevframe;
+    for (; tmp > frame; tmp--)
+      {
+	switch (tmp->kind)
+	  {
+	  case SPECPDL_LET:
+	  case SPECPDL_LET_DEFAULT:
+	  case SPECPDL_LET_LOCAL:
+	    {
+	      Lisp_Object sym = specpdl_symbol (tmp);
+	      Lisp_Object val = specpdl_old_value (tmp);
+	      if (EQ (sym, Qinternal_interpreter_environment))
+		{
+		  Lisp_Object env = val;
+		  for (; CONSP (env); env = XCDR (env))
+		    {
+		      Lisp_Object binding = XCAR (env);
+		      if (CONSP (binding))
+			result = Fcons (Fcons (XCAR (binding),
+					       XCDR (binding)),
+					result);
+		    }
+		}
+	      else
+		result = Fcons (Fcons (sym, val), result);
+	    }
+	  }
+      }
+  }
+
+  /* restore values from specpdl to original place */
+  backtrace_eval_unrewind (-distance);
+
+  return result;
+}
+
 \f
 void
 mark_specpdl (void)
@@ -3824,6 +3884,7 @@ alist of active lexical bindings.  */);
   defsubr (&Sbacktrace);
   defsubr (&Sbacktrace_frame);
   defsubr (&Sbacktrace_eval);
+  defsubr (&Sbacktrace_locals);
   defsubr (&Sspecial_variable_p);
   defsubr (&Sfunctionp);
 }

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: debugger-toggle-locals
  2013-12-02 11:04   ` debugger-toggle-locals Helmut Eller
@ 2013-12-02 14:46     ` Stefan Monnier
  0 siblings, 0 replies; 5+ messages in thread
From: Stefan Monnier @ 2013-12-02 14:46 UTC (permalink / raw)
  To: Helmut Eller; +Cc: emacs-devel

Thanks Helmut,

Looks good, installed, with minor tweaks (most visibly, using "v"
rather than "t").


        Stefan



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2013-12-02 14:46 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-23  8:57 debugger-toggle-locals Helmut Eller
2013-11-24 14:13 ` debugger-toggle-locals Stefan Monnier
2013-12-01 22:47 ` debugger-toggle-locals Stefan Monnier
2013-12-02 11:04   ` debugger-toggle-locals Helmut Eller
2013-12-02 14:46     ` debugger-toggle-locals Stefan Monnier

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).