Re: 'struct window' cleanup #2

[martin rudalics]

> Why a "hack"?  Lisp object are good precisely for this reason: that
> you can give them values of different types of object.

window_end_valid is documented as

    /* t if window_end_pos is truly valid.
       This is nil if nontrivial redisplay is preempted
       since in that case the frame image that window_end_pos
       did not get onto the frame.  */
    Lisp_Object window_end_valid;

but the code in xdisp.c checks it usually just as

     !NILP (w->window_end_valid)

Consequently, the assignment

      w->window_end_valid = w->buffer;

looks fragile (one has to be sure that the NILP checks don't happen
while it's set to the buffer since otherwise we end up trying again for
no use) and at least contradicts the initial comment of try_window_id

/* Try to redisplay window W by reusing its existing display.  W's
   current matrix must be up to date when this function is called,
   i.e. window_end_valid must not be nil.

but apparently the current matrix is not up to date when the value is
non-nil but some other buffer.  So unless this is better documented
and/or the corresponding code cleaned up it remains a "hack" IMHO.

>> Anyway, window_end_pos and window_end_vpos are the more
>> promising candidates (if Eli agrees).
>
> I don't object.  But if we think that Lisp integers cause any
> significant slowdown during GC, we could special-case them in
> mark_object, because that is a no-op for integers.  That would be less
> work and less potential bugs.

I don't care about GC.  But it seems to me that _all_ accesses of these
two structure members are via make_number and XFASTINT.  So we can only
gain from cleaning up this.

>>  Can you see whether the part
>>
>>        /* When splitting windows or for new windows, it happens that
>>         redisplay is called with a nil window_end_vpos or one being
>>         larger than the window.  This should really be fixed in
>>         window.c.  I don't have this on my list, now, so we do
>>         approximately the same as the old redisplay code.  --gerd.  */
>>        && INTEGERP (w->window_end_vpos)
>>
>> in xdisp.c still makes sense?
>
> No, there's no code anymore that sets it to nil.

Fine (apparently that was another "hack" in redisplay).

martin