bug#25295: 26.0.50; Represent eieio objects using object-print in backtraces and edebug

bug#25295: 26.0.50; Represent eieio objects using object-print in backtraces and edebug

[Eric Abrahamsen]

EIEIO class instances can either be represented using the read-only
#<class-name object string> style, or as vectors, dumping all the slot
information. For large or recursive objects, this can take some time, and
can also make for very large strings. In backtraces and edebug, in
particular, this can make it very difficult to see what's happening.

Objects have an `object-print' method which produces the #<foo>
representation, but this appears to only sometimes be used in edebug,
and never in backtraces.

It would be great if object-print could always be used, or if there were
an option to toggle this.

In GNU Emacs 26.0.50.9 (x86_64-unknown-linux-gnu, GTK+ Version 3.22.5)
 of 2016-12-26 built on clem
Repository revision: 65b997b95e284e2edc1266663e39791f68d76ad7
Windowing system distributor 'The X.Org Foundation', version 11.0.11804000
Recent messages:
 [6 times]
mwheel-scroll: Beginning of buffer [5 times]
Mark saved where search started
Checking new news...
Reading active file from Test via nngnorb...done
Reading active file from archive via nnfolder...done
Reading active file via nndraft...done
Reading active file from MTester via nnmairix...done
Checking new news...done
Processing kill file /home/eric/.emacs.d/News/gmane.emacs.devel.KILL...done

bug#25295: Acknowledgement (26.0.50; Represent eieio objects using object-print in backtraces and edebug)

[Eric Abrahamsen]

I guess this would require going into print.c and adding another branch
under the Lisp_Vectorlike case statement of object_print.

Is this sort of C code allowed to call back up to the lisp object-print
function? This is where I get lost...

bug#25295: Acknowledgement (26.0.50; Represent eieio objects using object-print in backtraces and edebug)

[npostavs]

severity 25295 wishlist
quit

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> I guess this would require going into print.c and adding another branch
> under the Lisp_Vectorlike case statement of object_print.

You mean print_object?  I thought the idea would be to change edebug and
backtrace code to (optionally?) use object-print instead of print.

bug#25295: Acknowledgement (26.0.50; Represent eieio objects using object-print in backtraces and edebug)

[Eric Abrahamsen]

On 12/29/16 22:42 PM, npostavs@users.sourceforge.net wrote:

> You mean print_object?  I thought the idea would be to change edebug and
> backtrace code to (optionally?) use object-print instead of print.

I do mean print_object, sorry. (And sorry I keep replying to you only!)

On 12/30/16 09:51 AM, Eli Zaretskii wrote:
>> From: Eric Abrahamsen <eric@ericabrahamsen.net>
>> Date: Thu, 29 Dec 2016 13:50:09 -0800
>> 
>> I guess this would require going into print.c and adding another branch
>> under the Lisp_Vectorlike case statement of object_print.
>> 
>> Is this sort of C code allowed to call back up to the lisp object-print
>> function?
>
> If that Lisp function will then call print.c again, that's not a good
> idea, since print.c internally uses a buffer by a certain fixed name.

`object-print' ends up using `format', which looks like it calls print.c
functions, so I guess that's out.

I don't know the right level at which to intervene. All other lisp
objects get a hard-coded #<obj representation from print_object in
print.c, only eieio objects "fake it" with a user-overrideable lisp
function. I suspect eieio objects won't be considered "fundamental" on
the same level as markers, buffers, etc., so maybe they don't belong in
print_object (plus the above problem of calling lisp-c-lisp-c).

I don't see how we could hijack at the lisp level, though. Functions
like `eval-expression' and `backtrace--print-frame' simply toss whole
lisp structures to prin1, there's no way to know that there's an eieio
object somewhere in that structure.

Personally, I'd be willing to lose the ability to customize object
representations with `object-print', if it meant that print_object could
produce a #<obj notation for eieio objects. That would mean writing a 
C test like INSTANCEP or what have you.

I don't know what the right solution is!

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

bug#25295: Acknowledgement (26.0.50; Represent eieio objects using object-print in backtraces and edebug)

[npostavs]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:
> On 12/30/16 09:51 AM, Eli Zaretskii wrote:
>>> From: Eric Abrahamsen <eric@ericabrahamsen.net>
>>> Date: Thu, 29 Dec 2016 13:50:09 -0800
>>> 
>>> I guess this would require going into print.c and adding another branch
>>> under the Lisp_Vectorlike case statement of object_print.
>>> 
>>> Is this sort of C code allowed to call back up to the lisp object-print
>>> function?
>>
>> If that Lisp function will then call print.c again, that's not a good
>> idea, since print.c internally uses a buffer by a certain fixed name.
>
> `object-print' ends up using `format', which looks like it calls print.c
> functions, so I guess that's out.
>
> I don't know the right level at which to intervene. All other lisp
> objects get a hard-coded #<obj representation from print_object in
> print.c, only eieio objects "fake it" with a user-overrideable lisp
> function. I suspect eieio objects won't be considered "fundamental" on
> the same level as markers, buffers, etc., so maybe they don't belong in
> print_object (plus the above problem of calling lisp-c-lisp-c).
>
> I don't see how we could hijack at the lisp level, though. Functions
> like `eval-expression' and `backtrace--print-frame' simply toss whole
> lisp structures to prin1, there's no way to know that there's an eieio
> object somewhere in that structure.
>

