Re: Hollow cursor under images

[Eli Zaretskii <eliz@gnu.org>]

> From: Evgeny Zajcev <lg.zevlg@gmail.com>
> Date: Tue, 28 Jan 2020 11:46:15 +0300
> Cc: Alan Third <alan@idiocy.org>, emacs-devel <emacs-devel@gnu.org>
> 
>  > +  if (CONSP (arg)
>  > +      && EQ (XCAR (arg), Qbox)
>  > +      && RANGED_FIXNUMP (0, XCDR (arg), INT_MAX))
>  > +    {
>  > +      *width = XFIXNUM (XCDR (arg));
> 
>  This calls XFIXNUM no less than 3 times.  I wonder if we could tweak
>  the code to do that only once.
> 
> Just for my info.  Should not such tweaks be done by compiler optimizer?  No value is declared as volatile, it is
> pretty clear that there is no need to call XFIXNUM 3 times.

You forget the unoptimized build case.  And in any case, it is
un-economical and inelegant, IMO, to write such code.  Granted, that
is a stylistic preference of mine to some extent, so if you feel
strongly about that, I don't think I will fight you.

> I'm afraid tweaking things in such way in source code will make code look ugly and less obvious

I don't think so.  Here's what I'd do in this case:

  if (CONSP (arg))
    {
      Lisp_Object style = XCAR (arg);
      Lisp_Object lval = XCDR (arg);
      ptrdiff_t val = FIXNUMP (lval) ? XFIXNUM (lval) : -1;
      if (0 <= val && val < INT_MAX)
        {
	  *width = val;
	  if (EQ (style, Qbox))
	    return FILLED_BOX_CURSOR;
	  else if (EQ (style, Qbar))
	    return BAR_CURSOR;
	  else if (EQ (style, Qhbar))
	    return HBAR_CURSOR;
	}
    }

Btw, one other advantage of the above is that you can easily display
in the debugger each intermediate value used by this calculation,
while stepping through the code.  Again, a minor convenience, but
convenience nonetheless.