On 03/22/2013 05:34 PM, Stefan Monnier wrote:

> For me, `payload' is associated with transport (or communication), so it
> sounds a bit odd here.  But I won't oppose it (whereas I do oppose "object").

OK, what about neutral `contained'?

>> +    /* Non-zero if this window is internal, e.g. used in either
>> +       horizontal or vertical combination.  */
>> +    unsigned combination : 1;
>
> Isn't that redundant with BUFFERP (payload)?
> I actually prefer the enum form than this one (which adds a fourth
> state, basically, instead of removing invalid states).
> But I think we can eliminate this `combination', which is better than either.

Yes; this makes the 'previous' (before nil was assigned to W->payload) state
undefined, but it shouldn't be a problem.

>> +#define WINDOW_HORIZONTAL_COMBINATION(W) \
>> +  ((W)->combination && (W)->horizontal)
>
> I think this should be (eassert (WINDOWP ((W)->payload)), (W)->horizontal)

This will require a lot of explicit checking whether W->payload is a window,
for example in Fwindow_top_child and Fwindow_left_child. I believe that we
should treat dead windows as 'typeless' (that is, all of WINDOW_LEAF_P and
WINDOW_xxx_COMBINATION_P should return false for them) and add explicit
eassert where leaf or dead window can cause serious problems.

Dmitry