Re: print hash table to disk and reread in hash table

Re: print hash table to disk and reread in hash table

[Ted Zlatanov]

On Fri, 29 Aug 2008 17:52:07 +0300 Eli Zaretskii <eliz@gnu.org> wrote: 

EZ> You mean, like bindat.el?
>> 
>> That looks useful for binary data.  I don't see from the code how to use
>> it to save and load a hashtable.

EZ> I never used it, I just advertised the closest hit for what you seemed
EZ> to be looking.

>> In addition, this library doesn't manage file I/O.  I'd expect the
>> library do do content coding and error handling internally.  This is,
>> actually, the most tedious part.

EZ> A cause for a small project, I guess. ;-)

I can just copy the code out of gnus-registry.el and abstract the
functionality to do a first cut of such a library.  For a large
hashtable, though, converting to a list on the way in and out is
inefficient compared to storing it in a more "native" format.  Is there
such a format, comparable to the native list read-eval format, as seen
in gnus-load:

  (with-temp-buffer
    (insert-file-contents file)
    (while (not (eobp))
      (condition-case type
	  (let ((form (read (current-buffer))))
	    (eval form))
;; ... much code omitted ...

cc to emacs-devel as I think this is of interest to that list.

Thanks
Ted

Re: print hash table to disk and reread in hash table

[tomas]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Fri, Aug 29, 2008 at 01:10:11PM -0500, Ted Zlatanov wrote:
> The following message is a courtesy copy of an article
> that has been posted to gnu.emacs.help as well.
> 
> On Fri, 29 Aug 2008 17:52:07 +0300 Eli Zaretskii <eliz@gnu.org> wrote: 
> 
> EZ> You mean, like bindat.el?
> >> 
> >> That looks useful for binary data.  I don't see from the code how to use
> >> it to save and load a hashtable.

I don't know the context -- but I have been musing on and off about a
persistent hash (backed by something like GDBM), similar to what Perl
offers.

Regards
- -- tomás
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFIuNgPBcgs9XrR2kYRAoxsAJ99r4ARwpnX1GQgaoX08E2iy86M8wCfUV4t
C81sI7b0SZFrOD6GkBw3Nb4=
=0/DR
-----END PGP SIGNATURE-----

Re: print hash table to disk and reread in hash table

[Ted Zlatanov]

On Sat, 30 Aug 2008 07:18:07 +0200 tomas@tuxteam.de wrote: 

t> On Fri, Aug 29, 2008 at 01:10:11PM -0500, Ted Zlatanov wrote:
>> The following message is a courtesy copy of an article
>> that has been posted to gnu.emacs.help as well.
>> 
>> On Fri, 29 Aug 2008 17:52:07 +0300 Eli Zaretskii <eliz@gnu.org> wrote: 
>> 
EZ> You mean, like bindat.el?
>> >> 
>> >> That looks useful for binary data.  I don't see from the code how to use
>> >> it to save and load a hashtable.

t> I don't know the context -- but I have been musing on and off about a
t> persistent hash (backed by something like GDBM), similar to what Perl
t> offers.

I was trying to serialize a hashtable to a file, and there's no good way
other than converting it to a list or making a file of (puthash)
statements.  bindat.el seems OK but doesn't seem to have a hashtable
spec in the format.

Persistent storage is useful and could easily be implemented with a
small sqlite database file.  It could be an option when creating a
hashtable.  It doesn't solve serialization, though.  The file is not
human-readable, which is the advantage of formats like the Gnus newsrc
file.  So it's not what I'm looking for, but still an useful addition to
Emacs IMO :)

Ted

Re: print hash table to disk and reread in hash table

[tomas]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Sat, Aug 30, 2008 at 04:17:54AM -0500, Ted Zlatanov wrote:
> On Sat, 30 Aug 2008 07:18:07 +0200 tomas@tuxteam.de wrote: 

> Persistent storage is useful and could easily be implemented with a
> small sqlite database file.  It could be an option when creating a
> hashtable.  It doesn't solve serialization, though.  The file is not
> human-readable, which is the advantage of formats like the Gnus newsrc
> file.  So it's not what I'm looking for, but still an useful addition to
> Emacs IMO :)

Ah, I see. Thanks for the clarification.

Regards
- -- tomás
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFIuTcHBcgs9XrR2kYRAtsUAJ95yoYMkIrCd0nmRihuTqVWqRrf4wCfU8CR
xW6I0HjK770iWnI9Q+HxPD8=
=HkLJ
-----END PGP SIGNATURE-----

Re: print hash table to disk and reread in hash table

[Ted Zlatanov]

On Sat, 30 Aug 2008 04:17:54 -0500 Ted Zlatanov <tzz@lifelogs.com> wrote: 

TZ> I was trying to serialize a hashtable to a file, and there's no good way
TZ> other than converting it to a list or making a file of (puthash)
TZ> statements.  bindat.el seems OK but doesn't seem to have a hashtable
TZ> spec in the format.

Steve Yegge published Ejacs over the weekend, which included some (IMHO
very reasonable) complaints about Emacs Lisp's lack of hashtable
serialization.

I propose the following functions (originals were contributed to Gnus by
Andreas Fuchs <asf@void.at>):

(defun hash-table-to-alist (hash)
  "Build an alist from the values in HASH."
  (let ((list nil))
    (maphash
     (lambda (key value)
       (setq list (cons (cons key value) list)))
     hash)
    list))

;; this would take the usual make-hash-table arguments (:test, :size,
;; :rehash-size, :rehash-threshold, :weakness) so those don't have to
;; get serialized with the alist.  This makes the implementation much
;; simpler, complicating life slightly for the API consumers

;; untested

(defun hash-table-from-alist (alist &rest options)
  "Build a hashtable from the values in ALIST."
  (let ((ht (apply 'make-hash-table options)))
    (mapc
     (lambda (kv-pair)
       (puthash (car kv-pair) (cdr kv-pair) ht))
     alist)
     ht))

Implemented in C these would probably be a bit faster.  They are pretty
trivial, but in the Emacs core they would provide a universal way to
serialize hashtables.  IMO this is better than trying to serialize the
hashtable directly, since there are so many ways to read and write lists
in Emacs Lisp, while hashtable support for the same will require a lot
of effort, new APIs, and little real gain.

Ted

hash-table-{to, from}-alist (was: print hash table to disk and reread in hash table)

[Ted Zlatanov]

On Mon, 17 Nov 2008 11:15:46 -0600 Ted Zlatanov <tzz@lifelogs.com> wrote: 

...
TZ> I propose the following functions (originals were contributed to Gnus by
TZ> Andreas Fuchs <asf@void.at>):

TZ> (defun hash-table-to-alist (hash)
TZ>   "Build an alist from the values in HASH."
TZ>   (let ((list nil))
TZ>     (maphash
TZ>      (lambda (key value)
TZ>        (setq list (cons (cons key value) list)))
TZ>      hash)
TZ>     list))

TZ> ;; this would take the usual make-hash-table arguments (:test, :size,
TZ> ;; :rehash-size, :rehash-threshold, :weakness) so those don't have to
TZ> ;; get serialized with the alist.  This makes the implementation much
TZ> ;; simpler, complicating life slightly for the API consumers

TZ> ;; untested

TZ> (defun hash-table-from-alist (alist &rest options)
TZ>   "Build a hashtable from the values in ALIST."
TZ>   (let ((ht (apply 'make-hash-table options)))
TZ>     (mapc
TZ>      (lambda (kv-pair)
TZ>        (puthash (car kv-pair) (cdr kv-pair) ht))
TZ>      alist)
TZ>      ht))

Even though the functions here are not by any means perfect (for
instance, a hashtable containing another hashtable will not be converted
correctly) I think they should go into Emacs.  No one has commented yet.

I can address the nested hashtable problem by recursion, but it won't
work when converting back into a hashtable because I won't know if the
original data was a hashtable or an alist.  Please let me know if
there's a better way that preserves the clean hashtable-to-alist
mapping (maybe I have to include metadata, after all, which I was hoping
to avoid).

If there are no objections or comments, I'll commit my version tomorrow
with a manual update to follow once the functions are stable.  Where
should this go under the lisp/ directory?

Thanks
Ted

Re: hash-table-{to, from}-alist

[Glenn Morris]

Ted Zlatanov wrote:

> If there are no objections or comments, I'll commit my version tomorrow

There is supposed to be a feature freeze, hence I would think you need
an explicit OK install new features. (I have no comments on the code.)

Re: hash-table-{to, from}-alist

[Ted Zlatanov]

On Wed, 19 Nov 2008 16:57:24 -0500 Glenn Morris <rgm@gnu.org> wrote: 

GM> Ted Zlatanov wrote:
>> If there are no objections or comments, I'll commit my version tomorrow

GM> There is supposed to be a feature freeze, hence I would think you need
GM> an explicit OK install new features. (I have no comments on the code.)

Ah, I was not thinking (nothing unusual :)

I'll wait until after the feature freeze, unless Chong or Stefan fall in
love with my suggestion and want to push it in.  Thank you.

Ted

Re: hash-table-{to, from}-alist

[Stefan Monnier]

TZ> I propose the following functions (originals were contributed to Gnus by
TZ> Andreas Fuchs <asf@void.at>):

TZ> (defun hash-table-to-alist (hash)
TZ> (defun hash-table-from-alist (alist &rest options)

> If there are no objections or comments, I'll commit my version tomorrow
> with a manual update to follow once the functions are stable.  Where
> should this go under the lisp/ directory?

They don't look too harmful, but I think we need more: we need to hook
this into prin1 and into read so it works transparently.

So all this looks pretty "post 23.1" to me.


        Stefan

Re: hash-table-{to, from}-alist

[Ted Zlatanov]

On Fri, 21 Nov 2008 17:02:06 -0500 Stefan Monnier <monnier@iro.umontreal.ca> wrote: 

TZ> I propose the following functions (originals were contributed to Gnus by
TZ> Andreas Fuchs <asf@void.at>):

TZ> (defun hash-table-to-alist (hash)
TZ> (defun hash-table-from-alist (alist &rest options)

>> If there are no objections or comments, I'll commit my version tomorrow
>> with a manual update to follow once the functions are stable.  Where
>> should this go under the lisp/ directory?

SM> They don't look too harmful, but I think we need more: we need to hook
SM> this into prin1 and into read so it works transparently.

We can't, because we're converting to an alist and there's no way to
convert back.  True serialization would require a new marker, similar to
() for lists and [] for vectors.  Braces would probably work, I don't
think they are used anywhere:

{ :prop1 prop-value1 :prop2 prop-value2 ... key1 val1 key2 val2 ... }

(:prop1 is :test for example)

WDYT?  It would cause read errors to have an odd number of elements, but
otherwise it's pretty clean.  The properties are the same as those you
can pass to make-hash-table, and are always serialized (but not required
on read).  You can have keys named :test for example, because the :test
property happens first and then any subsequent :test keys are assumed to
be hash keys.

Incidentally, the docs for prin1 should probably have examples for the
output of a vector and a series of simple examples of the other data
types, or a link to somewhere those are shown.  I didn't find it in the
manual.

SM> So all this looks pretty "post 23.1" to me.

OK.  If we agree on the format I can produce a patch for eventual
inclusion.

Ted

Re: hash-table-{to, from}-alist

[Stefan Monnier]

SM> They don't look too harmful, but I think we need more: we need to hook
SM> this into prin1 and into read so it works transparently.

> We can't, because we're converting to an alist and there's no way to
> convert back.  True serialization would require a new marker, similar to
> () for lists and [] for vectors.  Braces would probably work, I don't
> think they are used anywhere:

> { :prop1 prop-value1 :prop2 prop-value2 ... key1 val1 key2 val2 ... }

> (:prop1 is :test for example)

I was thinking of something more like #<hash-table ...> where "..."
would contain the props and then the key/value pairs.  I.e. an extension
of the current (un`read'able syntax).


        Stefan

Re: hash-table-{to, from}-alist

[tomas]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Fri, Nov 21, 2008 at 10:18:41PM -0500, Stefan Monnier wrote:
[...]
> I was thinking of something more like #<hash-table ...> where "..."
> would contain the props and then the key/value pairs.  I.e. an extension
> of the current (un`read'able syntax).

I like this one more: it is more extensible (not that lots of extensions
will spring up, but at least the syntax is not in the way if someday...
you know).

Regards
- -- tomás
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFJJ5xmBcgs9XrR2kYRAkZuAJ90ZOToHHIXZSfOJFDYMc/jNQd9SACeN/vY
ZbGq/mzgxgKTfnHeY/WZV+I=
=NUG/
-----END PGP SIGNATURE-----

Re: hash-table-{to, from}-alist

[Stephen J. Turnbull]

tomas@tuxteam.de writes:

 > On Fri, Nov 21, 2008 at 10:18:41PM -0500, Stefan Monnier wrote:
 > [...]
 > > I was thinking of something more like #<hash-table ...> where "..."
 > > would contain the props and then the key/value pairs.  I.e. an extension
 > > of the current (un`read'able syntax).
 > 
 > I like this one more: it is more extensible (not that lots of extensions
 > will spring up, but at least the syntax is not in the way if someday...
 > you know).

I'm not sure what's wrong with

        (let ((tbl (make-hash-table ...)))
          (puthash key1 val1 tbl)
          ...)

as a read syntax for hash tables.

N.B. XEmacs and SXEmacs already have lots of object types.  If you're
going to start proliferating `read'able print syntaxes, starting with
an extensible one would be nice.

Re: hash-table-{to, from}-alist

[tomas]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Sat, Nov 22, 2008 at 09:27:18PM +0900, Stephen J. Turnbull wrote:
> tomas@tuxteam.de writes:
> 
>  > On Fri, Nov 21, 2008 at 10:18:41PM -0500, Stefan Monnier wrote:
>  > [...]
>  > > I was thinking of something more like #<hash-table ...> where
>  > > "..."

[...]

>  > I like this one more: it is more extensible [...]

[...]

> I'm not sure what's wrong with
>
>         (let ((tbl (make-hash-table ...)))
>           (puthash key1 val1 tbl)
>           ...)
>
> as a read syntax for hash tables.

Hm. Abstraction? Separate "how it's spelt" from "how it's done"?

Not that I think it has much weight. Make-hash-table is an abstraction
itself, after all. I couldn't decide between both variants.

Regards
- -- tomás




>  > 
>  > I like this one more: it is more extensible (not that lots of extensions
>  > will spring up, but at least the syntax is not in the way if someday...
>  > you know).
> 
> I'm not sure what's wrong with
> 
>         (let ((tbl (make-hash-table ...)))
>           (puthash key1 val1 tbl)
>           ...)
> 
> as a read syntax for hash tables.
> 
> N.B. XEmacs and SXEmacs already have lots of object types.  If you're
> going to start proliferating `read'able print syntaxes, starting with
> an extensible one would be nice.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFJKCN2Bcgs9XrR2kYRAncCAJ48gH/pAj1pwGYC30ib8tif2YqLFQCeLitk
YgVNDIKOihglmb+j7a8PAec=
=oX27
-----END PGP SIGNATURE-----

Re: hash-table-{to, from}-alist

[Stephen J. Turnbull]

tomas@tuxteam.de writes:

 > >  > On Fri, Nov 21, 2008 at 10:18:41PM -0500, Stefan Monnier wrote:
 > >  > [...]
 > >  > > I was thinking of something more like #<hash-table ...> where
 > >  > > "..."

BTW, it turns out that in XEmacs and its descendants prin1 uses the CL
structure syntax when `print-readably' is bound to t:

    #s(hash-table size 1 data (x x-value))

This is not guaranteed to actually be acceptable to the reader, since
some contained objects may not be printable (eg, improper lists).
However, since most types in XEmacs do have readable print
representations, it does correctly recurse on composite types like
hash tables (as long as the expression is a tree; DAGs will contain
copies, cycles will be elided, etc).

Re: hash-table-{to, from}-alist

[Richard M Stallman]

    BTW, it turns out that in XEmacs and its descendants prin1 uses the CL
    structure syntax when `print-readably' is bound to t:

	#s(hash-table size 1 data (x x-value))

Being compatible with that seems like a good idea,
if there is no obstacle.

Re: hash-table-{to, from}-alist

[Stefan Monnier]

>     BTW, it turns out that in XEmacs and its descendants prin1 uses the CL
>     structure syntax when `print-readably' is bound to t:

> 	#s(hash-table size 1 data (x x-value))

> Being compatible with that seems like a good idea,
> if there is no obstacle.

Indeed.


        Stefan

Re: hash-table-{to, from}-alist

[Ted Zlatanov]

On Mon, 24 Nov 2008 10:44:44 -0500 Richard M Stallman <rms@gnu.org> wrote: 

RMS>     BTW, it turns out that in XEmacs and its descendants prin1 uses the CL
RMS>     structure syntax when `print-readably' is bound to t:

RMS> 	#s(hash-table size 1 data (x x-value))

RMS> Being compatible with that seems like a good idea,
RMS> if there is no obstacle.

The equality test is necessary if it was given originally (meaning, if
it's not 'eql).  I don't think we should ever omit it, even if it's the
default value, for readability.

The :weakness, :rehash-size, and :rehash-threshold options to
make-hash-table, can probably be omitted when they are the defaults.

Would this work?

#s(hash-table size 2 test equal data (k1 v1) (k2 v2))

Finally, the size is just a hint with make-hash-table; should the (read)
allow wrong sizes or throw an error?

Thanks
Ted

Re: hash-table-{to, from}-alist

[Stephen J. Turnbull]

Ted Zlatanov writes:

 > The equality test is necessary if it was given originally (meaning,
 > if it's not 'eql).  I don't think we should ever omit it, even if
 > it's the default value, for readability.

I disagree.  But it's not incompatible as long as you don't make the
:test option to make-hash-table obligatory. ;-)

 > Would this work?
 > 
 > #s(hash-table size 2 test equal data (k1 v1) (k2 v2))

Not quite.  The data field in XEmacs is a plist, and the construct
above signals an error.

#s(hash-table size 2 test equal data (k1 v1 k2 v2))

 > Finally, the size is just a hint with make-hash-table; should the (read)
 > allow wrong sizes or throw an error?

XEmacs allows wrong sizes.  I think this makes sense, but it probably
isn't deliberate (the structure-reading mechanism validates field
values one at a time; I don't see a provision for cross-field
consistency checks).

#s(hash-table size 10 data (y y-value x x-value))
#<hash-table size 2/29 data (y y-value x x-value) 0x587cb2e2>

Re: hash-table-{to, from}-alist

[Ted Zlatanov]

On Tue, 25 Nov 2008 10:50:53 +0900 "Stephen J. Turnbull" <stephen@xemacs.org> wrote: 

SJT> Ted Zlatanov writes:
>> The equality test is necessary if it was given originally (meaning,
>> if it's not 'eql).  I don't think we should ever omit it, even if
>> it's the default value, for readability.

SJT> I disagree.  But it's not incompatible as long as you don't make the
SJT> :test option to make-hash-table obligatory. ;-)

OK, I'll make :test optional.  Is size mandatory?  I ask since you put
it in your example, but you could have done so to illustrate
parameters.

>> Would this work?
>> 
>> #s(hash-table size 2 test equal data (k1 v1) (k2 v2))

SJT> Not quite.  The data field in XEmacs is a plist, and the construct
SJT> above signals an error.

SJT> #s(hash-table size 2 test equal data (k1 v1 k2 v2))

I'm OK with that format and it seems pretty good, since (read) can (I
think) reuse the plist read syntax for the contents, so there's not much
work for a simple implementation.  Of course, an optimized
implementation might be necessary, but a working simple solution is
always a good first step.

>> Finally, the size is just a hint with make-hash-table; should the (read)
>> allow wrong sizes or throw an error?

SJT> XEmacs allows wrong sizes.  I think this makes sense, but it probably
SJT> isn't deliberate (the structure-reading mechanism validates field
SJT> values one at a time; I don't see a provision for cross-field
SJT> consistency checks).

SJT> #s(hash-table size 10 data (y y-value x x-value))
SJT> #<hash-table size 2/29 data (y y-value x x-value) 0x587cb2e2>

That's fine.  I'll just hard-code the size to 42 then. (kidding)

Ted

Re: hash-table-{to, from}-alist

[Ted Zlatanov]

Here's a patch to print.c to implement printing out a hashtable's
properties and content as discussed.  I *think* I'm doing it right based
on what I see elsewhere in print.c and fns.c.  I rely on print_object to
do the work.  I know the indentation is off, sorry.

This patch does:

(let ((h (make-hash-table :rehash-size 20.0 :rehash-threshold 0.001 :weakness 'key))) 
  (puthash 1 2 h) 
  (puthash 5 'env h) 
  (print h))

--> #<hash-table 'eql key 2/65 0x83c49a0>

        #s<hash-table size 65 test 'eql weak 'key rehash-size 20.0 rehash-threshold 0.001 data (1 2 5 env )>

I know it's broken for circular references (e.g. (puthash 6 h h)
above).  How should I handle that?

The new output is offset and printed after the old-style output as a
sanity check.  Let me know if the code is OK and I'll make it the only
output of printing a hashtable, clean it up (based on any comments I get
and the TODO items), and go on to writing the read code.  I won't commit
it of course, just keep it as a patch until the trunk is open again.

Thanks
Ted

Index: print.c
===================================================================
RCS file: /sources/emacs/emacs/src/print.c,v
retrieving revision 1.253
diff -c -r1.253 print.c
*** print.c	31 Jul 2008 05:33:53 -0000	1.253
--- print.c	25 Nov 2008 23:40:54 -0000
***************
*** 2052,2057 ****
--- 2052,2108 ----
  	  sprintf (buf, " 0x%lx", (unsigned long) h);
  	  strout (buf, -1, -1, printcharfun, 0);
  	  PRINTCHAR ('>');
+ 
+ /*
+   implement a readable output:
+   #s(hash-table size 2 test equal data (k1 v1 k2 v2))
+ */
+           /* always print the size */
+           sprintf (buf, "\n\n\t#s<hash-table size %ld", (long) XVECTOR (h->next)->size);
+           strout (buf, -1, -1, printcharfun, 0);
+ 
+ 	  if (SYMBOLP (h->test))
+ 	    {
+               strout (" test '", -1, -1, printcharfun, 0);
+               print_object (h->test, printcharfun, 0);
+             }
+ 
+ 	  if (SYMBOLP (h->weak) && ! NILP(h->weak))
+ 	    {
+               strout (" weak '", -1, -1, printcharfun, 0);
+               print_object (h->weak, printcharfun, 0);
+             }
+ 
+ 	  if (FLOATP (h->rehash_size) || INTEGERP (h->rehash_size))
+ 	    {
+               strout (" rehash-size ", -1, -1, printcharfun, 0);
+               print_object (h->rehash_size, printcharfun, 0);
+             }
+ 
+           /* the rehash threshold must be 0 < threshold < 1 so it's never an integer */
+ 	  if (FLOATP (h->rehash_threshold))
+ 	    {
+               strout (" rehash-threshold ", -1, -1, printcharfun, 0);
+               print_object (h->rehash_threshold, printcharfun, 0);
+             }
+ 
+           strout (" data ", -1, -1, printcharfun, 0);
+ 
+           /* print the data here as a plist */
+           int i;
+ 
+           PRINTCHAR ('(');
+           for (i = 0; i < HASH_TABLE_SIZE (h); ++i)
+             if (!NILP (HASH_HASH (h, i)))
+               {
+                 print_object (HASH_KEY (h, i), printcharfun, 0);
+                 PRINTCHAR (' ');
+                 print_object (HASH_VALUE (h, i), printcharfun, 0);
+                 PRINTCHAR (' '); /* TODO: should not be printed at end */
+               }
+           PRINTCHAR (')');
+ 	  PRINTCHAR ('>');
+ 
  	}
        else if (BUFFERP (obj))
  	{

Re: hash-table-{to, from}-alist

[Stefan Monnier]

>         #s<hash-table size 65 test 'eql weak 'key rehash-size 20.0 rehash-threshold 0.001 data (1 2 5 env )>

Looks OK, except I don't think it's a good idea to include ' in front of
key and eql.  I know it's what we do right now, but in order to read it
back in, it's going to be more trouble.  Also I'd generally prefer using
":weakness" rather than "weak", so it's as close as possible to the key
args passed to make-hash-table.

> I know it's broken for circular references (e.g. (puthash 6 h h)
> above).  How should I handle that?

The same way as the other circularities.  I.e. it should mostly "just
work", except you'll have to change the code that does the first pass so
that it doesn't skip hash tables.

> + 	  if (SYMBOLP (h->test))
> + 	    {
> +               strout (" test '", -1, -1, printcharfun, 0);
> +               print_object (h->test, printcharfun, 0);
> +             }

Shouldn't it be !NILP (h->test) rather than SYMBOLP (h->test)?

> + 	  if (SYMBOLP (h->weak) && ! NILP(h->weak))
> + 	    {
> +               strout (" weak '", -1, -1, printcharfun, 0);
> +               print_object (h->weak, printcharfun, 0);
> +             }

Here as well, !NILP should be sufficient.

> + 	  if (FLOATP (h->rehash_size) || INTEGERP (h->rehash_size))
> + 	    {
> +               strout (" rehash-size ", -1, -1, printcharfun, 0);
> +               print_object (h->rehash_size, printcharfun, 0);
> +             }

And again !NILP sounds preferable.

Otherwise, it looks OK,


        Stefan

Re: hash-table-{to, from}-alist

[David De La Harpe Golden]

Stefan Monnier wrote:
>>         #s<hash-table size 65 test 'eql weak 'key rehash-size 20.0 rehash-threshold 0.001 data (1 2 5 env )>
> 
> Looks OK, except I don't think it's a good idea to include ' in front of
> key and eql.  I know it's what we do right now, but in order to read it
> back in, it's going to be more trouble.  Also I'd generally prefer using
> ":weakness" rather than "weak", so it's as close as possible to the key
> args passed to make-hash-table.
>

The #s kind of bothers me.  In common lisp, #s(a ...) is readable - as 
an instance of struct a [*]. Hence the "s". Now, hashtables may well use 
a struct for their metadata as an implementation detail, but the #s<...> 
does look to me like an odd mix of commmon lisp #s(...) 
struct-instance-literal syntax and #<...> 
printable-but-unreadable-object syntax.

I realise that for emacs lisp compatibility with common lisp is a 
nongoal and compatibility with xemacs lisp is of some interest, so it's 
not a huge argument against #s<...>,  I'm just saying I'm finding it a 
bit confusing coming from CL (mind you, CL anyway lacks any print/read
roundtripping hashtable syntax)

[*]
CL-USER> (defstruct a b c)
A
CL-USER> (make-a :b 1 :c 2)
#S(A :B 1 :C 2)
CL-USER> #s(a :b 2 :c 3)
#S(A :B 2 :C 3)
CL-USER>

Re: hash-table-{to, from}-alist

[Stefan Monnier]

> The #s kind of bothers me.  In common lisp, #s(a ...) is readable - as an
> instance of struct a [*]. Hence the "s". Now, hashtables may well use
> a struct for their metadata as an implementation detail, but the #s<...>
> does look to me like an odd mix of commmon lisp #s(...)
> struct-instance-literal syntax and #<...>
> printable-but-unreadable-object syntax.

For what it's worth:

   % clisp
     i i i i i i i       ooooo    o        ooooooo   ooooo   ooooo
     I I I I I I I      8     8   8           8     8     o  8    8
     I  \ `+' /  I      8         8           8     8        8    8
      \  `-+-'  /       8         8           8      ooooo   8oooo
       `-__|__-'        8         8           8           8  8
           |            8     o   8           8     o     8  8
     ------+------       ooooo    8oooooo  ooo8ooo   ooooo   8
   
   Welcome to GNU CLISP 2.44.1 (2008-02-23) <http://clisp.cons.org/>
   
   Copyright (c) Bruno Haible, Michael Stoll 1992, 1993
   Copyright (c) Bruno Haible, Marcus Daniels 1994-1997
   Copyright (c) Bruno Haible, Pierpaolo Bernardi, Sam Steingold 1998
   Copyright (c) Bruno Haible, Sam Steingold 1999-2000
   Copyright (c) Sam Steingold, Bruno Haible 2001-2008
   
   Type :h and hit Enter for context help.
   
   [1]> (let ((x (make-hash-table))) (setf (gethash 4 x) 6) x)
   #S(HASH-TABLE :TEST FASTHASH-EQL (4 . 6))
   [2]> 

So the question is should we use #s(...) as CL does, or #s<...> as
XEmacs does.


        Stefan

Re: hash-table-{to, from}-alist

[David De La Harpe Golden]

Stefan Monnier wrote:

 > So the question is should we use #s(...) as CL does,

(That's definitely implementation-specific, just what clisp in 
particular does)

> or #s<...> as XEmacs does.
> 

N.B. It's actually #s(...) in XEmacs too, at least when variable 
print-readably is t. Otherwise the not-readable #<...> (no "s") is used, 
as per Stephen J. Turnbull's posts to this thread, also shown in [1]

Since hashtables are typically used for largish numbers of keys, 
supporting such an XEmacs-like print-readably variable* and suppressing 
the output of key/value data (at least beyond a certain key count) for 
the non-readable #<...> case seems like it might be quite desirable, 
though I dunno if XEmacs actually does such suppression, CL
implementations tend to (regarding xemacs compat it can't matter much 
since it's only the non-readable case).

* itself similar to CL *print-readably*, though  there's no guarantee a 
particular CL implementation can actually print its hashtables readably 
no matter how you twiddle that. Clisp does its thing. SBCL spits out a 
#. read-time evaluation lump that constructs a hashtable. Clozure CL (at 
least the outdated version on my system) signals a print-not-readable 
error... [2]

[1] http://www.xemacs.org/Documentation/21.5/html/lispref_9.html#SEC67
[2] http://www.lispworks.com/documentation/HyperSpec/Body/v_pr_rda.htm

Re: hash-table-{to, from}-alist

[Stephen J. Turnbull]

Ted Zlatanov writes:
 > On Tue, 25 Nov 2008 10:50:53 +0900 "Stephen J. Turnbull" <stephen@xemacs.org> wrote: 
 > 
 > SJT> Ted Zlatanov writes:
 > >> The equality test is necessary if it was given originally (meaning,
 > >> if it's not 'eql).  I don't think we should ever omit it, even if
 > >> it's the default value, for readability.
 > 
 > SJT> I disagree.  But it's not incompatible as long as you don't make the
 > SJT> :test option to make-hash-table obligatory. ;-)
 > 
 > OK, I'll make :test optional.  Is size mandatory?  I ask since you put
 > it in your example, but you could have done so to illustrate
 > parameters.

It's not mandatory in the read syntax, but an accurate estimate will
avoid some consing at read time.  As an implementation detail, the
write syntax does include it if non-zero.

#s(hash-table)
--> #<hash-table size 0/29 0x5fa328ee>
(let ((print-readably t))
  (prin1 #s(hash-table)))
#s(hash-table)
--> #<hash-table size 0/29 0x5fa4b2ef>
#s(hash-table size 2)
--> #<hash-table size 0/29 0x5fa56456>
(let ((print-readably t))
  (prin1 #s(hash-table size 2)))
#s(hash-table)
--> #<hash-table size 0/29 0x5fa5a16d>

 > I'm OK with that format and it seems pretty good,

Good.  I would be very annoyed if we had to support an alternative
syntax.

Re: hash-table-{to, from}-alist

[David De La Harpe Golden]

Stephen J. Turnbull wrote:

>  > I'm OK with that format and it seems pretty good,
> 
> Good.  I would be very annoyed if we had to support an alternative
> syntax.

[I think I misunderstood xemacs in my last post, not a user of it], so:

"#s(hash-table ...)" with round brackets or
"#s<hash-table ...>" with angle brackets ?

Checking xemacs lisp ref and thread posts, looks like round brackets are 
consistent with xemacs. Not that it's a major change, just that Ted's
patch used <>  (I realise now that all participants in the thread before 
I chimed in cluelessly may simply have taken it as understood that would 
be changed later, sorry...)

Re: hash-table-{to, from}-alist

[Ted Zlatanov]

On Wed, 26 Nov 2008 11:29:06 +0900 "Stephen J. Turnbull" <stephen@xemacs.org> wrote: 

SJT> Ted Zlatanov writes:
>> OK, I'll make :test optional.  Is size mandatory?  I ask since you put
>> it in your example, but you could have done so to illustrate
>> parameters.

SJT> It's not mandatory in the read syntax, but an accurate estimate will
SJT> avoid some consing at read time.  As an implementation detail, the
SJT> write syntax does include it if non-zero.

Since it doesn't matter, I'll leave it in even if zero.  I think it
improves legibility at the cost of 7 bytes.

SJT> Good.  I would be very annoyed if we had to support an alternative
SJT> syntax.

Right, my goal here is to find the simplest solution that works for
everyone.  I changed it to #s(hash-table ...) as discussed.

On Tue, 25 Nov 2008 20:46:23 -0500 Stefan Monnier <monnier@iro.umontreal.ca> wrote: 

>> #s<hash-table size 65 test 'eql weak 'key rehash-size 20.0 rehash-threshold 0.001 data (1 2 5 env )>

SM> Looks OK, except I don't think it's a good idea to include ' in front of
SM> key and eql.  I know it's what we do right now, but in order to read it
SM> back in, it's going to be more trouble.  Also I'd generally prefer using
SM> ":weakness" rather than "weak", so it's as close as possible to the key
SM> args passed to make-hash-table.

OK, both done.

>> I know it's broken for circular references (e.g. (puthash 6 h h)
>> above).  How should I handle that?

SM> The same way as the other circularities.  I.e. it should mostly
SM> "just work", except you'll have to change the code that does the
SM> first pass so that it doesn't skip hash tables.

Sorry, I don't know what you mean.  My iterator doesn't skip hash
tables, does it?  Or are you discussing something else?

SM> Shouldn't it be !NILP (h->test) rather than SYMBOLP (h->test)?
SM> Here as well [weakness], !NILP should be sufficient.
SM> And again [rehash_size and rehash_threshold] !NILP sounds preferable.

All done.

On Wed, 26 Nov 2008 05:46:32 +0000 David De La Harpe Golden <david@harpegolden.net> wrote: 

DDLHG> N.B. It's actually #s(...) in XEmacs too, at least when variable
DDLHG> print-readably is t. Otherwise the not-readable #<...> (no "s") is
DDLHG> used, as per Stephen J. Turnbull's posts to this thread, also shown in
DDLHG> [1]

I am still printing the two versions of the object in the attached
patch, but I can of course make it conditional on print-readably.  IMHO
hashtables should always be printed completely and should be treated
like lists and vectors.  If, however, compatibility with XEmacs is
desired or people feel strongly for print-readably, I will implement
with print-readably.

It prints an extra space at the end, which I still have as a simple
TODO.  I also have to handle circular references, but otherwise it seems
pretty reasonable.

Ted

Index: print.c
===================================================================
RCS file: /sources/emacs/emacs/src/print.c,v
retrieving revision 1.253
diff -c -r1.253 print.c
*** print.c	31 Jul 2008 05:33:53 -0000	1.253
--- print.c	26 Nov 2008 16:01:57 -0000
***************
*** 2052,2057 ****
--- 2052,2107 ----
  	  sprintf (buf, " 0x%lx", (unsigned long) h);
  	  strout (buf, -1, -1, printcharfun, 0);
  	  PRINTCHAR ('>');
+ 
+ /*
+   implement a readable output, e.g.:
+   #s(hash-table size 2 test equal data (k1 v1 k2 v2))
+ */
+           /* always print the size */
+           sprintf (buf, "\n\n\t#s(hash-table size %ld", (long) XVECTOR (h->next)->size);
+           strout (buf, -1, -1, printcharfun, 0);
+ 
+ 	  if (!NILP(h->test))
+ 	    {
+               strout (" test ", -1, -1, printcharfun, 0);
+               print_object (h->test, printcharfun, 0);
+             }
+ 
+ 	  if (!NILP(h->weak))
+ 	    {
+               strout (" weakness ", -1, -1, printcharfun, 0);
+               print_object (h->weak, printcharfun, 0);
+             }
+ 
+ 	  if (!NILP(h->rehash_size))
+ 	    {
+               strout (" rehash-size ", -1, -1, printcharfun, 0);
+               print_object (h->rehash_size, printcharfun, 0);
+             }
+ 
+ 	  if (!NILP(h->rehash_threshold))
+ 	    {
+               strout (" rehash-threshold ", -1, -1, printcharfun, 0);
+               print_object (h->rehash_threshold, printcharfun, 0);
+             }
+ 
+           strout (" data ", -1, -1, printcharfun, 0);
+ 
+           /* print the data here as a plist */
+           int i;
+ 
+           PRINTCHAR ('(');
+           for (i = 0; i < HASH_TABLE_SIZE (h); ++i)
+             if (!NILP (HASH_HASH (h, i)))
+               {
+                 print_object (HASH_KEY (h, i), printcharfun, 0);
+                 PRINTCHAR (' ');
+                 print_object (HASH_VALUE (h, i), printcharfun, 0);
+                 PRINTCHAR (' '); /* TODO: should not be printed at end */
+               }
+           PRINTCHAR (')');
+ 	  PRINTCHAR (')');
+ 
  	}
        else if (BUFFERP (obj))
  	{

Re: hash-table-{to, from}-alist

[Stefan Monnier]

> everyone.  I changed it to #s(hash-table ...) as discussed.

Good, thanks.

SM> The same way as the other circularities.  I.e. it should mostly
SM> "just work", except you'll have to change the code that does the
SM> first pass so that it doesn't skip hash tables.
> Sorry, I don't know what you mean.  My iterator doesn't skip hash
> tables, does it?  Or are you discussing something else?

The print code does 2 passes over the data: a first one to collect the
set of objects it's going to print, and a second to do the actual print.
This 2-pass approach is done to handle cycles and sharing (so that when
an object appears several times in the output, we know it right when we
print the first occurrence).  IIUC your patch only patches the second
pass, but not the first.

> IMHO hashtables should always be printed completely and should be
> treated like lists and vectors.  If, however, compatibility with
> XEmacs is desired or people feel strongly for print-readably, I will
> implement with print-readably.

char tables are always printed in full and we haven't had any complaints
about that, so you're probably right: let's just always print them
in full.  If it turns out to be problematic in some cases, we can later
on add some extra variable to control the level of detail.


        Stefan

Re: hash-table-{to, from}-alist

[Ted Zlatanov]

On Wed, 26 Nov 2008 13:10:38 -0500 Stefan Monnier <monnier@iro.umontreal.ca> wrote: 

SM> The same way as the other circularities.  I.e. it should mostly
SM> "just work", except you'll have to change the code that does the
SM> first pass so that it doesn't skip hash tables.
>> Sorry, I don't know what you mean.  My iterator doesn't skip hash
>> tables, does it?  Or are you discussing something else?

SM> The print code does 2 passes over the data: a first one to collect the
SM> set of objects it's going to print, and a second to do the actual print.
SM> This 2-pass approach is done to handle cycles and sharing (so that when
SM> an object appears several times in the output, we know it right when we
SM> print the first occurrence).  IIUC your patch only patches the second
SM> pass, but not the first.

I'll research this, thanks for explaining.

>> IMHO hashtables should always be printed completely and should be
>> treated like lists and vectors.  If, however, compatibility with
>> XEmacs is desired or people feel strongly for print-readably, I will
>> implement with print-readably.

SM> char tables are always printed in full and we haven't had any complaints
SM> about that, so you're probably right: let's just always print them
SM> in full.  If it turns out to be problematic in some cases, we can later
SM> on add some extra variable to control the level of detail.

Great.  Maybe XEmacs should add print-unreadably ;)

Ted

Re: hash-table-{to, from}-alist

[Ted Zlatanov]

On Wed, 26 Nov 2008 14:37:28 -0600 Ted Zlatanov <tzz@lifelogs.com> wrote: 

TZ> On Wed, 26 Nov 2008 13:10:38 -0500 Stefan Monnier <monnier@iro.umontreal.ca> wrote: 
SM> The print code does 2 passes over the data: a first one to collect the
SM> set of objects it's going to print, and a second to do the actual print.
SM> This 2-pass approach is done to handle cycles and sharing (so that when
SM> an object appears several times in the output, we know it right when we
SM> print the first occurrence).  IIUC your patch only patches the second
SM> pass, but not the first.

TZ> I'll research this, thanks for explaining.

The patch below does all the previously discussed formatting plus
detection of circular references and Davis Herring's suggestion of
skipping the first preliminary space.  The old-style hashtable printout
is disabled with an #ifdef, so only the new style is available.

I tested it quite a bit, especially with compound data members, and it
seems to work all right.  If this patch is OK (Stefan, please make the
call since you've been looking at it) I'll go on to the reading code.

Thanks
Ted

Index: print.c
===================================================================
RCS file: /sources/emacs/emacs/src/print.c,v
retrieving revision 1.253
diff -c -r1.253 print.c
*** print.c	31 Jul 2008 05:33:53 -0000	1.253
--- print.c	26 Nov 2008 21:15:18 -0000
***************
*** 1341,1346 ****
--- 1341,1347 ----
   loop:
    if (STRINGP (obj) || CONSP (obj) || VECTORP (obj)
        || COMPILEDP (obj) || CHAR_TABLE_P (obj) || SUB_CHAR_TABLE_P (obj)
+       || HASH_TABLE_P (obj)
        || (! NILP (Vprint_gensym)
  	  && SYMBOLP (obj)
  	  && !SYMBOL_INTERNED_P (obj)))
***************
*** 1536,1541 ****
--- 1537,1543 ----
    /* Detect circularities and truncate them.  */
    if (STRINGP (obj) || CONSP (obj) || VECTORP (obj)
        || COMPILEDP (obj) || CHAR_TABLE_P (obj) || SUB_CHAR_TABLE_P (obj)
+       || HASH_TABLE_P (obj)
        || (! NILP (Vprint_gensym)
  	  && SYMBOLP (obj)
  	  && !SYMBOL_INTERNED_P (obj)))
***************
*** 2036,2041 ****
--- 2038,2044 ----
        else if (HASH_TABLE_P (obj))
  	{
  	  struct Lisp_Hash_Table *h = XHASH_TABLE (obj);
+ #if 0
  	  strout ("#<hash-table", -1, -1, printcharfun, 0);
  	  if (SYMBOLP (h->test))
  	    {
***************
*** 2052,2057 ****
--- 2055,2115 ----
  	  sprintf (buf, " 0x%lx", (unsigned long) h);
  	  strout (buf, -1, -1, printcharfun, 0);
  	  PRINTCHAR ('>');
+ #endif
+ 	  /*
+ 	    implement a readable output, e.g.:
+ 	    #s(hash-table size 2 test equal data (k1 v1 k2 v2))
+ 	  */
+ 	  /* always print the size */
+ 	  sprintf (buf, "#s(hash-table size %ld", (long) XVECTOR (h->next)->size);
+ 	  strout (buf, -1, -1, printcharfun, 0);
+ 
+ 	  if (!NILP(h->test))
+ 	    {
+ 	      strout (" test ", -1, -1, printcharfun, 0);
+ 	      print_object (h->test, printcharfun, 0);
+ 	    }
+ 
+ 	  if (!NILP(h->weak))
+ 	    {
+ 	      strout (" weakness ", -1, -1, printcharfun, 0);
+ 	      print_object (h->weak, printcharfun, 0);
+ 	    }
+ 
+ 	  if (!NILP(h->rehash_size))
+ 	    {
+ 	      strout (" rehash-size ", -1, -1, printcharfun, 0);
+ 	      print_object (h->rehash_size, printcharfun, 0);
+ 	    }
+ 
+ 	  if (!NILP(h->rehash_threshold))
+ 	    {
+ 	      strout (" rehash-threshold ", -1, -1, printcharfun, 0);
+ 	      print_object (h->rehash_threshold, printcharfun, 0);
+ 	    }
+ 
+ 	  strout (" data ", -1, -1, printcharfun, 0);
+ 
+ 	  /* print the data here as a plist */
+ 	  int i;
+ 	  int printed=0;
+ 
+ 	  PRINTCHAR ('(');
+ 	  for (i = 0; i < HASH_TABLE_SIZE (h); ++i)
+ 	    if (!NILP (HASH_HASH (h, i)))
+ 	      {
+ 		if (printed)
+ 		  {
+ 		    PRINTCHAR (' ');
+ 		  }
+ 		print_object (HASH_KEY (h, i), printcharfun, 0);
+ 		PRINTCHAR (' ');
+ 		print_object (HASH_VALUE (h, i), printcharfun, 0);
+ 		printed = 1;
+ 	      }
+ 	  PRINTCHAR (')');
+ 	  PRINTCHAR (')');
+ 
  	}
        else if (BUFFERP (obj))
  	{

Re: hash-table-{to, from}-alist

[Ted Zlatanov]

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

On Wed, 26 Nov 2008 15:16:41 -0600 Ted Zlatanov <tzz@lifelogs.com> wrote: 

TZ> The patch below does all the previously discussed formatting plus
TZ> detection of circular references and Davis Herring's suggestion of
TZ> skipping the first preliminary space.  The old-style hashtable printout
TZ> is disabled with an #ifdef, so only the new style is available.

The attached patch does reading and writing of hashtables.  I believe
it works correctly and handles the common error cases.  Please review.

Here are the issues:

- I don't know how to manage a dynamic array of Lisp_Object objects, as
  make-hash-table wants.  What I have right now is hacky and breaks for
  more than 5 parameters (e.g. reading #s(hash-table size 4 size 5 size
  4 size 5 size 4 weakness x) will ignore the weakness parameter.
  Ideally I'd put the arguments in a list and pass that off to
  make-hash-table, but I couldn't find an example of that in the source
  (casting a list of an array of Lisp_Object elements).  Additionally I
  use xmalloc without freeing the memory; I don't know if that will
  cause issues but I saw it used in other places without freeing the
  memory.

- it seems to me that
		      EQ(head, Qhash_table_size_marker) ||
		      EQ(head, Qhash_table_test_marker) ||
		      EQ(head, Qhash_table_weakness_marker) ||
		      EQ(head, Qhash_table_rehash_size_marker) ||
		      EQ(head, Qhash_table_rehash_threshold_marker))
  can be written better, but I don't know enough to do it right.  In
  addition, I don't know how to convert all those parameters to their :
  plist version, e.g. size => :size.  I'm sure it's trivial but couldn't
  find it in the source.  I really wish I could write all this in ELisp :)

- my code is very deliberate, using CAR_SAFE and CDR_SAFE a lot.  If
  that causes performance issues, please suggest improvements.  Ditto
  for anything else--remember this is my first trip to the C side of
  Emacs and treat everything with suspicion.

Thanks
Ted


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

? hashprint.patch
Index: lread.c
===================================================================
RCS file: /sources/emacs/emacs/src/lread.c,v
retrieving revision 1.401
diff -u -r1.401 lread.c
--- lread.c	7 Sep 2008 20:41:10 -0000	1.401
+++ lread.c	1 Dec 2008 21:48:58 -0000
@@ -80,6 +80,13 @@
 extern int errno;
 #endif
 
+/* hash table read constants */
+Lisp_Object Qhash_table_read_marker, Qhash_table_data_marker;
+Lisp_Object Qhash_table_test_marker, Qhash_table_size_marker;
+Lisp_Object Qhash_table_weakness_marker;
+Lisp_Object Qhash_table_rehash_size_marker;
+Lisp_Object Qhash_table_rehash_threshold_marker;
+
 Lisp_Object Qread_char, Qget_file_char, Qstandard_input, Qcurrent_load_list;
 Lisp_Object Qvariable_documentation, Vvalues, Vstandard_input, Vafter_load_alist;
 Lisp_Object Qascii_character, Qload, Qload_file_name;
@@ -2341,6 +2348,89 @@
 
     case '#':
       c = READCHAR;
+      if (c == 's')
+	{
+	  c = READCHAR;
+	  if (c == '(')
+	    {
+	      /*
+		Accept extended format for hashtables (extensible to
+		other types), e.g.
+		#s(hash-table size 2 test equal data (k1 v1 k2 v2))	     
+	      */
+	      Lisp_Object tmp = read_list (0, readcharfun);
+	      Lisp_Object head = CAR_SAFE(tmp);
+	      Lisp_Object data = Qnil;
+	      Lisp_Object val = Qnil;
+	      /* will this be freed automatically? */	      
+	      Lisp_Object* params = (Lisp_Object*) xmalloc (10 * sizeof(Lisp_Object));
+	      int param_count = 0;
+	      if (!EQ (head, Qhash_table_read_marker))
+		error ("Invalid extended read marker at head of #s list"
+		       "(only hash-table allowed)");
+
+	      while (!NILP(tmp))
+		{
+		  tmp = CDR_SAFE(tmp);
+		  head = CAR_SAFE(tmp);
+		  /* allowed parameters: size test weakness
+		     rehash-size rehash-threshold */
+		  if (EQ(head, Qhash_table_data_marker))
+		    {
+		      tmp = CDR_SAFE(tmp);
+		      data = CAR_SAFE(tmp);
+		      /* debug_print(data); */
+		    }
+
+		  if (
+		      param_count < 9 &&
+		      EQ(head, Qhash_table_size_marker) ||
+		      EQ(head, Qhash_table_test_marker) ||
+		      EQ(head, Qhash_table_weakness_marker) ||
+		      EQ(head, Qhash_table_rehash_size_marker) ||
+		      EQ(head, Qhash_table_rehash_threshold_marker))
+		    {
+		      tmp = CDR_SAFE(tmp);
+		      val = CAR_SAFE(tmp);
+		      /*
+			debug_print(head);
+			debug_print(val);
+		      */
+		      /* how do I turn head into a symbol with the same contents but beginning with ':'? */
+		      params[param_count] = head;
+		      params[param_count+1] = val;
+		      param_count+=2;
+		    }
+		}
+
+	      if (NILP(data))
+		error ("No data marker was found in the hash table");
+
+	      /* doesn't work because of 'size' vs. ':size' issue, see above
+		 Lisp_Object ht = Fmake_hash_table(param_count, params);
+	      */
+	      Lisp_Object ht = Fmake_hash_table(0, NULL);
+
+	      Lisp_Object key = Qnil;
+	      
+	      while (!NILP(data))
+		{
+		  key = CAR_SAFE(data);
+		  data = CDR_SAFE(data);
+		  val = CAR_SAFE(data);
+		  data = CDR_SAFE(data);
+		  if (NILP(val))
+		    error ("Odd number of elements in hashtable data");
+		  /*
+		    debug_print(key);
+		    debug_print(val);
+		  */
+		  Fputhash(key, val, ht);
+		}
+	      
+	      return ht;
+	    }	
+	}
       if (c == '^')
 	{
 	  c = READCHAR;
@@ -4432,6 +4522,21 @@
 
   Vloads_in_progress = Qnil;
   staticpro (&Vloads_in_progress);
+
+  Qhash_table_read_marker = intern ("hash-table");
+  staticpro (&Qhash_table_read_marker);
+  Qhash_table_data_marker = intern ("data");
+  staticpro (&Qhash_table_data_marker);
+  Qhash_table_test_marker = intern ("test");
+  staticpro (&Qhash_table_size_marker);
+  Qhash_table_test_marker = intern ("size");
+  staticpro (&Qhash_table_size_marker);
+  Qhash_table_weakness_marker = intern ("weakness");
+  staticpro (&Qhash_table_weakness_marker);
+  Qhash_table_rehash_size_marker = intern ("rehash-size");
+  staticpro (&Qhash_table_rehash_size_marker);
+  Qhash_table_rehash_threshold_marker = intern ("rehash-threshold");
+  staticpro (&Qhash_table_rehash_threshold_marker);
 }
 
 /* arch-tag: a0d02733-0f96-4844-a659-9fd53c4f414d
Index: print.c
===================================================================
RCS file: /sources/emacs/emacs/src/print.c,v
retrieving revision 1.253
diff -u -r1.253 print.c
--- print.c	31 Jul 2008 05:33:53 -0000	1.253
+++ print.c	1 Dec 2008 21:48:58 -0000
@@ -1341,6 +1341,7 @@
  loop:
   if (STRINGP (obj) || CONSP (obj) || VECTORP (obj)
       || COMPILEDP (obj) || CHAR_TABLE_P (obj) || SUB_CHAR_TABLE_P (obj)
+      || HASH_TABLE_P (obj)
       || (! NILP (Vprint_gensym)
 	  && SYMBOLP (obj)
 	  && !SYMBOL_INTERNED_P (obj)))
@@ -1536,6 +1537,7 @@
   /* Detect circularities and truncate them.  */
   if (STRINGP (obj) || CONSP (obj) || VECTORP (obj)
       || COMPILEDP (obj) || CHAR_TABLE_P (obj) || SUB_CHAR_TABLE_P (obj)
+      || HASH_TABLE_P (obj)
       || (! NILP (Vprint_gensym)
 	  && SYMBOLP (obj)
 	  && !SYMBOL_INTERNED_P (obj)))
@@ -2036,6 +2038,7 @@
       else if (HASH_TABLE_P (obj))
 	{
 	  struct Lisp_Hash_Table *h = XHASH_TABLE (obj);
+#if 0
 	  strout ("#<hash-table", -1, -1, printcharfun, 0);
 	  if (SYMBOLP (h->test))
 	    {
@@ -2052,6 +2055,61 @@
 	  sprintf (buf, " 0x%lx", (unsigned long) h);
 	  strout (buf, -1, -1, printcharfun, 0);
 	  PRINTCHAR ('>');
+#endif
+	  /*
+	    implement a readable output, e.g.:
+	    #s(hash-table size 2 test equal data (k1 v1 k2 v2))
+	  */
+	  /* always print the size */
+	  sprintf (buf, "#s(hash-table size %ld", (long) XVECTOR (h->next)->size);
+	  strout (buf, -1, -1, printcharfun, 0);
+
+	  if (!NILP(h->test))
+	    {
+	      strout (" test ", -1, -1, printcharfun, 0);
+	      print_object (h->test, printcharfun, 0);
+	    }
+
+	  if (!NILP(h->weak))
+	    {
+	      strout (" weakness ", -1, -1, printcharfun, 0);
+	      print_object (h->weak, printcharfun, 0);
+	    }
+
+	  if (!NILP(h->rehash_size))
+	    {
+	      strout (" rehash-size ", -1, -1, printcharfun, 0);
+	      print_object (h->rehash_size, printcharfun, 0);
+	    }
+
+	  if (!NILP(h->rehash_threshold))
+	    {
+	      strout (" rehash-threshold ", -1, -1, printcharfun, 0);
+	      print_object (h->rehash_threshold, printcharfun, 0);
+	    }
+
+	  strout (" data ", -1, -1, printcharfun, 0);
+
+	  /* print the data here as a plist */
+	  int i;
+	  int printed=0;
+
+	  PRINTCHAR ('(');
+	  for (i = 0; i < HASH_TABLE_SIZE (h); ++i)
+	    if (!NILP (HASH_HASH (h, i)))
+	      {
+		if (printed)
+		  {
+		    PRINTCHAR (' ');
+		  }
+		print_object (HASH_KEY (h, i), printcharfun, 0);
+		PRINTCHAR (' ');
+		print_object (HASH_VALUE (h, i), printcharfun, 0);
+		printed = 1;
+	      }
+	  PRINTCHAR (')');
+	  PRINTCHAR (')');
+
 	}
       else if (BUFFERP (obj))
 	{

Re: hash-table-{to, from}-alist

[Andreas Schwab]

Ted Zlatanov <tzz@lifelogs.com> writes:

>   I really wish I could write all this in ELisp :)

That should be fairly easy by introducing a variable, say,
hash-table-reader-function and just calling its value when set (and
bailing out if not).  That can then be implemented fully in Lisp.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: hash-table-{to, from}-alist

[Ted Zlatanov]

On Tue, 02 Dec 2008 01:13:49 +0100 Andreas Schwab <schwab@suse.de> wrote: 

AS> Ted Zlatanov <tzz@lifelogs.com> writes:
>> I really wish I could write all this in ELisp :)

AS> That should be fairly easy by introducing a variable, say,
AS> hash-table-reader-function and just calling its value when set (and
AS> bailing out if not).  That can then be implemented fully in Lisp.

Yes, I considered that, but I'm potentially reading thousands of entries
so I wanted to keep the code as fast as possible.  

Splitting the code into an ELisp setup phase and a C fast data filler is
probably the easiest solution if I go that route, but I have to say I
enjoyed my trip in the C side of Emacs so I'd like to figure out the C
way to do what I proposed.  Specifically, converting a C LispObject
holding the symbol "test" to the symbol ":test" without hard-coding, and
passing an array of LispObject elements to make-hash-table, are my main
issues right now.

Ted

Re: hash-table-{to, from}-alist

[Stefan Monnier]

>> I really wish I could write all this in ELisp :)
> That should be fairly easy by introducing a variable, say,
> hash-table-reader-function and just calling its value when set (and
> bailing out if not).  That can then be implemented fully in Lisp.

Yes, that would seem like a good idea, although I'm not sure how ready
the C reader code is to let elisp evaluation take place in the middle
of it, so we may find some trouble, but it's worth trying.


        Stefan

Re: hash-table-{to, from}-alist

[Andreas Schwab]

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

> Yes, that would seem like a good idea, although I'm not sure how ready
> the C reader code is to let elisp evaluation take place in the middle
> of it, so we may find some trouble, but it's worth trying.

You can already have arbitrary lisp functions for reading characters.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: hash-table-{to, from}-alist

[Ted Zlatanov]

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

The attached patch does print and read of hashtables.  It respects
print-length only, exactly like vector print and consistent with the
rest of GNU Emacs.  Please review and comment.  Some notes:

I ended up with this code to put a list into Fmake_hash_table format
('params' is a Lisp_Object that holds a bunch of conses).

	      int param_count = 0;
	      Lisp_Object *pa = (Lisp_Object*) xmalloc(XFASTINT(Flength(params)) * sizeof(Lisp_Object));

	      while (!NILP(params))
		{
		  pa[param_count++] = CAR_SAFE(params);
		  params = CDR_SAFE(params);
		}

	      Lisp_Object ht = Fmake_hash_table(param_count, pa);

I would prefer it if I could avoid all that work, but it seems (from
reading lisp.h and hunting for similar code) that it must be done as
above.  I don't see a macro to convert a Lisp_Object that holds a list
into the Lisp_Object* that a lot of functions need.

Also I don't know if what xmalloc() gives needs to be freed eventually.

To generate the Lisp symbol ":size" from the symbol "size" for example,
I used:

Lisp_Object colon = make_string (":", 1);
intern(SDATA(concat2(colon, SYMBOL_NAME(head))))

I don't know if there's a better way, but this works.

Thanks!
Ted


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

? hashprint.patch
Index: lread.c
===================================================================
RCS file: /sources/emacs/emacs/src/lread.c,v
retrieving revision 1.401
diff -u -r1.401 lread.c
--- lread.c	7 Sep 2008 20:41:10 -0000	1.401
+++ lread.c	2 Dec 2008 20:46:34 -0000
@@ -80,6 +80,13 @@
 extern int errno;
 #endif
 
+/* hash table read constants */
+Lisp_Object Qhash_table_read_marker, Qhash_table_data_marker;
+Lisp_Object Qhash_table_test_marker, Qhash_table_size_marker;
+Lisp_Object Qhash_table_weakness_marker;
+Lisp_Object Qhash_table_rehash_size_marker;
+Lisp_Object Qhash_table_rehash_threshold_marker;
+
 Lisp_Object Qread_char, Qget_file_char, Qstandard_input, Qcurrent_load_list;
 Lisp_Object Qvariable_documentation, Vvalues, Vstandard_input, Vafter_load_alist;
 Lisp_Object Qascii_character, Qload, Qload_file_name;
@@ -2341,6 +2348,94 @@
 
     case '#':
       c = READCHAR;
+      if (c == 's')
+	{
+	  c = READCHAR;
+	  if (c == '(')
+	    {
+	      /*
+		Accept extended format for hashtables (extensible to
+		other types), e.g.
+		#s(hash-table size 2 test equal data (k1 v1 k2 v2))	     
+	      */
+	      Lisp_Object tmp = read_list (0, readcharfun);
+	      Lisp_Object head = CAR_SAFE(tmp);
+	      Lisp_Object data = Qnil;
+	      Lisp_Object val = Qnil;
+	      Lisp_Object params = Qnil;
+	      Lisp_Object colon = make_string (":", 1);
+	      
+	      if (!EQ (head, Qhash_table_read_marker))
+		error ("Invalid extended read marker at head of #s list"
+		       "(only hash-table allowed)");
+
+	      while (!NILP(tmp))
+		{
+		  tmp = CDR_SAFE(tmp);
+		  head = CAR_SAFE(tmp);
+		  /* allowed parameters: size test weakness
+		     rehash-size rehash-threshold */
+		  if (EQ(head, Qhash_table_data_marker))
+		    {
+		      tmp = CDR_SAFE(tmp);
+		      data = CAR_SAFE(tmp);
+		    }
+
+		  if (
+		      EQ(head, Qhash_table_size_marker) ||
+		      EQ(head, Qhash_table_test_marker) ||
+		      EQ(head, Qhash_table_weakness_marker) ||
+		      EQ(head, Qhash_table_rehash_size_marker) ||
+		      EQ(head, Qhash_table_rehash_threshold_marker))
+		    {
+		      tmp = CDR_SAFE(tmp);
+		      val = CAR_SAFE(tmp);
+		      params = Fcons(val, params);
+		      params = Fcons(intern(SDATA(concat2(colon,
+							  SYMBOL_NAME(head)))),
+				     params);
+		      debug_print(params);
+		      /*
+			debug_print(head);
+			debug_print(val);
+		      */
+		    }
+		}
+
+	      if (NILP(data))
+		error ("No data marker was found in the hash table");
+
+	      int param_count = 0;
+	      Lisp_Object *pa = (Lisp_Object*) xmalloc(XFASTINT(Flength(params)) * sizeof(Lisp_Object));
+
+	      while (!NILP(params))
+		{
+		  pa[param_count++] = CAR_SAFE(params);
+		  params = CDR_SAFE(params);
+		}
+
+	      Lisp_Object ht = Fmake_hash_table(param_count, pa);
+
+	      Lisp_Object key = Qnil;
+	      
+	      while (!NILP(data))
+		{
+		  key = CAR_SAFE(data);
+		  data = CDR_SAFE(data);
+		  val = CAR_SAFE(data);
+		  data = CDR_SAFE(data);
+		  if (NILP(val))
+		    error ("Odd number of elements in hashtable data");
+		  /*
+		    debug_print(key);
+		    debug_print(val);
+		  */
+		  Fputhash(key, val, ht);
+		}
+	      
+	      return ht;
+	    }	
+	}
       if (c == '^')
 	{
 	  c = READCHAR;
@@ -4432,6 +4527,21 @@
 
   Vloads_in_progress = Qnil;
   staticpro (&Vloads_in_progress);
+
+  Qhash_table_read_marker = intern ("hash-table");
+  staticpro (&Qhash_table_read_marker);
+  Qhash_table_data_marker = intern ("data");
+  staticpro (&Qhash_table_data_marker);
+  Qhash_table_test_marker = intern ("test");
+  staticpro (&Qhash_table_size_marker);
+  Qhash_table_test_marker = intern ("size");
+  staticpro (&Qhash_table_size_marker);
+  Qhash_table_weakness_marker = intern ("weakness");
+  staticpro (&Qhash_table_weakness_marker);
+  Qhash_table_rehash_size_marker = intern ("rehash-size");
+  staticpro (&Qhash_table_rehash_size_marker);
+  Qhash_table_rehash_threshold_marker = intern ("rehash-threshold");
+  staticpro (&Qhash_table_rehash_threshold_marker);
 }
 
 /* arch-tag: a0d02733-0f96-4844-a659-9fd53c4f414d
Index: print.c
===================================================================
RCS file: /sources/emacs/emacs/src/print.c,v
retrieving revision 1.253
diff -u -r1.253 print.c
--- print.c	31 Jul 2008 05:33:53 -0000	1.253
+++ print.c	2 Dec 2008 20:46:34 -0000
@@ -1341,6 +1341,7 @@
  loop:
   if (STRINGP (obj) || CONSP (obj) || VECTORP (obj)
       || COMPILEDP (obj) || CHAR_TABLE_P (obj) || SUB_CHAR_TABLE_P (obj)
+      || HASH_TABLE_P (obj)
       || (! NILP (Vprint_gensym)
 	  && SYMBOLP (obj)
 	  && !SYMBOL_INTERNED_P (obj)))
@@ -1536,6 +1537,7 @@
   /* Detect circularities and truncate them.  */
   if (STRINGP (obj) || CONSP (obj) || VECTORP (obj)
       || COMPILEDP (obj) || CHAR_TABLE_P (obj) || SUB_CHAR_TABLE_P (obj)
+      || HASH_TABLE_P (obj)
       || (! NILP (Vprint_gensym)
 	  && SYMBOLP (obj)
 	  && !SYMBOL_INTERNED_P (obj)))
@@ -2036,6 +2038,7 @@
       else if (HASH_TABLE_P (obj))
 	{
 	  struct Lisp_Hash_Table *h = XHASH_TABLE (obj);
+#if 0
 	  strout ("#<hash-table", -1, -1, printcharfun, 0);
 	  if (SYMBOLP (h->test))
 	    {
@@ -2052,6 +2055,68 @@
 	  sprintf (buf, " 0x%lx", (unsigned long) h);
 	  strout (buf, -1, -1, printcharfun, 0);
 	  PRINTCHAR ('>');
+#endif
+	  /*
+	    implement a readable output, e.g.:
+	    #s(hash-table size 2 test equal data (k1 v1 k2 v2))
+	  */
+	  /* always print the size */
+	  sprintf (buf, "#s(hash-table size %ld", (long) XVECTOR (h->next)->size);
+	  strout (buf, -1, -1, printcharfun, 0);
+
+	  if (!NILP(h->test))
+	    {
+	      strout (" test ", -1, -1, printcharfun, 0);
+	      print_object (h->test, printcharfun, 0);
+	    }
+
+	  if (!NILP(h->weak))
+	    {
+	      strout (" weakness ", -1, -1, printcharfun, 0);
+	      print_object (h->weak, printcharfun, 0);
+	    }
+
+	  if (!NILP(h->rehash_size))
+	    {
+	      strout (" rehash-size ", -1, -1, printcharfun, 0);
+	      print_object (h->rehash_size, printcharfun, 0);
+	    }
+
+	  if (!NILP(h->rehash_threshold))
+	    {
+	      strout (" rehash-threshold ", -1, -1, printcharfun, 0);
+	      print_object (h->rehash_threshold, printcharfun, 0);
+	    }
+
+	  strout (" data ", -1, -1, printcharfun, 0);
+
+	  /* print the data here as a plist */
+	  int i;
+
+	  int real_size = HASH_TABLE_SIZE (h);
+	  int size = real_size;
+
+	  /* Don't print more elements than the specified maximum.  */
+	  if (NATNUMP (Vprint_length)
+	      && XFASTINT (Vprint_length) < size)
+	    size = XFASTINT (Vprint_length);
+	  
+	  PRINTCHAR ('(');
+	  for (i = 0; i < size; i++)
+	    if (!NILP (HASH_HASH (h, i)))
+	      {
+		if (i) PRINTCHAR (' ');
+		print_object (HASH_KEY (h, i), printcharfun, 0);
+		PRINTCHAR (' ');
+		print_object (HASH_VALUE (h, i), printcharfun, 0);
+	      }
+
+	  if (size < real_size)
+	    strout (" ...", 4, 4, printcharfun, 0);
+
+	  PRINTCHAR (')');
+	  PRINTCHAR (')');
+
 	}
       else if (BUFFERP (obj))
 	{

Re: hash-table-{to, from}-alist

[Andreas Schwab]

Ted Zlatanov <tzz@lifelogs.com> writes:

> I ended up with this code to put a list into Fmake_hash_table format
> ('params' is a Lisp_Object that holds a bunch of conses).
>
> 	      int param_count = 0;
> 	      Lisp_Object *pa = (Lisp_Object*) xmalloc(XFASTINT(Flength(params)) * sizeof(Lisp_Object));
>
> 	      while (!NILP(params))
> 		{
> 		  pa[param_count++] = CAR_SAFE(params);
> 		  params = CDR_SAFE(params);
> 		}
>
> 	      Lisp_Object ht = Fmake_hash_table(param_count, pa);

This is of course a memory leak, since pa is never freed.

> I would prefer it if I could avoid all that work, but it seems (from
> reading lisp.h and hunting for similar code) that it must be done as
> above.  I don't see a macro to convert a Lisp_Object that holds a list
> into the Lisp_Object* that a lot of functions need.

Since you know the max size of the array, just declare it as such.

(One more reason to implement it in Lisp.)

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: hash-table-{to, from}-alist

[Stefan Monnier]

> +	      /* will this be freed automatically? */
> +	      Lisp_Object* params = (Lisp_Object*) xmalloc (10 * sizeof(Lisp_Object));

No, it won't be freed automatically.  Why not use `alloca' instead?
Also rather than 10, why not use something like XINT (Flength (tmp)) ?

> +	      if (!EQ (head, Qhash_table_read_marker))

This constant should be called Qhash_table.

> +	      while (!NILP(tmp))
> +		{
> +		  tmp = CDR_SAFE(tmp);
> +		  head = CAR_SAFE(tmp);

If you replace !NILP with CONSP, you can replace CDR_SAFE with XCDR
(which is more efficient since it presumes you've done the CONSP test
earlier).

> +		  /* allowed parameters: size test weakness
> +		     rehash-size rehash-threshold */

Why not :weakness, :rehash-size, :rehash-threshold?
(maybe even :test and :size, although IIUC we may prefer just `size'
and `test' for compatibility with XEmacs, tho I'm not sure how
important that is for this kind of data).

> +		  if (EQ(head, Qhash_table_data_marker))

It should just be called Qdata.  Also you need to put spaces before all
your open parens.

> +		    {
> +		      tmp = CDR_SAFE(tmp);
> +		      data = CAR_SAFE(tmp);
> +		      /* debug_print(data); */
> +		    }
> +
> +		  if (
> +		      param_count < 9 &&
> +		      EQ(head, Qhash_table_size_marker) ||
> +		      EQ(head, Qhash_table_test_marker) ||
> +		      EQ(head, Qhash_table_weakness_marker) ||
> +		      EQ(head, Qhash_table_rehash_size_marker) ||
> +		      EQ(head, Qhash_table_rehash_threshold_marker))
> +		    {
> +		      tmp = CDR_SAFE(tmp);
> +		      val = CAR_SAFE(tmp);
> +		      /*
> +			debug_print(head);
> +			debug_print(val);
> +		      */
> +		      /* how do I turn head into a symbol with the same contents but beginning with ':'? */
> +		      params[param_count] = head;
> +		      params[param_count+1] = val;
> +		      param_count+=2;
> +		    }
> +		}

How 'bout:

   {
      Lisp_Object params[10];
      params[0] = QCsize;
      params[1] = Fplist_get (Qsize, tmp);
      params[2] = QCtest;
      params[3] = Fplist_get (Qtest, tmp);
      params[4] = QCweakness;
      params[5] = Fplist_get (Qweakness, tmp);
      params[6] = QCrehash_size;
      params[7] = Fplist_get (Qrehash_size, tmp);
      params[8] = QCrehash_threshold;
      params[9] = Fplist_get (Qrehash_threshold, tmp);
   }

and then use Fplist_get (Qdata, tmp) to get the data?

> +	      if (NILP(data))
> +		error ("No data marker was found in the hash table");

Why bother?

> +	      while (!NILP(data))
> +		{
> +		  key = CAR_SAFE(data);
> +		  data = CDR_SAFE(data);
> +		  val = CAR_SAFE(data);
> +		  data = CDR_SAFE(data);
> +		  if (NILP(val))
> +		    error ("Odd number of elements in hashtable data");

This test doesn't look right.  What if the value stored for `key' is
indeed nil?  You could write it like that instead:

      while (CONSP (data))
	{
	  key = XCAR (data);
	  data = XCDR (data);
	  if (!CONSP (data))
	    error ("Odd number of elements in hashtable data");
	  val = XCAR (data);
	  data = XCDR (data);


-- Stefan

Re: hash-table-{to, from}-alist

[Ted Zlatanov]

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

On Tue, 02 Dec 2008 16:58:41 -0500 Stefan Monnier <monnier@IRO.UMontreal.CA> wrote: 

>> +	      /* will this be freed automatically? */
>> +	      Lisp_Object* params = (Lisp_Object*) xmalloc (10 * sizeof(Lisp_Object));

SM> No, it won't be freed automatically.  Why not use `alloca' instead?
SM> Also rather than 10, why not use something like XINT (Flength (tmp)) ?

A static array was a much simpler solution in the end.

>> +	      if (!EQ (head, Qhash_table_read_marker))

SM> This constant should be called Qhash_table.

OK, all the constants are named appropriately.

>> +		  /* allowed parameters: size test weakness
>> +		     rehash-size rehash-threshold */

SM> Why not :weakness, :rehash-size, :rehash-threshold?
SM> (maybe even :test and :size, although IIUC we may prefer just `size'
SM> and `test' for compatibility with XEmacs, tho I'm not sure how
SM> important that is for this kind of data).

To be consistent.  If this will be used by many users and libraries,
it's important to provide a printed representation that doesn't mix
symbol conventions and is easy to parse.  Similarly, I'd rather provide
something compatible with XEmacs if there's a choice.

SM> Also you need to put spaces before all your open parens.

Done, thanks.

I took your other suggestions, except I don't pass Qnil parameters to
make-hash-table to keep things clean.  I hope this version of the patch
is acceptable.

Ted


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

? hashprint.patch
Index: lread.c
===================================================================
RCS file: /sources/emacs/emacs/src/lread.c,v
retrieving revision 1.401
diff -u -r1.401 lread.c
--- lread.c	7 Sep 2008 20:41:10 -0000	1.401
+++ lread.c	3 Dec 2008 19:17:59 -0000
@@ -80,6 +80,14 @@
 extern int errno;
 #endif
 
+/* hash table read constants */
+Lisp_Object Qhash_table, Qdata;
+Lisp_Object Qtest, Qsize;
+Lisp_Object Qweakness;
+Lisp_Object Qrehash_size;
+Lisp_Object Qrehash_threshold;
+extern Lisp_Object QCtest, QCsize, QCrehash_size, QCrehash_threshold, QCweakness;
+
 Lisp_Object Qread_char, Qget_file_char, Qstandard_input, Qcurrent_load_list;
 Lisp_Object Qvariable_documentation, Vvalues, Vstandard_input, Vafter_load_alist;
 Lisp_Object Qascii_character, Qload, Qload_file_name;
@@ -2341,6 +2349,72 @@
 
     case '#':
       c = READCHAR;
+      if (c == 's')
+	{
+	  c = READCHAR;
+	  if (c == '(')
+	    {
+	      /*
+		Accept extended format for hashtables (extensible to
+		other types), e.g.
+		#s(hash-table size 2 test equal data (k1 v1 k2 v2))	     
+	      */
+	      Lisp_Object tmp = read_list (0, readcharfun);
+	      Lisp_Object head = CAR_SAFE (tmp);
+	      Lisp_Object data = Qnil;
+	      Lisp_Object val = Qnil;
+	      /* 2 * number of allowed keywords to make-hash-table */
+	      Lisp_Object params[10]; 
+	      int param_count = 0;
+	      int i;
+	      
+	      if (!EQ (head, Qhash_table))
+		error ("Invalid extended read marker at head of #s list "
+		       "(only hash-table allowed)");
+	      
+	      tmp = CDR_SAFE (tmp);
+
+	      /* this is repetitive but fast and simple */
+	      params[param_count] = QCsize;
+	      params[param_count+1] = Fplist_get (tmp, Qsize);
+	      if (!NILP (params[param_count+1])) param_count+=2;
+
+	      params[param_count] = QCtest;
+	      params[param_count+1] = Fplist_get (tmp, Qtest);
+	      if (!NILP (params[param_count+1])) param_count+=2;
+
+	      params[param_count] = QCweakness;
+	      params[param_count+1] = Fplist_get (tmp, Qweakness);
+	      if (!NILP (params[param_count+1])) param_count+=2;
+
+	      params[param_count] = QCrehash_size;
+	      params[param_count+1] = Fplist_get (tmp, Qrehash_size);
+	      if (!NILP (params[param_count+1])) param_count+=2;
+
+	      params[param_count] = QCrehash_threshold;
+	      params[param_count+1] = Fplist_get (tmp, Qrehash_threshold);
+	      if (!NILP (params[param_count+1])) param_count+=2;
+
+	      data = Fplist_get (tmp, Qdata); /* this is the hashtable data */
+
+	      /* now use params to make a new hashtable and fill it */
+	      Lisp_Object ht = Fmake_hash_table (param_count, params);
+	      Lisp_Object key = Qnil;
+	      
+	      while (CONSP (data))
+	      	{
+	      	  key = XCAR (data);
+	      	  data = XCDR (data);
+	      	  if (!CONSP (data))
+	      	    error ("Odd number of elements in hashtable data");
+	      	  val = XCAR (data);
+	      	  data = XCDR (data);
+	      	  Fputhash (key, val, ht);
+	      	}
+	      
+	      return ht;
+	    }
+	}
       if (c == '^')
 	{
 	  c = READCHAR;
@@ -4432,6 +4506,21 @@
 
   Vloads_in_progress = Qnil;
   staticpro (&Vloads_in_progress);
+
+  Qhash_table = intern ("hash-table");
+  staticpro (&Qhash_table);
+  Qdata = intern ("data");
+  staticpro (&Qdata);
+  Qtest = intern ("test");
+  staticpro (&Qtest);
+  Qsize = intern ("size");
+  staticpro (&Qsize);
+  Qweakness = intern ("weakness");
+  staticpro (&Qweakness);
+  Qrehash_size = intern ("rehash-size");
+  staticpro (&Qrehash_size);
+  Qrehash_threshold = intern ("rehash-threshold");
+  staticpro (&Qrehash_threshold);
 }
 
 /* arch-tag: a0d02733-0f96-4844-a659-9fd53c4f414d
Index: print.c
===================================================================
RCS file: /sources/emacs/emacs/src/print.c,v
retrieving revision 1.253
diff -u -r1.253 print.c
--- print.c	31 Jul 2008 05:33:53 -0000	1.253
+++ print.c	3 Dec 2008 19:17:59 -0000
@@ -1341,6 +1341,7 @@
  loop:
   if (STRINGP (obj) || CONSP (obj) || VECTORP (obj)
       || COMPILEDP (obj) || CHAR_TABLE_P (obj) || SUB_CHAR_TABLE_P (obj)
+      || HASH_TABLE_P (obj)
       || (! NILP (Vprint_gensym)
 	  && SYMBOLP (obj)
 	  && !SYMBOL_INTERNED_P (obj)))
@@ -1536,6 +1537,7 @@
   /* Detect circularities and truncate them.  */
   if (STRINGP (obj) || CONSP (obj) || VECTORP (obj)
       || COMPILEDP (obj) || CHAR_TABLE_P (obj) || SUB_CHAR_TABLE_P (obj)
+      || HASH_TABLE_P (obj)
       || (! NILP (Vprint_gensym)
 	  && SYMBOLP (obj)
 	  && !SYMBOL_INTERNED_P (obj)))
@@ -2036,6 +2038,7 @@
       else if (HASH_TABLE_P (obj))
 	{
 	  struct Lisp_Hash_Table *h = XHASH_TABLE (obj);
+#if 0
 	  strout ("#<hash-table", -1, -1, printcharfun, 0);
 	  if (SYMBOLP (h->test))
 	    {
@@ -2052,6 +2055,68 @@
 	  sprintf (buf, " 0x%lx", (unsigned long) h);
 	  strout (buf, -1, -1, printcharfun, 0);
 	  PRINTCHAR ('>');
+#endif
+	  /*
+	    implement a readable output, e.g.:
+	    #s(hash-table size 2 test equal data (k1 v1 k2 v2))
+	  */
+	  /* always print the size */
+	  sprintf (buf, "#s(hash-table size %ld", (long) XVECTOR (h->next)->size);
+	  strout (buf, -1, -1, printcharfun, 0);
+
+	  if (!NILP (h->test))
+	    {
+	      strout (" test ", -1, -1, printcharfun, 0);
+	      print_object (h->test, printcharfun, 0);
+	    }
+
+	  if (!NILP (h->weak))
+	    {
+	      strout (" weakness ", -1, -1, printcharfun, 0);
+	      print_object (h->weak, printcharfun, 0);
+	    }
+
+	  if (!NILP (h->rehash_size))
+	    {
+	      strout (" rehash-size ", -1, -1, printcharfun, 0);
+	      print_object (h->rehash_size, printcharfun, 0);
+	    }
+
+	  if (!NILP (h->rehash_threshold))
+	    {
+	      strout (" rehash-threshold ", -1, -1, printcharfun, 0);
+	      print_object (h->rehash_threshold, printcharfun, 0);
+	    }
+
+	  strout (" data ", -1, -1, printcharfun, 0);
+
+	  /* print the data here as a plist */
+	  int i;
+
+	  int real_size = HASH_TABLE_SIZE (h);
+	  int size = real_size;
+
+	  /* Don't print more elements than the specified maximum.  */
+	  if (NATNUMP (Vprint_length)
+	      && XFASTINT (Vprint_length) < size)
+	    size = XFASTINT (Vprint_length);
+	  
+	  PRINTCHAR ('(');
+	  for (i = 0; i < size; i++)
+	    if (!NILP (HASH_HASH (h, i)))
+	      {
+		if (i) PRINTCHAR (' ');
+		print_object (HASH_KEY (h, i), printcharfun, 0);
+		PRINTCHAR (' ');
+		print_object (HASH_VALUE (h, i), printcharfun, 0);
+	      }
+
+	  if (size < real_size)
+	    strout (" ...", 4, 4, printcharfun, 0);
+
+	  PRINTCHAR (')');
+	  PRINTCHAR (')');
+
 	}
       else if (BUFFERP (obj))
 	{

Re: hash-table-{to, from}-alist

[Stefan Monnier]

> I took your other suggestions, except I don't pass Qnil parameters to
> make-hash-table to keep things clean.  I hope this version of the patch
> is acceptable.

Some nitpicks, see below.
BTW, have you tried to delegate to some Elisp code?

> +	      /*
> +		Accept extended format for hashtables (extensible to
> +		other types), e.g.
> +		#s(hash-table size 2 test equal data (k1 v1 k2 v2))	     
> +	      */

We like to avoid putting comment markers on their own line.  So we'd write

	      /* Accept extended format for hashtables (extensible to
		 other types), e.g.
		 #s(hash-table size 2 test equal data (k1 v1 k2 v2))  */

> +	      /* 2 * number of allowed keywords to make-hash-table */

We like to terminate our comments like sentences: with a sot followed by
2 spaces.

> +	      if (!NILP (params[param_count+1])) param_count+=2;

We like to put the body of the `if' on a separate line.

> +	      data = Fplist_get (tmp, Qdata); /* this is the hashtable data */

Comments should be capitalized:  /* This is the hashtable data.  */

> +	      Lisp_Object ht = Fmake_hash_table (param_count, params);
> +	      Lisp_Object key = Qnil;

We recently decided it was OK to use ANSI C syntax for function headers,
but I don't think ANSI C allows such variable declarations in the middle
of a block.  So we should probably move the delcaration of those 2 vars
higher up, or open up a new block.

Other than that, it looks good.


        Stefan

Re: hash-table-{to, from}-alist

[Stephen J. Turnbull]

Stefan Monnier writes:

 > > +	      Lisp_Object ht = Fmake_hash_table (param_count, params);
 > > +	      Lisp_Object key = Qnil;
 > 
 > We recently decided it was OK to use ANSI C syntax for function headers,
 > but I don't think ANSI C allows such variable declarations in the middle
 > of a block.

ISTR that C99 does allow those declarations (which is a GoodThang[tm]
IMHO), but C89 definitely does not allow them (and portability is a
GooderThang[tm], so let's not do that).

Re: hash-table-{to, from}-alist

[Miles Bader]

"Stephen J. Turnbull" <stephen@xemacs.org> writes:
>  > > +	      Lisp_Object ht = Fmake_hash_table (param_count, params);
>  > > +	      Lisp_Object key = Qnil;
>  > 
>  > We recently decided it was OK to use ANSI C syntax for function headers,
>  > but I don't think ANSI C allows such variable declarations in the middle
>  > of a block.
>
> ISTR that C99 does allow those declarations (which is a GoodThang[tm]
> IMHO), but C89 definitely does not allow them (and portability is a
> GooderThang[tm], so let's not do that).

BTW, is it now OK to use initializers with Lisp_Object variable declarations?

As I understand it, those used to be proscribed in emacs because they
didn't work correctly in some case (IIRC, some ancient IBM compiler
generated incorrect code), but enforcement has always been spotty, and I
expect the codebase is so chock full of such initializations now that
it's very unlikely it's really still a problem...

-Miles

-- 
Suburbia: where they tear out the trees and then name streets after them.

Re: hash-table-{to, from}-alist

[Andreas Schwab]

Miles Bader <miles@gnu.org> writes:

> BTW, is it now OK to use initializers with Lisp_Object variable declarations?

This was never a problem, Lisp_Object is just an integer (if you make it
a union you need to compile with a good compiler anyway).

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: hash-table-{to, from}-alist

[Stefan Monnier]

> BTW, is it now OK to use initializers with Lisp_Object variable declarations?

Yes,


        Stefan

Re: hash-table-{to, from}-alist

[Ted Zlatanov]

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

On Wed, 03 Dec 2008 21:05:54 -0500 Stefan Monnier <monnier@iro.umontreal.ca> wrote: 

>> I took your other suggestions, except I don't pass Qnil parameters to
>> make-hash-table to keep things clean.  I hope this version of the patch
>> is acceptable.

SM> Some nitpicks, see below.
SM> BTW, have you tried to delegate to some Elisp code?

Why at this point?  The C code is written, is there a problem with it?

SM> We like to avoid putting comment markers on their own line.  We like
SM> to terminate our comments like sentences: with a [d]ot followed by 2
SM> spaces.  We like to put the body of the `if' on a separate line.
SM> Comments should be capitalized.

All these are fixed, thanks for pointing them out.

>> +	      Lisp_Object ht = Fmake_hash_table (param_count, params);
>> +	      Lisp_Object key = Qnil;

SM> We recently decided it was OK to use ANSI C syntax for function headers,
SM> but I don't think ANSI C allows such variable declarations in the middle
SM> of a block.  So we should probably move the delcaration of those 2 vars
SM> higher up, or open up a new block.

Fixed.  Revised patch attached.

Thanks
Ted


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

? hashprint.patch
Index: lread.c
===================================================================
RCS file: /sources/emacs/emacs/src/lread.c,v
retrieving revision 1.401
diff -u -r1.401 lread.c
--- lread.c	7 Sep 2008 20:41:10 -0000	1.401
+++ lread.c	4 Dec 2008 15:02:39 -0000
@@ -80,6 +80,14 @@
 extern int errno;
 #endif
 
+/* hash table read constants */
+Lisp_Object Qhash_table, Qdata;
+Lisp_Object Qtest, Qsize;
+Lisp_Object Qweakness;
+Lisp_Object Qrehash_size;
+Lisp_Object Qrehash_threshold;
+extern Lisp_Object QCtest, QCsize, QCrehash_size, QCrehash_threshold, QCweakness;
+
 Lisp_Object Qread_char, Qget_file_char, Qstandard_input, Qcurrent_load_list;
 Lisp_Object Qvariable_documentation, Vvalues, Vstandard_input, Vafter_load_alist;
 Lisp_Object Qascii_character, Qload, Qload_file_name;
@@ -2341,6 +2349,78 @@
 
     case '#':
       c = READCHAR;
+      if (c == 's')
+	{
+	  c = READCHAR;
+	  if (c == '(')
+	    {
+	      /* Accept extended format for hashtables (extensible to
+		 other types), e.g.
+		 #s(hash-table size 2 test equal data (k1 v1 k2 v2)) */
+	      Lisp_Object tmp = read_list (0, readcharfun);
+	      Lisp_Object head = CAR_SAFE (tmp);
+	      Lisp_Object data = Qnil;
+	      Lisp_Object val = Qnil;
+	      /* The size is 2 * number of allowed keywords to
+		 make-hash-table. */
+	      Lisp_Object params[10]; 
+	      Lisp_Object ht;
+	      Lisp_Object key = Qnil;
+	      int param_count = 0;
+	      int i;
+	      
+	      if (!EQ (head, Qhash_table))
+		error ("Invalid extended read marker at head of #s list "
+		       "(only hash-table allowed)");
+	      
+	      tmp = CDR_SAFE (tmp);
+
+	      /* This is repetitive but fast and simple. */
+	      params[param_count] = QCsize;
+	      params[param_count+1] = Fplist_get (tmp, Qsize);
+	      if (!NILP (params[param_count+1]))
+		param_count+=2;
+
+	      params[param_count] = QCtest;
+	      params[param_count+1] = Fplist_get (tmp, Qtest);
+	      if (!NILP (params[param_count+1]))
+		param_count+=2;
+
+	      params[param_count] = QCweakness;
+	      params[param_count+1] = Fplist_get (tmp, Qweakness);
+	      if (!NILP (params[param_count+1]))
+		param_count+=2;
+
+	      params[param_count] = QCrehash_size;
+	      params[param_count+1] = Fplist_get (tmp, Qrehash_size);
+	      if (!NILP (params[param_count+1]))
+		param_count+=2;
+
+	      params[param_count] = QCrehash_threshold;
+	      params[param_count+1] = Fplist_get (tmp, Qrehash_threshold);
+	      if (!NILP (params[param_count+1]))
+		param_count+=2;
+
+	      /* This is the hashtable data. */
+	      data = Fplist_get (tmp, Qdata);
+
+	      /* Now use params to make a new hashtable and fill it. */
+	      ht = Fmake_hash_table (param_count, params);
+	      
+	      while (CONSP (data))
+	      	{
+	      	  key = XCAR (data);
+	      	  data = XCDR (data);
+	      	  if (!CONSP (data))
+	      	    error ("Odd number of elements in hashtable data");
+	      	  val = XCAR (data);
+	      	  data = XCDR (data);
+	      	  Fputhash (key, val, ht);
+	      	}
+	      
+	      return ht;
+	    }
+	}
       if (c == '^')
 	{
 	  c = READCHAR;
@@ -4432,6 +4512,21 @@
 
   Vloads_in_progress = Qnil;
   staticpro (&Vloads_in_progress);
+
+  Qhash_table = intern ("hash-table");
+  staticpro (&Qhash_table);
+  Qdata = intern ("data");
+  staticpro (&Qdata);
+  Qtest = intern ("test");
+  staticpro (&Qtest);
+  Qsize = intern ("size");
+  staticpro (&Qsize);
+  Qweakness = intern ("weakness");
+  staticpro (&Qweakness);
+  Qrehash_size = intern ("rehash-size");
+  staticpro (&Qrehash_size);
+  Qrehash_threshold = intern ("rehash-threshold");
+  staticpro (&Qrehash_threshold);
 }
 
 /* arch-tag: a0d02733-0f96-4844-a659-9fd53c4f414d
Index: print.c
===================================================================
RCS file: /sources/emacs/emacs/src/print.c,v
retrieving revision 1.253
diff -u -r1.253 print.c
--- print.c	31 Jul 2008 05:33:53 -0000	1.253
+++ print.c	4 Dec 2008 15:02:39 -0000
@@ -1341,6 +1341,7 @@
  loop:
   if (STRINGP (obj) || CONSP (obj) || VECTORP (obj)
       || COMPILEDP (obj) || CHAR_TABLE_P (obj) || SUB_CHAR_TABLE_P (obj)
+      || HASH_TABLE_P (obj)
       || (! NILP (Vprint_gensym)
 	  && SYMBOLP (obj)
 	  && !SYMBOL_INTERNED_P (obj)))
@@ -1536,6 +1537,7 @@
   /* Detect circularities and truncate them.  */
   if (STRINGP (obj) || CONSP (obj) || VECTORP (obj)
       || COMPILEDP (obj) || CHAR_TABLE_P (obj) || SUB_CHAR_TABLE_P (obj)
+      || HASH_TABLE_P (obj)
       || (! NILP (Vprint_gensym)
 	  && SYMBOLP (obj)
 	  && !SYMBOL_INTERNED_P (obj)))
@@ -2036,6 +2038,7 @@
       else if (HASH_TABLE_P (obj))
 	{
 	  struct Lisp_Hash_Table *h = XHASH_TABLE (obj);
+#if 0
 	  strout ("#<hash-table", -1, -1, printcharfun, 0);
 	  if (SYMBOLP (h->test))
 	    {
@@ -2052,6 +2055,67 @@
 	  sprintf (buf, " 0x%lx", (unsigned long) h);
 	  strout (buf, -1, -1, printcharfun, 0);
 	  PRINTCHAR ('>');
+#endif
+	  /* Implement a readable output, e.g.:
+	    #s(hash-table size 2 test equal data (k1 v1 k2 v2)) */
+	  /* Always print the size. */
+	  sprintf (buf, "#s(hash-table size %ld",
+		   (long) XVECTOR (h->next)->size);
+	  strout (buf, -1, -1, printcharfun, 0);
+
+	  if (!NILP (h->test))
+	    {
+	      strout (" test ", -1, -1, printcharfun, 0);
+	      print_object (h->test, printcharfun, 0);
+	    }
+
+	  if (!NILP (h->weak))
+	    {
+	      strout (" weakness ", -1, -1, printcharfun, 0);
+	      print_object (h->weak, printcharfun, 0);
+	    }
+
+	  if (!NILP (h->rehash_size))
+	    {
+	      strout (" rehash-size ", -1, -1, printcharfun, 0);
+	      print_object (h->rehash_size, printcharfun, 0);
+	    }
+
+	  if (!NILP (h->rehash_threshold))
+	    {
+	      strout (" rehash-threshold ", -1, -1, printcharfun, 0);
+	      print_object (h->rehash_threshold, printcharfun, 0);
+	    }
+
+	  strout (" data ", -1, -1, printcharfun, 0);
+
+	  /* Print the data here as a plist. */
+	  int i;
+
+	  int real_size = HASH_TABLE_SIZE (h);
+	  int size = real_size;
+
+	  /* Don't print more elements than the specified maximum.  */
+	  if (NATNUMP (Vprint_length)
+	      && XFASTINT (Vprint_length) < size)
+	    size = XFASTINT (Vprint_length);
+	  
+	  PRINTCHAR ('(');
+	  for (i = 0; i < size; i++)
+	    if (!NILP (HASH_HASH (h, i)))
+	      {
+		if (i) PRINTCHAR (' ');
+		print_object (HASH_KEY (h, i), printcharfun, 0);
+		PRINTCHAR (' ');
+		print_object (HASH_VALUE (h, i), printcharfun, 0);
+	      }
+
+	  if (size < real_size)
+	    strout (" ...", 4, 4, printcharfun, 0);
+
+	  PRINTCHAR (')');
+	  PRINTCHAR (')');
+
 	}
       else if (BUFFERP (obj))
 	{

Re: hash-table-{to, from}-alist

[Stefan Monnier]

SM> BTW, have you tried to delegate to some Elisp code?
> Why at this point?  The C code is written, is there a problem with it?

No problem with it, just curious.

> Fixed.  Revised patch attached.

Thank you.  Looks good.


        Stefan

Re: hash-table-{to, from}-alist

[Ted Zlatanov]

On Thu, 04 Dec 2008 14:34:23 -0500 Stefan Monnier <monnier@IRO.UMontreal.CA> wrote: 

SM> BTW, have you tried to delegate to some Elisp code?
>> Why at this point?  The C code is written, is there a problem with it?

SM> No problem with it, just curious.

I wanted to learn a bit about the Emacs internals, now and for the
future.  Also the C code is probably going to be a little faster.

(let's not forget my LOC stats shot up too :)

Ted

Re: hash-table-{to, from}-alist

[Ted Zlatanov]

On Thu, 04 Dec 2008 09:02:56 -0600 Ted Zlatanov <tzz@lifelogs.com> wrote: 

TZ> Fixed.  Revised patch attached.

Hi,

I would like to commit and start using the hashtable print/read support
I proposed last December.  

The Gnus registry is my first target since I control that code.  My
question is, what's the right way to tell applications that hashtable
print/read support is available?  Is it with featurep and provide?  If
so, what name should I use and where do I put the provide statement
(looks like lisp/bindings.el from a quick look)?

I'll add that change to the patch, verify it works against the CVS
trunk, and resubmit for a final review.

Thanks
Ted

Re: hash-table-{to, from}-alist

[Chong Yidong]

Ted Zlatanov <tzz@lifelogs.com> writes:

> The Gnus registry is my first target since I control that code.  My
> question is, what's the right way to tell applications that hashtable
> print/read support is available?  Is it with featurep and provide?  If
> so, what name should I use and where do I put the provide statement
> (looks like lisp/bindings.el from a quick look)?

Put the `provide' call in whichever file the code for the hastable
print/read support resides (which I guess will be one of the C files).

Re: hash-table-{to, from}-alist

[Ted Zlatanov]

On Thu, 30 Jul 2009 15:08:01 -0400 Chong Yidong <cyd@stupidchicken.com> wrote: 

CY> Ted Zlatanov <tzz@lifelogs.com> writes:
>> The Gnus registry is my first target since I control that code.  My
>> question is, what's the right way to tell applications that hashtable
>> print/read support is available?  Is it with featurep and provide?  If
>> so, what name should I use and where do I put the provide statement
>> (looks like lisp/bindings.el from a quick look)?

CY> Put the `provide' call in whichever file the code for the hastable
CY> print/read support resides (which I guess will be one of the C files).

OK.  Can I use two names: "hashtable-read" and "hashtable-print" for the
feature?  I can't think of a single name that doesn't sound clumsy
(suggestions welcome, of course).

Ted

Re: hash-table-{to, from}-alist

[Chong Yidong]

Ted Zlatanov <tzz@lifelogs.com> writes:

> OK.  Can I use two names: "hashtable-read" and "hashtable-print" for the
> feature?  I can't think of a single name that doesn't sound clumsy
> (suggestions welcome, of course).

Just hashtable-read, I think.  After all, hashtables already have a
print representation (all Lisp objects do)---the present difficulty is
that this print representation isn't readable.

Re: hash-table-{to, from}-alist

[Stefan Monnier]

>> OK.  Can I use two names: "hashtable-read" and "hashtable-print" for the
>> feature?  I can't think of a single name that doesn't sound clumsy
>> (suggestions welcome, of course).

> Just hashtable-read, I think.  After all, hashtables already have a
> print representation (all Lisp objects do)---the present difficulty is
> that this print representation isn't readable.

Or hashtable-print-readable.


        Stefan

Re: hash-table-{to, from}-alist

[Stefan Monnier]

>>> OK.  Can I use two names: "hashtable-read" and "hashtable-print" for the
>>> feature?  I can't think of a single name that doesn't sound clumsy
>>> (suggestions welcome, of course).

>> Just hashtable-read, I think.  After all, hashtables already have a
>> print representation (all Lisp objects do)---the present difficulty is
>> that this print representation isn't readable.

> Or hashtable-print-readable.

Of, course there's always another (more reliable) way:

   (hash-table-p (read-from-string (prin1-to-string (make-hash-table))))


-- Stefan

Re: hash-table-{to, from}-alist

[Ted Zlatanov]

On Thu, 30 Jul 2009 21:30:48 -0400 Stefan Monnier <monnier@iro.umontreal.ca> wrote: 

>>>> OK.  Can I use two names: "hashtable-read" and "hashtable-print" for the
>>>> feature?  I can't think of a single name that doesn't sound clumsy
>>>> (suggestions welcome, of course).

>>> Just hashtable-read, I think.  After all, hashtables already have a
>>> print representation (all Lisp objects do)---the present difficulty is
>>> that this print representation isn't readable.

>> Or hashtable-print-readable.

SM> Of, course there's always another (more reliable) way:

SM>    (hash-table-p (read-from-string (prin1-to-string (make-hash-table))))

That was my first thought, but it seemed like a really hacky approach
when featurep exists.

Ted

Re: hash-table-{to, from}-alist

[Stefan Monnier]

SM> Of, course there's always another (more reliable) way:
SM> (hash-table-p (read-from-string (prin1-to-string (make-hash-table))))
> That was my first thought, but it seemed like a really hacky approach
> when featurep exists.

It's definitely not hacky.  It's more reliable (tho it probably requires
a bit of fiddling such as wrapping it in ignore-errors) and tests more
directly the presence of the feature you need.

The advantage of the featurep call is that it's likely to be
more efficient (which only matters if the test is made within a loop,
of course).


        Stefan

Re: hash-table-{to, from}-alist

[Ted Zlatanov]

On Fri, 31 Jul 2009 15:03:48 -0400 Stefan Monnier <monnier@iro.umontreal.ca> wrote: 

SM> Of, course there's always another (more reliable) way:
SM> (hash-table-p (read-from-string (prin1-to-string (make-hash-table))))
...
SM> It's definitely not hacky.  It's more reliable (tho it probably requires
SM> a bit of fiddling such as wrapping it in ignore-errors) and tests more
SM> directly the presence of the feature you need.

SM> The advantage of the featurep call is that it's likely to be
SM> more efficient (which only matters if the test is made within a loop,
SM> of course).

If it's all right, can I stick to provide/featurep?  It simplifies the
client code, plus there are some performance improvements as you noted.

Ted

Re: hash-table-{to, from}-alist

[Stephen J. Turnbull]

Ted Zlatanov writes:
 > On Fri, 31 Jul 2009 15:03:48 -0400 Stefan Monnier <monnier@iro.umontreal.ca> wrote: 
 > 
 > SM> Of, course there's always another (more reliable) way:
 > SM> (hash-table-p (read-from-string (prin1-to-string (make-hash-table))))

 > If it's all right, can I stick to provide/featurep?  It simplifies the
 > client code, plus there are some performance improvements as you noted.

Durn.  And here I was hoping this would eventually turn into a
full-blown replacement for Autoconf.

Re: hash-table-{to, from}-alist

[Chong Yidong]

Ted Zlatanov <tzz@lifelogs.com> writes:

> If it's all right, can I stick to provide/featurep?  It simplifies the
> client code, plus there are some performance improvements as you noted.

Yes, please use `provide'.

Re: hash-table-{to, from}-alist

[Ted Zlatanov]

On Sat, 01 Aug 2009 07:35:44 -0400 Chong Yidong <cyd@stupidchicken.com> wrote: 

CY> Ted Zlatanov <tzz@lifelogs.com> writes:
>> If it's all right, can I stick to provide/featurep?  It simplifies the
>> client code, plus there are some performance improvements as you noted.

CY> Yes, please use `provide'.

In that case the patch is fine as is.  Can you please commit it if it is
all right, or let me know and I'll commit it?

Thanks
Ted

Re: hash-table-{to, from}-alist

[Ted Zlatanov]

On Mon, 03 Aug 2009 09:53:38 -0500 Ted Zlatanov <tzz@lifelogs.com> wrote: 

TZ> On Sat, 01 Aug 2009 07:35:44 -0400 Chong Yidong <cyd@stupidchicken.com> wrote: 
CY> Ted Zlatanov <tzz@lifelogs.com> writes:
>>> If it's all right, can I stick to provide/featurep?  It simplifies the
>>> client code, plus there are some performance improvements as you noted.

CY> Yes, please use `provide'.

TZ> In that case the patch is fine as is.  Can you please commit it if it is
TZ> all right, or let me know and I'll commit it?

I've committed the patch with the usual ChangeLog information.

Ted

Re: hash-table-{to, from}-alist

[Chong Yidong]

Ted Zlatanov <tzz@lifelogs.com> writes:

> I've committed the patch with the usual ChangeLog information.

Please add a NEWS entry as well.

Re: hash-table-{to, from}-alist

[Ted Zlatanov]

On Wed, 05 Aug 2009 09:48:43 -0400 Chong Yidong <cyd@stupidchicken.com> wrote: 

CY> Ted Zlatanov <tzz@lifelogs.com> writes:
>> I've committed the patch with the usual ChangeLog information.

CY> Please add a NEWS entry as well.

Would this be sufficient?

Ted

Index: NEWS
===================================================================
RCS file: /sources/emacs/emacs/etc/NEWS,v
retrieving revision 1.2042
diff -c -r1.2042 NEWS
*** NEWS	16 Jul 2009 21:04:18 -0000	1.2042
--- NEWS	5 Aug 2009 16:47:25 -0000
***************
*** 24,29 ****
--- 24,37 ----
  \f
  * Changes in Emacs 23.2
  
+ ** Hashtables can be printed and read back directly.  The feature
+ 'hashtable-print-readable identifies this new functionality.
+ Hashtables are printed as alists, but with some hashtable-specific
+ properties and the actual data as an alist in a data property.  Be
+ aware the test property could have a bogus value when it is read back,
+ if the symbol it refers to doesn't exist.
+ 
+ +++
  ** The default value of `trash-directory' has changed to nil, which
  means that `move-file-to-trash' trashes files according to
  freedesktop.org specifications, the same method used by the Gnome,

Re: hash-table-{to, from}-alist

[Stefan Monnier]

> + ** Hashtables can be printed and read back directly.  The feature
> + 'hashtable-print-readable identifies this new functionality.

Good.

> + Hashtables are printed as alists, but with some hashtable-specific
> + properties and the actual data as an alist in a data property.  Be
> + aware the test property could have a bogus value when it is read back,
> + if the symbol it refers to doesn't exist.

Superfluous.  Don't forget: the NEWS is already too long usually.
It's just there to advertize (and warn of) new features.  The details
are in the manul and the docstrings.


        Stefan

Re: hash-table-{to, from}-alist

[Ted Zlatanov]

On Thu, 06 Aug 2009 12:30:10 -0400 Stefan Monnier <monnier@iro.umontreal.ca> wrote: 

>> + ** Hashtables can be printed and read back directly.  The feature
>> + 'hashtable-print-readable identifies this new functionality.

SM> Good.

>> + Hashtables are printed as alists, but with some hashtable-specific
>> + properties and the actual data as an alist in a data property.  Be
>> + aware the test property could have a bogus value when it is read back,
>> + if the symbol it refers to doesn't exist.

SM> Superfluous.  Don't forget: the NEWS is already too long usually.
SM> It's just there to advertize (and warn of) new features.  The details
SM> are in the manul and the docstrings.

OK.  Can you or someone else commit the short version plus the featurep
fix as Andreas suggested earlier?  I won't be able to commit until
tomorrow, unfortunately, and I hate to see the build broken.  Also there
are two new Emacs bugs related to this that can then be closed.

Thanks
Ted

Re: hash-table-{to, from}-alist

[Chong Yidong]

Ted Zlatanov <tzz@lifelogs.com> writes:

> SM> Superfluous.  Don't forget: the NEWS is already too long usually.
> SM> It's just there to advertize (and warn of) new features.  The details
> SM> are in the manul and the docstrings.
>
> OK.  Can you or someone else commit the short version plus the featurep
> fix as Andreas suggested earlier?

I've checked in the NEWS entry.

Re: hash-table-{to, from}-alist

[Dan Nicolaescu]

Ted Zlatanov <tzz@lifelogs.com> writes:

  > On Mon, 03 Aug 2009 09:53:38 -0500 Ted Zlatanov <tzz@lifelogs.com> wrote: 
  > 
  > TZ> On Sat, 01 Aug 2009 07:35:44 -0400 Chong Yidong <cyd@stupidchicken.com> wrote: 
  > CY> Ted Zlatanov <tzz@lifelogs.com> writes:
  > >>> If it's all right, can I stick to provide/featurep?  It simplifies the
  > >>> client code, plus there are some performance improvements as you noted.
  > 
  > CY> Yes, please use `provide'.
  > 
  > TZ> In that case the patch is fine as is.  Can you please commit it if it is
  > TZ> all right, or let me know and I'll commit it?
  > 
  > I've committed the patch with the usual ChangeLog information.

This patch breaks the build, temacs crashes on startup.

You cannot call Fprovide in print.c, it's initialized later than that.

Re: hash-table-{to, from}-alist

[Florian Beck]

Ted Zlatanov <tzz@lifelogs.com> writes:

> I've committed the patch with the usual ChangeLog information.

I was in eager anticipation for this patch, but I can't get it to work
with strings as keys:

(let ((hashtable (make-hash-table :test 'equal)))
  (puthash "key" "value" hashtable)
  (gethash "key" (car (read-from-string (prin1-to-string hashtable)))))

  => nil

Why?  

-- 
Florian Beck

Re: hash-table-{to, from}-alist

[Stefan Monnier]

> If it's all right, can I stick to provide/featurep?

Of course.  I was just pointing out that the other option is not a hack.


        Stefan

Re: hash-table-{to, from}-alist

[Ted Zlatanov]

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

On Thu, 30 Jul 2009 17:04:22 -0400 Stefan Monnier <monnier@IRO.UMontreal.CA> wrote: 

>>> OK.  Can I use two names: "hashtable-read" and "hashtable-print" for the
>>> feature?  I can't think of a single name that doesn't sound clumsy
>>> (suggestions welcome, of course).

>> Just hashtable-read, I think.  After all, hashtables already have a
>> print representation (all Lisp objects do)---the present difficulty is
>> that this print representation isn't readable.

SM> Or hashtable-print-readable.

Attached is the patch for review.  I've been using it for a while with
no issues, FWIW.

Thanks
Ted


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

? hashprint.patch
Index: lread.c
===================================================================
RCS file: /sources/emacs/emacs/src/lread.c,v
retrieving revision 1.406
diff -c -r1.406 lread.c
*** lread.c	29 Apr 2009 03:03:00 -0000	1.406
--- lread.c	31 Jul 2009 18:09:25 -0000
***************
*** 80,85 ****
--- 80,93 ----
  extern int errno;
  #endif
  
+ /* hash table read constants */
+ Lisp_Object Qhash_table, Qdata;
+ Lisp_Object Qtest, Qsize;
+ Lisp_Object Qweakness;
+ Lisp_Object Qrehash_size;
+ Lisp_Object Qrehash_threshold;
+ extern Lisp_Object QCtest, QCsize, QCrehash_size, QCrehash_threshold, QCweakness;
+ 
  Lisp_Object Qread_char, Qget_file_char, Qstandard_input, Qcurrent_load_list;
  Lisp_Object Qvariable_documentation, Vvalues, Vstandard_input, Vafter_load_alist;
  Lisp_Object Qascii_character, Qload, Qload_file_name;
***************
*** 2341,2346 ****
--- 2349,2426 ----
  
      case '#':
        c = READCHAR;
+       if (c == 's')
+ 	{
+ 	  c = READCHAR;
+ 	  if (c == '(')
+ 	    {
+ 	      /* Accept extended format for hashtables (extensible to
+ 		 other types), e.g.
+ 		 #s(hash-table size 2 test equal data (k1 v1 k2 v2)) */
+ 	      Lisp_Object tmp = read_list (0, readcharfun);
+ 	      Lisp_Object head = CAR_SAFE (tmp);
+ 	      Lisp_Object data = Qnil;
+ 	      Lisp_Object val = Qnil;
+ 	      /* The size is 2 * number of allowed keywords to
+ 		 make-hash-table. */
+ 	      Lisp_Object params[10]; 
+ 	      Lisp_Object ht;
+ 	      Lisp_Object key = Qnil;
+ 	      int param_count = 0;
+ 	      int i;
+ 	      
+ 	      if (!EQ (head, Qhash_table))
+ 		error ("Invalid extended read marker at head of #s list "
+ 		       "(only hash-table allowed)");
+ 	      
+ 	      tmp = CDR_SAFE (tmp);
+ 
+ 	      /* This is repetitive but fast and simple. */
+ 	      params[param_count] = QCsize;
+ 	      params[param_count+1] = Fplist_get (tmp, Qsize);
+ 	      if (!NILP (params[param_count+1]))
+ 		param_count+=2;
+ 
+ 	      params[param_count] = QCtest;
+ 	      params[param_count+1] = Fplist_get (tmp, Qtest);
+ 	      if (!NILP (params[param_count+1]))
+ 		param_count+=2;
+ 
+ 	      params[param_count] = QCweakness;
+ 	      params[param_count+1] = Fplist_get (tmp, Qweakness);
+ 	      if (!NILP (params[param_count+1]))
+ 		param_count+=2;
+ 
+ 	      params[param_count] = QCrehash_size;
+ 	      params[param_count+1] = Fplist_get (tmp, Qrehash_size);
+ 	      if (!NILP (params[param_count+1]))
+ 		param_count+=2;
+ 
+ 	      params[param_count] = QCrehash_threshold;
+ 	      params[param_count+1] = Fplist_get (tmp, Qrehash_threshold);
+ 	      if (!NILP (params[param_count+1]))
+ 		param_count+=2;
+ 
+ 	      /* This is the hashtable data. */
+ 	      data = Fplist_get (tmp, Qdata);
+ 
+ 	      /* Now use params to make a new hashtable and fill it. */
+ 	      ht = Fmake_hash_table (param_count, params);
+ 	      
+ 	      while (CONSP (data))
+ 	      	{
+ 	      	  key = XCAR (data);
+ 	      	  data = XCDR (data);
+ 	      	  if (!CONSP (data))
+ 	      	    error ("Odd number of elements in hashtable data");
+ 	      	  val = XCAR (data);
+ 	      	  data = XCDR (data);
+ 	      	  Fputhash (key, val, ht);
+ 	      	}
+ 	      
+ 	      return ht;
+ 	    }
+ 	}
        if (c == '^')
  	{
  	  c = READCHAR;
***************
*** 4436,4441 ****
--- 4516,4536 ----
  
    Vloads_in_progress = Qnil;
    staticpro (&Vloads_in_progress);
+ 
+   Qhash_table = intern ("hash-table");
+   staticpro (&Qhash_table);
+   Qdata = intern ("data");
+   staticpro (&Qdata);
+   Qtest = intern ("test");
+   staticpro (&Qtest);
+   Qsize = intern ("size");
+   staticpro (&Qsize);
+   Qweakness = intern ("weakness");
+   staticpro (&Qweakness);
+   Qrehash_size = intern ("rehash-size");
+   staticpro (&Qrehash_size);
+   Qrehash_threshold = intern ("rehash-threshold");
+   staticpro (&Qrehash_threshold);
  }
  
  /* arch-tag: a0d02733-0f96-4844-a659-9fd53c4f414d
Index: print.c
===================================================================
RCS file: /sources/emacs/emacs/src/print.c,v
retrieving revision 1.256
diff -c -r1.256 print.c
*** print.c	3 Apr 2009 06:23:49 -0000	1.256
--- print.c	31 Jul 2009 18:09:25 -0000
***************
*** 1341,1346 ****
--- 1341,1347 ----
   loop:
    if (STRINGP (obj) || CONSP (obj) || VECTORP (obj)
        || COMPILEDP (obj) || CHAR_TABLE_P (obj) || SUB_CHAR_TABLE_P (obj)
+       || HASH_TABLE_P (obj)
        || (! NILP (Vprint_gensym)
  	  && SYMBOLP (obj)
  	  && !SYMBOL_INTERNED_P (obj)))
***************
*** 1536,1541 ****
--- 1537,1543 ----
    /* Detect circularities and truncate them.  */
    if (STRINGP (obj) || CONSP (obj) || VECTORP (obj)
        || COMPILEDP (obj) || CHAR_TABLE_P (obj) || SUB_CHAR_TABLE_P (obj)
+       || HASH_TABLE_P (obj)
        || (! NILP (Vprint_gensym)
  	  && SYMBOLP (obj)
  	  && !SYMBOL_INTERNED_P (obj)))
***************
*** 2031,2036 ****
--- 2033,2039 ----
        else if (HASH_TABLE_P (obj))
  	{
  	  struct Lisp_Hash_Table *h = XHASH_TABLE (obj);
+ #if 0
  	  strout ("#<hash-table", -1, -1, printcharfun, 0);
  	  if (SYMBOLP (h->test))
  	    {
***************
*** 2047,2052 ****
--- 2050,2116 ----
  	  sprintf (buf, " 0x%lx", (unsigned long) h);
  	  strout (buf, -1, -1, printcharfun, 0);
  	  PRINTCHAR ('>');
+ #endif
+ 	  /* Implement a readable output, e.g.:
+ 	    #s(hash-table size 2 test equal data (k1 v1 k2 v2)) */
+ 	  /* Always print the size. */
+ 	  sprintf (buf, "#s(hash-table size %ld",
+ 		   (long) XVECTOR (h->next)->size);
+ 	  strout (buf, -1, -1, printcharfun, 0);
+ 
+ 	  if (!NILP (h->test))
+ 	    {
+ 	      strout (" test ", -1, -1, printcharfun, 0);
+ 	      print_object (h->test, printcharfun, 0);
+ 	    }
+ 
+ 	  if (!NILP (h->weak))
+ 	    {
+ 	      strout (" weakness ", -1, -1, printcharfun, 0);
+ 	      print_object (h->weak, printcharfun, 0);
+ 	    }
+ 
+ 	  if (!NILP (h->rehash_size))
+ 	    {
+ 	      strout (" rehash-size ", -1, -1, printcharfun, 0);
+ 	      print_object (h->rehash_size, printcharfun, 0);
+ 	    }
+ 
+ 	  if (!NILP (h->rehash_threshold))
+ 	    {
+ 	      strout (" rehash-threshold ", -1, -1, printcharfun, 0);
+ 	      print_object (h->rehash_threshold, printcharfun, 0);
+ 	    }
+ 
+ 	  strout (" data ", -1, -1, printcharfun, 0);
+ 
+ 	  /* Print the data here as a plist. */
+ 	  int i;
+ 
+ 	  int real_size = HASH_TABLE_SIZE (h);
+ 	  int size = real_size;
+ 
+ 	  /* Don't print more elements than the specified maximum.  */
+ 	  if (NATNUMP (Vprint_length)
+ 	      && XFASTINT (Vprint_length) < size)
+ 	    size = XFASTINT (Vprint_length);
+ 	  
+ 	  PRINTCHAR ('(');
+ 	  for (i = 0; i < size; i++)
+ 	    if (!NILP (HASH_HASH (h, i)))
+ 	      {
+ 		if (i) PRINTCHAR (' ');
+ 		print_object (HASH_KEY (h, i), printcharfun, 0);
+ 		PRINTCHAR (' ');
+ 		print_object (HASH_VALUE (h, i), printcharfun, 0);
+ 	      }
+ 
+ 	  if (size < real_size)
+ 	    strout (" ...", 4, 4, printcharfun, 0);
+ 
+ 	  PRINTCHAR (')');
+ 	  PRINTCHAR (')');
+ 
  	}
        else if (BUFFERP (obj))
  	{
***************
*** 2354,2359 ****
--- 2418,2425 ----
    Qfloat_output_format = intern ("float-output-format");
    staticpro (&Qfloat_output_format);
  
+   Fprovide (intern ("hashtable-print-readable"), Qnil);
+   
    DEFVAR_LISP ("print-length", &Vprint_length,
  	       doc: /* Maximum length of list to print before abbreviating.
  A value of nil means no limit.  See also `eval-expression-print-length'.  */);

Re: hash-table-{to, from}-alist

[Stephen J. Turnbull]

Ted Zlatanov writes:
 > On Wed, 26 Nov 2008 13:10:38 -0500 Stefan Monnier <monnier@iro.umontreal.ca> wrote: 

 > >> IMHO hashtables should always be printed completely and should be
 > >> treated like lists and vectors.

XEmacs abbreviates all "large" atomic objects in Lisp backtraces,
AFAIK.  This includes anything with a plist.  But you can have your
preference by setting `print-readably' globally.

 > >> If, however, compatibility with XEmacs is desired or people feel
 > >> strongly for print-readably, I will implement with
 > >> print-readably.
 > 
 > SM> char tables are always printed in full and we haven't had any complaints
 > SM> about that, so you're probably right: let's just always print them
 > SM> in full.  If it turns out to be problematic in some cases, we can later
 > SM> on add some extra variable to control the level of detail.
 > 
 > Great.  Maybe XEmacs should add print-unreadably ;)

We already have that. (^^;

Re: hash-table-{to, from}-alist

[Ted Zlatanov]

On Thu, 27 Nov 2008 09:24:34 +0900 "Stephen J. Turnbull" <stephen@xemacs.org> wrote: 

SJT> Ted Zlatanov writes:
>> On Wed, 26 Nov 2008 13:10:38 -0500 Stefan Monnier <monnier@iro.umontreal.ca> wrote: 

>> >> IMHO hashtables should always be printed completely and should be
>> >> treated like lists and vectors.

SJT> XEmacs abbreviates all "large" atomic objects in Lisp backtraces,
SJT> AFAIK.  This includes anything with a plist.  But you can have your
SJT> preference by setting `print-readably' globally.

>> >> If, however, compatibility with XEmacs is desired or people feel
>> >> strongly for print-readably, I will implement with
>> >> print-readably.
>> 
SM> char tables are always printed in full and we haven't had any complaints
SM> about that, so you're probably right: let's just always print them
SM> in full.  If it turns out to be problematic in some cases, we can later
SM> on add some extra variable to control the level of detail.
>> 
>> Great.  Maybe XEmacs should add print-unreadably ;)

SJT> We already have that. (^^;

Can you and Stefan figure out whether to have print-readably or not?  I
was against it, and Stefan supported that.  But let's see the pro
argument and discuss this.  I'd like to make your life easier in
addition to providing features for GNU Emacs.  Will the current state of
things cause XEmacs breakage anywhere?

One idea I had is to check if print-readably is bound; if not then we
act as if it was bound and t.  That will DTRT in GNU Emacs and in
XEmacs, I hope.

Thanks
Ted

Re: hash-table-{to, from}-alist

[Stephen J. Turnbull]

Ted Zlatanov writes:

 > Can you and Stefan figure out whether to have print-readably or
 > not?  I was against it, and Stefan supported that.  But let's see
 > the pro argument and discuss this.

The pro argument is that unless you're debugging the hash table, it's
a PITA to have a hash table that gets recursively passed to each
function down the line taking up 20 screens for each frame in a
backtrace.  "Not debugging the hash table" is the overwhelmingly
common occurance for me; usually I'm more interested in where in the
stack the value I'm going to be puthash'ing turned from gold into
garbage.  OTOH, about half the time when I've got a chartable in the
stack, the chartable is buggy (in my experience almost all chartables
are write-once).

 > Will the current state of things cause XEmacs breakage anywhere?

No.  This is a user interface thing.  If I'm right, XEmacs will get a
mild popularity boost if you always print the hash table.  I'm not
going to complain about that! ;-)

 > One idea I had is to check if print-readably is bound; if not then
 > we act as if it was bound and t.  That will DTRT in GNU Emacs and
 > in XEmacs, I hope.

Hm?  XEmacs already has the feature in C; your code will either get
ignored in XEmacs syncs, or its algorithms will be ported to our C
code.  Definitely not something for you to worry about.

Re: hash-table-{to, from}-alist

[Ted Zlatanov]

On Tue, 02 Dec 2008 18:05:52 +0900 "Stephen J. Turnbull" <stephen@xemacs.org> wrote: 

SJT> Ted Zlatanov writes:
>> Can you and Stefan figure out whether to have print-readably or
>> not?  I was against it, and Stefan supported that.  But let's see
>> the pro argument and discuss this.

SJT> The pro argument is that unless you're debugging the hash table, it's
SJT> a PITA to have a hash table that gets recursively passed to each
SJT> function down the line taking up 20 screens for each frame in a
SJT> backtrace.  "Not debugging the hash table" is the overwhelmingly
SJT> common occurance for me; usually I'm more interested in where in the
SJT> stack the value I'm going to be puthash'ing turned from gold into
SJT> garbage.  OTOH, about half the time when I've got a chartable in the
SJT> stack, the chartable is buggy (in my experience almost all chartables
SJT> are write-once).

Isn't the same argument valid for lists?  At least in the Emacs
backtrace, lists get shortened for display; I don't know if that's done
at the print level or at the display level.  If it's at the display
level, a lot of cycles are wasted for large lists.

Basically I want to treat hashtables like lists as much as possible for
printing and reading, so print-readably would introduce inconsistency.
Otherwise I agree with your argument, I am just reluctant to make
hashtables an exception to the general print behavior.

>> Will the current state of things cause XEmacs breakage anywhere?

SJT> No.  This is a user interface thing.  If I'm right, XEmacs will get a
SJT> mild popularity boost if you always print the hash table.  I'm not
SJT> going to complain about that! ;-)

Heh.

>> One idea I had is to check if print-readably is bound; if not then
>> we act as if it was bound and t.  That will DTRT in GNU Emacs and
>> in XEmacs, I hope.

SJT> Hm?  XEmacs already has the feature in C; your code will either get
SJT> ignored in XEmacs syncs, or its algorithms will be ported to our C
SJT> code.  Definitely not something for you to worry about.

Great.

Ted

Re: hash-table-{to, from}-alist

[Stephen J. Turnbull]

Ted Zlatanov writes:
 > On Tue, 02 Dec 2008 18:05:52 +0900 "Stephen J. Turnbull" <stephen@xemacs.org> wrote: 
 > 
 > SJT> Ted Zlatanov writes:
 > >> Can you and Stefan figure out whether to have print-readably or
 > >> not?  I was against it, and Stefan supported that.  But let's see
 > >> the pro argument and discuss this.
 > 
 > SJT> The pro argument is that unless you're debugging the hash table, it's
 > SJT> a PITA to have a hash table that gets recursively passed to each
 > SJT> function down the line taking up 20 screens for each frame in a
 > SJT> backtrace.
 > 
 > Isn't the same argument valid for lists?

No.  You don't know whether a list is data or an executable expression
in Lisp until you eval it.  Lists are special.  A comparison to
vectors might be more appropriate.

Re: hash-table-{to, from}-alist

[Ted Zlatanov]

On Wed, 03 Dec 2008 00:54:49 +0900 "Stephen J. Turnbull" <stephen@xemacs.org> wrote: 

SJT> Ted Zlatanov writes:
>> On Tue, 02 Dec 2008 18:05:52 +0900 "Stephen J. Turnbull" <stephen@xemacs.org> wrote: 
>> 
SJT> Ted Zlatanov writes:
>> >> Can you and Stefan figure out whether to have print-readably or
>> >> not?  I was against it, and Stefan supported that.  But let's see
>> >> the pro argument and discuss this.
>> 
SJT> The pro argument is that unless you're debugging the hash table, it's
SJT> a PITA to have a hash table that gets recursively passed to each
SJT> function down the line taking up 20 screens for each frame in a
SJT> backtrace.
>> 
>> Isn't the same argument valid for lists?

SJT> No.  You don't know whether a list is data or an executable expression
SJT> in Lisp until you eval it.  Lists are special.  A comparison to
SJT> vectors might be more appropriate.

OK, that makes sense.  I checked the source and I think the relevant GNU
Emacs variables are eval-expression-print-length and print-length, which
it affects.  They control how many vector/bitvector elements are
printed.  Would it work to say:

;; pseudo-lisp
(when (and (bound-and-not-nil print-readably) 
           (not (natnump print-length)))
 (setq print-length 10))

Ted

Re: hash-table-{to, from}-alist

[Stephen J. Turnbull]

Ted Zlatanov writes:

 > OK, that makes sense.  I checked the source and I think the relevant GNU
 > Emacs variables are eval-expression-print-length and print-length, which
 > it affects.  They control how many vector/bitvector elements are
 > printed.  Would it work to say:

Well, that's not really for me to say.  I can advise you on the basis
of XEmacs experience and the rationales I've heard from the people who
wrote the code, but whether that makes sense/is to the taste of Emacs
users is something that ultimately Stefan and Yidong have to judge.

If you haven't heard something *convincing* to you by now, I think you
should take the easy way out, and just print everything until somebody
complains.  That's what you personally need anyway, right?

IOW, I think at this point we're discussing the color of the trim on
the bikeshed.... :-)

Re: hash-table-{to, from}-alist

[Stefan Monnier]

> Can you and Stefan figure out whether to have print-readably or not?  I

As I said, let's go with a "always print it all" approach for now.
You may want to make it obey print-length and print-depth, tho.


        Stefan

Re: hash-table-{to, from}-alist

[Ted Zlatanov]

On Tue, 02 Dec 2008 17:19:55 -0500 Stefan Monnier <monnier@IRO.UMontreal.CA> wrote: 

>> Can you and Stefan figure out whether to have print-readably or not?  I
SM> As I said, let's go with a "always print it all" approach for now.
SM> You may want to make it obey print-length and print-depth, tho.

OK, my latest patch does that.

Ted

Re: hash-table-{to, from}-alist

[Davis Herring]

> It prints an extra space at the end, which I still have as a simple
> TODO.  I also have to handle circular references, but otherwise it seems
> pretty reasonable.

I don't mean to assume that it's news to you, but if it happens to be,
it's quite easy to fix this:

int printed=0;
for(...) {
  if(print_this_one) {
    if(printed) PRINTCHAR(' ');
    printed=1;
    print_it();
    /* no trailing space here */
  }
}

Move declaration to block beginning, adjust coding style, and salt to taste.

Davis

-- 
This product is sold by volume, not by mass.  If it appears too dense or
too sparse, it is because mass-energy conversion has occurred during
shipping.

Re: hash-table-{to, from}-alist

[Ted Zlatanov]

On Wed, 26 Nov 2008 11:44:32 -0800 (PST) "Davis Herring" <herring@lanl.gov> wrote: 

>> It prints an extra space at the end, which I still have as a simple
>> TODO.  I also have to handle circular references, but otherwise it seems
>> pretty reasonable.

DH> I don't mean to assume that it's news to you, but if it happens to be,
DH> it's quite easy to fix this:

DH> int printed=0;
DH> for(...) {
DH>   if(print_this_one) {
DH>     if(printed) PRINTCHAR(' ');
DH>     printed=1;
DH>     print_it();
DH>     /* no trailing space here */
DH>   }
DH> }

DH> Move declaration to block beginning, adjust coding style, and salt to taste.

Thanks, I used your suggestion.  I'll post the updated patch once I get
circular references cleared up.

Ted

Re: hash-table-{to, from}-alist

[Stefan Monnier]

RMS> #s(hash-table size 1 data (x x-value))

RMS> Being compatible with that seems like a good idea,
RMS> if there is no obstacle.

> The equality test is necessary if it was given originally (meaning, if
> it's not 'eql).  I don't think we should ever omit it, even if it's the
> default value, for readability.

I don't see any reason to treat this specially, actually.

> The :weakness, :rehash-size, and :rehash-threshold options to
> make-hash-table, can probably be omitted when they are the defaults.

Indeed.

> Would this work?
> #s(hash-table size 2 test equal data (k1 v1) (k2 v2))
> Finally, the size is just a hint with make-hash-table; should the (read)
> allow wrong sizes or throw an error?

I don't know what "size" is meant to represent in XEmacs's or CL's use
of this representation.  Is it the expected average number of elements
in the table?  or the expected max number of elements?  or the current
number of elements?


        Stefan

Re: hash-table-{to, from}-alist

[Stephen J. Turnbull]

Stefan Monnier writes:

 > I don't know what "size" is meant to represent in XEmacs's or CL's use
 > of this representation.  Is it the expected average number of elements
 > in the table?  or the expected max number of elements?  or the current
 > number of elements?

Size is the actual number of elements currently in the table.  In
XEmacs, weak elements that are "no longer there" are not guaranteed to
be removed from the table at any particular time, and I suspect size
is ambiguous for weak tables.  Ask Mike Sperber (XEmacs.org mailbox
"mike") and/or Martin Buchholz ("martin") if you want to know the
details and the design.

In the XEmacs implementation, the parameters of #s(hash-table ...) are
simply passed to make-hash-table, so as Ted says, it's just a hint.

Re: hash-table-{to, from}-alist

[Miles Bader]

Stefan Monnier <monnier@iro.umontreal.ca> writes:
> I was thinking of something more like #<hash-table ...> where "..."
> would contain the props and then the key/value pairs.  I.e. an extension
> of the current (un`read'able syntax).

Isn't there a general lisp convention that all syntaxes of the form
#<...> are output-only, not intended to be readable?

-Miles

-- 
History, n. An account mostly false, of events mostly unimportant, which are
brought about by rulers mostly knaves, and soldiers mostly fools.

Re: hash-table-{to, from}-alist

[tomas]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Sat, Nov 22, 2008 at 03:07:04PM +0900, Miles Bader wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> > I was thinking of something more like #<hash-table ...> where "..."
> > would contain the props and then the key/value pairs.  I.e. an extension
> > of the current (un`read'able syntax).
> 
> Isn't there a general lisp convention that all syntaxes of the form
> #<...> are output-only, not intended to be readable?

Right -- that would be one point more iin favour of Stephen's proposal
upthread. More lispy, it seems.

Regards
- -- tomás
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFJKCTeBcgs9XrR2kYRAo1oAJ41PEmsRrp5XH9GZrfs/nWOV3VjzQCfVtT3
s8tTYe9RU5M2UqtaBWeGZWE=
=I5NL
-----END PGP SIGNATURE-----

Re: hash-table-{to, from}-alist

[Ted Zlatanov]

On Fri, 21 Nov 2008 22:18:41 -0500 Stefan Monnier <monnier@iro.umontreal.ca> wrote: 

Ted Zlatanov wrote:
>> { :prop1 prop-value1 :prop2 prop-value2 ... key1 val1 key2 val2 ... }

>> (:prop1 is :test for example)

SM> I was thinking of something more like #<hash-table ...> where "..."
SM> would contain the props and then the key/value pairs.  I.e. an extension
SM> of the current (un`read'able syntax).

Why would you want to make it unreadable if (as above, and in the other
proposals) you can make it readable?  The whole point of this discussion
was serializing hash tables; pretty-printing them is trivial.

On Sat, 22 Nov 2008 21:27:18 +0900 "Stephen J. Turnbull" <stephen@xemacs.org> wrote: 

SJT> I'm not sure what's wrong with

SJT>         (let ((tbl (make-hash-table ...)))
SJT>           (puthash key1 val1 tbl)
SJT>           ...)

SJT> as a read syntax for hash tables.

It's verbose, and doesn't follow what vectors do (they use [] instead of
(make-vector ...), setting some precedent for special read syntax).
OTOH it's readable, extensible, and easy to set up your way.

SJT> N.B. XEmacs and SXEmacs already have lots of object types.  If you're
SJT> going to start proliferating `read'able print syntaxes, starting with
SJT> an extensible one would be nice.

I only want some format that lets me easily print and read back a hash
table; the actual syntax is not important to me.  My proposal was only a
suggestion.

On Sun, 23 Nov 2008 02:38:59 +0900 "Stephen J. Turnbull" <stephen@xemacs.org> wrote: 

SJT> BTW, it turns out that in XEmacs and its descendants prin1 uses the CL
SJT> structure syntax when `print-readably' is bound to t:

SJT>     #s(hash-table size 1 data (x x-value))

SJT> This is not guaranteed to actually be acceptable to the reader, since
SJT> some contained objects may not be printable (eg, improper lists).
SJT> However, since most types in XEmacs do have readable print
SJT> representations, it does correctly recurse on composite types like
SJT> hash tables (as long as the expression is a tree; DAGs will contain
SJT> copies, cycles will be elided, etc).

That's fine too, and it would make sense to use something that has been
proven to work already.  As I said, the syntax doesn't matter to me.
Whatever the Emacs maintainers and developers like is OK, it just has to
work :)

Ted