Re: space leak from `values'

[Kim F. Storm]

Stefan Monnier <address@hidden> writes:

> > I bet most people don't know that `values' even exists, let alone what
> > conses onto it!
> 
> Completely agreed.

I know it -- I just fail to see why it is useful.

> 
> > It seems basically unclean to let something like that grow
> > unboundedly, even if it doesn't keep a significant part of the heap
> > live, and especially as it's pretty obscure.
> 
> I'd vote for removing it, but IIRC some people argued for keeping it last
> time this came up.  So I suggest we replace the lisp with a ring.


Here is a patch which reduces the values list to max 10 elements (not
configurable -- should it be?)

The major part of the patch is the addition of a new C-level function
`setnthcdr' which is used by other C and lisp level code to truncate
the `values' list.

This function seems generally useful so I think we should add it 
in any case...

> Still, it should have a different name to avoid surprises (like
> `values-of-last-interactive-evaluations').

Or `last-eval-results'.  However, I didn't change that.


Index: src/fns.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/fns.c,v
retrieving revision 1.372
diff -c -r1.372 fns.c
*** src/fns.c   6 Jul 2004 19:36:56 -0000       1.372
--- src/fns.c   28 Jul 2004 23:02:02 -0000
***************
*** 1415,1420 ****
--- 1415,1447 ----
    return Fcar (Fnthcdr (n, list));
  }
  
+ DEFUN ("setnthcdr", Fsetnthcdr, Ssetnthcdr, 2, 3, 0,
+        doc: /* Set cdr of Nth element of LIST to VALUE (nil if omitted), 
returns the result.
+ If N is negative, count from end of LIST.
+ If list has less than N elements, do not modify list.  */)
+   (n, list, value)
+      Lisp_Object n, list, value;
+ {
+   register Lisp_Object elt;
+   register int num;
+   CHECK_NUMBER (n);
+   num = XINT (n);
+   if (num < 0)
+     num += XFASTINT (Flength (list));
+   if (num <= 0)
+     return Qnil;
+   for (elt = list; --num > 0 && !NILP (elt);)
+     {
+       QUIT;
+       if (! CONSP (elt))
+       wrong_type_argument (Qlistp, elt);
+       elt = XCDR (elt);
+     }
+   if (CONSP (elt))
+     XSETCDR (elt, value);
+   return list;
+ }
+ 
  DEFUN ("elt", Felt, Selt, 2, 2, 0,
         doc: /* Return element of SEQUENCE at index N.  */)
       (sequence, n)
***************
*** 5715,5720 ****
--- 5742,5748 ----
    defsubr (&Ssubstring_no_properties);
    defsubr (&Snthcdr);
    defsubr (&Snth);
+   defsubr (&Ssetnthcdr);
    defsubr (&Selt);
    defsubr (&Smember);
    defsubr (&Smemq);
Index: src/lisp.h
===================================================================
RCS file: /cvsroot/emacs/emacs/src/lisp.h,v
retrieving revision 1.501
diff -c -r1.501 lisp.h
*** src/lisp.h  30 Jun 2004 13:09:05 -0000      1.501
--- src/lisp.h  28 Jul 2004 23:02:04 -0000
***************
*** 2271,2276 ****
--- 2271,2277 ----
  extern Lisp_Object substring_both P_ ((Lisp_Object, int, int, int, int));
  EXFUN (Fnth, 2);
  EXFUN (Fnthcdr, 2);
+ EXFUN (Fsetnthcdr, 3);
  EXFUN (Fmemq, 2);
  EXFUN (Fassq, 2);
  EXFUN (Fassoc, 2);
