[PATCH] Add new function to test whether a key is present in a hash table.

[PATCH] Add new function to test whether a key is present in a hash table.

[Philipp Stephani]

Such a function is useful because in Emacs Lisp, 'gethash' cannot
return whether the key is present as in Common Lisp, and using
'gethash' alone to test for presence is nontrivial.

* src/fns.c (Fhash_table_contains_p): New function.
* test/src/fns-tests.el (hash-table-contains-p): New unit test.
* doc/lispref/hash.texi (Hash Access): Document new function.
---
 doc/lispref/hash.texi | 12 ++++++++++++
 etc/NEWS              |  3 +++
 src/fns.c             | 10 ++++++++++
 test/src/fns-tests.el |  8 ++++++++
 4 files changed, 33 insertions(+)

diff --git a/doc/lispref/hash.texi b/doc/lispref/hash.texi
index ddd46a55ed..8848f0462e 100644
--- a/doc/lispref/hash.texi
+++ b/doc/lispref/hash.texi
@@ -195,6 +195,18 @@ Hash Access
 association in @var{table}.
 @end defun
 
+While it might be tempting to use @code{gethash} to check whether a
+key is present in a hash table, keep in mind that you often need to
+distinguish between @var{key} being absent and @var{key} being mapped
+to @var{default}.  To easily distinguish between these two cases,
+there's another function to explicitly check whether a key is present:
+
+@defun hash-table-contains-p key table
+This function looks up @var{key} in @var{table}; it returns @code{t}
+if @var{key} is present, and @code{nil} otherwise.  The associated
+value is ignored.
+@end defun
+
 @defun puthash key value table
 This function enters an association for @var{key} in @var{table}, with
 value @var{value}.  If @var{key} already has an association in
diff --git a/etc/NEWS b/etc/NEWS
index 71569c95ad..5c9363c052 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -282,6 +282,9 @@ depending on the new customizable variable 'read-answer-short'.
 Specifically, it puts the module name into 'load-history', prints
 loading messages if requested, and protects against recursive loads.
 
+** New function 'hash-table-contains-p' to check whether a hash table
+contains a certain key.
+
 \f
 * Changes in Emacs 27.1 on Non-Free Operating Systems
 
diff --git a/src/fns.c b/src/fns.c
index 47457e44c8..6075544c69 100644
--- a/src/fns.c
+++ b/src/fns.c
@@ -4650,6 +4650,15 @@ DEFUN ("clrhash", Fclrhash, Sclrhash, 1, 1, 0,
 }
 
 
+DEFUN ("hash-table-contains-p", Fhash_table_contains_p, Shash_table_contains_p,
+       2, 2, NULL,
+       doc: /* Return t if TABLE contains KEY, nil otherwise.  */)
+  (Lisp_Object key, Lisp_Object table)
+{
+  return hash_lookup (check_hash_table (table), key, NULL) >= 0 ? Qt : Qnil;
+}
+
+
 DEFUN ("gethash", Fgethash, Sgethash, 2, 3, 0,
        doc: /* Look up KEY in TABLE and return its associated value.
 If KEY is not found, return DFLT which defaults to nil.  */)
@@ -5145,6 +5154,7 @@ syms_of_fns (void)
   defsubr (&Shash_table_weakness);
   defsubr (&Shash_table_p);
   defsubr (&Sclrhash);
+  defsubr (&Shash_table_contains_p);
   defsubr (&Sgethash);
   defsubr (&Sputhash);
   defsubr (&Sremhash);
diff --git a/test/src/fns-tests.el b/test/src/fns-tests.el
index f8554636ba..f651ee8985 100644
--- a/test/src/fns-tests.el
+++ b/test/src/fns-tests.el
@@ -575,4 +575,12 @@ dot2
                                :type 'wrong-type-argument)
                  '(wrong-type-argument plistp (:foo 1 . :bar)))))
 
