Re: [AC 21.5] Shut up some clang warnings
Ar an t-ochtú lá is fiche de mí na Samhain, scríobh Stephen J. Turnbull:
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.
Right. Without bignum support, we’ll still get an error, just a more
confusing one. Which is well and good, it’s motivation to build with 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.
It doesn’t. Test code here:
https://parhasard.net/xemacs/very-long-comment.el.html
Adjust very-long-comment-length and very-long-comment-readchar-prologue to
taste; the limiting factor is likely your appetite for the heap-allocation
frenzy as each increment and each numeric comparison creates a new bignum
when the index gets above most-positive-fixnum.
> > - 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.
We silence all these warnings all the time, and they’re usually false
positives. The whole point of turning them on is the occasional bug they
flush out. And this is an actual bug, and now I grep for them, *all* of the
natnum and integer specifiers in the C code would benefit from a similar
restriction, they’re all pixel widths. It’s not just one place.
Here’s a doozy:
xemacs-21.5-b34 -vanilla &
C-x 2 ;; (split the *scratch* window)
(set-specifier vertical-divider-shadow-thickness `((global (nil .
,most-positive-fixnum)))) C-j
;; Watch XEmacs crash
--
‘As I sat looking up at the Guinness ad, I could never figure out /
How your man stayed up on the surfboard after forty pints of stout’
(C. Moore)