Package: emacs;
Reported by: Drew Adams <drew.adams <at> oracle.com>
Date: Sun, 1 Jan 2023 18:09:01 UTC
Severity: normal
Found in version 28.2
To reply to this bug, email your comments to 60471 AT debbugs.gnu.org.
Toggle the display of automated, internal messages from the tracker.
View this report as an mbox folder, status mbox, maintainer mbox
bug-gnu-emacs <at> gnu.org
:bug#60471
; Package emacs
.
(Sun, 01 Jan 2023 18:09:01 GMT) Full text and rfc822 format available.Drew Adams <drew.adams <at> oracle.com>
:bug-gnu-emacs <at> gnu.org
.
(Sun, 01 Jan 2023 18:09:02 GMT) Full text and rfc822 format available.Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
From: Drew Adams <drew.adams <at> oracle.com> To: "bug-gnu-emacs <at> gnu.org" <bug-gnu-emacs <at> gnu.org> Subject: 28.2; Doc (and behavior) of `cl-case' 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)
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.