+(ert-deftest hash-table-contains-p ()
+  (let ((h (make-hash-table)))
+    (puthash 1.5 'foo h)
+    (puthash 2.5 nil h)
+    (should (equal (hash-table-contains-p 1.5 h) t))
+    (should (equal (hash-table-contains-p 2.5 h) t))
+    (should (equal (hash-table-contains-p 2 h) nil))))
+
 (provide 'fns-tests)
-- 
2.16.1

Re: [PATCH] Add new function to test whether a key is present in a hash table.

[Clément Pit-Claudel]

On 2018-02-15 15:34, Philipp Stephani wrote:
> Such a function is useful because in Emacs Lisp, 'gethash' cannot
> return whether the key is present as in Common Lisp, and using
> 'gethash' alone to test for presence is nontrivial.

Looks useful to me :) Maybe the function should be called …-contains-key-p instead of …-contains-key?

Clément.

RE: [PATCH] Add new function to test whether a key is present in a hash table.

[Drew Adams]

> Such a function is useful because in Emacs Lisp, 'gethash' cannot
> return whether the key is present as in Common Lisp, and using
> 'gethash' alone to test for presence is nontrivial.

Glad to see this.  Everyone was coding their own (in Lisp).

> +While it might be tempting to use @code{gethash} to check whether a
> +key is present in a hash table, keep in mind that you often need to
> +distinguish between @var{key} being absent and @var{key} being mapped
> +to @var{default}.  To easily distinguish between these two cases,
> +there's another function to explicitly check whether a key is present:
> +
> +@defun hash-table-contains-p key table
> +This function looks up @var{key} in @var{table}; it returns @code{t}
> +if @var{key} is present, and @code{nil} otherwise.  The associated
> +value is ignored.

1. I don't think we need to say "The associated value is
ignored".  Nothing suggests that it would be used in any way.

2. The function name should have `key' in it, I think.  Suggestion:
`hash-table-contains-key-p' or even just `hash-table-key-p'.

3. Another possibility could be to return, as the true value,
a cons of the value associated with the key.  IOW, instead
of returning `t' we would return `(THE-VALUE)'.   If this
is not particularly costly it might be a bit more useful.

___

Related: Bug #30458 asks for a function that tests whether
a hash table contains an entry with a given value.  More
precisely, it would return the list of keys with that value,
so nil would mean the value is absent altogether.

Re: [PATCH] Add new function to test whether a key is present in a hash table.

[Stefan Monnier]

> Such a function is useful because in Emacs Lisp, 'gethash' cannot
> return whether the key is present as in Common Lisp, and using
> 'gethash' alone to test for presence is nontrivial.

We could also implement it in Elisp:

    (defun hash-table-contains-p (key table)
      (let ((x '(:hash-table-contains-p)))
        (not (eq x (gethash key table x)))))


-- Stefan

Re: [PATCH] Add new function to test whether a key is present in a hash table.

[Eric Abrahamsen]

Drew Adams <drew.adams@oracle.com> writes:

>> Such a function is useful because in Emacs Lisp, 'gethash' cannot
>> return whether the key is present as in Common Lisp, and using
>> 'gethash' alone to test for presence is nontrivial.
>
> Glad to see this.  Everyone was coding their own (in Lisp).

+1

>> +While it might be tempting to use @code{gethash} to check whether a
>> +key is present in a hash table, keep in mind that you often need to
>> +distinguish between @var{key} being absent and @var{key} being mapped
>> +to @var{default}.  To easily distinguish between these two cases,
>> +there's another function to explicitly check whether a key is present:
>> +
>> +@defun hash-table-contains-p key table
>> +This function looks up @var{key} in @var{table}; it returns @code{t}
>> +if @var{key} is present, and @code{nil} otherwise.  The associated
>> +value is ignored.
>
> 1. I don't think we need to say "The associated value is
> ignored".  Nothing suggests that it would be used in any way.
>
> 2. The function name should have `key' in it, I think.  Suggestion:
> `hash-table-contains-key-p' or even just `hash-table-key-p'.
>
> 3. Another possibility could be to return, as the true value,
> a cons of the value associated with the key.  IOW, instead
> of returning `t' we would return `(THE-VALUE)'.   If this
> is not particularly costly it might be a bit more useful.

I would really like to see a function that didn't require us to:

(and (hash-table-key-p "key" <table>) (gethash "key" <table>))

Ie, an all-in-one function (happy to see it implemented in elisp, a la
Stefan's solution) that gives us both pieces of information at once.
Drew's cons-cell seems like a good middle ground. We could even call it
`hash-table-get', to contrast with `gethash', and still have it accept
(and return) the optional DFLT argument.

Eric

RE: [PATCH] Add new function to test whether a key is present in a hash table.

[Drew Adams]

> > Such a function is useful because in Emacs Lisp, 'gethash'
> > cannot return whether the key is present as in Common Lisp,
> > and using 'gethash' alone to test for presence is nontrivial.
> 
> We could also implement it in Elisp:
> (defun hash-table-contains-p (key table)
>   (let ((x '(:hash-table-contains-p)))
>     (not (eq x (gethash key table x)))))

Yes, as I said:
  Everyone was coding their own (in Lisp).

using a unique cons, uninterned symbol, or
some other unique object.

Philipp used an uninterned symbol:
(let ((uniq-symb  '#:void))
  (not (eq uniq-symb (gethash key table uniq-symb))))

I (like you) used a unique cons:
(let ((uniq-cons  (cons 1 1)))
  (not (eq uniq-cons (gethash key table uniq-cons))))

But isn't it better to define this in C?

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28753

Re: [PATCH] Add new function to test whether a key is present in a hash table.

[Stefan Monnier]

> But isn't it better to define this in C?

To me the answer is usually no, unless it's speed-critical.


        Stefan


PS: I think a more interesting function to provide would be some
gethash-ref which would return some kind of "reference" to the entry, so
we can afterwards update that hash-table entry without (re)computing the
hash (i.e. some kind of equivalent to `intern` after which you can just
do `set` which doesn't involve hashing any more).  But that would imply
a fairly significant amount of design to make it work.

RE: [PATCH] Add new function to test whether a key is present in a hash table.

[Drew Adams]

> > But isn't it better to define this in C?
> 
> To me the answer is usually no, unless it's speed-critical.

To me too, the answer is usually no.  If this is done
in Lisp it is presumably because it might be useful to
someone to modify or advise, or at least serve as food
for thought.  In that case, the code should be as clear
as possible.

> PS: I think a more interesting function to provide would be some
> gethash-ref which would return some kind of "reference" to the entry, so
> we can afterwards update that hash-table entry without (re)computing the
> hash (i.e. some kind of equivalent to `intern` after which you can just
> do `set` which doesn't involve hashing any more).

Definitely more interesting.

> But that would imply
> a fairly significant amount of design to make it work.

Re: [PATCH] Add new function to test whether a key is present in a hash table.

[John Wiegley]

>>>>> "DA" == Drew Adams <drew.adams@oracle.com> writes:

DA> 2. The function name should have `key' in it, I think. Suggestion:
DA> `hash-table-contains-key-p' or even just `hash-table-key-p'.

It seems a bit strange that the name isn't something like "hashash", since
knowing "puthash" and "gethash" would never help you guess
hash-table-contains-key-p.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

Re: [PATCH] Add new function to test whether a key is present in a hash table.

[Richard Copley]

On 16 February 2018 at 06:51, John Wiegley <johnw@gnu.org> wrote:
>>>>>> "DA" == Drew Adams <drew.adams@oracle.com> writes:
>
> DA> 2. The function name should have `key' in it, I think. Suggestion:
> DA> `hash-table-contains-key-p' or even just `hash-table-key-p'.
>
> It seems a bit strange that the name isn't something like "hashash", since
> knowing "puthash" and "gethash" would never help you guess
> hash-table-contains-key-p.

Or "testhash"? And maybe "tryhash" or "try-gethash" for a function that
returns a cons or "slot" of some kind.

Re: [PATCH] Add new function to test whether a key is present in a hash table.

[John Wiegley]

>>>>> "RC" == Richard Copley <rcopley@gmail.com> writes:

RC> Or "testhash"? And maybe "tryhash" or "try-gethash" for a function that
RC> returns a cons or "slot" of some kind.

I like tryhash. All of the hash-table functions right now are a bit
unfortunately inconsistent in their naming. I'd rather have hash-table-get,
hash-table-has-key, etc.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

RE: [PATCH] Add new function to test whether a key is present in a hash table.

[Drew Adams]

> DA> 2. The function name should have `key' in it, I think. Suggestion:
> DA> `hash-table-contains-key-p' or even just `hash-table-key-p'.
> 
> It seems a bit strange that the name isn't something like "hashash",
> since knowing "puthash" and "gethash" would never help you guess
> hash-table-contains-key-p.

Right.  And knowing `hash-table-contains-key-p' wouldn't
help you guess `puthash' or `get-hash'.

But that's the fault of `puthash' and `gethash'. ;-)

It's `puthash' and `gethash' that are not so conventional
for Emacs.  They are borrowed from Common Lisp.  Likewise,
`maphash', `remhash', and `clrhash'.  Those names predate
Common Lisp even.

`puthash', `gethash', `remhash', etc. could be made
aliases of, say, `hash-table-put', `hash-table-get',
`hash-table-remove', etc.  (I'm not saying they should.)

`puthash' seems to be the only Emacs `put*' function
whose name doesn't start with `put-' (apart from `put'
itself).  Same thing for `gethash' (except that there
are also `getenv' and `getf').  The Emacs convention is
to use prefix `put-' or `get-'.

Another (more common?) convention in Emacs is to put the
object type first: `string-match', `bool-vector-union',
`mode-line-next-buffer', `menu-bar-open', `marker-buffer',
`buffer-size', `window-child', `frame-edges', etc.,
especially for functions that extract parts of an object.

Yet another convention is to put the verb first,
especially for "utility" verbs: `map-*', `make-*',
`put-*', `get-*', `copy-*', `delete-*', etc.

Finally, a library prefix is often used: `ad-get-*',
`ange-ftp-get-*', `bookmark-get-*', `comint-get-*',
`dired-get-*', `ediff-get-*', `gnus-get-*' ...


These are the functions I see with "hash" in their names:

0. Those that (I guess) have nothing to do with hash tables:

   ange-ftp-guess-hash-mark-size
   ange-ftp-process-handle-hash
   buffer-hash
   gnutls-hash-digest
   hashcash-insert-payment
   hashcash-insert-payment-async
   hashcash-verify-payment
   rfc2104-hash
   sxhash
   sxhash-eq
   sxhash-eql
   sxhash-equal

   I mention these to point out that names like `puthash'
   don't really, on their own, make clear that they have
   to do with a hash table.

1. Those that are about hash-table hashing but (I guess)
   don't have a hash table as the main object:

   gnus-create-hash-size
   gnutls-hash-mac
   secure-hash
   secure-hash-algorithms

2. Those (non-CL) that have a hash table as the main object:

   ange-ftp-hash-entry-exists-p
   ange-ftp-hash-table-keys
   clrhash
   copy-hash-table
   define-hash-table-test
   define-translation-hash-table
   gethash
   gnus-make-hashtable
   hash-table-count
   hash-table-p
   hash-table-rehash-size
   hash-table-rehash-threshold
   hash-table-size
   hash-table-test
   hash-table-weakness
   make-hash-table
   maphash
   puthash
   remhash
   ucs-normalize-make-hash-table-from-alist

3. Common-Lisp emulation functions for hash tables:

   cl-clrhash
   cl-gethash
   cl-hash-table-count
   cl-hash-table-p
   cl-make-hash-table
   cl-maphash
   cl-not-hash-table 
   cl-puthash
   cl-remhash

   Note that except for those names inherited from older
   Lisps, Common Lisp spells out `hash-table' in the
   function names.

The most common convention, so far, for Emacs hash-table
functions is to start the function name with `hash-table'.
That accords with the convention of putting the object
name first.

Re: [PATCH] Add new function to test whether a key is present in a hash table.

[Philipp Stephani]

Am Do., 15. Feb. 2018 um 23:05 Uhr schrieb Clément Pit-Claudel
<cpitclaudel@gmail.com>:
>
> On 2018-02-15 15:34, Philipp Stephani wrote:
> > Such a function is useful because in Emacs Lisp, 'gethash' cannot
> > return whether the key is present as in Common Lisp, and using
> > 'gethash' alone to test for presence is nontrivial.
>
> Looks useful to me :) Maybe the function should be called …-contains-key-p instead of …-contains-key?

It's called ...-contains-p, I think it's clear enough that this tests
for key presence and not value presence.
Any objections to pushing this one-liner to master?