How to fix byte compiler warnings for "Unused lexical variable" in a macro?

Question

I have a macro and a function which uses it:

(defmacro oauth2-auto--query-case (&rest cases)
  "Handle HTTP queries based on the keys present in ‘query-alist’.
‘query-alist’ is a free variable, bound by the caller of this macro.  Each
element of CASES has the format ‘(symbols msg body)'. For each element of CASES:

- ‘symbols' is a list of at least one symbol, which should be keys in
  ‘query-alist’.
- Extract and bind keys `symbols' from `query-alist'.
- If all of them are present, respond with `msg' and runs `body'.

For example of usage see ‘oauth2-auto--httpd-filter’."
  (declare
   (debug (&rest ((symbolp &rest symbolp) form &rest form))))
  `(cond
    ,@(mapcar (lambda (case)
                (let ((symbols (car case))
                      (msg (cadr case))
                      (body (cddr case)))
                  `((and ,@(--map `(cdr (assoc ',it query-alist)) symbols))
                    (let* (,@(--map `(,it (cdr (assoc ',it query-alist))) symbols)
                           (msg ,msg))
                      (oauth2-auto--httpd-respond process msg)
                      ,@body))))
              cases)))

(defun oauth2-auto--httpd-filter (process input)
  "The HTTP handler for the OAuth2 challenge-response server.
PROCESS is the server process created in ‘oauth2-auto--browser-request’.
INPUT is the raw HTTP request."
  (let ((query-alist
         (with-temp-buffer
           (insert input)
           (goto-char (point-min))
           (re-search-forward
            "^[[:space:]]*GET[[:space:]]+[/?]+\\([[:graph:]]*\\)[[:space:]]+HTTP/[0-9.]+[[:space:]]*$")
           (mapcar
            (lambda (it) (cons (intern (car it)) (cadr it)))
            (url-parse-query-string (match-string 1))))))
    (oauth2-auto--query-case
     ((error error_description)
      (format "Error %s: %s" error error_description)
      (error msg)
      nil)
     ((code state)
      "Authentication token successfully obtained by Emacs! You may close this page now."
      query-alist)
     ((favicon.ico)
      ""
      nil)  ; just return empty list if favicon.ico is requested
     (()
      (format "Could not parse query string %s" (pp-to-string query-alist))
      (error msg)
      nil))))

Despite my declare declaration in the macro, however, I still get byte compilation errors when I try to use the macro:

