GNU bug report logs - #36370
27.0.50; XFIXNAT called on negative numbers

Previous Next

Package: emacs;

Reported by: Pip Cet <pipcet <at> gmail.com>

Date: Tue, 25 Jun 2019 05:37:02 UTC

Severity: normal

Tags: patch

Found in version 27.0.50

Done: Paul Eggert <eggert <at> cs.ucla.edu>

Bug is archived. No further changes may be made.

Full log


Message #27 received at 36370-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Pip Cet <pipcet <at> gmail.com>
Cc: 36370-done <at> debbugs.gnu.org
Subject: Re: bug#36370: 27.0.50; XFIXNAT called on negative numbers
Date: Thu, 27 Jun 2019 12:38:10 -0700
[Message part 1 (text/plain, inline)]
>> if it were up to me we'd get rid of XFIXNAT entirely,
>> and just use XFIXNUM. But old habits die hard....
> 
> I actually think that would be best, so we're only disagreeing about
> what the second-best solution is :-)

Removing XFIXNAT would be outside the scope of this patch. However, if 
we're already fixing the code for some other reason and if the XFIXNATs 
are confusing that code, we might as well replace them with XFIXNUMs. 
The attached patch does that.

> valid_image_p only returns true if ->valid_p returns true, which only
> happens when parse_image_spec returns true, which only happens if
> :ascent is not present, Qcenter, or a fixnum in the 0..100 range.

Thanks for explaining. The attached patch removes the unnecessary range 
checks that I proposed.

> My idea is to define eassume as follows:
> 
> #define eassume(cond) (__builtin_constant_p (!(cond) == !(cond)) ?
> ((cond) ? 0 : __builtin_unreachable ()) : 0)

That would generate worse code in some cases, since after (say) "eassume 
(i >= 0); return i/2;" where i is a variable, GCC would not be able to 
optimize i/2 into i>>1 because GCC would not know that i is nonnegative. 
The main point of eassume (as opposed to eassert) is to enable 
optimizations like that.

>  think this code is a bit hard to read:

The attached patch changes that to use XFIXNUM instead of XFIXNAT, along 
the lines discussed above.

Thanks for the review. In addition to the already-mentioned patches, I 
installed the attached and am closing the bug report.
[0001-Omit-a-few-minor-unnecessary-range-checks.patch (text/x-patch, attachment)]

This bug report was last modified 6 years and 38 days ago.

Previous Next


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