[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#66863: [PATCH] Add two docstrings in cl-macs.el
From: |
Jeremy Bryant |
Subject: |
bug#66863: [PATCH] Add two docstrings in cl-macs.el |
Date: |
Tue, 31 Oct 2023 23:40:04 +0000 |
0001-Add-two-docstrings-in-cl-macs.el.patch
Description: Text Data
Hi Stefan,
Revised patch attached and comments below
Kind regards
Jeremy Bryant
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> * lisp/emacs-lisp/cl-macs.el (cl--simple-exprs-p)
>> (cl--const-expr-p): Add docstrings.
>
> Hmm... tackling some tricky ones, eh?
Apparently yes!
>
>> (defun cl--simple-exprs-p (xs)
>> + "Check if no side effects, and executes quickly, for each element of list
>> XS."
>> (while (and xs (cl--simple-expr-p (car xs)))
>> (setq xs (cdr xs)))
>> (not xs))
>
> I think I'd use a ref to `cl--simple-expr-p`, so we don't duplicate the
> poorly defined "no side effects, and executes quickly".
See revised patch which follows this advice
>
> Wait... `grep` says this function is not used any more, so maybe the
> better option is to delete it.
>
>> @@ -118,6 +119,7 @@ cl--safe-expr-p
>>
>> ;;; Check if constant (i.e., no side effects or dependencies).
>> (defun cl--const-expr-p (x)
>> + "Check if X is constant (i.e., no side effects or dependencies)."
>> (cond ((consp x)
>> (or (eq (car x) 'quote)
>> (and (memq (car x) '(function cl-function))
>
> Yeah..hmm.. by order of increasing preference:
> - Move the comment to the docstring rather that just copying it.
Done
> - Clarify the difference with `macroexp-const-p`.
Added reference to this with my interpretation of the difference (which
could need change or not)
> - Declare the function obsoleted by `macroexp-const-p`.
> - Delete the function altogether.
>
>
> Stefan