[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Emacs-diffs] master f18af6c: Audit use of lsh and fix glitches

From: Pip Cet
Subject: Re: [Emacs-diffs] master f18af6c: Audit use of lsh and fix glitches
Date: Wed, 22 Aug 2018 15:50:45 +0000

On Wed, Aug 22, 2018 at 1:44 PM Paul Eggert <address@hidden> wrote:
> True, but even bignums should be faster than the slow function, so the code
> really shouldn't be worrying about integer width. Fixing this, though, will 
> be a
> bit trickier than simply removing the width check, as I expect the code uses 
> eq
> on integers. Until that assumption is fixed (or until we start hashing 
> bignums)
> the integer width check still makes sense.

Maybe we ought to warn (in debug builds) about eq being used to
compare several integers, at least one of which is outside a narrow
interval of guaranteed fixnums (and ditto for memq and hashtest_eq). I
wonder whether Emacs would even build with MOST_POSITIVE_FIXNUM =
0x100000, but it might be worth it to try and run it that way...

That said, vc-hg.el doesn't contain any suspicious-looking instances
of `eq', `memq', or `hash'; I think all the author was worried about
was reading an u32 into an Emacs integer, which now works.

Again, I'm not suggesting to remove `most-positive-fixnum', just not
to use it in this particular file. It's "dangerous", but so are many
useful Emacs functions (the worst of which is probably kill-emacs,
which can lead to a user using a different editor, but minor
catastrophes such as segfaults or data loss are also possible).

As for `eq' and `eql', what I thought Stefan proposed was to keep a
hash value in struct Lisp_Bignum, which is calculated lazily the first
time the bignum is compared to another bignum with `eq', at which
point we might as well point out to the user that portable code would
require `eql'. That sounds to me like it would be much cheaper than a
hash table, but if there are objections to it we can still implement
it without guaranteeing it for the future in the documentation.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]