Hi Stefan, Revised patch attached and comments below Kind regards Jeremy Bryant Stefan Monnier 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