Aidan Kehoe writes:
This doesn’t have to fit into a fixnum, except for the error
message, which will work ok with make_unsigned_integer and bignum
support.
(1) I don't think we're guaranteed to have bignum support.
(2) In that range the file is in error (character indicies *do* have
to fit in a fixnum), but no error is reported. This will blow up
in line 2672.
> - abs ((!WINDOW_HAS_MODELINE_P (win) \
> - ? ((XFIXNUM (win->modeline_shadow_thickness) > 1) \
> - ? XFIXNUM (win->modeline_shadow_thickness) - 1 \
> + EMACS_INT_ABS ((!WINDOW_HAS_MODELINE_P (win) \
> + ? ((XFIXNUM (win->modeline_shadow_thickness) > 1) \
> + ? XFIXNUM (win->modeline_shadow_thickness) - 1 \
I had been holding off on making that change because all these
values are pixel widths, they really shouldn’t get values up to
#x3fffffffffffffff, so abs() should be fine with appropriate range
restrictions. It would be nice if there were integer specifiers
that allowed more limitations on values. The natnum specifier is
the closest thing at the moment, and “greater than -1” isn’t much
of a restriction.
Yeah, I thought about that, but as you say, we don't have a good way
to say "no pixel size bigger than 8K exists outside of a hospital" on
the one hand, and on the other they're both declared as functions
(presumably inlined). I guess there may be a slightly higher cost for
long than int on 64-bit machines, but I doubt this function is called
enough to make a big difference. Ditto space savings.
I'll revert it if you want, but I really dislike warnings, and think
we probably have better things to do than avoid labs() in one function.
Regards,
Steve