unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Re: [nongnu] elpa/racket-mode 363246ac70 1/2: Fix cl-loop byte-compiler warnings
       [not found] ` <20230917170003.62A4FC05816@vcs2.savannah.gnu.org>
@ 2023-09-17 18:09   ` Stefan Monnier
  2023-09-18 14:29     ` Greg Hendershott
  0 siblings, 1 reply; 5+ messages in thread
From: Stefan Monnier @ 2023-09-17 18:09 UTC (permalink / raw)
  To: Greg Hendershott; +Cc: emacs-devel

>     Fix cl-loop byte-compiler warnings
>     
>     Frustratingly, the work-around of introducing a dummy `let` binding
>     for the cl-loop index variable works on older versions of Emacs... but
>     on bleeding edge Emacs starting c. a week ago starts to cause a
>     different warning unless the variable is used.

I plead guilty.  Sorry 'bout that.

Note that your workaround for the workaround could receive a warning in
the future (that code looks very odd and we were recently talking about
adding warnings for such unusual codes where an expression whose return
value is ignored consist in just a constant or a variable lookup).

So you might like to pass your dummy references to `i` to `ignore`,
which is the "standard" idiom to say "don't bother warning me if I don't
use this variable" and which the compiler already treats specially.

>     So to make this work across all versions (so far) of Emacs, we need a
>     work-around for the work-around.

The upside is that you should be able to drop the workaround as soon as
you don't need to support Emacs<30 any more :-)

You can also use a silly macro

    (defmacro my-workaround-for-cl-loop-bug65833 (var &rest body)
      (if (>= emacs-major-version 30)
          (macroexp-progn body)
        `(let (,var) ,@body)))


-- Stefan




^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [nongnu] elpa/racket-mode 363246ac70 1/2: Fix cl-loop byte-compiler warnings
  2023-09-17 18:09   ` [nongnu] elpa/racket-mode 363246ac70 1/2: Fix cl-loop byte-compiler warnings Stefan Monnier
@ 2023-09-18 14:29     ` Greg Hendershott
  2023-09-18 16:18       ` Stefan Monnier
  0 siblings, 1 reply; 5+ messages in thread
From: Greg Hendershott @ 2023-09-18 14:29 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel


>>     Fix cl-loop byte-compiler warnings
>>
>>     Frustratingly, the work-around of introducing a dummy `let` binding
>>     for the cl-loop index variable works on older versions of Emacs... but
>>     on bleeding edge Emacs starting c. a week ago starts to cause a
>>     different warning unless the variable is used.
>
> I plead guilty.  Sorry 'bout that.

No worries. That's an expected cost of enabling warnings-as-errors,
combined with having CI that runs tests against the main branch.

> Note that your workaround for the workaround could receive a warning in
> the future (that code looks very odd and we were recently talking about
> adding warnings for such unusual codes where an expression whose return
> value is ignored consist in just a constant or a variable lookup).
>
> So you might like to pass your dummy references to `i` to `ignore`,
> which is the "standard" idiom to say "don't bother warning me if I don't
> use this variable" and which the compiler already treats specially.

Ah. I tend to forget about `ignore` as a signal, or at least confuse it
with an "ignore" from some other lisp where it's just an s-expression
comment form.

>>     So to make this work across all versions (so far) of Emacs, we need a
>>     work-around for the work-around.
>
> The upside is that you should be able to drop the workaround as soon as
> you don't need to support Emacs<30 any more :-)
>
> You can also use a silly macro
>
>     (defmacro my-workaround-for-cl-loop-bug65833 (var &rest body)
>       (if (>= emacs-major-version 30)
>           (macroexp-progn body)
>         `(let (,var) ,@body)))

Does that mean `cl-loop` will handle this itself in its expansion?
Because that was going to be my Captain Obvious suggestion. :)

p.s. Maybe there's even a way to address this for macros in general? I
don't know Emacs Lisp expansion internals, but could macroexpand add
whatever remediation(s) like `ignore` that may be required -- and that's
one place to adjust as the optimizer or warnings are fine-tuned from
time to time? The theory being that such warnings are N/A and
non-actionable to _users_ of a macro. Just maybe the author. But...
maybe I'm over-thinking and/or misunderstanding the problem here.



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [nongnu] elpa/racket-mode 363246ac70 1/2: Fix cl-loop byte-compiler warnings
  2023-09-18 14:29     ` Greg Hendershott
@ 2023-09-18 16:18       ` Stefan Monnier
  2023-09-18 18:46         ` Greg Hendershott
  0 siblings, 1 reply; 5+ messages in thread
From: Stefan Monnier @ 2023-09-18 16:18 UTC (permalink / raw)
  To: Greg Hendershott; +Cc: emacs-devel

>> You can also use a silly macro
>>
>>     (defmacro my-workaround-for-cl-loop-bug65833 (var &rest body)
         (declare (indent 1) (debug (symbolp body)))
>>       (if (>= emacs-major-version 30)
>>           (macroexp-progn body)
>>         `(let (,var) ,@body)))
>
> Does that mean `cl-loop` will handle this itself in its expansion?

