bug#36370: 27.0.50; XFIXNAT called on negative numbers

[Paul Eggert]

> > 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
Description: Text Data