bug#56641: Deprecate `lsh`

bug#56641: Deprecate `lsh`

[Mattias Engdegård]

The function `lsh` works almost like `ash` but is much slower, cannot be constant-folded by the compiler, and above all has confusing semantics for historical reasons. Using `lsh` instead of `ash` is always a mistake.

For these reasons we should deprecate `lsh`. It is not important to remove it as soon as possible, but to dissuade people from accidentally using it in their code.

bug#56641: Deprecate `lsh`

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Tue, 19 Jul 2022 15:38:29 +0200
> 
> The function `lsh` works almost like `ash` but is much slower, cannot be constant-folded by the compiler, and above all has confusing semantics for historical reasons. Using `lsh` instead of `ash` is always a mistake.
> 
> For these reasons we should deprecate `lsh`. It is not important to remove it as soon as possible, but to dissuade people from accidentally using it in their code.

The ELisp manual already says that ash is a better alternative, but
lsh has its niche, and I see no reason to deprecate it more than the
manual already does.

So, no, please let's not.

bug#56641: Deprecate `lsh`

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> The ELisp manual already says that ash is a better alternative, but
> lsh has its niche,

When would I want to use `lsh' over `ash'?

bug#56641: Deprecate `lsh`

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Tue, 19 Jul 2022 10:44:18 -0400
> Cc: 56641@debbugs.gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > The ELisp manual already says that ash is a better alternative, but
> > lsh has its niche,
> 
> When would I want to use `lsh' over `ash'?

When you want the results that lsh gives you.

They don't yield the same results in all situations.

bug#56641: Deprecate `lsh`

[Mattias Engdegård]

19 juli 2022 kl. 16.44 skrev Stefan Kangas <stefankangas@gmail.com>:

> When would I want to use `lsh' over `ash'?

Never, of course, but we shouldn't remove `lsh` as long as it's needed for compatibility with old packages that make pre-bignum assumptions, which is the reason for its existence (and semantics) in the first place.

And that's fine -- `lsh` can be kept indefinitely. However, I just removed several calls to `lsh` from new code in Emacs so evidently we need to up our information efforts a notch.

Adding a few words to the doc string may suffice, but a byte-compiler warning would help.

bug#56641: Deprecate `lsh`

[Stefan Kangas]

Mattias Engdegård <mattiase@acm.org> writes:

> Adding a few words to the doc string may suffice, but a byte-compiler
> warning would help.

What we have done in such cases is declare it obsolete but add a comment
that we want to keep it around for longer.  See `interactive-p', that
was not deleted in Emacs 28 despite having been obsolete for over a
decade.

bug#56641: Deprecate `lsh`

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Tue, 19 Jul 2022 12:20:01 -0400
> Cc: Eli Zaretskii <eliz@gnu.org>, 56641@debbugs.gnu.org
> 
> Mattias Engdegård <mattiase@acm.org> writes:
> 
> > Adding a few words to the doc string may suffice, but a byte-compiler
> > warning would help.
> 
> What we have done in such cases is declare it obsolete but add a comment
> that we want to keep it around for longer.  See `interactive-p', that
> was not deleted in Emacs 28 despite having been obsolete for over a
> decade.

This is not what "obsolete" means and is about, so let's not abuse it
for this purpose.

I'm okay with introducing some new special attribute for such cases,
i.e. when the use of a function or a variable is discouraged for
whatever reasons, so that the byte-compiler could warn about that.
Just let's not called them "obsolete", because they aren't.

bug#56641: Deprecate `lsh`

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> I'm okay with introducing some new special attribute for such cases,
> i.e. when the use of a function or a variable is discouraged for
> whatever reasons, so that the byte-compiler could warn about that.
> Just let's not called them "obsolete", because they aren't.

Having the byte compiler warn when someone uses lsh would be very
annoying.

bug#56641: Deprecate `lsh`

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: Stefan Kangas <stefankangas@gmail.com>,  mattiase@acm.org,
>   56641@debbugs.gnu.org
> Date: Wed, 20 Jul 2022 09:25:49 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I'm okay with introducing some new special attribute for such cases,
> > i.e. when the use of a function or a variable is discouraged for
> > whatever reasons, so that the byte-compiler could warn about that.
> > Just let's not called them "obsolete", because they aren't.
> 
> Having the byte compiler warn when someone uses lsh would be very
> annoying.

If it's rare enough (as it should be), I don't see the annoyance.  Am
I missing something?

