GNU bug report logs - #66863
[PATCH] Add two docstrings in cl-macs.el

Previous Next

Package: emacs;

Reported by: Jeremy Bryant <jb <at> jeremybryant.net>

Date: Tue, 31 Oct 2023 22:42:01 UTC

Severity: normal

Tags: patch

Done: Stefan Kangas <stefankangas <at> gmail.com>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: Jeremy Bryant <jb <at> jeremybryant.net>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 66863 <at> debbugs.gnu.org
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 (text/x-diff, attachment)]
[Message part 2 (text/plain, inline)]
Hi Stefan,

Revised patch attached and comments below

Kind regards
Jeremy Bryant 

Stefan Monnier <monnier <at> 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


This bug report was last modified 1 year and 201 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.