I think the only way to integrate `object-print' with the existing
`print' functions, would be to make it follow the same protocol.  That
is, currently `object-print' is really `object-to-string', it should be
changed (or perhaps a new function (e.g., `print-object') would be a
better idea, so as not to break existing code too much) to accept a
PRINTCHARFUN argument, and print to it.

> Personally, I'd be willing to lose the ability to customize object
> representations with `object-print', if it meant that print_object could
> produce a #<obj notation for eieio objects. That would mean writing a 
> C test like INSTANCEP or what have you.
>

That's easier, of course, but a non-customized representation would be
pretty uninformative.

bug#25295: Acknowledgement (26.0.50; Represent eieio objects using object-print in backtraces and edebug)

[Eric Abrahamsen]

On 12/31/16 00:48 AM, npostavs@users.sourceforge.net wrote:
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:

[...]

>> I don't see how we could hijack at the lisp level, though. Functions
>> like `eval-expression' and `backtrace--print-frame' simply toss whole
>> lisp structures to prin1, there's no way to know that there's an eieio
>> object somewhere in that structure.
>>
>
> I think the only way to integrate `object-print' with the existing
> `print' functions, would be to make it follow the same protocol.  That
> is, currently `object-print' is really `object-to-string', it should be
> changed (or perhaps a new function (e.g., `print-object') would be a
> better idea, so as not to break existing code too much) to accept a
> PRINTCHARFUN argument, and print to it.

The problem is that pretty much all of the printing happens at the C
level. Whole lisp structures are sent directly to C, and it's the C code
that recurses through them and decides how to print everything it finds
inside. Lisp code never gets a chance (except in a few very specific
situations).

For example: when an error is raised, `backtrace--print-frame' gets all
the contents of the error as a single argument. It simply punts that to
`prin1', and then it's done. There's no chance to pick apart that single
argument and see if there is an object inside. `eval-expression'
essentially does the same thing.

>> Personally, I'd be willing to lose the ability to customize object
>> representations with `object-print', if it meant that print_object could
>> produce a #<obj notation for eieio objects. That would mean writing a 
>> C test like INSTANCEP or what have you.
>>
>
> That's easier, of course, but a non-customized representation would be
> pretty uninformative.

Having looked at the code, I'm not too optimistic about achieving the
ideal solution. Getting eval-expression and backtraces to stop exploding
seems like enough for now.

bug#25295: Acknowledgement (26.0.50; Represent eieio objects using object-print in backtraces and edebug)

[npostavs]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> On 12/31/16 00:48 AM, npostavs@users.sourceforge.net wrote:
>> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>>
>> I think the only way to integrate `object-print' with the existing
>> `print' functions, would be to make it follow the same protocol.  That
>> is, currently `object-print' is really `object-to-string', it should be
>> changed (or perhaps a new function (e.g., `print-object') would be a
>> better idea, so as not to break existing code too much) to accept a
>> PRINTCHARFUN argument, and print to it.
>
> The problem is that pretty much all of the printing happens at the C
> level. Whole lisp structures are sent directly to C, and it's the C code
> that recurses through them and decides how to print everything it finds
> inside. Lisp code never gets a chance (except in a few very specific
> situations).
>
> For example: when an error is raised, `backtrace--print-frame' gets all
> the contents of the error as a single argument. It simply punts that to
> `prin1', and then it's done. There's no chance to pick apart that single
> argument and see if there is an object inside. `eval-expression'
> essentially does the same thing.
>

You would need to add a branch in print_object to detect eieio objects,
same as for the other alternative.

>>> Personally, I'd be willing to lose the ability to customize object
>>> representations with `object-print', if it meant that print_object could
>>> produce a #<obj notation for eieio objects. That would mean writing a 
>>> C test like INSTANCEP or what have you.
>>>
>>
>> That's easier, of course, but a non-customized representation would be
>> pretty uninformative.
>
> Having looked at the code, I'm not too optimistic about achieving the
> ideal solution. Getting eval-expression and backtraces to stop exploding
> seems like enough for now.

bug#25295: Acknowledgement (26.0.50; Represent eieio objects using object-print in backtraces and edebug)

[Eric Abrahamsen]

On 12/31/16 14:23 PM, npostavs@users.sourceforge.net wrote:
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> On 12/31/16 00:48 AM, npostavs@users.sourceforge.net wrote:
>>> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>>>
>>> I think the only way to integrate `object-print' with the existing
>>> `print' functions, would be to make it follow the same protocol.  That
>>> is, currently `object-print' is really `object-to-string', it should be
>>> changed (or perhaps a new function (e.g., `print-object') would be a
>>> better idea, so as not to break existing code too much) to accept a
>>> PRINTCHARFUN argument, and print to it.
>>
>> The problem is that pretty much all of the printing happens at the C
>> level. Whole lisp structures are sent directly to C, and it's the C code
>> that recurses through them and decides how to print everything it finds
>> inside. Lisp code never gets a chance (except in a few very specific
>> situations).
>>
>> For example: when an error is raised, `backtrace--print-frame' gets all
>> the contents of the error as a single argument. It simply punts that to
>> `prin1', and then it's done. There's no chance to pick apart that single
>> argument and see if there is an object inside. `eval-expression'
>> essentially does the same thing.
>>
>
> You would need to add a branch in print_object to detect eieio objects,
> same as for the other alternative.

Well sure, that's what this whole thread started out with.

bug#25295: Acknowledgement (26.0.50; Represent eieio objects using object-print in backtraces and edebug)

[Eli Zaretskii]

> From: Eric Abrahamsen <eric@ericabrahamsen.net>
> Date: Thu, 29 Dec 2016 13:50:09 -0800
> 
> I guess this would require going into print.c and adding another branch
> under the Lisp_Vectorlike case statement of object_print.
> 
> Is this sort of C code allowed to call back up to the lisp object-print
> function?

If that Lisp function will then call print.c again, that's not a good
idea, since print.c internally uses a buffer by a certain fixed name.

bug#25295: Represent eieio objects using object-print in backtraces and edebug

[Stefan Monnier]

> I guess this would require going into print.c and adding another branch
> under the Lisp_Vectorlike case statement of object_print.

That's right.

> Is this sort of C code allowed to call back up to the lisp object-print
> function? This is where I get lost...

The Elisp-level printer and the interaction between the C code and
the Elisp ill have to be worked out, indeed.

E.g. you probably won't want to have Elisp-level printer to just call
`format` or `prin1` since that does more than just print (it sets up
a printing destination, initializes the cycle-detection table, then
checks for cycles, ...).

This said, making `prin1` reentrant will probably be helpful.

One more thing: there have already been other desires/needs to tweak the
printing from Elisp.  E.g. to special case printing of objects that
can't be printed readably (e.g. buffers, markers, overlays, ...).


        Stefan


PS: One more thing; currently cl-structs and EIEIO objects are
represented as plain normal arrays (and they mostly use the same
representation now, where the first field contains a symbol whose
function cell contains `:quick-object-witness-check` and whose value
cell is the actual class object, which is a cl-struct that inherits
from cl--class).

It'd be good to instead provide a new kind of array just for
objects/records/structs (just like we have the `compiled-function` type
which is basically a normal array just with a different tag).
This would let us get rid of the "symbol with
a :quick-object-witness-check" indirection and would make it easier for
the print.c code to delegate to the Elisp-level printer since it
wouldn't need to know about the hackish representation choices of EIEIO
and cl-structs.

bug#25295: Represent eieio objects using object-print in backtraces and edebug

[Eric Abrahamsen]

On 01/03/17 13:21 PM, Stefan Monnier wrote:
>> I guess this would require going into print.c and adding another branch
>> under the Lisp_Vectorlike case statement of object_print.
>
> That's right.
>
>> Is this sort of C code allowed to call back up to the lisp object-print
>> function? This is where I get lost...
>
> The Elisp-level printer and the interaction between the C code and
> the Elisp ill have to be worked out, indeed.
>
> E.g. you probably won't want to have Elisp-level printer to just call
> `format` or `prin1` since that does more than just print (it sets up
> a printing destination, initializes the cycle-detection table, then
> checks for cycles, ...).
>
> This said, making `prin1` reentrant will probably be helpful.

If an Elisp-level printer is available to users and developers, I think
we'd have to assume they will throw any old thing in there, including
`format' and `prin1'. Would making `prin1' reentrant solve all of the
resulting problems?

> One more thing: there have already been other desires/needs to tweak the
> printing from Elisp.  E.g. to special case printing of objects that
> can't be printed readably (e.g. buffers, markers, overlays, ...).

That's good! But the previous issue would still need to be resolved.

>
>         Stefan
>
>
> PS: One more thing; currently cl-structs and EIEIO objects are
> represented as plain normal arrays (and they mostly use the same
> representation now, where the first field contains a symbol whose
> function cell contains `:quick-object-witness-check` and whose value
> cell is the actual class object, which is a cl-struct that inherits
> from cl--class).
>
> It'd be good to instead provide a new kind of array just for
> objects/records/structs (just like we have the `compiled-function` type
> which is basically a normal array just with a different tag).
> This would let us get rid of the "symbol with
> a :quick-object-witness-check" indirection and would make it easier for
> the print.c code to delegate to the Elisp-level printer since it
> wouldn't need to know about the hackish representation choices of EIEIO
> and cl-structs.

I really wish I could help, but I've never written a line of C, and
in twenty minutes of looking through the sources earlier today, I
couldn't even find where the array types were defined.

It sounds like this would be Step One of solving the problem, but I
would not be able to do it without some substantial hand-holding.

bug#25295: Represent eieio objects using object-print in backtraces and edebug

[Stefan Monnier]

> If an Elisp-level printer is available to users and developers, I think
> we'd have to assume they will throw any old thing in there, including
> `format' and `prin1'. Would making `prin1' reentrant solve all of the
> resulting problems?

Maybe the better solution would be to make `prin1` (and `format`, ..)
signal errors.  Instead, we could provide the printer with a special
prin1-like function which basically "continues" the print job in the
C code.

>> One more thing: there have already been other desires/needs to tweak the
>> printing from Elisp.  E.g. to special case printing of objects that
>> can't be printed readably (e.g. buffers, markers, overlays, ...).

> That's good! But the previous issue would still need to be resolved.

Yes, and they're largely independent: the printer you're talking about
would be used when we don't want to print readably, whereas this case
would apply to the situation where we want to print readably (in which
case we don't want to go through custom pretty printers like the one you
need).


        Stefan

bug#25295: Represent eieio objects using object-print in backtraces and edebug

[Stefan Monnier]

> Maybe the better solution would be to make `prin1` (and `format`, ..)
> signal errors.  Instead, we could provide the printer with a special
> prin1-like function which basically "continues" the print job in the
> C code.

Actually, print.c is already setup to be (at least partially) reentrant
(for the case `printcharfun` is a function), so maybe making it fully
reentrant won't be too hard.  But we'd still want to provide some ad-hoc
prin1-like function which continues the print job rather than starting
a new nested one; this is needed e.g. if the custom printer wants to
print a value V but V has already been printed so it should be printed
as a reference #NN# to the previous one.


        Stefan

bug#25295: Represent eieio objects using object-print in backtraces and edebug

[Stefan Monnier]

> Actually, print.c is already setup to be (at least partially) reentrant
> (for the case `printcharfun` is a function), so maybe making it fully
> reentrant won't be too hard.  But we'd still want to provide some ad-hoc
> prin1-like function which continues the print job rather than starting
> a new nested one; this is needed e.g. if the custom printer wants to
> print a value V but V has already been printed so it should be printed
> as a reference #NN# to the previous one.

Looking further at the code, maybe this isn't even needed, since the
print-number-table can survive calls to print.  But that can get ugly
when your printer code also want to call `print` for "unrelated"
purposes (e.g. to print debug messages in a side buffer).
And I'll stop thinking about interaction with concurrency.


        Stefan

bug#25295: Represent eieio objects using object-print in backtraces and edebug

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Wed, 04 Jan 2017 23:37:26 -0500
> Cc: 25295@debbugs.gnu.org
> 
> And I'll stop thinking about interaction with concurrency.

There should be none, since only one thread running Lisp could be
doing that at any given time.  Right?

bug#25295: Represent eieio objects using object-print in backtraces and edebug

[Stefan Monnier]

>> And I'll stop thinking about interaction with concurrency.
> There should be none, since only one thread running Lisp could be
> doing that at any given time.  Right?

I'm thinking of

   prin1
   -> enters print.c
   -> calls back the custom Elisp-side object-print
   -> which decides to accept-process-output


-- Stefan

bug#25295: Represent eieio objects using object-print in backtraces and edebug

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: eric@ericabrahamsen.net,  25295@debbugs.gnu.org
> Date: Thu, 05 Jan 2017 19:42:55 -0500
> 
>    prin1
>    -> enters print.c
>    -> calls back the custom Elisp-side object-print
>    -> which decides to accept-process-output

This will cause problems without any threads as well, because the last
line might cause timers to run.

bug#25295: Represent eieio objects using object-print in backtraces and edebug

[npostavs]

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

>> Actually, print.c is already setup to be (at least partially) reentrant
>> (for the case `printcharfun` is a function), so maybe making it fully
>> reentrant won't be too hard.  But we'd still want to provide some ad-hoc
>> prin1-like function which continues the print job rather than starting
>> a new nested one; this is needed e.g. if the custom printer wants to
>> print a value V but V has already been printed so it should be printed
>> as a reference #NN# to the previous one.
>
> Looking further at the code, maybe this isn't even needed, since the
> print-number-table can survive calls to print.  But that can get ugly
> when your printer code also want to call `print` for "unrelated"
> purposes (e.g. to print debug messages in a side buffer).
> And I'll stop thinking about interaction with concurrency.

Can we allow overriding printing of primitive types too?  I'm wanting
that for e.g., printing byte code functions in nicer ways.

Looking to Common Lisp for inspiration, there is generic function
print-object[1], and also *print-pprint-dispatch* (an alist of type
specifiers -> printer functions).

Cycle detection state seems to be packed in with STREAM, which is
convenient, although dispatching on STREAM becomes harder.

[1]: http://www.ai.mit.edu/projects/iiip/doc/CommonLISP/HyperSpec/Body/stagenfun_print-object.html
[2]: http://www.ai.mit.edu/projects/iiip/doc/CommonLISP/HyperSpec/Body/sec_22-2-1-4.html

bug#25295: Represent eieio objects using object-print in backtraces and edebug

[Stefan Monnier]

> Can we allow overriding printing of primitive types too?
> I'm wanting that for e.g., printing byte code functions in nicer ways.

Maybe we should just switch to an Elisp version of printing, in that case.
We could keep the C code for the "print-readably" case only.
The main question is whether it's fast enough.


        Stefan


;;; cl-print.el --- Generic printer facilies         -*- lexical-binding: t; -*-

;; Copyright (C) 2017  Stefan Monnier

;; Author: Stefan Monnier <monnier@iro.umontreal.ca>
;; Keywords: 

;; This program is free software; you can redistribute it and/or modify
;; it under the terms of the GNU General Public License as published by
;; the Free Software Foundation, either version 3 of the License, or
;; (at your option) any later version.

;; This program is distributed in the hope that it will be useful,
;; but WITHOUT ANY WARRANTY; without even the implied warranty of
;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
;; GNU General Public License for more details.

;; You should have received a copy of the GNU General Public License
;; along with this program.  If not, see <http://www.gnu.org/licenses/>.

;;; Commentary:

;;; Code:

;;;###autoload
(cl-defgeneric cl-print-object (object stream)
  "Dispatcher to print OBJECT on STREAM according to its type."
  (prin1 object stream))

(cl-defmethod cl-print-object ((object cons) stream)
  (let ((car (pop object)))
    (if (and (memq car '(\, quote \` \,@ \,.))
             (consp object)
             (null (cdr object)))
        (progn
          (princ (if (eq car 'quote) '\' car) stream)
          (cl-print-object (car object) stream))
      (princ "(" stream)
      (cl-print-object car stream)
      (while (consp object)
        (princ " " stream)
        (cl-print-object (pop object) stream))
      (when object
        (princ " . ") (cl-print-object object stream))
      (princ ")"))))

(cl-defmethod cl-print-object ((object vector) stream)
  (princ "[" stream)
  (dotimes (i (length object))
    (unless (zerop i) (princ " " stream))
    (cl-print-object (aref object i) stream))
  (princ "]" stream))

(cl-defmethod cl-print-object ((object compiled-function) stream)
  ;; FIXME: Give a prettier representation.
  (princ "#<compiled-function>" stream))

(cl-defmethod cl-print-object ((object cl-structure-object) stream)
  (princ "#s(")
  (let* ((class (symbol-value (aref object 0)))
         (slots (cl--struct-class-slots class)))
    (princ (cl--struct-class-name class) stream)
    (dotimes (i (length slots))
      (let ((slot (aref slots i)))
        (princ " :" stream)
        (princ (cl--slot-descriptor-name slot) stream)
        (princ " " stream)
        (cl-print-object (aref object (1+ i)) stream))))
  (princ ")"))

;;; Circularity and sharing.

;; I don't try to support the `print-continuous-numbering', because
;; I think it's ill defined anyway: if an object appears only once in each call
;; its sharing can't be properly preserved!

(defvar cl-print--number-index nil)
(defvar cl-print--number-table nil)

(cl-defmethod cl-print-object :around (object stream)
  ;; FIXME: Only put such an :around method on types where it's relevant.
  (let ((n (if cl-print--number-table (gethash object cl-print--number-table))))
    (if (not (numberp n))
        (cl-call-next-method)
      (if (> n 0)
          ;; Already printed.  Just print a reference.
          (progn (princ "#" stream) (princ n stream) (princ "#" stream))
        (puthash object (- n) cl-print--number-table)
        (princ "#" stream) (princ (- n) stream) (princ "=" stream)
        (cl-call-next-method)))))

(defun cl-print--find-sharing (object table)
  (unless
      ;; Skip objects which don't have identity!
      (or (floatp object) (numberp object))
    (let ((n (gethash object table)))
      (cond
       ((numberp n))                   ;All done.
       (n                              ;Already seen, but only once.
        (let ((n (1+ cl-print--number-index)))
          (setq cl-print--number-index n)
          (puthash object (- n) table)))
       (t
         (puthash object t table)
         (pcase object
           (`(,car . ,cdr)
            (cl-print--find-sharing car table)
            (cl-print--find-sharing cdr table))
           ((pred stringp)
            ;; We presumably won't print its text-properties.
            nil)
           ((pred arrayp)             ;FIXME: Inefficient for char-tables!
            (dotimes (i (length object))
              (cl-print--find-sharing (aref object i) table)))))))))

;;;###autoload
(defun cl-prin1 (object &optional stream)
  (if (not print-circle)
      (cl-print-object object stream)
    (let ((cl-print--number-table (make-hash-table :test 'eq))
          (cl-print--number-index 0))
      (cl-print--find-sharing object cl-print--number-table)
      (cl-print-object object stream))))

(provide 'cl-print)
;;; cl-print.el ends here

bug#25295: Represent eieio objects using object-print in backtraces and edebug

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

Most of the time, the speed of printing is not important.
But I suspect there are cases where people print a lot of data
and the speed of printing is crucial.

-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.

bug#25295: Represent eieio objects using object-print in backtraces and edebug

[Stefan Monnier]

> Most of the time, the speed of printing is not important.
> But I suspect there are cases where people print a lot of data
> and the speed of printing is crucial.

Indeed.  I know `C-h v load-history` is often sluggish, for example,
because of the size of the value.

I've been trying my cl-print code and the speed is clearly inferior to
the C code, but at least in the case of C-h v the main slowdown comes
from the prettifying step that happens after printing (where we insert
newlines and indent the result), so while it gets measurably slower, the
impact might be tolerable.

Clearly, for cases such as when the byte-compiler wants to print the
result into a .elc file, or when we want to save an undo-log in a file,
speed will be very important and my cl-print is unlikely to be
good enough.  That's why I was suggesting that maybe we should have
2 printers: a fast one in C for the `print-readably` case, and
a slower but customizable one in Elisp for "human consumption".


        Stefan

Elisp printer (was: bug#25295: Represent eieio objects using object-print in backtraces and edebug)

[Michael Heerdegen]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> Maybe we should just switch to an Elisp version of printing, in that
> case.

That's a very nice idea, not just for the eieio case.


> ;;; cl-print.el --- Generic printer facilies         -*- lexical-binding: t; -*-

Does "cl" stand for "command line" or for "Common Lisp"?  I find the
name hard to remember in both cases, because that's not what comes to my
mind when I think of this library (when I internet-search for
"cl-print", I find mostly shops that print business cards and such
stuff...)

Anyway, we really really need to define a printer for thunk.el thunks,
and maybe an extra printer for streams (that's why I CC Nicolas), and
probably some similar other stuff!


Michael.

Re: Elisp printer

[Stefan Monnier]

> Does "cl" stand for "command line" or for "Common Lisp"?

It's for "Common Lisp", since it uses the Common Lisp names and
calling conventions (as well as reliance on CLOS).

> I find the name hard to remember in both cases, because that's not
> what comes to my mind when I think of this library (when
> I internet-search for "cl-print", I find mostly shops that print
> business cards and such stuff...)

It would probably make sense to get rid of the "cl-" prefix at some
point, but that will cause `cl-prin1` and `prin1` to collide, so it
would require to merge the two somehow.  We might want to do it down the
road, but I figured it as better to move forward with a separate name
space so we don't have to solve that problem before we can use the
new functionality.

> Anyway, we really really need to define a printer for thunk.el thunks,
> and maybe an extra printer for streams (that's why I CC Nicolas), and
> probably some similar other stuff!

If you look in the cl-print.el I installed into the master branch,
you'll see it already has support for printing advice objects, so it
should be fairly easy to use the same approach to add support for
thunks, streams, iterators, ... (and contrary to what I did for advice,
that support can be placed in thunk.el, stream.el, ...).  Basically, the
question is to figure out how to *recognize* those functions.


        Stefan


PS: I wish we could have funcallable defstructs, so we could
    efficiently dispatch on the "type" of functions when those functions
    are used as objects.

Re: Elisp printer

[Michael Heerdegen]

Hi Stefan,

I noticed that pp.el doesn't handle the #<...> print syntax very well.
I asked myself whether for the purpose of font-lock, indenting, moving
by parens etc - it would make sense to switch to a print syntax that is
`read'able (pseudo) Lisp so that we could just use Emacs-Lisp mode to
present/work with the print results?  I think that could simplify the
work with the new printer.


Michael.

Re: Elisp printer

[Stefan Monnier]

> I noticed that pp.el doesn't handle the #<...> print syntax very well.

Which ones in particular have you noticed (it's used in different ways:
some are very old some are more recent (e.g. new in cl-print), some are
very simple, others more complex, ...)?

> I asked myself whether for the purpose of font-lock, indenting, moving
> by parens etc - it would make sense to switch to a print syntax that is
> `read'able (pseudo) Lisp so that we could just use Emacs-Lisp mode to
> present/work with the print results?  I think that could simplify the
> work with the new printer.

We could use a syntax more like that of structs, i.e. something of the
form #s(...).  For those objects which really aren't structs at all, we
could use a similar notation with another letter (e.g. #f(...) for
function objects such as advice thingies)?


        Stefan

Re: Elisp printer

[Michael Heerdegen]

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

> > I noticed that pp.el doesn't handle the #<...> print syntax very well.
>
> Which ones in particular have you noticed (it's used in different ways:
> some are very old some are more recent (e.g. new in cl-print), some are
> very simple, others more complex, ...)?

I tried with (symbol-function 'byte-compile-arglist-warn), where I have
advised that function, and got

#+begin_src emacs-lisp
#<advice-wrapper :around #<compiled-function
(name arglist macrop)
#<bytecode> > my-byte-compile-arglist-warn--around-ad>
#+end_src

which not really looks neat (especially with the unindented broken
lines).  I produced that with a hacked `pp-to-string' that uses
`cl-prin1' as printer.

> We could use a syntax more like that of structs, i.e. something of the
> form #s(...).  For those objects which really aren't structs at all, we
> could use a similar notation with another letter (e.g. #f(...) for
> function objects such as advice thingies)?

Is just one letter enough?  How would thunks and streams look like with
this scheme?


Michael.

Re: Elisp printer

[Stefan Monnier]

> I tried with (symbol-function 'byte-compile-arglist-warn), where I have
> advised that function, and got

> #+begin_src emacs-lisp
> #<advice-wrapper :around #<compiled-function
> (name arglist macrop)
> #<bytecode> > my-byte-compile-arglist-warn--around-ad>
> #+end_src

I see, yes, that's ugly.

>> We could use a syntax more like that of structs, i.e. something of the
>> form #s(...).  For those objects which really aren't structs at all, we
>> could use a similar notation with another letter (e.g. #f(...) for
>> function objects such as advice thingies)?
> Is just one letter enough?

Of course: the "#s(..)" is really "#s(<type> ...)" so the above example
could look like

    #f(advice-wrapper :around #f(compiled-function (name arglist macrop)
                                                   #<bytecode>)
                       my-byte-compile-arglist-warn--around-ad)

> How would thunks and streams look like with
> this scheme?

You'd get to choose when you implement their corresponding methods but
they could look like

    #f(thunk ...)
and
    #f(stream ...)


-- Stefan

Re: Elisp printer

[Tom Tromey]

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

Stefan> PS: I wish we could have funcallable defstructs, so we could
Stefan>     efficiently dispatch on the "type" of functions when those functions
Stefan>     are used as objects.

I was wondering about this a while ago and thinking that maybe it could
be done without C changes by repurposing the extra arguments to
make-byte-code.  The idea here would be to let cl-defsubst take a new
:type argument other than 'vector or 'list, meaning "use
make-byte-code"; and set the :initial-offset to skip over the stuff
necessary for the bytecode.

It's kind of hacky though.  I think it's probably better to just add
funcallable instances directly, and real types of some kind at the same
time.

Tom

Re: Elisp printer

[Stefan Monnier]

> I was wondering about this a while ago and thinking that maybe it could
> be done without C changes by repurposing the extra arguments to
> make-byte-code.  The idea here would be to let cl-defsubst take a new
> :type argument other than 'vector or 'list, meaning "use
> make-byte-code"; and set the :initial-offset to skip over the stuff
> necessary for the bytecode.

Yes, that can be done fairly easily.  But that doesn't give you callable
functions: it just gives you structs represented by those special
"compiled-function" vectors.

E.g. thunk.el creates its thunks with:

    (defmacro thunk-delay (&rest body)
      "Delay the evaluation of BODY."
      (declare (debug t))
      (let ((forced (make-symbol "forced"))
            (val (make-symbol "val")))
        `(let (,forced ,val)
           (lambda (&optional check)
             (if check
                 ,forced
               (unless ,forced
                 (setf ,val (progn ,@body))
                 (setf ,forced t))
               ,val)))))

so we'd need some way to specify both the function's body and its
"struct" at the same time.  What I was thinking of was something like

    (callable-defstruct thunk
      val forced)

    (defmacro thunk-delay (&rest body)
      "Delay the evaluation of BODY."
      (declare (debug t))
      `(make-thunk (&optional check)
         (if check
             forced
           (unless forced
             (setq val (progn ,@body))
             (setq forced t))
           val)))

The idea would be that `forced` and `val` would be fields of the
"callable-struct" and would be accessible directly from the body of the
function (as well as from the outside via thunk-val and thunk-forced
accessors).  That would require changes to the byte-compiler (mostly in
cconv.el) but it can probably be made to work without significant
changes at the C level.


        Stefan

Re: Elisp printer

[Tom Tromey]

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

>> I was wondering about this a while ago and thinking that maybe it could
>> be done without C changes by repurposing the extra arguments to
>> make-byte-code.  The idea here would be to let cl-defsubst take a new
>> :type argument other than 'vector or 'list, meaning "use
>> make-byte-code"; and set the :initial-offset to skip over the stuff
>> necessary for the bytecode.

Stefan> Yes, that can be done fairly easily.  But that doesn't give you
Stefan> callable functions: it just gives you structs represented by
Stefan> those special "compiled-function" vectors.

What I forgot to mention is that this would be coupled with a slot in
the base class that holds the function to "actually call", and the
underlying call to make-byte-code would install some bytecode that would
simply funcall whatever is in this slot, maybe passing in the object as
a first argument.

I'm not 100% sure this addresses what you're looking for; if not I'd
appreciate it if you could explain more.

One thing I note is that this doesn't provide the "sugar" of being able
to refer to slots using their bare name, you'd have to use accessors.
I'm not sure that this omission is so bad, but maybe make-thunk could
wrap the body of the generated function in some cl-symbol-macrolet forms

Tom

Re: Elisp printer

[Stefan Monnier]

> What I forgot to mention is that this would be coupled with a slot in
> the base class that holds the function to "actually call", and the
> underlying call to make-byte-code would install some bytecode that would
> simply funcall whatever is in this slot, maybe passing in the object as
> a first argument.

But that adds a funcall-indirection.  Given the cost of funcalls in
Elisp, this is a fairly major price to pay.  In that case, you're better
off representing your functions as symbols, so you store the actual
function in the function slot, and the data slots in the symbol-plist or
symbol-value slots.

> I'm not 100% sure this addresses what you're looking for; if not I'd
> appreciate it if you could explain more.

Yes, it would, except I don't like its performance.

> One thing I note is that this doesn't provide the "sugar" of being able
> to refer to slots using their bare name, you'd have to use accessors.

That sugar is just an attractive idea, not indispensable.


        Stefan

Re: Elisp printer

[Lars Brinkhoff]

Tom Tromey <tom@tromey.com> writes:
> It's kind of hacky though.  I think it's probably better to just add
> funcallable instances directly, and real types of some kind at the same
> time.

I tried to submit a patch for user-defined record types some years ago.
Instances were pseudovectors, with the first element being a symbol
naming its type.

Re: Elisp printer

[Stefan Monnier]

>> It's kind of hacky though.  I think it's probably better to just add
>> funcallable instances directly, and real types of some kind at the same
>> time.
> I tried to submit a patch for user-defined record types some years ago.
> Instances were pseudovectors, with the first element being a symbol
> naming its type.

Yes, I think we're pretty much ready to accept such a patch,


        Stefan

Re: Elisp printer

[Lars Brinkhoff]

Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>> It's kind of hacky though.  I think it's probably better to just add
>>> funcallable instances directly, and real types of some kind at the same
>>> time.
>> I tried to submit a patch for user-defined record types some years ago.
>> Instances were pseudovectors, with the first element being a symbol
>> naming its type.
> Yes, I think we're pretty much ready to accept such a patch,

Great, I'll see if I can dust if off and send it in again.

User-defined record types

[Lars Brinkhoff]

Stefan Monnier wrote:
>> Tom Tromey wrote:
>>> It's kind of hacky though.  I think it's probably better to just add
>>> funcallable instances directly, and real types of some kind at the
>>> same time.
>> I tried to submit a patch for user-defined record types some years
>> ago.  Instances were pseudovectors, with the first element being a
>> symbol naming its type.
> Yes, I think we're pretty much ready to accept such a patch

This is my old patch dusted off and rebased to current master.
It's just a raw material posted for review.

A test case would be:

  (let ((x (make-record 'foo 3 nil)))
    (aset x 1 1)
    (aset x 2 2)
    (aset x 3 3)
    (list (read-from-string (with-output-to-string (prin1 x)))
    (recordp x)
    (type-of x)
    (aref x 0)
    (aref x 3)
    (length x)))

This evalates to ((#%[foo 1 2 3] . 13) t foo foo 3 4).


diff --git a/src/alloc.c b/src/alloc.c
index ae3e151..de08276 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -3392,6 +3392,46 @@ struct buffer *
   return b;
 }
 
+static void
+check_record_type (Lisp_Object type)
+{
+  if (!SYMBOLP(type))
+    error ("Invalid type; must be symbol");
+}
+
+static struct Lisp_Vector *
+allocate_record (int count)
+{
+  if (count >= (1 << PSEUDOVECTOR_SIZE_BITS))
+    error ("Record too large");
+
+  struct Lisp_Vector *p = allocate_vector (count);
+  XSETPVECTYPE (p, PVEC_RECORD);
+  return p;
+}
+
+DEFUN ("make-record", Fmake_record, Smake_record, 3, 3, 0,
+       doc: /* Create a new record of type TYPE with SLOTS elements, each initialized to INIT.  */)
+  (Lisp_Object type, Lisp_Object slots, Lisp_Object init)
+{
+  Lisp_Object vector;
+  ptrdiff_t size, i;
+  struct Lisp_Vector *p;
+
+  CHECK_NATNUM (slots);
+  check_record_type (type);
+
+  size = XFASTINT (slots) + 1;
+  p = allocate_record (size);
+  p->contents[0] = type;
+  for (i = 1; i < size; i++)
+    p->contents[i] = init;
+
+  XSETVECTOR (vector, p);
+  return vector;
+}
+
+
 DEFUN ("make-vector", Fmake_vector, Smake_vector, 2, 2, 0,
        doc: /* Return a newly created vector of length LENGTH, with each element being INIT.
 See also the function `vector'.  */)
@@ -7465,6 +7505,7 @@ This means that certain objects should be allocated in shared (pure) space.
   defsubr (&Smake_byte_code);
   defsubr (&Smake_list);
   defsubr (&Smake_vector);
+  defsubr (&Smake_record);
   defsubr (&Smake_string);
   defsubr (&Smake_bool_vector);
   defsubr (&Smake_symbol);
diff --git a/src/data.c b/src/data.c
index ae8dd97..eceb752 100644
--- a/src/data.c
+++ b/src/data.c
@@ -267,6 +267,7 @@ static void swap_in_symval_forwarding (struct Lisp_Symbol *,
         case PVEC_MUTEX: return Qmutex;
         case PVEC_CONDVAR: return Qcondition_variable;
         case PVEC_TERMINAL: return Qterminal;
+        case PVEC_RECORD: return AREF (object, 0);
         /* "Impossible" cases.  */
         case PVEC_XWIDGET:
         case PVEC_OTHER:
@@ -359,6 +360,15 @@ static void swap_in_symval_forwarding (struct Lisp_Symbol *,
   return Qnil;
 }
 
+DEFUN ("recordp", Frecordp_p, Srecordp, 1, 1, 0,
+       doc: /* Return t if OBJECT is a record.  */)
+  (Lisp_Object object)
+{
+  if (RECORDP (object))
+    return Qt;
+  return Qnil;
+}
+
 DEFUN ("stringp", Fstringp, Sstringp, 1, 1, 0,
        doc: /* Return t if OBJECT is a string.  */
        attributes: const)
@@ -2287,7 +2297,7 @@ If the current binding is global (the default), the value is nil.  */)
       ptrdiff_t size = 0;
       if (VECTORP (array))
 	size = ASIZE (array);
-      else if (COMPILEDP (array))
+      else if (COMPILEDP (array) || RECORDP (array))
 	size = ASIZE (array) & PSEUDOVECTOR_SIZE_MASK;
       else
 	wrong_type_argument (Qarrayp, array);
@@ -2308,7 +2318,8 @@ If the current binding is global (the default), the value is nil.  */)
 
   CHECK_NUMBER (idx);
   idxval = XINT (idx);
-  CHECK_ARRAY (array, Qarrayp);
+  if (! RECORDP (array))
+    CHECK_ARRAY (array, Qarrayp);
 
   if (VECTORP (array))
     {
@@ -2328,7 +2339,14 @@ If the current binding is global (the default), the value is nil.  */)
       CHECK_CHARACTER (idx);
       CHAR_TABLE_SET (array, idxval, newelt);
     }
-  else
+  else if (RECORDP (array))
+    {
+      ptrdiff_t size = ASIZE (array) & PSEUDOVECTOR_SIZE_MASK;
+      if (idxval < 0 || idxval >= size)
+	args_out_of_range (array, idx);
+      ASET (array, idxval, newelt);
+    }
+  else /* STRINGP */
     {
       int c;
 
@@ -3714,6 +3732,7 @@ enum bool_vector_op { bool_vector_exclusive_or,
   DEFSYM (Qbuffer, "buffer");
   DEFSYM (Qframe, "frame");
   DEFSYM (Qvector, "vector");
+  DEFSYM (Qrecord, "record");
   DEFSYM (Qchar_table, "char-table");
   DEFSYM (Qbool_vector, "bool-vector");
   DEFSYM (Qhash_table, "hash-table");
@@ -3750,6 +3769,7 @@ enum bool_vector_op { bool_vector_exclusive_or,
   defsubr (&Sstringp);
   defsubr (&Smultibyte_string_p);
   defsubr (&Svectorp);
+  defsubr (&Srecordp);
   defsubr (&Schar_table_p);
   defsubr (&Svector_or_char_table_p);
   defsubr (&Sbool_vector_p);
diff --git a/src/fns.c b/src/fns.c
index 1065355..36bde20 100644
--- a/src/fns.c
+++ b/src/fns.c
@@ -104,7 +104,7 @@ static void sort_vector_copy (Lisp_Object, ptrdiff_t,
     XSETFASTINT (val, MAX_CHAR);
   else if (BOOL_VECTOR_P (sequence))
     XSETFASTINT (val, bool_vector_size (sequence));
-  else if (COMPILEDP (sequence))
+  else if (COMPILEDP (sequence) || RECORDP (sequence))
     XSETFASTINT (val, ASIZE (sequence) & PSEUDOVECTOR_SIZE_MASK);
   else if (CONSP (sequence))
     {
diff --git a/src/lisp.h b/src/lisp.h
index ab4db4c..fb5fed1 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -874,6 +874,7 @@ enum pvec_type
   PVEC_TERMINAL,
   PVEC_WINDOW_CONFIGURATION,
   PVEC_SUBR,
+  PVEC_RECORD,
   PVEC_OTHER,            /* Should never be visible to Elisp code.  */
   PVEC_XWIDGET,
   PVEC_XWIDGET_VIEW,
@@ -2728,6 +2729,12 @@ enum char_bits
   return PSEUDOVECTORP (a, PVEC_FRAME);
 }
 
+INLINE bool
+RECORDP (Lisp_Object a)
+{
+  return PSEUDOVECTORP (a, PVEC_RECORD);
+}
+
 /* Test for image (image . spec)  */
 INLINE bool
 IMAGEP (Lisp_Object x)
diff --git a/src/lread.c b/src/lread.c
index 5c6a7f9..1fcbc37 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -2762,6 +2762,19 @@ BUFFER is the buffer to evaluate (nil means use current buffer),
 	  make_byte_code (vec);
 	  return tmp;
 	}
+      if (c == '%')
+	{
+	  c = READCHAR;
+	  if (c == '[')
+	    {
+	      Lisp_Object tmp;
+	      tmp = read_vector (readcharfun, 1);
+	      XSETPVECTYPE (XVECTOR(tmp), PVEC_RECORD);
+	      return tmp;
+	    }
+	  UNREAD (c);
+	  invalid_syntax ("#");
+	}
       if (c == '(')
 	{
 	  Lisp_Object tmp;
diff --git a/src/print.c b/src/print.c
index e857761..f7ecd3c 100644
--- a/src/print.c
+++ b/src/print.c
@@ -1966,6 +1966,7 @@
       case PVEC_SUB_CHAR_TABLE:
       case PVEC_COMPILED:
       case PVEC_CHAR_TABLE:
+      case PVEC_RECORD:
       case PVEC_NORMAL_VECTOR: ;
 	{
 	  ptrdiff_t size = ASIZE (obj);
@@ -1974,6 +1975,12 @@
 	      printchar ('#', printcharfun);
 	      size &= PSEUDOVECTOR_SIZE_MASK;
 	    }
+	  if (RECORDP (obj))
+	    {
+	      printchar ('#', printcharfun);
+	      printchar ('%', printcharfun);
+	      size &= PSEUDOVECTOR_SIZE_MASK;
+	    }
 	  if (CHAR_TABLE_P (obj) || SUB_CHAR_TABLE_P (obj))
 	    {
 	      /* We print a char-table as if it were a vector,

Re: User-defined record types

[Lars Brinkhoff]

Lars Brinkhoff wrote:
> This is my old patch dusted off and rebased to current master.
> It's just a raw material posted for review.

This is how cl-defstruct could be modified to optionally make record
instances.  More work would probably be needed in cl-preloaded.el and
cl-generic.el.

Test case:

  (cl-defstruct (foo (:type record)) x y z)
  (let ((x (make-foo :y 1)))
    (list (type-of x)
          (foo-p x)
          (recordp x)
          (foo-y x)
          x))



diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el
index 40342f3..dead86e 100644
--- a/lisp/emacs-lisp/cl-macs.el
+++ b/lisp/emacs-lisp/cl-macs.el
@@ -2544,6 +2544,12 @@ cl--sublis
       (cons (cl--sublis alist (car tree)) (cl--sublis alist (cdr tree))))
      (t tree))))
 
+(defun record (type &rest elements)
+  (let ((result (make-record type (length elements) nil))
+	(i 0))
+    (dolist (elt elements result)
+      (aset result (cl-incf i) elt))))
+
 ;;; Structures.
 
 (defmacro cl--find-class (type)
@@ -2656,6 +2662,8 @@ cl-defstruct
 				  descs)))
 	      (t
 	       (error "Structure option %s unrecognized" opt)))))
+    (if (eq type 'record)
+        (setq named t))
     (unless (or include-name type)
       (setq include-name cl--struct-default-parent))
     (when include-name (setq include (cl--struct-get-class include-name)))
@@ -2684,7 +2692,7 @@ cl-defstruct
 	  (if (cl--struct-class-named include) (setq tag name named t)))
       (if type
 	  (progn
-	    (or (memq type '(vector list))
+	    (or (memq type '(vector list record))
 		(error "Invalid :type specifier: %s" type))
 	    (if named (setq tag name)))
 	(setq named 'true)))
@@ -2700,6 +2708,9 @@ cl-defstruct
                              `(and (vectorp cl-x)
                                    (>= (length cl-x) ,(length descs))
                                    (memq (aref cl-x ,pos) ,tag-symbol)))
+                            ((eq type 'record)
+                             `(and (recordp cl-x)
+                                   (memq (type-of cl-x) ,tag-symbol)))
                             ((= pos 0) `(memq (car-safe cl-x) ,tag-symbol))
                             (t `(and (consp cl-x)
 				     (memq (nth ,pos cl-x) ,tag-symbol))))))
@@ -2740,7 +2751,7 @@ cl-defstruct
 			      (list `(or ,pred-check
                                          (signal 'wrong-type-argument
                                                  (list ',name cl-x)))))
-                       ,(if (memq type '(nil vector)) `(aref cl-x ,pos)
+                       ,(if (memq type '(nil vector record)) `(aref cl-x ,pos)
                           (if (= pos 0) '(car cl-x)
                             `(nth ,pos cl-x))))
                     forms)

Re: User-defined record types

[Lars Brinkhoff]

Stefan Monnier wrote:
>> diff --git a/src/data.c b/src/data.c
>> index ae8dd97..eceb752 100644
>> --- a/src/data.c
>> +++ b/src/data.c
>> @@ -267,6 +267,7 @@ static void swap_in_symval_forwarding (struct
>> Lisp_Symbol *,
>>          case PVEC_MUTEX: return Qmutex;
>>          case PVEC_CONDVAR: return Qcondition_variable;
>>          case PVEC_TERMINAL: return Qterminal;
>> +        case PVEC_RECORD: return AREF (object, 0);
>>          /* "Impossible" cases.  */
>>          case PVEC_XWIDGET:
>>          case PVEC_OTHER:
>
> `type-of` is expected to return a symbol.  So the above code implies
> that `record` objects should have a symbol their slot 0 and that this
> symbol should be the record's type name.

Right.  Fmake_record does check that slot 0 is initialized to a symbol.

> Currently EIEIO and `cl-defstruct` indeed puts a symbol in slot 0 but
> this symbol is not exactly the struct type, so returning AREF (object,
> 0) wouldn't return quite the right value.

I don't understand everything about cl-defstruct, and even less about
Emacs flavor of generic functions or EIEIO.  I have a patch which
demostrates how to make cl-defstruct create record instances if you
explicitly ask for it.  It does put the type name in slot 0.

If you pass (:named foo) to cl-defstruct, foo will be put in list or
vector slot 0, so I suppose that should work for records too.

If you don't use the :named option, it seems there will be no type
information in the instances.

Re: User-defined record types

[Lars Brinkhoff]

> If you pass (:named foo) to cl-defstruct, foo will be put in list or
> vector slot 0, so I suppose that should work for records too.
>
> If you don't use the :named option, it seems there will be no type
> information in the instances.

I see now this is wrong, and there are more cases to consider.
This is the situation as of now:

- If you don't pass the :type option to cl-defstruct, you get a
cl-struct-FOO type tag in slot 0.

- If you pass :type but not :named, you don't get any type tag.

- If you pass :type and :named, you get a type tag in slot 0, which is
the same symbol as the struct name.

Re: User-defined record types

[Stefan Monnier]

>> If you pass (:named foo) to cl-defstruct, foo will be put in list or
>> vector slot 0, so I suppose that should work for records too.
>> If you don't use the :named option, it seems there will be no type
>> information in the instances.
> I see now this is wrong, and there are more cases to consider.
> This is the situation as of now:
> - If you don't pass the :type option to cl-defstruct, you get a
> cl-struct-FOO type tag in slot 0.

That's right.  And this symbol's value is the class object.
The same holds for EIEIO.

These are the main candidates for use of the new `record` type.

> - If you pass :type but not :named, you don't get any type tag.

And those should not use `record` but `vector` or `list`
depending on the :type that was specified.

> - If you pass :type and :named, you get a type tag in slot 0, which is
> the same symbol as the struct name.

But again, these shouldn't use `record`.


        Stefan

Re: User-defined record types

[Lars Brinkhoff]

Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> - If you don't pass the :type option to cl-defstruct, you get a
>> cl-struct-FOO type tag in slot 0.
>
> That's right.  And this symbol's value is the class object.
> The same holds for EIEIO.
>
> These are the main candidates for use of the new `record` type.

Yes, at first I tried to make cl-deftype do this by default.  But I ran
into problems in cl-preloaded.el and cl-generic.el.

>> - If you pass :type but not :named, you don't get any type tag.
>
> And those should not use `record` but `vector` or `list`
> depending on the :type that was specified.
>
>> - If you pass :type and :named, you get a type tag in slot 0, which is
>> the same symbol as the struct name.
>
> But again, these shouldn't use `record`.

Right, I just wanted to list all possible variations to gain some
understanding.  For example, I don't know if it makes a difference if
the slot 0 type tag has the form cl-struct-FOO as is the default, or
just FOO as when you specify :type.  But now I know there might be.

Re: User-defined record types

[Stefan Monnier]

>>> - If you don't pass the :type option to cl-defstruct, you get a
>>> cl-struct-FOO type tag in slot 0.
>> That's right.  And this symbol's value is the class object.
>> The same holds for EIEIO.
>> These are the main candidates for use of the new `record` type.
> Yes, at first I tried to make cl-deftype do this by default.  But I ran
> into problems in cl-preloaded.el and cl-generic.el.

I can help with that.

> Right, I just wanted to list all possible variations to gain some
> understanding.  For example, I don't know if it makes a difference if
> the slot 0 type tag has the form cl-struct-FOO as is the default, or
> just FOO as when you specify :type.  But now I know there might be.

The cl-struct-FOO symbol was used instead of just FOO in order to try
and avoid false-positives (i.e. to try and avoid mistakenly recognizing
some random array as being a cl-struct object).

With `record` you don't need that trick any more, so records could have
the FOO symbol in their slot 0.

For EIEIO objects, it would slow things down a bit, because EIEIO
accessors needs to consult the class object, so the time it takes to get
from (aref obj 0) to the actual class object is important: if we put the
actual type symbol, that means that we'd need something like

    (get (aref obj 0) 'cl--class)

whereas the current code uses

    (symbol-value (aref obj 0))

The problem being that the identifier used for the type name might
already be used as a variable as well as as function, so we can't use
the `symbol-value` or `symbol-function` slot.

As mentioned, ideally, we'd want to store the class object directly in
the slot 0.  The downside is that prin1 would then dump the class object
as well, so when reading dumped objects we'd end up creating another
copy of the class object rather than reusing an existing class object.
And this will break cl-generic dispatch which compares class objects
with `eq`.


        Stefan

Re: User-defined record types

[Lars Brinkhoff]

Stefan Monnier wrote:
>> Yes, at first I tried to make cl-deftype do this by default.  But I
>> ran into problems in cl-preloaded.el and cl-generic.el.
> I can help with that.

Thanks.  I got past that and made it through to a dumped emacs which
uses record types for defstruct by default.

However, there is still EIEIO which may well need some expert guidance
to update.

> As mentioned, ideally, we'd want to store the class object directly in
> the slot 0.  The downside is that prin1 would then dump the class object
> as well, so when reading dumped objects we'd end up creating another
> copy of the class object rather than reusing an existing class object.
> And this will break cl-generic dispatch which compares class objects
> with `eq`.

Understood.  I'll look into storing the class object in slot 0.  I
imagine there will be some circular bootstrapping problem, e.g. creating
the first record object requires a class object which is a record
object.

Some suggestions:

- type-of looks into the class object and returns the symbol naming the
  class.

- class-of could be introduced to return a class object, like CLOS.

- The read/print syntax for records uses the symbol for the type slot.
  Reading a record would then maybe only work right if the class object
  has been defined first.  Not sure if that's ok.

Re: User-defined record types

[Stefan Monnier]

> Thanks.  I got past that and made it through to a dumped emacs which
> uses record types for defstruct by default.

Cool.

> However, there is still EIEIO which may well need some expert guidance
> to update.

I can take care of that.  Can you put your code on a branch like
scratch/record?

>> As mentioned, ideally, we'd want to store the class object directly in
>> the slot 0.  The downside is that prin1 would then dump the class object
>> as well, so when reading dumped objects we'd end up creating another
>> copy of the class object rather than reusing an existing class object.
>> And this will break cl-generic dispatch which compares class objects
>> with `eq`.
> Understood.  I'll look into storing the class object in slot 0.
> I imagine there will be some circular bootstrapping problem, e.g. creating
> the first record object requires a class object which is a record
> object.

Yes, cl-preloaded.el would likely need some extra care to get the system
to bootstrap, but I'm not worried about that.

> Some suggestions:
> - type-of looks into the class object and returns the symbol naming the
>   class.

Yes, that would make a lot of sense, but that means we have to impose
a particular shape on the contents of slot 0.  IOW it means that the
representation of classes is at least somewhat known/imposed by the
C code.  It's probably OK.

> - class-of could be introduced to return a class object, like CLOS.

Sure.

> - The read/print syntax for records uses the symbol for the type slot.
>   Reading a record would then maybe only work right if the class object
>   has been defined first.  Not sure if that's ok.

Not sure either.  Of course, another option is for prin1 to print the
whole class object in slot 0, and when we read it in, we handle it
specially by looking at the class name inside the class object and
reusing the corresponding class if it already exists.  The would likely
hard-code even more of the notion of class-objects into the C code.


        Stefan

Re: User-defined record types

[Lars Brinkhoff]

Stefan Monnier wrote:
>> However, there is still EIEIO which may well need some expert guidance
>> to update.
>
> I can take care of that.  Can you put your code on a branch like
> scratch/record?

Certainly.  Do I need to apply for project membership or write
permission or something like that?

Re: User-defined record types

[Stefan Monnier]

> Certainly.  Do I need to apply for project membership or write
> permission or something like that?

If you don't have write access yet, then yes.  Just go to your Savannah
account and request membership in the `emacs` group.


        Stefan

Re: User-defined record types

[Lars Brinkhoff]

Stefan Monnier wrote:
>> However, there is still EIEIO which may well need some expert guidance
>> to update.
> I can take care of that.  Can you put your code on a branch like
> scratch/record?

Done, scratch/record it is.

Note that this doesn't survive a full build due to EIEIO breakage.
I will usually build from master, then checkout the branch and rebuild
the changed files.

Re: User-defined record types

[Stefan Monnier]

> Note that this doesn't survive a full build due to EIEIO breakage.

I'll see about fixing this, thanks,


        Stefan

Re: User-defined record types

[Stefan Monnier]

>> However, there is still EIEIO which may well need some expert guidance
>> to update.
> I can take care of that.  Can you put your code on a branch like
> scratch/record?

OK, it's not working.
I found a problem with cl-defstruct objects, tho:

We're going to have trouble preserving backward compatibility with
pre-existing .elc files.  If you look at the macro expansion of current
cl-defstruct:

    (macroexpand '(cl-defstruct (sm-test) a b))
  =>
    (progn
      (defvar cl-struct-sm-test-tags)
      (define-inline sm-test-p (x) (declare (side-effect-free error-free))
        (inline-letevals (x)
                         (inline-quote
                          (and (vectorp #1=,x) (>= (length #1#) 3)
                               (memq (aref #1# 0) cl-struct-sm-test-tags)
                               t))))
      (put 'sm-test 'cl-deftype-satisfies 'sm-test-p)
      (define-inline sm-test-a #2=(x)
        "Access slot \"a\" of `(sm-test)' struct CL-X."
        #3=(declare (side-effect-free t))
        (inline-letevals #4=(x)
          (inline-quote
           (progn
             (or (memq (aref #5=,x 0) cl-struct-sm-test-tags)
                 (signal #6='wrong-type-argument (list 'sm-test . #7=(,x))))
             (aref #8=,x 1)))))
      (define-inline sm-test-b #2# "Access slot \"b\" of `(sm-test)' struct CL-X." #3#
        (inline-letevals #4#
          (inline-quote
           (progn
             (or (memq (aref #5# 0) cl-struct-sm-test-tags)
                 (signal #6# (list 'sm-test . #7#)))
             (aref #8# 2)))))
      (defalias 'copy-sm-test (function copy-sequence))
      (cl-defsubst make-sm-test
          (&cl-defs (nil . #9=((cl-tag-slot) (a) (b))) &key a b)
        "Constructor for objects of type `sm-test'."
        (declare (side-effect-free t))
        (vector 'cl-struct-sm-test a b))
      (eval-and-compile
        (cl-struct-define 'sm-test nil 'cl-structure-object 'nil nil '#9# 'cl-struct-sm-test-tags 'cl-struct-sm-test 't))
      'sm-test)

As you can see, the code says it inherits from `cl-structure-object'
(which now is of `record` type), but its constructor `make-sm-test`
creates a vector rather than a record, so (cl-typep 'cl-structure-object)
will fail if it only considers `type-of'.

Even if we could arrange for cl-struct-define to override the
definition of make-sm-test, that wouldn't help with all the places
where make-sm-test has been inlined already.

[ Of course, the reverse could happen as well: a new struct type (using
  `record` in its constructor) could inherit from a parent that's still
  using `vector`.  But I think we can arrange for that to happen only in
  cases we don't care to support.  ]


        Stefan

Re: User-defined record types

[Stefan Monnier]

>>> However, there is still EIEIO which may well need some expert guidance
>>> to update.
>> I can take care of that.  Can you put your code on a branch like
>> scratch/record?
> OK, it's not working.
           ^^^
           now
Duh!


        Stefan

Re: User-defined record types

[Lars Brinkhoff]

Stefan Monnier wrote:
> OK, it's [now] working.

A full bootstrap build still fails for me:

cedet/ede.el:46:1:Error: Wrong type argument: sequencep, #%[cl-slot-descriptor expanded nil boolean ((:documentation . "State of an object being expanded in speedbar."))]

> We're going to have trouble preserving backward compatibility with
> pre-existing .elc files.  As you can see, the code says it inherits
> from `cl-structure-object' (which now is of `record` type), but its
> constructor `make-sm-test` creates a vector rather than a record, so
> (cl-typep 'cl-structure-object) will fail if it only considers
> `type-of'.

What if we relax the type check to use (aref object 0) instead?  That
works for both old and new instances, as long as :initial-offset is 0.

Re: User-defined record types

[Stefan Monnier]

> A full bootstrap build still fails for me:
> cedet/ede.el:46:1:Error: Wrong type argument: sequencep,
> #%[cl-slot-descriptor expanded nil boolean ((:documentation . "State of an
> object being expanded in speedbar."))]

Looks like I missed a place where a copy-sequence needs to be turned
into a copy-record?  I know there's still this bug in cl-defstruct's
definition of the copy-<foo> function, but I think this is never used,
so it's probably somewhere else.

>> We're going to have trouble preserving backward compatibility with
>> pre-existing .elc files.  As you can see, the code says it inherits
>> from `cl-structure-object' (which now is of `record` type), but its
>> constructor `make-sm-test` creates a vector rather than a record, so
>> (cl-typep 'cl-structure-object) will fail if it only considers
>> `type-of'.
> What if we relax the type check to use (aref object 0) instead?

But we need an arrayp or recordp check before, so it'd be something like

   (and (or (arrayp x) (recordp x)) (aref x 0))

Problem is that now we're slower than we were before the introduction of
`record`.  One option could be to change `type-of' so as to know about
the old struct format, and to make this backward compatibility hack
dependent on a boolean var which defaults to nil but would be set to
t as soon as we detect the use of an old-style struct.

> That works for both old and new instances, as long as :initial-offset
> is 0.

:initial-offset is always 0 if :type is not `vector` or `list`, so we
don't have to worry about this issue.


        Stefan

Re: User-defined record types

[Lars Brinkhoff]

Stefan Monnier wrote:
> Looks like I missed a place where a copy-sequence needs to be turned
> into a copy-record?  I know there's still this bug in cl-defstruct's
> definition of the copy-<foo> function, but I think this is never used,
> so it's probably somewhere else.

I fixed that, but it was probably the one in EIEIO's clone method.

Full bootstrap works now.

Re: User-defined record types

[Lars Brinkhoff]

Stefan Monnier wrote:
>> What if we relax the type check to use (aref object 0) instead?
>
> One option could be to change `type-of' so as to know about the old
> struct format, and to make this backward compatibility hack dependent
> on a boolean var which defaults to nil but would be set to t as soon
> as we detect the use of an old-style struct.

I have now implemented that in the latest commit to the scratch/record
branch.

Re: User-defined record types

[Stefan Monnier]

We could auto-detect the use of old-style structs (and set the
compatibility var) when cl-struct-define is called with a nil value of
the `type` argument.

[ Of course, this means we need to use a different value for the new
  `record` format.  ]


        Stefan

Re: User-defined record types

[Stefan Monnier]

One more thing: what's up with "record2"?  Which branch should I pay
attention to?  If you don't intend to keep using scratch/record, then
it's better to just replace scratch/record with scratch/record2: you can
rebase/rewrite those `scratch` branches at will.


        Stefan


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

> We could auto-detect the use of old-style structs (and set the
> compatibility var) when cl-struct-define is called with a nil value of
> the `type` argument.

> [ Of course, this means we need to use a different value for the new
>   `record` format.  ]


>         Stefan

Re: User-defined record types

[Lars Brinkhoff]

Stefan Monnier wrote:
> We could auto-detect the use of old-style structs (and set the
> compatibility var) when cl-struct-define is called with a nil value of
> the `type` argument.

Great, I'll add that.

> Of course, this means we need to use a different value for the new
> `record` format.

The symbol `record' springs to mind.

> One more thing: what's up with "record2"?  Which branch should I pay
> attention to?  If you don't intend to keep using scratch/record, then
> it's better to just replace scratch/record with scratch/record2: you
> can rebase/rewrite those `scratch` branches at will.

I'm happy to do that.  I was not sure what the policy is with replaced
branched.  But if that is ok, I'll mercilessly rewrite it until it's
readly for merging.  I have deleted record2 and updated scratch/record.

Re: User-defined record types

[Stefan Monnier]

>> Of course, this means we need to use a different value for the new
>> `record` format.
> The symbol `record' springs to mind.

I'd prefer using a symbol like `blue-sky`, to avoid confusion.

> I'm happy to do that.  I was not sure what the policy is with replaced
> branched.  But if that is ok, I'll mercilessly rewrite it until it's
> readly for merging.  I have deleted record2 and updated scratch/record.

Great, thanks,


        Stefan

bug#25295: 26.0.50; Represent eieio objects using object-print in backtraces and edebug

[Stefan Monnier]

> Objects have an `object-print' method which produces the #<foo>
> representation, but this appears to only sometimes be used in edebug,
> and never in backtraces.

I've installed a cl-print.el package into Emacs's master branch which is
like EIEIO's object-print but more thorough.  This is now used in
edebug, so it should hopefully fix the "only sometimes" part (and it
additionally knows about cl-defstructs as well, rather than only EIEIO
objects).

Using it in backtraces should be a simple matter of applying the patch
below.  I haven't installed it because I'm afraid of the potential
breakage and slowdown of using cl-prin1, so I'd rather wait and see how
it fares in Edebug first.  My experiments seem to indicate that it might
be "good enough" but it might also suffer from the overhead of
cl-generic (mostly the fact that a call to a generic-function ends up
going through some &rest+apply which in cl-print's case end up
generating a lot of garbage: in one of my tests, the cl-print code was
twice slower than the C code but the added time was almost entirely
spent in the GC and that seemed to come exclusively from the lists
allocated for &rest.  I.e. if we could change cl-generic so it doesn't
use &rest+apply as much there's a chance it could potentially be about
as fast as the C code).


        Stefan


diff --git a/lisp/subr.el b/lisp/subr.el
index ea159e733c..68bfceb8d2 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -4660,10 +4660,10 @@ backtrace--print-frame
   (princ (if (plist-get flags :debug-on-exit) "* " "  "))
   (cond
    ((and evald (not debugger-stack-frame-as-list))
-    (prin1 func)
-    (if args (prin1 args) (princ "()")))
+    (cl-prin1 func)
+    (if args (cl-prin1 args) (princ "()")))
    (t
-    (prin1 (cons func args))))
+    (cl-prin1 (cons func args))))
   (princ "\n"))
 
 (defun backtrace ()

bug#25295: 26.0.50; Represent eieio objects using object-print in backtraces and edebug

[npostavs]

Stefan Monnier <monnier@iro.umontreal.ca> writes:
> My experiments seem to indicate that it might
> be "good enough" but it might also suffer from the overhead of
> cl-generic (mostly the fact that a call to a generic-function ends up
> going through some &rest+apply which in cl-print's case end up
> generating a lot of garbage: in one of my tests, the cl-print code was
> twice slower than the C code but the added time was almost entirely
> spent in the GC and that seemed to come exclusively from the lists
> allocated for &rest.  I.e. if we could change cl-generic so it doesn't
> use &rest+apply as much there's a chance it could potentially be about
> as fast as the C code).

I have an idea to use this for the pretty printer so it that produces
the correct spacing while printing.  I think this would end up faster
than calling indent-sexp afterwards.  But it would involve dispatching
on STREAM, and according to the comments in cl-generic.el multiple
dispatch adds more apply calls.  Thoughts?

bug#25295: 26.0.50; Represent eieio objects using object-print in backtraces and edebug

[Stefan Monnier]

> I have an idea to use this for the pretty printer so it that produces
> the correct spacing while printing.  I think this would end up faster
> than calling indent-sexp afterwards.  But it would involve dispatching
> on STREAM, and according to the comments in cl-generic.el multiple
> dispatch adds more apply calls.  Thoughts?

No idea, how would that work?


        Stefan

bug#25295: 26.0.50; Represent eieio objects using object-print in backtraces and edebug

[npostavs]

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

>> I have an idea to use this for the pretty printer so it that produces
>> the correct spacing while printing.  I think this would end up faster
>> than calling indent-sexp afterwards.  But it would involve dispatching
>> on STREAM, and according to the comments in cl-generic.el multiple
>> dispatch adds more apply calls.  Thoughts?
>
> No idea, how would that work?

I posted an initial draft at
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=25122#21, I'm not seeing
as much speedup as I'd hoped, though I haven't tried optimizing it yet.

New pp (was: bug#25295: 26.0.50; Represent eieio objects using object-print in backtraces and edebug)

[Stefan Monnier]

> I posted an initial draft at
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=25122#21, I'm not seeing
> as much speedup as I'd hoped, though I haven't tried optimizing it yet.

Thanks.  I answered there about the performance aspect.

> +(defun pp-prin1 (object &optional stream)
> +  (let ((cl-print-readably nil)
> +        (stream (make-pp-state (or stream standard-output))))
> +    (pp--scan :open-block stream)
> +    (prog1 (cl-prin1 object (cons :pprint stream))
                               ^^^^^^^^^^^^^^^^^^^^^
Why not just pass `stream' since it's a struct and we can hence dispatch
based on its type?

> +;; fallback to standard `cl-print-object'.
> +(cl-defmethod cl-print-object (object (stream (head :pprint)))
> +  (pp--scan (cl-prin1-to-string object) (cdr stream))
> +  object)

Hmm... but if we use such a pseudo-stream here, doesn't it break all
calls to `princ' within other cl-print-object methods (i.e. forcing us
the override pretty much all existing cl-print-object methods with
a pprint-specific one)?


        Stefan

Re: New pp (was: bug#25295: 26.0.50; Represent eieio objects using object-print in backtraces and edebug)

[Noam Postavsky]

On Sat, Mar 11, 2017 at 10:38 AM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
>> +(defun pp-prin1 (object &optional stream)
>> +  (let ((cl-print-readably nil)
>> +        (stream (make-pp-state (or stream standard-output))))
>> +    (pp--scan :open-block stream)
>> +    (prog1 (cl-prin1 object (cons :pprint stream))
>                                ^^^^^^^^^^^^^^^^^^^^^
> Why not just pass `stream' since it's a struct and we can hence dispatch
> based on its type?

Oh, hmm, that makes sense. I took a few wrong turns while implementing
this, probably what happened is that when I decided to use (cons
:pprint) I hadn't decided on a struct yet.

>
>> +;; fallback to standard `cl-print-object'.
>> +(cl-defmethod cl-print-object (object (stream (head :pprint)))
>> +  (pp--scan (cl-prin1-to-string object) (cdr stream))
>> +  object)
>
> Hmm... but if we use such a pseudo-stream here, doesn't it break all
> calls to `princ' within other cl-print-object methods (i.e. forcing us
> the override pretty much all existing cl-print-object methods with
> a pprint-specific one)?

It doesn't break anything, AFAIK, it just means that you don't get
control over newlines vs spaces. So we would need pprint-specific
methods for "big" structures only.

Oh wait, did you possibly miss a close paren? Might be clearer this way:

(cl-defmethod cl-print-object (object (pprint-state (head :pprint)))
  (pp--scan (cl-prin1-to-string object)
            (cdr pprint-state))
  object)

Re: New pp

[Stefan Monnier]

>> Hmm... but if we use such a pseudo-stream here, doesn't it break all
>> calls to `princ' within other cl-print-object methods (i.e. forcing us
>> the override pretty much all existing cl-print-object methods with
>> a pprint-specific one)?
> It doesn't break anything, AFAIK, it just means that you don't get
> control over newlines vs spaces.

But from my reading of the code, we may sometimes end up calling `princ'
with a pp-state object, and I expect it won't know what to do with it.

> Oh wait, did you possibly miss a close paren? Might be clearer this way:
> (cl-defmethod cl-print-object (object (pprint-state (head :pprint)))
>   (pp--scan (cl-prin1-to-string object)
>             (cdr pprint-state))
>   object)

No, the problem is when we run for example the

    (cl-defmethod cl-print-object ((object vector) stream)

method (which AFAIK takes precedence over the

   (cl-defmethod cl-print-object (object (pprint-state (head :pprint)))

method, so it will receive a pp-state stream and pass it on to `princ`).


        Stefan

Re: New pp

[Noam Postavsky]

On Sat, Mar 11, 2017 at 11:40 AM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
>
> No, the problem is when we run for example the
>
>     (cl-defmethod cl-print-object ((object vector) stream)
>
> method (which AFAIK takes precedence over the
>
>    (cl-defmethod cl-print-object (object (pprint-state (head :pprint)))
>
> method, so it will receive a pp-state stream and pass it on to `princ`).

Oh yeah, that is a problem. Is there any way to get a higher precedence?

Re: New pp

[Stefan Monnier]

>> No, the problem is when we run for example the
>>
>> (cl-defmethod cl-print-object ((object vector) stream)
>>
>> method (which AFAIK takes precedence over the
>>
>> (cl-defmethod cl-print-object (object (pprint-state (head :pprint)))
>>
>> method, so it will receive a pp-state stream and pass it on to `princ`).
> Oh yeah, that is a problem. Is there any way to get a higher precedence?

We could change the cl-defgeneric to specify that the second arg comes
before the first when ordering methods with (:argument-precedence-order
stream object).
But it still feels brittle.


        Stefan

Re: New pp

[Noam Postavsky]

On Sat, Mar 11, 2017 at 11:57 AM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
>
> We could change the cl-defgeneric to specify that the second arg comes
> before the first when ordering methods with (:argument-precedence-order
> stream object).

Works.

> But it still feels brittle.

Any other ideas?

Re: New pp

[Stefan Monnier]

>> We could change the cl-defgeneric to specify that the second arg comes
>> before the first when ordering methods with (:argument-precedence-order
>> stream object).
> Works.
>> But it still feels brittle.
> Any other ideas?

Add cl-princ?
Use a function for `stream`?
Extend the print.c code to accept other kinds of streams?
Don't try to do it all within cl-print-object but use a separate
function on top of it?

The second option should work right now, except you can't easily
dispatch on them.  The problem with it is that it might prove slow (one
function call per character), but maybe we can fix this by changing the
print.c code so those function-valued streams can be called with
a string rather than with a single char.




        Stefan

Re: New pp

[Noam Postavsky]

On Sun, Mar 12, 2017 at 10:06 AM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
> Don't try to do it all within cl-print-object but use a separate
> function on top of it?

Hmm, right, why *did* I try to use cl-print-object? Maybe I had some
vague idea about recursing back into prettyprinting for subelements,
but of course that can't work.

New patch posted at [1] uses pp-print-object instead.

[1]: https://debbugs.gnu.org/cgi/bugreport.cgi?att=1;filename=v2-0001-New-pretty-printer-Bug-25122.patch;msg=60;bug=25122

bug#25295: 26.0.50; Represent eieio objects using object-print in backtraces and edebug

[Eric Abrahamsen]

On 03/02/17 02:00 AM, Stefan Monnier wrote:
>> Objects have an `object-print' method which produces the #<foo>
>> representation, but this appears to only sometimes be used in edebug,
>> and never in backtraces.
>
> I've installed a cl-print.el package into Emacs's master branch which is
> like EIEIO's object-print but more thorough.  This is now used in
> edebug, so it should hopefully fix the "only sometimes" part (and it
> additionally knows about cl-defstructs as well, rather than only EIEIO
> objects).

Very cool! How does this interact with the existing `object-print'
method? That method seems to be getting called at some point, but I
don't see where. Is the intention that we'd stop using that?

bug#25295: 26.0.50; Represent eieio objects using object-print in backtraces and edebug

[Stefan Monnier]

> Very cool! How does this interact with the existing `object-print'
> method?

The default cl-print-object method for EIEIO objects calls object-print.
So you can add methods to either of object-print or cl-print-object
(modulo issues with ordering between methods if one is applied to
cl-print-object and the other to object-print).

> That method seems to be getting called at some point, but I
> don't see where. Is the intention that we'd stop using that?

Yes, object-print is now obsolete.


        Stefan

bug#25295: 26.0.50; Represent eieio objects using object-print in backtraces and edebug

[Eric Abrahamsen]

On 03/02/17 02:00 AM, Stefan Monnier wrote:
>> Objects have an `object-print' method which produces the #<foo>
>> representation, but this appears to only sometimes be used in edebug,
>> and never in backtraces.
>
> I've installed a cl-print.el package into Emacs's master branch which is
> like EIEIO's object-print but more thorough.  This is now used in
> edebug, so it should hopefully fix the "only sometimes" part (and it
> additionally knows about cl-defstructs as well, rather than only EIEIO
> objects).
>
> Using it in backtraces should be a simple matter of applying the patch
> below.  I haven't installed it because I'm afraid of the potential
> breakage and slowdown of using cl-prin1, so I'd rather wait and see how
> it fares in Edebug first.  My experiments seem to indicate that it might
> be "good enough" but it might also suffer from the overhead of
> cl-generic (mostly the fact that a call to a generic-function ends up
> going through some &rest+apply which in cl-print's case end up
> generating a lot of garbage: in one of my tests, the cl-print code was
> twice slower than the C code but the added time was almost entirely
> spent in the GC and that seemed to come exclusively from the lists
> allocated for &rest.  I.e. if we could change cl-generic so it doesn't
> use &rest+apply as much there's a chance it could potentially be about
> as fast as the C code).
>
>
>         Stefan
>
>
> diff --git a/lisp/subr.el b/lisp/subr.el
> index ea159e733c..68bfceb8d2 100644
> --- a/lisp/subr.el
> +++ b/lisp/subr.el
> @@ -4660,10 +4660,10 @@ backtrace--print-frame
>    (princ (if (plist-get flags :debug-on-exit) "* " "  "))
>    (cond
>     ((and evald (not debugger-stack-frame-as-list))
> -    (prin1 func)
> -    (if args (prin1 args) (princ "()")))
> +    (cl-prin1 func)
> +    (if args (cl-prin1 args) (princ "()")))
>     (t
> -    (prin1 (cons func args))))
> +    (cl-prin1 (cons func args))))
>    (princ "\n"))
>  
>  (defun backtrace ()

How about we just apply this and see what happens? Developing with EIEIO
is pretty painful -- oftentimes the *Messages* buffer is such a mess you
can't even tell if there's been an error or not.

I've been running the above patch, as well as an equivalent change in
`eval-expression', and nothing bad has happened. Perhaps there's a
slowdown, but from my own selfish point of view any slowdown is far, far
preferable to the several seconds you need to wait for a class object to
be spat out into *Messages*.

BTW, this works in edebug but only at the top level: if a nested
structure is printed, interior objects are still represented as the full
struct.

bug#25295: 26.0.50; Represent eieio objects using object-print in backtraces and edebug

[Stefan Monnier]

> How about we just apply this and see what happens?

AFAIK, that's exactly what's been done in Emacs-26 already.


        Stefan

bug#25295: 26.0.50; Represent eieio objects using object-print in backtraces and edebug

[Eric Abrahamsen]

This is either done, or thoroughly out of date.

On 10/21/17 23:21 PM, Stefan Monnier wrote:
>> How about we just apply this and see what happens?
>
> AFAIK, that's exactly what's been done in Emacs-26 already.
>
>
>         Stefan