In oauth2-auto--httpd-filter:
oauth2-auto.el:358:13: Warning: Unused lexical variable `code'
oauth2-auto.el:358:13: Warning: Unused lexical variable `state'
oauth2-auto.el:361:8: Warning: Unused lexical variable `favicon\.ico'

What is the right way to declare the macro to get rid of the byte compilation errors?

EDIT: I'm running Emacs 28.1. Here is the first macroexpand-step of oauth2-auto--query-case in oauth2-auto--httpd-filter:

(defun oauth2-auto--httpd-filter (process input)
  "The HTTP handler for the OAuth2 challenge-response server.
PROCESS is the server process created in ‘oauth2-auto--browser-request’.
INPUT is the raw HTTP request."
  (let ((query-alist
         (with-temp-buffer
           (insert input)
           (goto-char (point-min))
           (re-search-forward
            "^[[:space:]]*GET[[:space:]]+[/?]+\\([[:graph:]]*\\)[[:space:]]+HTTP/[0-9.]+[[:space:]]*$")
           (mapcar
            (lambda (it) (cons (intern (car it)) (cadr it)))
            (url-parse-query-string (match-string 1))))))
    (cond
     ((and
       (cdr
        (assoc 'error query-alist))
       (cdr
        (assoc 'error_description query-alist)))
      (let*
          ((error
            (cdr
             (assoc 'error query-alist)))
           (error_description
            (cdr
             (assoc 'error_description query-alist)))
           (msg
            (format "Error %s: %s" error error_description)))
        (oauth2-auto--httpd-respond process msg)
        (error msg)
        nil))
     ((and
       (cdr
        (assoc 'code query-alist))
       (cdr
        (assoc 'state query-alist)))
      (let*
          ((code
            (cdr
             (assoc 'code query-alist)))
           (state
            (cdr
             (assoc 'state query-alist)))
           (msg "Authentication token successfully obtained by Emacs! You may close this page now."))
        (oauth2-auto--httpd-respond process msg)
        query-alist))
     ((and
       (cdr
        (assoc 'favicon\.ico query-alist)))
      (let*
          ((favicon\.ico
            (cdr
             (assoc 'favicon\.ico query-alist)))
           (msg ""))
        (oauth2-auto--httpd-respond process msg)
        nil))
     ((and)
      (let*
          ((msg
            (format "Could not parse query string %s"
                    (pp-to-string query-alist))))
        (oauth2-auto--httpd-respond process msg)
        (error msg)
        nil)))))

Which version of Emacs? The byte-compilation errors I see when I byte-compile that are: In oauth2-auto--query-case: test.el:26:57:Warning: reference to free variable ‘it’ test.el:32:1:Error: Symbol’s value as variable is void: it" — phils, Jul 25 '22 at 22:29
Also, your only `declare` form is `(debug...)` which is relevant only when stepping through the macro with edebug, so your references to `declare` are also confusing. — phils, Jul 25 '22 at 22:32
Please review and update your question so that it makes sense. — phils, Jul 25 '22 at 22:33
@phils Is there a way to declare to the byte compiler that some argument to a macro should not be interpreted as a variable, but as a literal symbol? For example, `(setq foo "bar")` does not require quoting `foo`. — telotortium, Jul 25 '22 at 22:53
ALL arguments to macros are unevaluated. They get evaluated only if you explicitly evaluate them. So if you pass a symbol argument, it is treated similarly to `foo` in `(setq foo "bar")`. I suggest you macroexpand your call to your macro to see what you've actually ended up with, and (again) fix the question so that it aligns with the code being shown. — phils, Jul 25 '22 at 22:58
Also, an additional tip: you should probably avoid declaring the `msg` variable, since that could shadow an existing variable that the user wants to use. I recommend using `gensym` to create a new variable name that won’t conflict with any in use by the user. This is more complicated and makes the code harder to read, but it is better than the alternative. — db48x, Jul 26 '22 at 05:37

score 1 · Answer 1 · answered Jul 26 '22 at 05:07

The first two errors come from here:

(let*
    ((code
      (cdr
       (assoc 'code query-alist)))
     (state
      (cdr
       (assoc 'state query-alist)))
     (msg "Authentication token successfully obtained by Emacs! You may close this page now."))
  (oauth2-auto--httpd-respond process msg)
  query-alist)

Note how it defines several lexical variables, but then never uses two of them, state and code. Change the code inserted into the body of this let* so that it either uses all the variables it declares, avoids declaring variables it doesn’t use, or uses ignore to mark them as deliberately ignored and then the warnings will go away.

Likewise, the third warning comes from here:

(let*
    ((favicon\.ico
      (cdr
       (assoc 'favicon\.ico query-alist)))
     (msg ""))
  (oauth2-auto--httpd-respond process msg)
  nil)

Same problem, same fix.

score 0 · Answer 2 · answered Jul 26 '22 at 05:05

As @phils said above, the (declare (debug (&rest ((symbolp &rest symbolp) form &rest form)))) form has nothing to do with this error. The error was actually apparent from the macroexpansion - the code, state, and favicon.ico variables declared in the let* of the macroexpansion were in fact unused, causing the error. To fix this error, I've added an ignore call to the macro to suppress this error:

(defmacro oauth2-auto--query-case (&rest cases)
  "Handle HTTP queries based on the keys present in ‘query-alist’.
‘query-alist’ is a free variable, bound by the caller of this macro.  Each
element of CASES has the format ‘(symbols msg body)'. For each element of CASES:

- ‘symbols' is a list of at least one symbol, which should be keys in
  ‘query-alist’.
- Extract and bind keys `symbols' from `query-alist'.
- If all of them are present, respond with `msg' and runs `body'.

For example of usage see ‘oauth2-auto--httpd-filter’."
  (declare
   (debug (&rest ((symbolp &rest symbolp) form &rest form))))
  `(cond
    ,@(mapcar (lambda (case)
                (let ((symbols (car case))
                      (msg (cadr case))
                      (body (cddr case)))
                  `((and ,@(--map `(cdr (assoc ',it query-alist)) symbols))
                    (let* (,@(--map `(,it (cdr (assoc ',it query-alist))) symbols)
                           (msg ,msg))
                      ;; **ADDED**: ‘ignore’ suppresses byte compiler warnings if the macro
                      ;; caller doesn’t use the variables declared in the ‘let*’
                      ;; above.
                      (ignore msg ,@symbols)
                      (oauth2-auto--httpd-respond process msg)
                      ,@body))))
              cases)))

I think this is a poor choice. By inserting a blanket `ignore` of every variable, you are denying a useful diagnostic tool to the user of the macro. The warning is trying to tell the user that they left something important out of their code; if they want to ignore the warning then make them put in an explicit `ignore` statement. — db48x, Jul 26 '22 at 05:32
If the variables are unused, why not edit the code to not define them? — phils, Jul 26 '22 at 06:28
Failing that, deliberately-unused lexical variables can be renamed with the prefix `_` to make the byte-compiler not complain about them. *Usually* that would be for function arguments or in destructuring statements where you can't simply not include them; but you *can* use it for other situations if you really want to. — phils, Jul 27 '22 at 08:00

How to fix byte compiler warnings for "Unused lexical variable" in a macro?

2 Answers2