bug#47368: [External] : bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN

[Drew Adams]

> > Nothing about eq being the default, and nothing about
> > testing being also possible with the others you mention.
> >
> > Not only that, but the doc string says that TESTFN
> > is deprecated, but there's no other mention of TESTFN.
> >
> > What's TESTFN?  Where is it specified?  It's not part
> > of the function signature that's shown.  How can you
> > refer to it if there's no indication anywhere here of
> > what it is?  This makes no sense to me.
> 
> All of these points are already being discussed in this thread.

And yet in this thread you asked, seeming to suggest
that the doc here is fine as is:

  What would you like to see clarified in the documentation?

Seems pretty clear that this doc has more than one
problem, as I guess (hope) you acknowledge now.

If the function handles different kinds of data
(alists, hash tables, ... whatnot), and if its
behavior can depend on an equality predicate that
it can't know (can't even be passed as an arg),
then _that_, at the very least, should be stated
in the doc.

I'd think that if a test function _can_ be used for
at least some types of data, such as alists, then it
should be allowed as an optional arg.

Just because a function is generic, that doesn't mean
it has to always act with lowest-common-denominator
behavior.  A hash table has a given comparer.  But so
what?

Is `eq' the _default_ comparer, or is it the only one
for some data types?

  "its default depends on the MAP argument".

Maybe that too isn't generic enough?  If `eq' is all
that's allowed for some types then it makes no sense
to speak of a "default" those cases.

You said:

  That's what the docstring is trying to warn about:
  alists default to testing with eq, but can also use
  eql, equal, or anything else.

  Hash tables, OTOH, are limited to the test function
  that they were created with.

  So TESTFN doesn't always work as expected depending
  on the map type.

Why not allow TESTFN, and state that it applies only
to some types (naming some of them)?  Why cripple it
just because it's limited for some types?

Are you sure deprecating TESTFN was a great idea?

If it will remain deprecated, then at least add a
statement like what you wrote (above), to the doc.

And let users know how to use other than `eq' with
an alist etc., since you tell them that `eq' is
only the default and they can use other comparers.

And especially if the types are limited to alist,
hash table, and array, just _how_ a key is found
should be specified.  It makes no sense (to me)
for the doc to say only "If KEY is found".

FWIW, a cursory look at the code suggests the type
can also be a plist, but the doc currently mentions
only the other 3 types.  That too seems wrong.