bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#69108: false-positive warning "variable ‘_’ not left unused" in if-l


From: Eli Zaretskii
Subject: bug#69108: false-positive warning "variable ‘_’ not left unused" in if-let* and if-let
Date: Sat, 17 Feb 2024 10:04:11 +0200

> Cc: 69108@debbugs.gnu.org
> Date: Sat, 17 Feb 2024 01:28:55 +0100
> From:  Michael Heerdegen via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> --- a/lisp/subr.el
> +++ b/lisp/subr.el
> @@ -2575,12 +2575,12 @@ delay-mode-hooks
>  (defun internal--build-binding (binding prev-var)
>    "Check and build a single BINDING with PREV-VAR."
>    (setq binding
> -        (cond
> -         ((symbolp binding)
> +        (pcase binding
> +         ((pred symbolp)
>            (list binding binding))
> -         ((null (cdr binding))
> -          (list (make-symbol "s") (car binding)))
> -         (t binding)))
> +         ((or `(,test) `(_ ,test))
> +          (list (make-symbol "s") test))
> +         (_ binding)))

Thanks, but can we please leave this as 'cond', instead of converting
it to a 'pcase'?  It doesn't seem to be justified here, and even less
so since you need to rewrite all the existing conditions.

>  (defmacro if-let (spec then &rest else)
>    "Bind variables according to SPEC and evaluate THEN or ELSE.
> -Evaluate each binding in turn, as in `let*', stopping if a
> -binding value is nil.  If all are non-nil return the value of
> -THEN, otherwise the last form in ELSE.
> +Evaluate each binding in turn, as in `let*', stopping if a binding value
> +is nil.  If all are non-nil return the value of THEN, otherwise the
> +value of the last ELSE form or nil if there are none.
> 
>  Each element of SPEC is a list (SYMBOL VALUEFORM) that binds
>  SYMBOL to the value of VALUEFORM.  An element can additionally be
> @@ -2642,9 +2642,9 @@ if-let
>  interest.  It can also be of the form SYMBOL, then the binding of
>  SYMBOL is checked for nil.
> 
> -As a special case, interprets a SPEC of the form \(SYMBOL SOMETHING)
> -like \((SYMBOL SOMETHING)).  This exists for backward compatibility
> -with an old syntax that accepted only one binding."
> +As a special case that exists for backward compatibility only, a
> +complete SPEC of the form \(SYMBOL SOMETHING) is interpreted like
> +\((SYMBOL SOMETHING))."
>    (declare (indent 2)
>             (debug ([&or (symbolp form)  ; must be first, Bug#48489
>                          (&rest [&or symbolp (symbolp form) (form)])]

This hunk seems to be unrelated?  And it is not necessarily for the
better, IMO, at least not all of it (replaces active tense with
passive, refills text that doesn't need refilling, and other minor
issues, like the confusing use of construct state in "last ELSE
form").





reply via email to

[Prev in Thread] Current Thread [Next in Thread]