GNU bug report logs - #24354
Address some compiler warnings in undo-tree

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 24354 in the body.
You can then email your comments to 24354 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jonas Bernoulli <jonas <at> bernoul.li>
To: bug-gnu-emacs <at> gnu.org, Toby Cubitt <toby-undo-tree <at> dr-qubit.org>
Subject: Address some compiler warnings in undo-tree
Date: Fri, 02 Sep 2016 15:32:26 +0200

Hello,

The below two commits address some compiler warnings in `undo-tree',
which is available from GNU Elpa.  They should be applied to the
upstream repository, which should then be merged into its Elpa
subtree.

Earlier I have pointed out that `undo-tree' isn't currently being
properly merged into the Elpa repository.  See debbugs#24303.  It
would probably be a good idea to take care of that first.

----------------------------------------------------------------
After applying these two patches the byte-compiler still complains
about some other things:

    In undo-tree-pull-undo-in-region-branch:
    undo-tree.el:2279:36:Warning: ‘undo-elt-crosses-region’ is an obsolete
        function (as of 25.1).

    In undo-tree-pull-redo-in-region-branch:
    undo-tree.el:2476:25:Warning: ‘undo-elt-crosses-region’ is an obsolete
        function (as of 25.1).

    In end of data:
    undo-tree.el:4381:1:Warning: the function ‘gv--defsetter’ might not be
        defined at runtime.

Maybe `undo-elt-crosses-region' should not be declared to be obsolete
since `undo-tree' still uses it, or maybe `undo-tree' should stop using
it.  I don't know.

`undo-tree.el' requires `cl' and uses `defsetf' which is defined there.
The `defsetf' macro uses some functions from `gv.el', without requiring
`gv'.  Simply requiring `gv' in `undo-tree' should be avoided because
that would break compatibility with older Emacsen.  Also it does not
actually make that warning go away.  However it might be the case that
without explicitly requiring `gv' when using Emacsen that come with that
library might mean that `gv' might end up not being loaded at run-time,
and should therefore be explicitly loaded for such Emacsen.  I don't
know.

----------------------------------------------------------------
The following changes since commit a3e81b682053a81e082139300ef0a913a7a610a2
are available in the git repository at:

  git <at> github.com:tarsius/undo-tree.git silencio

for you to fetch.

----------------------------------------------------------------
Jonas Bernoulli (2):
      Use _ as VAR in calls to dotimes.
      Declare two mode variables to silence byte-compiler.

 undo-tree.el | 34 +++++++++++++++++++---------------
 1 file changed, 19 insertions(+), 15 deletions(-)

----------------------------------------------------------------
Toby, I am aware that I have suggested before that you use `_' as VAR in
calls to `dotimes' instead of `i'.  You rejected that suggestion,
claiming that the byte-compiler complaining about `i' not being used is
a bug.

That is not so, it is a feature, which is helpful when you accidentially
don't use `i'.  The byte-compiler *not* complaining when you use `_'
instead is also a feature, it allows using `dotimes' even when you
intentionally don't use `_' without getting these warnings which would
be bogus in this case.  This is not unique to `dotimes' and I think that
now that `undo-tree' is part of Emacs/Elpa you should respect those
conventions / use those features, even if you should think that they are
misguided.

  Best regards,
  Jonas

Message #12 received at 24354 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Alex Branham <alex.branham <at> gmail.com>
Cc: 30235 <at> debbugs.gnu.org, 24354 <at> debbugs.gnu.org
Subject: Re: bug#30235: [patch] undo-tree byte-compiler warnings
Date: Tue, 25 Jun 2019 13:51:14 +0200

Alex Branham <alex.branham <at> gmail.com> writes:

> I'm not sure if this is the right place to submit patches for ELPA
> packages, but I couldn't find anywhere else to go. Let me know if this
> should go some place else.
>
> This patch silences several byte compiler warnings, mostly having to do
> with unused lexical variables in loops.

The unused dolist fixes look OK, but they could also be rewritten as

(cl-loop repeat ...
         do ...)

to avoid the binding altogether.

The other changes (time-subtract/subtract-time) I'm not so sure about,
because ELPA things are supposed to work across a large number of Emacs
versions, and time-subtract is newer than subtract-time.

The same with the other obsolete functions -- you're bound to get those
warnings in ELPA packages.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #15 received at 24354 <at> debbugs.gnu.org (full text, mbox):

From: Richard Stallman <rms <at> gnu.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: alex.branham <at> gmail.com, 30235 <at> debbugs.gnu.org, 24354 <at> debbugs.gnu.org
Subject: Re: bug#30235: [patch] undo-tree byte-compiler warnings
Date: Tue, 25 Jun 2019 23:24:32 -0400

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > The unused dolist fixes look OK, but they could also be rewritten as

  > (cl-loop repeat ...
  >          do ...)

  > to avoid the binding altogether.

Warnings are meant to be our helpers, not our masters.
We should not get in the habit of jumping through hoops
to satisfy a demanding, rigid program.

-- 
Dr Richard Stallman
President, Free Software Foundation (https://gnu.org, https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.