Problems with debug-on-entry in the Lisp debugger.

Problems with debug-on-entry in the Lisp debugger.

[Lute Kamstra]

Debug-on-entry currently changes the definition of functions by adding
code that enters the debugger.  This code needs be hidden from the
user of the debugger, which can be hard.  Redefining the function also
removes this code and thus cancels debug-on-entry, which is
inconvenient.  I'll give some more details on these problems below and
propose an new implementation of debug-on-entry that (hopefully)
solves these problems.

I'm currently working on three bugs in the debugger that have to do
with debug-on-entry:

1. When using `d' in the debugger to step through code, you can
   encounter the code added by debug-on-entry.  For example: you have
   two functions, f-outer and f-inner, and f-outer calls f-inner, and
   you set both to debug-on-entry.  If a call to f-outer puts you in
   the debugger and you decide to step through the function with `d',
   then you will encounter the code added by debug-on-entry.  This is
   not only bad because it shows the debugger's internals and can thus
   cause confusion, but it also bad because it effectively stops you
   from stepping into f-inner since you will just step into the
   debugger.

   The only solution (within the current implementation) that I can
   think of, is to temporarily remove all debug-on-entry code while
   stepping with `d'.  This could change the definition of the
   function currently being debugged, which is rather tricky.

2. The debugger keeps a list of functions, debug-function-list, that
   are set to debug-on-entry.  Redefining a function set to
   debug-on-entry effectively disables debug-on-entry but doesn't
   remove it from debug-function-list.

   Richard proposed to change defun, defmacro, defsubst or defalias to
   let them put in debug-on-entry code when a function is in
   debug-function-list.  Stefan suggested to use advice to cause
   functions to debug-on-entry.

3. Debug-on-entry for macros is currently broken: debug-on-entry just
   strips the car (i.e. `macro') of the macro definition and treats it
   as if it was a function.  This not only makes debug-on-entry for
   macros useless, but it also breaks the macro definition itself;
   even a subsequent cancel-debug-on-entry will not put `macro' back.

   I can think of two points in a macro to set a break for the
   debugger: just before macro expansion and just after it, right
   before the evaluation of the resulting sexp.  In both cases, hiding
   the debug-on-entry code from the user of the debugger seems not
   possible.

When I was thinking about these three problems, it seemed to me that
the easiest and simplest thing to do, is to move support for
debug-on-entry into the C implementation of the Lisp interpreter.  To
mark a function for debug-on-entry, you could set the debug-on-entry
property of the function's symbol and the Lisp interpreter would then
call the debugger.

This doesn't change the definition of functions so it hides the
debuggers internals much better.  So it solves problem 1.  Redefining
a function does not effect the debug-on-entry property on the symbol
so problem 2 is solved as well.  From the lisp interpreter it would
also be easy to call the debugger before and/or after macro expansion,
thus solving 3 as well.

The big disadvantage is that the Lisp interpreter has to check the
properties of every functions it evaluates.  This slows down the
overall speed of the interpreter.  This could be mostly solved by
using a `debug-on-entry' variable that defaults to nil.  The Lisp
interpreter will only consider the debug-on-entry property of
functions if the debug-on-entry variable is non-nil.  The debugger
would then only set this variable to a non-nil value if the
debug-function-list is not empty.  So the overhead of moving
debug-on-entry support into the Lisp interpreter would then be one
variable check per function call.  This seems acceptable to me.

Shall I go ahead and try to implement this, or do people think this is
a bad idea?

Lute.

Re: Problems with debug-on-entry in the Lisp debugger.

[Stefan Monnier]

>    I can think of two points in a macro to set a break for the
>    debugger: just before macro expansion and just after it, right
>    before the evaluation of the resulting sexp.  In both cases, hiding
>    the debug-on-entry code from the user of the debugger seems not
>    possible.

To me "entry of a macro" is very clearly "just before expansion".
Especially if you think of macro-expansion occurring because of explicit
calls to macroexpand (e.g. in the byte-compiler) rather than as a direct
part of interpretation.

> Shall I go ahead and try to implement this, or do people think this is
> a bad idea?

I think it's a bad idea.  The interpreter is already way too slow, I'd
rather not make it worse.


        Stefan

Re: Problems with debug-on-entry in the Lisp debugger.

[Kim F. Storm]

Lute Kamstra <Lute.Kamstra.lists@xs4all.nl> writes:

> When I was thinking about these three problems, it seemed to me that
> the easiest and simplest thing to do, is to move support for
> debug-on-entry into the C implementation of the Lisp interpreter.  To
> mark a function for debug-on-entry, you could set the debug-on-entry
> property of the function's symbol and the Lisp interpreter would then
> call the debugger.

