Il 18/07/2013 17:57, Laszlo Ersek ha scritto:
> On 07/18/13 16:56, Paolo Bonzini wrote:
>> Il 18/07/2013 15:59, Laszlo Ersek ha scritto:
>>> The new modes are equal-rank, exclusive sub-modes of LM_IN_PROGRESS. Teach
>>> opts_next_list(), opts_type_int() and opts_type_uint64() to handle them.
>>
>> Perhaps you could use a bitmap then:
>>
>> LM_NONE = 0
>> LM_STARTED = 1
>> LM_IN_PROGRESS = 2
>> LM_SIGNED_INTERVAL = LM_IN_PROGRESS | 4
>> LM_UNSIGNED_INTERVAL = LM_IN_PROGRESS | 8
>>
>> I think the only change would be that this hunk:
>>
>>> @@ -211,7 +238,10 @@ opts_end_list(Visitor *v, Error **errp)
>>> {
>>> OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);
>>>
>>> - assert(ov->list_mode == LM_STARTED || ov->list_mode == LM_IN_PROGRESS);
>>> + assert(ov->list_mode == LM_STARTED ||
>>> + ov->list_mode == LM_IN_PROGRESS ||
>>> + ov->list_mode == LM_SIGNED_INTERVAL ||
>>> + ov->list_mode == LM_UNSIGNED_INTERVAL);
>>> ov->repeated_opts = NULL;
>>> ov->list_mode = LM_NONE;
>>> }
>>
>> could be changed to
>>
>> assert(ov->list_mode == LM_STARTED ||
>> (ov->list_mode & LM_IN_PROGRESS));
>
> If you don't insist (please don't :)), I wouldn't like to do this.
No, I won't.
> (a) I wanted to represent and query each individual mode explicitly
> (helps with code search),
>
> (b) the "sub-mode" nature is a theoretical thing. It only applies to the
> two interval modes. This small orthogonality is limited, it doesn't
> cause "combinatorial explosion" (I didn't have to double the states or
> such). Most importantly, I specifically wanted one state in general to
> exclude any other state. Bitmaps beget thinking about the meaning of
> arbitrary variations, like 1|4, 0|2 etc; I intended to prevent such
> thoughts right in the type.
Fair enough. But please find a way to put the "sub-mode" thing in the
code too (that's the redeeming grace of bitmaps)---even better if, at
the same time, the phrasing will calm the urge to say "bitmap!".
Paolo