bug#56641: Deprecate `lsh`

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> If it's rare enough (as it should be), I don't see the annoyance.  Am
> I missing something?

It might be rare, but there's nothing wrong with using lsh for its
intended purpose, which is handling negative numbers correctly in
several different situations.

Byte-compiler warnings are supposed to point out problems.  Using lsh is
not a problem, using lsh incorrectly is, so it should not issue warnings
for all uses of lsh.

bug#56641: Deprecate `lsh`

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: stefankangas@gmail.com,  mattiase@acm.org,  56641@debbugs.gnu.org
> Date: Wed, 20 Jul 2022 11:20:14 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > If it's rare enough (as it should be), I don't see the annoyance.  Am
> > I missing something?
> 
> It might be rare, but there's nothing wrong with using lsh for its
> intended purpose, which is handling negative numbers correctly in
> several different situations.

The point is that people could use lsh because they don't realize they
could use ash instead.  If one really must use lsh, there's always
with-no-warnings, which is IMO fine for the rare situations where it's
needed.

> Byte-compiler warnings are supposed to point out problems.

And this could indeed be a problem: that the programmer is not aware
of a better alternative.

So I don't see a problem with warning in this case, I just don't want
us to pretend this function is "obsolete", because it isn't.

bug#56641: Deprecate `lsh`

[Mattias Engdegård]

20 juli 2022 kl. 05.20 skrev Po Lu <luangruo@yahoo.com>:

> It might be rare, but there's nothing wrong with using lsh for its
> intended purpose, which is handling negative numbers correctly in
> several different situations.

The purpose of `lsh` is not breaking old code. (It used to have a different purpose but it vanished when bignums were introduced.)
There is no reasonable use for it in new code. Even in the unlikely event of code that just happens to need that sort of quirky behaviour, it is much better implemented explicitly where it's used, in terms of `ash`.

> Byte-compiler warnings are supposed to point out problems.  Using lsh is
> not a problem