No, you'd have to manually wrap your `cl-loop` with

    (my-workaround-for-cl-loop-bug65833 i
      ...)

:-(

> p.s. Maybe there's even a way to address this for macros in general?

I don't think there can be.

> time to time? The theory being that such warnings are N/A and
> non-actionable to _users_ of a macro. Just maybe the author.

The macroexpander can't know whether the use of a free `i` variable is
part of the official documented behavior of the macro (and hence it's
a bug in the user's code that forgot to let-bind that var) or a bug in
the macro :-(


        Stefan




^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [nongnu] elpa/racket-mode 363246ac70 1/2: Fix cl-loop byte-compiler warnings
  2023-09-18 16:18       ` Stefan Monnier
@ 2023-09-18 18:46         ` Greg Hendershott
  2023-09-18 20:36           ` Stefan Monnier
  0 siblings, 1 reply; 5+ messages in thread
From: Greg Hendershott @ 2023-09-18 18:46 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

Genuine apologies for not really knowing the mechanics of Emacs Lisp
macro expansion. Or the specific implementation of cl-loop... or even
really much of its usage beyond a few examples I've tried in Emacs Lisp.

I guess my dumb question is this:

When I pp-macroexpand one example from my code:

    (cl-loop for i being the intervals from beg to end
             do
             (racket--region-set-face (car i) (cdr i)
                                      (funcall func
                                               (or (get-text-property (car i) 'face)
                                                   'default))
                                      'force))

I get:

    (cl-block nil
      (cl-block --cl-finish--
        (cl--map-intervals
         (lambda
           (--cl-var1-- --cl-var2--)
           (setq i
                 (cons --cl-var1-- --cl-var2--))
           (racket--region-set-face
            (car i)
            (cdr i)
            (funcall func
                     (or
                      (get-text-property
                       (car i)
                       'face)
                      'default))
            'force))
         nil nil beg end))
      nil)

If instead of `setq` cl-loop expanded to use `let`

         (lambda
           (--cl-var1-- --cl-var2--)
           (let ((i (cons --cl-var1-- --cl-var2--))
             (racket--region-set-face
              (car i)
              (cdr i)
              (funcall func
                       (or
                        (get-text-property
                         (car i)
                         'face)
                        'default))
              'force))))

would that avoid the byte-compiler warning? If not, can it add an
`ignore`?

I guess my general question is: If I can hand-write code to avoid the
warning, and macros can write any code that I can write by hand, why
can't cl-loop do so for me?



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [nongnu] elpa/racket-mode 363246ac70 1/2: Fix cl-loop byte-compiler warnings
  2023-09-18 18:46         ` Greg Hendershott
@ 2023-09-18 20:36           ` Stefan Monnier
  0 siblings, 0 replies; 5+ messages in thread
From: Stefan Monnier @ 2023-09-18 20:36 UTC (permalink / raw)
  To: Greg Hendershott; +Cc: emacs-devel

> If instead of `setq` cl-loop expanded to use `let`
>
>          (lambda
>            (--cl-var1-- --cl-var2--)
>            (let ((i (cons --cl-var1-- --cl-var2--))
>              (racket--region-set-face
>               (car i)
>               (cdr i)
>               (funcall func
>                        (or
>                         (get-text-property
>                          (car i)
>                          'face)
>                         'default))
>               'force))))

Yes, that's pretty much what Emacs-30 generates now.

> would that avoid the byte-compiler warning?

Yup.

> If not, can it add an `ignore`?

Yes and no:

- It can add one within the `let` but that would be useless
  because the internal `i` *is* used already.
- It can't really add one outside the `let` because that would cause
  warnings again if the surrounding code doesn't use an extra `let` to
  define that (external) `i`.

> I guess my general question is: If I can hand-write code to avoid the
> warning, and macros can write any code that I can write by hand, why
> can't cl-loop do so for me?

Because as human you get to make a decision based on more information
than the macro so you'd sometimes put an `ignore` and sometimes not but
the macro can't guess when to put one and when not to.


        Stefan




^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-09-18 20:36 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <169497000203.14597.6218117918193558526@vcs2.savannah.gnu.org>
     [not found] ` <20230917170003.62A4FC05816@vcs2.savannah.gnu.org>
2023-09-17 18:09   ` [nongnu] elpa/racket-mode 363246ac70 1/2: Fix cl-loop byte-compiler warnings Stefan Monnier
2023-09-18 14:29     ` Greg Hendershott
2023-09-18 16:18       ` Stefan Monnier
2023-09-18 18:46         ` Greg Hendershott
2023-09-18 20:36           ` Stefan Monnier

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).