possibly spurious bytecomp warning:

possibly spurious bytecomp warning:

[T.V Raman]

Building my own libs with emacs 30 built from @HEAD, I get this
warning that looks spurious:


emacspeak-eww.el:599:7: Warning: value from call to ‘delete’ is unused

-- 

Thanks,

--Raman(I Search, I Find, I Misplace, I Research)
♉ Id: kg:/m/0285kf1  🦮

-- 

Thanks,

--Raman(I Search, I Find, I Misplace, I Research)
♉ Id: kg:/m/0285kf1  🦮

Re: possibly spurious bytecomp warning:

[Michael Heerdegen]

"T.V Raman" <raman@google.com> writes:

> Building my own libs with emacs 30 built from @HEAD, I get this
> warning that looks spurious:
>
> emacspeak-eww.el:599:7: Warning: value from call to ‘delete’ is unused

A new type of compiler warnings had been added recently, this looks like
it.  See 8c9377b6c4e "Try declaring `delq` and `delete`
important-return-value (bug#61730)".

These warn about calls of `delq' and `delete' whose return value is not
used: a common pitfall is to expect that these functions could be used to
modify a sequence in place, which is only correct in few cases.

Michael.

Re: possibly spurious bytecomp warning:

[Robin Tarsiger]

Michael Heerdegen wrote:
> "T.V Raman" <raman@google.com> writes:
>> emacspeak-eww.el:599:7: Warning: value from call to ‘delete’ is unused
> 
> These warn about calls of `delq' and `delete' whose return value is not
> used: a common pitfall is to expect that these functions could be used to
> modify a sequence in place, which is only correct in few cases.

Yes. In the context it's in, I believe it's actually subtly correct, though
doing the setq would be much clearer. Here's the form I see around that
line (arrow mine):

   (cl-loop
    for c in
    '(?I ?o)
    do
    (when (assoc  c eww-link-keymap)
      (delete (assoc  c eww-link-keymap) eww-link-keymap))) ; <--

Here, the list being modified is a keymap, so the first element
is the keymap type symbol and assoc never returns it. And notably,
delete's _docstring_ doesn't guarantee in-place modification for
non-head deletions, but the Info node for delq/delete describes both
functions as doing cdr splicing in that case, so assuming that's meant
as normative, this will operate correctly.

It probably wouldn't be reasonable for bytecomp to try to distinguish
this, though...

-RTT

Re: possibly spurious bytecomp warning:

[Michael Heerdegen]

Robin Tarsiger <rtt@dasyatidae.com> writes:

>   (cl-loop
>    for c in
>    '(?I ?o)
>    do
>    (when (assoc  c eww-link-keymap)
>      (delete (assoc  c eww-link-keymap) eww-link-keymap))) ; <--
>
> Here, the list being modified is a keymap, so the first element
> is the keymap type symbol and assoc never returns it. And notably,
> delete's _docstring_ doesn't guarantee in-place modification for
> non-head deletions, but the Info node for delq/delete describes both
> functions as doing cdr splicing in that case, so assuming that's meant
> as normative, this will operate correctly.

I think this is correct.

Maybe using higher-level functions for keymaps (e.g. `lookup-key') would
be better.

> It probably wouldn't be reasonable for bytecomp to try to distinguish
> this, though...

I would not harm if it could, but how would it do that?

Michael.

Re: possibly spurious bytecomp warning:

[T.V Raman]

Thanks for the explanation -- makes sense. Perhaps we should have a
keymap related API call that allows one to remove keymap entries?
-- 

Thanks,

--Raman(I Search, I Find, I Misplace, I Research)
♉ Id: kg:/m/0285kf1  🦮

--

Thanks,

--Raman(I Search, I Find, I Misplace, I Research)
♉ Id: kg:/m/0285kf1  🦮

Re: possibly spurious bytecomp warning:

[Michael Heerdegen]

"T.V Raman" <raman@google.com> writes:

> Thanks for the explanation -- makes sense. Perhaps we should have a
> keymap related API call that allows one to remove keymap entries?

Doesn't `define-key' suffice?


Let me add that these warnings were discussed in
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=61730

If anyone has a considered opinion why such warnings are a good/ a bad
idea: It is not yet cast in concrete.

Michael.

Re: possibly spurious bytecomp warning:

[T.V Raman]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=gb18030, Size: 436 bytes --]

Having now fixed my code, I think this warning is good and useful:-)

re define-key -- not sure how I would use that to remove a keybinding.

In my usage  I wanted to unbind the keys in the eww link map which gets
applied as a  keymap property -- since it was shadowing a binding that I
wanted everywhere in the EWW buffer.

-- 

Thanks,

--Raman(I Search, I Find, I Misplace, I Research)
7©4 Id: kg:/m/0285kf1  •0Ü8

Re: possibly spurious bytecomp warning:

[Michael Heerdegen]

"T.V Raman" <raman@google.com> writes:

> re define-key -- not sure how I would use that to remove a keybinding.

> In my usage  I wanted to unbind the keys in the eww link map which gets
> applied as a  keymap property -- since it was shadowing a binding that I
> wanted everywhere in the EWW buffer.

`define-key' to nil should be good enough - AFAIR, a nil entry doesn't
shadow other keymaps.

`define-key' also has an optional REMOVE argument that actually removes
the binding and (see docstring) "makes a difference if the KEYMAP has a
parent, and KEY is shadowing the same binding in the parent.  With
REMOVE, subsequent lookups will return the binding in the parent, and
with a nil DEF, the lookups will return nil."  So this is only necessary
when your keymap has a parent.

Oh, and now there is also the new `keymap-unset' with a slightly different
interface.

Michael.

Re: possibly spurious bytecomp warning:

[T.V Raman]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=gb18030, Size: 216 bytes --]

keymap-unset is the new API I was asking for -- had forgotten to look at
the newly arrived keymap-* calls.


-- 

Thanks,

--Raman(I Search, I Find, I Misplace, I Research)
7©4 Id: kg:/m/0285kf1  •0Ü8