You could define a bit in the Lisp_Symbol, like this:

struct Lisp_Symbol
{
  unsigned gcmarkbit : 1;

  /* Non-zero means symbol serves as a variable alias.  The symbol
     holding the real value is found in the value slot.  */
  unsigned indirect_variable : 1;

  /* Non-zero means call debugger on entry.  */
  unsigned debug_on_entry;

  ...

};


Then you would define debug-on-entry and cancel-debug-on-entry
in C as well, which simply set or clear that bit.

Re: Problems with debug-on-entry in the Lisp debugger.

[drkm]

storm@cua.dk (Kim F. Storm) writes:

> You could define a bit in the Lisp_Symbol, like this:

> struct Lisp_Symbol
> {

  [...]

>   unsigned debug_on_entry;
                          ^^^

  Really?-)

--drkm

Re: Problems with debug-on-entry in the Lisp debugger.

[Kim F. Storm]

drkm <darkman_spam@yahoo.fr> writes:

>   Really?-)

Ouch. That should have been:

  unsigned debug_on_entry : 1;

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Problems with debug-on-entry in the Lisp debugger.

[Lute Kamstra]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>    I can think of two points in a macro to set a break for the
>>    debugger: just before macro expansion and just after it, right
>>    before the evaluation of the resulting sexp.  In both cases,
>>    hiding the debug-on-entry code from the user of the debugger
>>    seems not possible.
>
> To me "entry of a macro" is very clearly "just before expansion".
> Especially if you think of macro-expansion occurring because of
> explicit calls to macroexpand (e.g. in the byte-compiler) rather
> than as a direct part of interpretation.

I see you implemented this.  This makes debug-on-entry for macros a
lot better, of course.  Thanks.  But the problem I mentioned remains:
the debug-entry-code is visible.  For example:

(defmacro inc (var)
  (list 'setq var (list '1+ var)))
