Re: Example use of macro to minimize and generalize the code

[Yuri Khan]

On Wed, 2 Jun 2021 at 00:27, Jean Louis <bugs@gnu.support> wrote:

> I have removed `id' argument and now I have:
>
> (macroexpand '(when-tabulated-id "people" (ignore)))
> ⇒ (if id (progn (ignore))
>     (if (or (eq "people" 'any) (string-equal "people" rcd-current-table))
>         (let ((id (tabulated-list-get-id)))
>           (if id (progn (ignore)) (message "Did not get ID")))
>       (message "This function is for table `%s' only" "people")))
>
> If I would use ,d it would expand into number or nil, it seem it would end up 
> same.
>
> Now I don't have compiler warnings, and functions work fine.
>
> Let me know if I am making errors in this.

Yes you are. Your macro expands to a form that refers to a symbol
named “id” from outer scope. Next, there are several possibilities:

* The symbol is not defined, in which case the result of expansion
will signal (void-variable id):

    (when-tabulated-id "people" (ignore))
    ⇒ Debugger entered--Lisp error: (void-variable id)

* The user of your macro has to define a variable named “id” globally
or buffer-locally. This is bad because the name “id” is not
namespaced. In your own package, any variable names you define should
start with a prefix unique to your package.

    (setq id t)
    (when-tabulated-id "people" (ignore))
    ⇒ nil

* The user has to use your macro in a context where “id” is bound as a
local variable or a function argument. If that’s your intent, you
should document it in the docstring of the macro, but it’s fragile
because nobody expects forms to behave this way.

    (let ((id t))
      (when-tabulated-id "people" (ignore)))
    ⇒ nil

    (defun foo (id)
      (when-tabulated-id "people" (ignore)))
    (foo t)
    ⇒ nil

* Lastly, you might have intended this macro to be used only by you.
In that case, it should be namespaced to your package with a double
hyphen after the prefix. Anyway, three month later, you forget that
the macro depends on “id” and you stare at your (let ((id t)) …) or
(defun foo (id) …) which doesn’t obviously use “id” and remove the
binding. Suddenly everything breaks.