magnars / dash.el

A modern list library for Emacs
GNU General Public License v3.0
1.66k stars 138 forks source link

-setq always sets last variable #307

Open alphapapa opened 5 years ago

alphapapa commented 5 years ago

This -setq form expands to set _rest, even though it shouldn't. If . _rest is removed, it expands to set _two-char-construct-syntax. This causes byte-compile warnings like Warning: variable ‘_rest’ not left unused.

(-setq (depth _start-of-innermost-list _start-of-last-complete-sexp-terminated
              in-string-p comment-level-p _following-quote-p
              _min-paren-depth _comment-style comment-or-string-start
              _open-parens-list _two-char-construct-syntax . _rest)
  (syntax-ppss))

Expansion:

(let (--dash-binding-depth--
      --dash-binding-in-string-p--
      --dash-binding-comment-level-p--
      --dash-binding-comment-or-string-start--
      --dash-binding-_rest--)
  (let* ((--dash-source-309-- (syntax-ppss))
         (depth (pop --dash-source-309--))
         (in-string-p (prog1 (nth 2 --dash-source-309--)
                        (setq --dash-source-309-- (nthcdr 3 --dash-source-309--))))
         (comment-level-p (pop --dash-source-309--))
         (comment-or-string-start (prog1 (nth 3 --dash-source-309--)
                                    (setq --dash-source-309-- (nthcdr 4 --dash-source-309--))))
         (_rest (nthcdr 2 --dash-source-309--)))
    (setq --dash-binding-depth-- depth
          --dash-binding-in-string-p-- in-string-p
          --dash-binding-comment-level-p-- comment-level-p
          --dash-binding-comment-or-string-start-- comment-or-string-start
          --dash-binding-_rest-- _rest))
  (setq depth --dash-binding-depth--
        in-string-p --dash-binding-in-string-p--
        comment-level-p --dash-binding-comment-level-p--
        comment-or-string-start --dash-binding-comment-or-string-start--
        _rest --dash-binding-_rest--))

I can work around it by removing everything after the last non-_-prefixed variable, but I feel like this is a bug.

Thanks for your work on Dash!

Fuco1 commented 5 years ago

I don't understand why shouldn't it set the last variable? Can you give a more minimal example, I'm kind of getting lost in the long names :blush:

alphapapa commented 5 years ago

Maybe a more complete example will help. Byte-compile this function:

(defun compile-this-get-warning ()
  (let (depth in-string-p comment-level-p comment-or-string-start)
    (-setq (depth _start-of-innermost-list _start-of-last-complete-sexp-terminated
                  in-string-p comment-level-p _following-quote-p
                  _min-paren-depth _comment-style comment-or-string-start
                  _open-parens-list _two-char-construct-syntax . _rest)
      (syntax-ppss))))

And you'll get this warning:

In compile-this-get-warning:
charlie-0ac22.el:6:66:Warning: assignment to free variable ‘_rest’

-setq is very helpful in when using the results of syntax-ppss multiple times in a function in the same scope, like https://github.com/alphapapa/prism.el/blob/428b9b3cc38b1d6cce6d230b2eab6115fd125b78/prism.el#L214