Index: src/lread.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/lread.c,v
retrieving revision 1.322
diff -c -r1.322 lread.c
*** src/lread.c 26 Apr 2004 21:28:40 -0000      1.322
--- src/lread.c 28 Jul 2004 23:02:03 -0000
***************
*** 1377,1383 ****
  
        if (printflag)
        {
!         Vvalues = Fcons (val, Vvalues);
          if (EQ (Vstandard_output, Qt))
            Fprin1 (val, Qnil);
          else
--- 1377,1384 ----
  
        if (printflag)
        {
!         Vvalues = Fsetnthcdr (make_number (10),
!                               Fcons (val, Vvalues), Qnil);
          if (EQ (Vstandard_output, Qt))
            Fprin1 (val, Qnil);
          else
Index: lisp/simple.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/simple.el,v
retrieving revision 1.651
diff -c -r1.651 simple.el
*** lisp/simple.el      23 Jul 2004 11:52:03 -0000      1.651
--- lisp/simple.el      28 Jul 2004 23:02:05 -0000
***************
*** 825,836 ****
         current-prefix-arg))
  
    (if (null eval-expression-debug-on-error)
!       (setq values (cons (eval eval-expression-arg) values))
      (let ((old-value (make-symbol "t")) new-value)
        ;; Bind debug-on-error to something unique so that we can
        ;; detect when evaled code changes it.
        (let ((debug-on-error old-value))
!       (setq values (cons (eval eval-expression-arg) values))
        (setq new-value debug-on-error))
        ;; If evaled code has changed the value of debug-on-error,
        ;; propagate that change to the global binding.
--- 825,836 ----
         current-prefix-arg))
  
    (if (null eval-expression-debug-on-error)
!       (setq values (setnthcdr 10 (cons (eval eval-expression-arg) values)))
      (let ((old-value (make-symbol "t")) new-value)
        ;; Bind debug-on-error to something unique so that we can
        ;; detect when evaled code changes it.
        (let ((debug-on-error old-value))
!       (setq values (setnthcdr 10 (cons (eval eval-expression-arg) values)))
        (setq new-value debug-on-error))
        ;; If evaled code has changed the value of debug-on-error,
        ;; propagate that change to the global binding.
Index: lisp/emacs-lisp/edebug.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/emacs-lisp/edebug.el,v
retrieving revision 3.69
diff -c -r3.69 edebug.el
*** lisp/emacs-lisp/edebug.el   10 Jun 2004 04:18:04 -0000      3.69
--- lisp/emacs-lisp/edebug.el   28 Jul 2004 23:02:07 -0000
***************
*** 3716,3722 ****
                      'read-expression-history)))
    (princ
     (edebug-outside-excursion
!     (setq values (cons (edebug-eval edebug-expr) values))
      (concat (edebug-safe-prin1-to-string (car values))
              (eval-expression-print-format (car values))))))
  
--- 3716,3722 ----
                      'read-expression-history)))
    (princ
     (edebug-outside-excursion
!     (setq values (setnthcdr 10 (cons (edebug-eval edebug-expr) values)))
      (concat (edebug-safe-prin1-to-string (car values))
              (eval-expression-print-format (car values))))))
  
Index: lisp/emacs-lisp/pp.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/emacs-lisp/pp.el,v
retrieving revision 1.23
diff -c -r1.23 pp.el
*** lisp/emacs-lisp/pp.el       28 Jun 2004 23:08:31 -0000      1.23
--- lisp/emacs-lisp/pp.el       28 Jul 2004 23:02:07 -0000
***************
*** 100,106 ****
  instead.  The value is also consed onto the front of the list
  in the variable `values'."
    (interactive "xPp-eval: ")
!   (setq values (cons (eval expression) values))
    (let* ((old-show-function temp-buffer-show-function)
         ;; Use this function to display the buffer.
         ;; This function either decides not to display it at all
--- 100,106 ----
  instead.  The value is also consed onto the front of the list
  in the variable `values'."
    (interactive "xPp-eval: ")
!   (setq values (setnthcdr 10 (cons (eval expression) values)))
    (let* ((old-show-function temp-buffer-show-function)
         ;; Use this function to display the buffer.
         ;; This function either decides not to display it at all


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