(debug-on-entry 'inc)
(progn (setq x 0) (inc x))

gives a backtrace like this:

------ Buffer: *Backtrace* ------ 
Debugger entered--entering a function:
* (lambda (var) (if (or inhibit-debug-on-entry debugger-jumping-flag) nil (debug ...)) (list (quote setq) var (list ... var)))(x)
  (inc x)
  (progn (setq x 0) (inc x))
  eval((progn (setq x 0) (inc x)))
  eval-last-sexp-1(nil)
  eval-last-sexp(nil)
  call-interactively(eval-last-sexp)
------ Buffer: *Backtrace* ------ 

>> Shall I go ahead and try to implement this, or do people think this
>> is a bad idea?
>
> I think it's a bad idea.  The interpreter is already way too slow,
> I'd rather not make it worse.

I think the effect on performance will be very minimal.  Do you know
of a good way to test the performance of the interpreter so that I can
measure the impact of my proposed change?  Can you indicate how much
performance loss would be acceptable for you?

Lute.

Re: Problems with debug-on-entry in the Lisp debugger.

[Stefan Monnier]

> I see you implemented this.  This makes debug-on-entry for macros a
> lot better, of course.  Thanks.  But the problem I mentioned remains:
> the debug-entry-code is visible.
[...]
> Debugger entered--entering a function:
> * (lambda (var) (if (or inhibit-debug-on-entry debugger-jumping-flag) nil (debug ...)) (list (quote setq) var (list ... var)))(x)
>   (inc x)
[...]

Other than the aesthetic aspect (which we can fix by just removing the
offending line in an ad-hoc way), does it have any real impact?

> I think the effect on performance will be very minimal.

But I see no compelling reason to pay this price.

After all, we've live for many years with this elisp implementation without
nearly any complaint.  If the aesthetic aspect is just more serious now that
we replace (debug 'debug) with (if (or inhibit-debug-on-entry
debugger-jumping-flag) nil (debug 'debug)), we can define a function named
e.g. `debug-entering' that will do the checking of inhibit-debug-on-entry
and debugger-jumping-flag.


        Stefan

Re: Problems with debug-on-entry in the Lisp debugger.

[Richard Stallman]

       The only solution (within the current implementation) that I can
       think of, is to temporarily remove all debug-on-entry code while
       stepping with `d'.

Would setting inhibit-debug-on-entry temporarily do the job?

       I can think of two points in a macro to set a break for the
       debugger: just before macro expansion and just after it, right
       before the evaluation of the resulting sexp.

The correct place to do it is before macro expansion.
This is a very evident bug, so please just fix it if you can.

						     In both cases, hiding
       the debug-on-entry code from the user of the debugger seems not
       possible.

I am not sure what that means.

    When I was thinking about these three problems, it seemed to me that
    the easiest and simplest thing to do, is to move support for
    debug-on-entry into the C implementation of the Lisp interpreter.  To
    mark a function for debug-on-entry, you could set the debug-on-entry
    property of the function's symbol and the Lisp interpreter would then
    call the debugger.

I agree this is undesirable due to slowness.
I don't see a need for this big a change.

Re: Problems with debug-on-entry in the Lisp debugger.

[Lute Kamstra]

Richard Stallman <rms@gnu.org> writes:

>        The only solution (within the current implementation) that I can
>        think of, is to temporarily remove all debug-on-entry code while
>        stepping with `d'.
>
> Would setting inhibit-debug-on-entry temporarily do the job?

inhibit-debug-on-entry cannot be set from within the debugger because
it is shadowed by a let binding.  That's what I introduced
debugger-jumping-flag for.  Temporary setting debugger-jumping-flag
would solve the problem of stepping into the code of the debugger.
(I'll implement this fix.)  The debug-entry-code would still be
visible in the backtrace however.

>        I can think of two points in a macro to set a break for the
>        debugger: just before macro expansion and just after it, right
>        before the evaluation of the resulting sexp.
>
> The correct place to do it is before macro expansion.
> This is a very evident bug, so please just fix it if you can.

Stefan already did this.

> 						     In both cases, hiding
>        the debug-on-entry code from the user of the debugger seems not
>        possible.
>
> I am not sure what that means.

Consider this example (with Stefan's fix for macro's installed):

(defmacro inc (var)
  (list 'setq var (list '1+ var)))
(debug-on-entry 'inc)
(progn (setq x 0) (inc x))

This gives a backtrace like this:

------ Buffer: *Backtrace* ------ 
Debugger entered--entering a function:
* (lambda (var) (if (or inhibit-debug-on-entry debugger-jumping-flag) nil (debug ...)) (list (quote setq) var (list ... var)))(x)
  (inc x)
  (progn (setq x 0) (inc x))
  eval((progn (setq x 0) (inc x)))
  eval-last-sexp-1(nil)
  eval-last-sexp(nil)
  call-interactively(eval-last-sexp)
------ Buffer: *Backtrace* ------ 

where you can see the debug-entry-code (if (or inhibit-debug-on-entry
debugger-jumping-flag) nil (debug ...)).  I would prefer to hide the
internals of the debugger from its users.  Moving support for
debug-on-entry into the lisp interpreter (just like support for
stepping is in the lisp interpreter) would make this possible (and
easy).

>     When I was thinking about these three problems, it seemed to me that
>     the easiest and simplest thing to do, is to move support for
>     debug-on-entry into the C implementation of the Lisp interpreter.  To
>     mark a function for debug-on-entry, you could set the debug-on-entry
>     property of the function's symbol and the Lisp interpreter would then
>     call the debugger.
>
> I agree this is undesirable due to slowness.

My proposed change would add one test like
 
  if (debug_on_entry) ...

to Feval and Ffuncall.  And when no functions are set to break on
entry (i.e., the normal situation), debug_on_entry will be zero.  Do
you think that this will have a significant impact on performance?

> I don't see a need for this big a change.

You proposed to change defun, defsubst, defalias and defmacro to add
debug-entry-code when their argument was in debug-function-list.  That
is a similarly big change.

Below is a quick "proof-of-concept" patch for src/eval.c and
lisp/emacs-lisp/debug.el to get a better idea of what I mean.  As you
can see, the change to eval.c isn't that big.  The change in debug.el
is more substantial, but the code becomes a lot simpler.

I also did a quick-and-dirty test to measure the performance impact of
the patch but I did not see any effect on speed; so I suppose it is
negligible.

Lute.


Index: src/eval.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/eval.c,v
retrieving revision 1.234
diff -u -r1.234 eval.c
--- src/eval.c	6 Mar 2005 16:02:47 -0000	1.234
+++ src/eval.c	8 Mar 2005 14:58:49 -0000
@@ -1,6 +1,6 @@
 /* Evaluator for GNU Emacs Lisp interpreter.
-   Copyright (C) 1985, 86, 87, 93, 94, 95, 99, 2000, 2001, 02, 2004
-     Free Software Foundation, Inc.
+   Copyright (C) 1985, 1986, 1987, 1993, 1994, 1995, 1999, 2000, 2001,
+     2002, 2004, 2005 Free Software Foundation, Inc.
 
 This file is part of GNU Emacs.
 
@@ -91,6 +91,7 @@
 Lisp_Object Qautoload, Qmacro, Qexit, Qinteractive, Qcommandp, Qdefun;
 Lisp_Object Qinhibit_quit, Vinhibit_quit, Vquit_flag;
 Lisp_Object Qand_rest, Qand_optional;
+Lisp_Object Qdebug_on_entry, Qdebug;
 Lisp_Object Qdebug_on_error;
 Lisp_Object Qdeclare;
 
@@ -135,6 +136,14 @@
 
 int debug_on_next_call;
 
+/* Nonzero means check the debug-on-entry property of functions. */
+
+int debug_on_entry;
+
+/* Nonzero means dont't check the debug-on-entry property of functions. */
+
+int suspend_debug_on_entry;
+
 /* Non-zero means debugger may continue.  This is zero when the
    debugger is called during redisplay, where it might not be safe to
    continue the interrupted redisplay. */
@@ -206,6 +215,8 @@
   specpdl_ptr = specpdl;
   max_specpdl_size = 1000;
   max_lisp_eval_depth = 300;
+  debug_on_entry = 0;
+  suspend_debug_on_entry = 0;
 
   Vrun_hooks = Qnil;
 }
@@ -2051,6 +2062,11 @@
 
   if (debug_on_next_call)
     do_debug_on_call (Qt);
+  else if (debug_on_entry && 
+	   ! suspend_debug_on_entry && 
+	   SYMBOLP (original_fun) &&
+	   ! NILP (Fget (original_fun, Qdebug_on_entry)))
+    do_debug_on_call (Qdebug);
 
   /* At this point, only original_fun and original_args
      have values that will be used below */
@@ -2741,6 +2757,11 @@
 
   if (debug_on_next_call)
     do_debug_on_call (Qlambda);
+  else if (debug_on_entry && 
+	   ! suspend_debug_on_entry && 
+	   SYMBOLP (args[0]) &&
+	   ! NILP (Fget (args[0], Qdebug_on_entry)))
+    do_debug_on_call (Qdebug);
 
  retry:
 
@@ -3379,6 +3400,12 @@
   Qexit = intern ("exit");
   staticpro (&Qexit);
 
+  Qdebug_on_entry = intern ("debug-on-entry");
+  staticpro (&Qdebug_on_entry);
+
+  Qdebug = intern ("debug");
+  staticpro (&Qdebug);
+
   Qinteractive = intern ("interactive");
   staticpro (&Qinteractive);
 
@@ -3432,6 +3459,18 @@
   DEFVAR_BOOL ("debug-on-next-call", &debug_on_next_call,
 	       doc: /* Non-nil means enter debugger before next `eval', `apply' or `funcall'.  */);
 
+  DEFVAR_BOOL ("debug-on-entry", &debug_on_entry,
+	       doc: /* Non-nil means debug-on-entry is enabled.
+When debug-on-entry is enabled, the debugger in entered when functions
+are called that have the debug-on-entry property set.  */);
+
+  DEFVAR_BOOL ("suspend-debug-on-entry", &suspend_debug_on_entry,
+	       doc: /* Non-nil means debug-on-entry is disabled.
+When this variable is nil, the variable `debug-on-entry' determines
+whether debug-on-entry is enabled.  When debug-on-entry is enabled,
+the debugger in entered when functions are called that have the
+debug-on-entry property set.  */);
+
   DEFVAR_BOOL ("debugger-may-continue", &debugger_may_continue,
 	       doc: /* Non-nil means debugger may continue execution.
 This is nil when the debugger is called under circumstances where it
Index: lisp/emacs-lisp/debug.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/emacs-lisp/debug.el,v
retrieving revision 1.73
diff -u -r1.73 debug.el
--- lisp/emacs-lisp/debug.el	7 Mar 2005 14:12:26 -0000	1.73
+++ lisp/emacs-lisp/debug.el	8 Mar 2005 14:58:50 -0000
@@ -89,21 +89,6 @@
 (defvar debugger-outer-inhibit-redisplay)
 (defvar debugger-outer-cursor-in-echo-area)
 
-(defvar inhibit-debug-on-entry nil
-  "Non-nil means that debug-on-entry is disabled.")
-
-(defvar debugger-jumping-flag nil
-  "Non-nil means that debug-on-entry is disabled.
-This variable is used by `debugger-jump' and `debugger-reenable'.")
-
-;; When you change this, you may also need to change the number of
-;; frames that the debugger skips.
-(defconst debug-entry-code
-  '(if (or inhibit-debug-on-entry debugger-jumping-flag)
-       nil
-     (debug 'debug))
-  "Code added to a function to cause it to call the debugger upon entry.")
-
 ;;;###autoload
 (setq debugger 'debug)
 ;;;###autoload
@@ -124,6 +109,7 @@
     (let (debugger-value
 	  (debug-on-error nil)
 	  (debug-on-quit nil)
+	  (debug-on-entry nil)
 	  (debugger-buffer (let ((default-major-mode 'fundamental-mode))
 			     (get-buffer-create "*Backtrace*")))
 	  (debugger-old-buffer (current-buffer))
@@ -159,7 +145,6 @@
       ;; Don't let these magic variables affect the debugger itself.
       (let ((last-command nil) this-command track-mouse
 	    (inhibit-trace t)
-	    (inhibit-debug-on-entry t)
 	    unread-command-events
 	    unread-post-input-method-events
 	    last-input-event last-command-event last-nonmenu-event
@@ -198,9 +183,8 @@
 		  (message "%s" (buffer-string))
 		  (kill-emacs))
 		(if (eq (car debugger-args) 'debug)
-		    ;; Skip the frames for backtrace-debug, byte-code,
-		    ;; and debug-entry-code.
-		    (backtrace-debug 4 t))
+		    ;; Skip the frames for backtrace-debug and byte-code.
+		    (backtrace-debug 3 t))
 		(message "")
 		(let ((standard-output nil)
 		      (buffer-read-only t))
@@ -262,9 +246,7 @@
   (delete-region (point)
 		 (progn
 		   (search-forward "\n  debug(")
-		   (forward-line (if (eq (car debugger-args) 'debug)
-				     2	; Remove debug-entry-code frame.
-				   1))
+		   (forward-line 1)
 		   (point)))
   (insert "Debugger entered")
   ;; lambda is for debug-on-call when a function call is next.
@@ -409,7 +391,7 @@
   "Continue to exit from this frame, with all debug-on-entry suspended."
   (interactive)
   (debugger-frame)
-  (setq debugger-jumping-flag t)
+  (setq suspend-debug-on-entry t)
   (add-hook 'post-command-hook 'debugger-reenable)
   (message "Continuing through this frame")
   (exit-recursive-edit))
@@ -418,7 +400,7 @@
   "Turn all debug-on-entry functions back on.
 This function is put on `post-command-hook' by `debugger-jump' and
 removes itself from that hook."
-  (setq debugger-jumping-flag nil)
+  (setq suspend-debug-on-entry nil)
   (remove-hook 'post-command-hook 'debugger-reenable))
 
 (defun debugger-frame-number ()
@@ -429,9 +411,6 @@
 	  (count 0))
       (while (not (eq (cadr (backtrace-frame count)) 'debug))
 	(setq count (1+ count)))
-      ;; Skip debug-entry-code frame.
-      (when (member '(debug (quote debug)) (cdr (backtrace-frame (1+ count))))
-	(setq count (1+ count)))
       (goto-char (point-min))
       (when (looking-at "Debugger entered--\\(Lisp error\\|returning value\\):")
 	(goto-char (match-end 0))
@@ -624,29 +603,16 @@
 (defun debug-on-entry (function)
   "Request FUNCTION to invoke debugger each time it is called.
 If you tell the debugger to continue, FUNCTION's execution proceeds.
-This works by modifying the definition of FUNCTION,
-which must be written in Lisp, not predefined.
-Use \\[cancel-debug-on-entry] to cancel the effect of this command.
-Redefining FUNCTION also cancels it."
+This works by setting the debug-on-entry property of FUNCTION.
+Use \\[cancel-debug-on-entry] to cancel the effect of this command."
   (interactive "aDebug on entry (to function): ")
   ;; Handle a function that has been aliased to some other function.
   (if (and (subrp (symbol-function function))
 	   (eq (cdr (subr-arity (symbol-function function))) 'unevalled))
       (error "Function %s is a special form" function))
-  (if (or (symbolp (symbol-function function))
-	  (subrp (symbol-function function)))
-      ;; Create a wrapper in which we can then add the necessary debug call.
-      (fset function `(lambda (&rest debug-on-entry-args)
-			,(interactive-form (symbol-function function))
-			(apply ',(symbol-function function)
-			       debug-on-entry-args))))
-  (or (consp (symbol-function function))
-      (debug-convert-byte-code function))
-  (or (consp (symbol-function function))
-      (error "Definition of %s is not a list" function))
-  (fset function (debug-on-entry-1 function (symbol-function function) t))
-  (or (memq function debug-function-list)
-      (push function debug-function-list))
+  (put function 'debug-on-entry t)
+  (add-to-list 'debug-function-list function)
+  (setq debug-on-entry t)
   function)
 
 ;;;###autoload
@@ -661,56 +627,14 @@
 	   (if name (intern name)))))
   (if (and function (not (string= function "")))
       (progn
-	(let ((f (debug-on-entry-1 function (symbol-function function) nil)))
-	  (condition-case nil
-	      (if (and (equal (nth 1 f) '(&rest debug-on-entry-args))
-		       (eq (car (nth 3 f)) 'apply))
-		  ;; `f' is a wrapper introduced in debug-on-entry.
-		  ;; Get rid of it since we don't need it any more.
-		  (setq f (nth 1 (nth 1 (nth 3 f)))))
-	    (error nil))
-	  (fset function f))
+	(put function 'debug-on-entry nil)
 	(setq debug-function-list (delq function debug-function-list))
+	(unless debug-function-list
+	  (setq debug-on-entry nil))
 	function)
     (message "Cancelling debug-on-entry for all functions")
-    (mapcar 'cancel-debug-on-entry debug-function-list)))
-
-(defun debug-convert-byte-code (function)
-  (let ((defn (symbol-function function)))
-    (if (not (consp defn))
-	;; Assume a compiled code object.
-	(let* ((contents (append defn nil))
-	       (body
-		(list (list 'byte-code (nth 1 contents)
-			    (nth 2 contents) (nth 3 contents)))))
-	  (if (nthcdr 5 contents)
-	      (setq body (cons (list 'interactive (nth 5 contents)) body)))
-	  (if (nth 4 contents)
-	      ;; Use `documentation' here, to get the actual string,
-	      ;; in case the compiled function has a reference
-	      ;; to the .elc file.
-	      (setq body (cons (documentation function) body)))
-	  (fset function (cons 'lambda (cons (car contents) body)))))))
-
-(defun debug-on-entry-1 (function defn flag)
-  (let ((tail defn))
-    (if (subrp tail)
-	(error "%s is a built-in function" function)
-      (if (eq (car tail) 'macro) (setq tail (cdr tail)))
-      (if (eq (car tail) 'lambda) (setq tail (cdr tail))
-	(error "%s not user-defined Lisp function" function))
-      ;; Skip the docstring.
-      (when (and (stringp (cadr tail)) (cddr tail))
-	(setq tail (cdr tail)))
-      ;; Skip the interactive form.
-      (when (eq 'interactive (car-safe (cadr tail)))
-	(setq tail (cdr tail)))
-      (unless (eq flag (equal (cadr tail) debug-entry-code))
-	;; Add/remove debug statement as needed.
-	(if flag
-	    (setcdr tail (cons debug-entry-code (cdr tail)))
-	  (setcdr tail (cddr tail))))
-      defn)))
+    (setq debug-function-list nil)
+    (setq debug-on-entry nil)))
 
 (defun debugger-list-functions ()
   "Display a list of all the functions now set to debug on entry."
@@ -726,10 +650,7 @@
 	  (make-text-button (point) (progn (prin1 fun) (point))
 			    'type 'help-function
 			    'help-args (list fun))
-	  (terpri))
-	(terpri)
-	(princ "Note: if you have redefined a function, then it may no longer\n")
-	(princ "be set to debug on entry, even if it is in the list.")))))
+	  (terpri))))))
 
 (provide 'debug)

Re: Problems with debug-on-entry in the Lisp debugger.

[Lute Kamstra]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> I see you implemented this.  This makes debug-on-entry for macros a
>> lot better, of course.  Thanks.  But the problem I mentioned remains:
>> the debug-entry-code is visible.
> [...]
>> Debugger entered--entering a function:
>> * (lambda (var) (if (or inhibit-debug-on-entry debugger-jumping-flag) nil (debug ...)) (list (quote setq) var (list ... var)))(x)
>>   (inc x)
> [...]
>
> Other than the aesthetic aspect (which we can fix by just removing the
> offending line in an ad-hoc way),

Actually, I'd like to see the line, but not the "(if (or
inhibit-debug-on-entry debugger-jumping-flag) nil (debug ...))".

> does it have any real impact?

It can be confusing for new users of the debugger: Hey, what's that
doing in my function?  Did I put that there?

>> I think the effect on performance will be very minimal.
>
> But I see no compelling reason to pay this price.
>
> After all, we've live for many years with this elisp implementation
> without nearly any complaint.

That's not really a valid argument.  The macro handling bug was in
there for years without any complaint too.  Yet you fixed it.  ;-)

> If the aesthetic aspect is just more serious now that we replace
> (debug 'debug) with (if (or inhibit-debug-on-entry
> debugger-jumping-flag) nil (debug 'debug)), we can define a function
> named e.g. `debug-entering' that will do the checking of
> inhibit-debug-on-entry and debugger-jumping-flag.

That would be somewhat better.

However, I still feel that moving debug-on-entry to the lisp
interpreter is "the right thing".  It makes the hole thing better a
lot simpler.

Lute.

Re: Problems with debug-on-entry in the Lisp debugger.

[Stefan Monnier]

> This gives a backtrace like this:

> ------ Buffer: *Backtrace* ------ 
> Debugger entered--entering a function:
> * (lambda (var) (if (or inhibit-debug-on-entry debugger-jumping-flag) nil (debug ...)) (list (quote setq) var (list ... var)))(x)
>   (inc x)
>   (progn (setq x 0) (inc x))
>   eval((progn (setq x 0) (inc x)))
>   eval-last-sexp-1(nil)
>   eval-last-sexp(nil)
>   call-interactively(eval-last-sexp)
> ------ Buffer: *Backtrace* ------ 

> where you can see the debug-entry-code (if (or inhibit-debug-on-entry
> debugger-jumping-flag) nil (debug ...)).  I would prefer to hide the
> internals of the debugger from its users.

debug.el already hides its internals.  See debugger-setup-buffer.
It just has to be updated to hide this part of the internals.

> You proposed to change defun, defsubst, defalias and defmacro to add
> debug-entry-code when their argument was in debug-function-list.  That
> is a similarly big change.

There's no need to do that.  The hooks are already present for defadvice, so
we should simply use them.  Better yet, we should use defadvice directly:

   (defadvice <FOO> (before debug-on-entry activate)
     (if inhibit-debug-on-entry nil (debug 'debug)))

This will properly survive function redefinitions.


        Stefan

Re: Problems with debug-on-entry in the Lisp debugger.

[Lute Kamstra]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> This gives a backtrace like this:
>
>> ------ Buffer: *Backtrace* ------ 
>> Debugger entered--entering a function:
>> * (lambda (var) (if (or inhibit-debug-on-entry debugger-jumping-flag) nil (debug ...)) (list (quote setq) var (list ... var)))(x)
>>   (inc x)
>>   (progn (setq x 0) (inc x))
>>   eval((progn (setq x 0) (inc x)))
>>   eval-last-sexp-1(nil)
>>   eval-last-sexp(nil)
>>   call-interactively(eval-last-sexp)
>> ------ Buffer: *Backtrace* ------ 
>
>> where you can see the debug-entry-code (if (or inhibit-debug-on-entry
>> debugger-jumping-flag) nil (debug ...)).  I would prefer to hide the
>> internals of the debugger from its users.
>
> debug.el already hides its internals.  See debugger-setup-buffer.
> It just has to be updated to hide this part of the internals.

In this case, hiding the internals is possible.  It will be clumsy
though.  You have to search for debug-entry-code as text, which can be
abbreviated like in "(if (or inhibit-debug-on-entry
debugger-jumping-flag) nil (debug ...))".  You probably can only do
that easily for a specific example of debug-entry-code.  So the next
time debug-entry-code is changed, debug.el breaks in yet another
place.  (Remember that when you introduced the (if ...) test in
debug-entry-code, debug.el already had to be changed in 3 defuns to
compensate for this?)  So debug.el will become more difficult to
maintain.

In the other case I mentioned, hiding the debugger's internals is not
possible.  When you encounter debug-entry-code while stepping with
`d', it will not be possible to hide this.  You could hide the code
visually by deleting it from the backtrace buffer, but then the user
would still have to press `d' a couple of times (seemingly without any
effect) to step through the debug-entry-code.

>> You proposed to change defun, defsubst, defalias and defmacro to
>> add debug-entry-code when their argument was in
>> debug-function-list.  That is a similarly big change.
>
> There's no need to do that.  The hooks are already present for
> defadvice, so we should simply use them.

Hmm, I'm don't see how you can use these hooks except by using
defadvice: they seem to be designed to handle only advice.  I'd have
to change Ffset to deal with debug-on-entry.  Am I missing something?

> Better yet, we should use defadvice directly:
>
>    (defadvice <FOO> (before debug-on-entry activate)
>      (if inhibit-debug-on-entry nil (debug 'debug)))
>
> This will properly survive function redefinitions.

It does survive function redefinition.  I think this solution is worse
than the problem it tries to solve, however.  The defadvice changes a
function definition quite a bit.  The hapless debugger user might not
recognize his own function anymore:

(defun fun () "Return one." 1)
(symbol-function 'fun)
  => (lambda nil "Return one." 1)
(defadvice fun (before debug-on-entry activate)
  (if inhibit-debug-on-entry nil (debug 'debug)))
(symbol-function 'fun)
  => (lambda nil "$ad-doc: fun$" 
       (let (ad-return-value) 
	 (if inhibit-debug-on-entry 
	     nil 
	   (debug (quote debug))) 
	 (setq ad-return-value (ad-Orig-fun)) 
	 ad-return-value))

Stepping through this will cause some confusion, I think.  Can you
think of an easy way to hide the debugger's internals when you use
advice like this?

Lute.

Re: Problems with debug-on-entry in the Lisp debugger.

[Richard Stallman]

      Temporary setting debugger-jumping-flag
    would solve the problem of stepping into the code of the debugger.
    (I'll implement this fix.)

Thanks.

				The debug-entry-code would still be
    visible in the backtrace however.

That is not a problem.  It might even help users learn to understand
that programs are data.

Re: Problems with debug-on-entry in the Lisp debugger.

[Richard Stallman]

    Below is a quick "proof-of-concept" patch for src/eval.c and
    lisp/emacs-lisp/debug.el to get a better idea of what I mean.  As you
    can see, the change to eval.c isn't that big.

The code is not unclean, but I don't think it is a real problem 
that the debug-on-entry code is visible.  So I'd rather not
go in this direction.

This is not the part of Emacs that is important to improve.
Hackers tend to focus their attention on the parts of Emacs
that make it seem more elegant in programmers' terms,
but this doesn't make Emacs more powerful or a better editor.

Could you possibly look at implementing something listed in etc/TODO?

Re: Problems with debug-on-entry in the Lisp debugger.

[Richard Stallman]

    > does it have any real impact?

    It can be confusing for new users of the debugger: Hey, what's that
    doing in my function?  Did I put that there?

The user probably rembemers having set "debug on entry" for the function,
and has just been reminded by entering the debugger, so that ought
to help him understand.

    > If the aesthetic aspect is just more serious now that we replace
    > (debug 'debug) with (if (or inhibit-debug-on-entry
    > debugger-jumping-flag) nil (debug 'debug)), we can define a function
    > named e.g. `debug-entering' that will do the checking of
    > inhibit-debug-on-entry and debugger-jumping-flag.

    That would be somewhat better.

Using the name implement-debug-on-entry will help the user
figure out why it is there.

the user to understand.

Re: Problems with debug-on-entry in the Lisp debugger.

[Lute Kamstra]

Richard Stallman <rms@gnu.org> writes:

>     > does it have any real impact?
>
>     It can be confusing for new users of the debugger: Hey, what's
>     that doing in my function?  Did I put that there?
>
> The user probably rembemers having set "debug on entry" for the
> function, and has just been reminded by entering the debugger, so
> that ought to help him understand.

True.

>     > If the aesthetic aspect is just more serious now that we
>     > replace (debug 'debug) with (if (or inhibit-debug-on-entry
>     > debugger-jumping-flag) nil (debug 'debug)), we can define a
>     > function named e.g. `debug-entering' that will do the checking
>     > of inhibit-debug-on-entry and debugger-jumping-flag.
>
>     That would be somewhat better.
>
> Using the name implement-debug-on-entry will help the user figure
> out why it is there.

Ok, I'll implement this.

Lute.

Re: Problems with debug-on-entry in the Lisp debugger.

[Lute Kamstra]

Richard Stallman <rms@gnu.org> writes:

>     Below is a quick "proof-of-concept" patch for src/eval.c and
>     lisp/emacs-lisp/debug.el to get a better idea of what I mean.
>     As you can see, the change to eval.c isn't that big.
>
> The code is not unclean, but I don't think it is a real problem 
> that the debug-on-entry code is visible.  So I'd rather not
> go in this direction.

Ok, I'll cease my efforts to convince you then.  ;-)

> This is not the part of Emacs that is important to improve.
> Hackers tend to focus their attention on the parts of Emacs
> that make it seem more elegant in programmers' terms,
> but this doesn't make Emacs more powerful or a better editor.
>
> Could you possibly look at implementing something listed in etc/TODO?

I'll take a look to see if there's anything that strikes my fancy.

Lute.

Re: Problems with debug-on-entry in the Lisp debugger.

[Kevin Rodgers]

Richard Stallman wrote:
> This is not the part of Emacs that is important to improve.
> Hackers tend to focus their attention on the parts of Emacs
> that make it seem more elegant in programmers' terms,
> but this doesn't make Emacs more powerful or a better editor.

Maybe not now, but an elegant rewrite may enable changes to make Emacs
more powerful and better in the future.

-- 
Kevin Rodgers

Re: Problems with debug-on-entry in the Lisp debugger.

[Richard Stallman]

    Maybe not now, but an elegant rewrite may enable changes to make Emacs
    more powerful and better in the future.

I am not saying these are useless, I am saying that people focus too
much on making Emacs "more powerful", disregarding other kinds of
improvements that we need.  Most of the TODO list items are not of
that kind.

The fact is that very few of the TODO list entries have ever been
done.