You are right that most uses of `lsh` are perfectly innocuous but that's because they are used in a way where `ash` would give the same result. This is often true even when the return values of `lsh` and `ash` differ, as in

  (logand (lsh x -16) #xff)

which would give the same result even for negative x if `lsh` were replaced with `ash`.

However there is a potential problem since the behaviour of `lsh` is strongly platform-dependent, and it's impossible to write correct code without taking that into account (consider replacing -16 with -24 above). Users may not be aware of how it works and mistakenly think it is the 'unsigned' or 'logical' shift that they know from other languages.

bug#56641: Deprecate `lsh`

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Mattias Engdegård <mattiase@acm.org> writes:

> The purpose of `lsh` is not breaking old code. (It used to have a
> different purpose but it vanished when bignums were introduced.)
> There is no reasonable use for it in new code.

So you're saying there is no possible use for expecting the value of
this:

  (lsh most-negative-fixnum -1)

to become:

  #x1000000000000000

> Even in the unlikely event of code that just happens to need that sort
> of quirky behaviour, it is much better implemented explicitly where
> it's used, in terms of `ash`.

Why is that better? What business does the byte compiler, or some idea
of "platform independence" have in dictating that I should write extra
code (instead of using something built-in) to implement a network
protocol?

bug#56641: Deprecate `lsh`

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  stefankangas@gmail.com,
>   56641@debbugs.gnu.org
> Date: Wed, 20 Jul 2022 20:21:59 +0800
> 
> What business does the byte compiler, or some idea of "platform
> independence" have in dictating that I should write extra code
> (instead of using something built-in) to implement a network
> protocol?

It isn't the byte compiler, it's us the Emacs developers.

And no, you don't need to write any extra code, as long as you are
prepared to ignore the warnings.

bug#56641: Deprecate `lsh`

[Mattias Engdegård]

20 juli 2022 kl. 14.21 skrev Po Lu <luangruo@yahoo.com>:

> So you're saying there is no possible use for expecting the value of
> this:
> 
>  (lsh most-negative-fixnum -1)
> 
> to become:
> 
>  #x1000000000000000

That is only true when Emacs has been built for 62-bit fixnums. And no, I don't see an obvious use other than for compatibility.

> What business does the byte compiler, or some idea
> of "platform independence" have in dictating that I should write extra
> code (instead of using something built-in) to implement a network
> protocol?

It sounds as if we are talking past one another. If you have a network protocol that is somehow defined in terms of the size of fixnums in a particular build of Emacs, then yes, I'd say that most people would think that your code should make that explicit.

But you certainly don't need lsh to handle arbitrary binary data; it's not even convenient. I'm not saying that you are wrong; perhaps I don't understand your needs.

We could add functions for depositing and extracting bit fields in an integer, basically encapsulating the common shift-and-mask operations. That would make bit field manipulation less error-prone without compromising speed or portability.

bug#56641: Deprecate `lsh`

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Mattias Engdegård <mattiase@acm.org> writes:

> That is only true when Emacs has been built for 62-bit fixnums. And
> no, I don't see an obvious use other than for compatibility.

Which is the obvious use? For compatibility with programs (not
necessarily within Emacs) that expect different values based on the
number of VALBITs in the Emacs machine file?

bug#56641: Deprecate `lsh`

[Mattias Engdegård]

21 juli 2022 kl. 03.43 skrev Po Lu <luangruo@yahoo.com>:

> Which is the obvious use? For compatibility with programs (not
> necessarily within Emacs) that expect different values based on the
> number of VALBITs in the Emacs machine file?

Honestly I don't know. Even before Emacs 27, `lsh` seems a bit questionable since the width is both platform-dependent and unlike C, say, isn't a particularly useful size. It doesn't correspond to anything in hardware, isn't a power of two or anything like that.

It could be code that expects (lsh x -29) to return the sign bit as 1 or 0, as some kind of 'optimisation', what do I know.

Also see bug#32463 for some discussion about lsh during the transition to bignums. Basically everyone agreed that it's useless but that it probably needs to be kept for compatibility in some way.

bug#56641: Deprecate `lsh`

[Lars Ingebrigtsen]

Mattias Engdegård <mattiase@acm.org> writes:

> The function `lsh` works almost like `ash` but is much slower, cannot
> be constant-folded by the compiler, and above all has confusing
> semantics for historical reasons. Using `lsh` instead of `ash` is
> always a mistake.
>
> For these reasons we should deprecate `lsh`. It is not important to
> remove it as soon as possible, but to dissuade people from
> accidentally using it in their code.

I agree.  I think the presence of `lsh' just confuses people.  Even when
Emacs didn't have bignums it was pretty odd, but I have absolutely no
idea what any theoretical use case would be today.

bug#56641: Deprecate `lsh`

[Mattias Engdegård]

There is now a compiler warning about calls to `lsh`.

Although we usually defer changes in verilog-mode.el to the maintainer for keeping in sync with the upstream version, I made the necessary (trivial) adjustments to keep warnings away.

bug#56641: Deprecate `lsh`

[Lars Ingebrigtsen]

Mattias Engdegård <mattiase@acm.org> writes:

> There is now a compiler warning about calls to `lsh`.

Eli didn't agree with that, I think?  I was just expressing an opinion,
and not, er, like saying "go forth and deprecate".  :-/  Sorry for being
unclear.

Let's see what Eli says...

bug#56641: Deprecate `lsh`

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: 56641@debbugs.gnu.org,  Wilson Snyder <wsnyder@wsnyder.org>, "'Eli
>  Zaretskii'" <eliz@gnu.org>
> Date: Sat, 23 Jul 2022 13:20:22 +0200
> 
> Mattias Engdegård <mattiase@acm.org> writes:
> 
> > There is now a compiler warning about calls to `lsh`.
> 
> Eli didn't agree with that, I think?  I was just expressing an opinion,
> and not, er, like saying "go forth and deprecate".  :-/  Sorry for being
> unclear.
> 
> Let's see what Eli says...

No, I just said the warning shouldn't be about obsoletion.  What's in
the code is fine by me, it just says to use 'ash' instead.

bug#56641: Deprecate `lsh`

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> No, I just said the warning shouldn't be about obsoletion.  What's in
> the code is fine by me, it just says to use 'ash' instead.

Ah, OK.

bug#56641: Deprecate `lsh`

[Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Mattias Engdegård [2022-07-23 12:32 +0200] wrote:

> There is now a compiler warning about calls to `lsh`.

Thanks, but the warnings are being stubborn with and not listening to
me.  Could you please have a word with them?

First, they are a bit repetitive in parallel 'make -j8 check':

In toplevel form:
lisp/international/ccl-tests.el:29:15: Warning: avoid `lsh'; use `ash' instead
lisp/international/ccl-tests.el:29:15: Warning: avoid `lsh'; use `ash' instead
lisp/international/ccl-tests.el:33:15: Warning: avoid `lsh'; use `ash' instead
lisp/international/ccl-tests.el:33:15: Warning: avoid `lsh'; use `ash' instead
lisp/international/ccl-tests.el:37:15: Warning: avoid `lsh'; use `ash' instead
lisp/international/ccl-tests.el:37:15: Warning: avoid `lsh'; use `ash' instead
lisp/international/ccl-tests.el:41:15: Warning: avoid `lsh'; use `ash' instead
lisp/international/ccl-tests.el:41:15: Warning: avoid `lsh'; use `ash' instead
lisp/international/ccl-tests.el:29:15: Warning: avoid `lsh'; use `ash' instead
lisp/international/ccl-tests.el:33:15: Warning: avoid `lsh'; use `ash' instead
lisp/international/ccl-tests.el:37:15: Warning: avoid `lsh'; use `ash' instead
lisp/international/ccl-tests.el:41:15: Warning: avoid `lsh'; use `ash' instead
lisp/international/ccl-tests.el:29:15: Warning: avoid `lsh'; use `ash' instead
lisp/international/ccl-tests.el:33:15: Warning: avoid `lsh'; use `ash' instead
lisp/international/ccl-tests.el:37:15: Warning: avoid `lsh'; use `ash' instead
lisp/international/ccl-tests.el:41:15: Warning: avoid `lsh'; use `ash' instead
lisp/international/ccl-tests.el:29:15: Warning: avoid `lsh'; use `ash' instead
lisp/international/ccl-tests.el:33:15: Warning: avoid `lsh'; use `ash' instead
lisp/international/ccl-tests.el:37:15: Warning: avoid `lsh'; use `ash' instead
lisp/international/ccl-tests.el:41:15: Warning: avoid `lsh'; use `ash' instead

Note that each warning is emitted five times.  The same happens with
src/data-tests.el.  Some ERT quirk perhaps?

Second, wrapping the respective ert-deftest bodies in
(with-suppressed-warnings ((suspicious lsh)) ...) or
(with-no-warnings ...) does not actually silence the warnings.
Interestingly, suppressing warnings around the whole ert-deftest form
eliminates the duplicate warning lines, but still emits the originals.

Third, the docstring of with-suppressed-warnings says:

  The types that can be suppressed with this macro are
  `free-vars', `callargs', `redefine', `obsolete',
  `interactive-only', `lexical', `mapcar', `constants' and
  `suspicious'.

  For the `mapcar' case, only the `mapcar' function can be used in
  the symbol list.  For `suspicious', only `set-buffer' can be used.
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Fourth, the warnings seem to be nondeterministic, sometimes being
emitted only for one of the two files in question, and sometimes for
neither file (particularly with serial 'make check').

How can these issues be addressed?

Thanks,

-- 
Basil

bug#56641: Deprecate `lsh`

[Mattias Engdegård]

23 juli 2022 kl. 17.42 skrev Basil L. Contovounesios <contovob@tcd.ie>:

> Thanks, but the warnings are being stubborn with and not listening to
> me.  Could you please have a word with them?

A serious conversation was held and some very frank words were pronounced.

> First, they are a bit repetitive in parallel 'make -j8 check':

That was made on purpose to really hammer the point home.

>  For the `mapcar' case, only the `mapcar' function can be used in
>  the symbol list.  For `suspicious', only `set-buffer' can be used.
>                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Clearly you aren't suspicious enough.

> How can these issues be addressed?

Why, `git pull` of course!

(Sorry about all this, and thanks a lot!)

bug#56641: Deprecate `lsh`

[Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Mattias Engdegård [2022-07-23 18:51 +0200] wrote:

> 23 juli 2022 kl. 17.42 skrev Basil L. Contovounesios <contovob@tcd.ie>:
>
>> Thanks, but the warnings are being stubborn with and not listening to
>> me.  Could you please have a word with them?
>
> A serious conversation was held and some very frank words were pronounced.

Is there an RFC for that protocol?  It seems to be effective.

>> First, they are a bit repetitive in parallel 'make -j8 check':
>
> That was made on purpose to really hammer the point home.

Is it a bug that we don't consistently do that for all warnings?

>>  For the `mapcar' case, only the `mapcar' function can be used in
>>  the symbol list.  For `suspicious', only `set-buffer' can be used.
>>                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> Clearly you aren't suspicious enough.

But enough to not be suspiciously unsuspicious.

>> How can these issues be addressed?
>
> Why, `git pull` of course!

I picked some of that up from my local pharmacy, and...

> (Sorry about all this, and thanks a lot!)

...WFM, thanks!  No need to apologise,

-- 
Basil