Subject: 28.2; Doc (and behavior) of `cl-case'

From: Drew Adams <[email-masked]>
Date: Sun, 1 Jan 2023 18:07:51 +0000

1. The `cl-case' doc string talks about evaluating and evaluating and
evaluating...  But it doesn't ever say that the values in KEYLIST are
_not_ evaluated.  Please consider improving that doc, to make clear that
neither the KEYLIST arg nor any of its keys gets evaluated during macro
expansion.

According to Stefan M., this is a common source of confusion, with users
"very frequently" mistakenly quoting keys.

Keys in KEYLIST are _literal values_ to be compared against the value of
EXPR using `eql'.  They are not sexps to be evaluated.  It's important
to point this out explicitly, I think.

The whole point of `cl-case' - its particular use case - is to (1)
evaluate a sexp only once and (2) test that value with `eql' against
values that you provide as literals.  It's essentially a simplification
of using `let' plus `cond' to support only that use case: single EXPR
evaluation and `eql' comparisons with multiple sets of values.

Besides the doc string, there's the CL manual, node `Conditionals':

 "This macro evaluates KEYFORM, then compares
  it with the key values listed in the various
  CLAUSEs."

It doesn't compare KEYFORM with the values.  It compares the _result of
evaluating_ KEYFORM with the values.  Maybe this is a nit, but it's
important to be clear about which of the macro's args get evaluated etc.

 "Whichever clause matches the key is executed;
  comparison is done by 'eql'."

What does it mean for a _clause_ to match a key (a value in the KEYLIST
of the clause)?  The doc should instead talk about using `eql' to
compare the value of KEYFORM against values in the KEYLIST of each
clause.

[BTW, why use the name KEYFORM for something that isn't about keys,
especially since there are keys in the KEYLISTs of the clauses?  That
just invites confusion, I think.]

 "The clauses are of the form
  (KEYLIST BODY-FORMS...) where KEYLIST is a
  list of key values."

This is better than the doc-string text, at least.  It says that KEYLIST
_is_ a list of values, not sexps to be evaluated.  And KEYLIST _is_ a
(literal) list, not a sexp to be evaluated to a list.

By comparison, note that "Common Lisp The Language, 2nd Edition"
(CLTL2), is very clear about specifying what gets evaluated and what
doesn't.

https://www.cs.cmu.edu/Groups/AI/html/cltl/clm/node84.html

 "If KEY is in the KEYLIST (that is, is `eql'
  to any item in the KEYLIST)..."

 "The keys in the keylists are _not_ evaluated;
  literal key values must appear in the keylists."

Emphasis on "not".  _Literal_ values.  (It might also have mentioned
that KEYLIST is a literal list, not a sexp that's evaluated to give a
list.)

That's what our doc should also say, whatever the words we choose to use
to say it.  In the syntax, KEYLIST and its keys are literal values;
they're not evaluated by the macro.
__________

2. There are also some additional wording problems that make the text
unclear:

 "Eval EXPR and choose among clauses on that value.

Clauses on a value?  Maybe it should say something like "based on that
value".

 "Each clause looks like (KEYLIST BODY...).
  EXPR is evaluated and compared against each
  key in each KEYLIST; the corresponding BODY
  is evaluated.

`eql is the predicate used for testing.  It would be clearer here to
point out that "corresponding" means with a key that is `eql'....

 "If no clause succeeds, cl-case returns nil.
  A single non-nil atom may be used in place
  of a KEYLIST of one atom.  A KEYLIST of t
  or `otherwise' is allowed only in the final
  clause, and matches if no other keys match.
  Key values are compared by `eql'."

This should say that any clauses after the use of KEYLIST `t' or
`otherwise' are just _ignored_.  If they were really "allowed only in
the final clause" then any clause after them would raise an error
instead of being ignored, but no error is raised.

[What about the syntax case of just (KEYLIST)?  Does that fit the
template (KEYLIST BODY...)?  Does "BODY..." mean that BODY can be
missing altogether?  I can never remember the Emacs doc convention for
this, sorry.  (KEYLIST) here acts the same as (KEYLIST nil) - it returns
nil if one of its keys matches.]
__________

3. `cl-case' seems to be mentioned in the Elisp manual only in the
section about `pcase'.  Please consider adding `cl-case' to the Elisp
manual's Index.

In node `pcase Macro' there's a subsection called "Example: Advantage
Over `cl-case'".  Instead of trying to only show an advantage, it would
be better to contrast the two by pointing out the specific use case of
`cl-case': testing a given value against sets of values using `eql'.
`cl-case' is _all about `eql'_ (e.g., it's not for lists, strings, and
vectors).

In that section we say this, which I may not understand, but I think is
incorrect.  After showing a `pcase' example which starts with "(pcase
(get-return-code x)" we say:

 "With 'cl-case', you would need to explicitly
  declare a local variable 'code' to hold the
  return value of 'get-return-code'."

I think that's trying to say that _you need to use a variable_ as the
first arg to `cl-case'.  If so, that's simply wrong.  (If that's not
what it's trying to say then I don't know what is.)

`cl-case' accepts any sexp as its first arg.  It evaluates that sexp,
and it's the resulting _value_ that's tested against the keys in the
clauses.  There's nothing about the sexp having any relation to any
_variable_, local or nonlocal.  There's no need to introduce any var.

The language there is also unclear.  What does it mean to "declare" a
variable in Elisp (other than a vacuous defvar: (defvar x))?  I'm
guessing that the text is trying to say that you need to _bind_ a local
varible to the value returned by the expression (get-return-code x), and
then use that variable as the first arg to `cl-case'.  If so, that's
just not true.

Here's a `cl-case' example that corresponds to the `pcase' example in
the text.  It doesn't bind/declare any local variable corresponding to
(get-return-code x):

 (defun get-return-code (arg) (if (< arg 42) 'success 'failure))

If x is set or bound to, say, 13, this prints "Done!", and with x=56, it
prints "Unknown!":

 (cl-case (get-return-code x)
   (success (message "Done!"))
   (t       (message "Unknown!"))))
__________

4. Beyond the doc string, there seems to also be a bug in the `cl-case'
implementation.

CLTL2 says also this, as part of the spec for `case':

 "It is an error for the same key to appear in
  more than one clause; a consequence is that
  the order of the clauses does not affect the
  behavior of the case construct."

Apparently Emacs's `cl-case' doesn't raise such an error.  To emulate CL
`case' it should.  If we really don't want the specified `case' behavior
then I think the doc should point out this departure from a faithful
emulation of CL `case'.

In GNU Emacs 28.2 (build 2, x86_64-w64-mingw32)
 of 2022-09-13 built on AVALON
Windowing system distributor 'Microsoft Corp.', version 10.0.19044
System Description: Microsoft Windows 10 Pro (v10.0.2009.19044.2364)

Configured using:
 'configure --with-modules --without-dbus --with-native-compilation
 --without-compress-install CFLAGS=-O2'

Configured features:
ACL GIF GMP GNUTLS HARFBUZZ JPEG JSON LCMS2 LIBXML2 MODULES NATIVE_COMP
NOTIFY W32NOTIFY PDUMPER PNG RSVG SOUND THREADS TIFF TOOLKIT_SCROLL_BARS
XPM ZLIB

(NATIVE_COMP present but libgccjit not available)