Re: [elpa] externals/ebdb f7b50402cf: Final round of compiler quieting

[Eric Abrahamsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>     Not all the way there, but I don't know what to do about the format
>>     complaints.
>
> The suspect that problem is that your definition of `ebdb-add-to-list`
> duplicates its argument, so
>
>     (ebdb-add-to-list X (format FOO BAR))
>
> becomes
>
>     (when (format FOO BAR) (push (format FOO BAR) X))
>
> Since `format` always returns a string, the `when` can be optimized away
> so the above becomes:
>
>     (progn (format FOO BAR) (push (format FOO BAR) X))
>
> Beside the useless call to `format` there are other minor problems with
> this definition of `ebdb-add-to-list`, such as the fact that its
> arguments are not evaluated in the expected order (left to right), and
> the fact that the second argument gets evaluated twice, so:
>
>     (ebdb-add-to-list (progn (message "hello") 'x)
>                       (progn (message "world") 3))
>
> ends up adding 3 to x and emitting
>
>     world
>     world
>     hello
>
> instead of
>
>     hello
>     world
>
> But the more serious problem is that `ebdb-add-to-list` is defined as
> a function yet it can't work as a function because its first argument is
> expected to be a "place" not a value.
> So you have to define it as a macro rather than a (define-inline) function.
>
> Also, personally I'd recommend you rename the to use a word like "push"
> (and to switch its args) rather than `add-to-list`, so the reader can
> guess that it takes an place, like `(cl-)push(new)`, rather than
> a symbol, like `add-to-list`.

Thank you very much for the detailed explanation, and for the patch!
This is an area of Elisp I still don't have much feel for, and I don't
think would have come up with the right gv-/macroexp- invocations
myself.

I'll get this released in the next few days.

Thanks again,
Eric