Eric Botcazou <ebotca...@adacore.com> writes:
>> Now that we're in C++, I think we should be using iterators that are much
>> more in the style of libstdc++. I agree that the .next interface used here
>> is a bit confusing.
>>
>> I'd expect to see something like
>>
>> for (bit_field_mode_iterator i(x,y,z); i ; ++i)
>> {
>> enum machine_mode mode = *i;
>> ...
>> }
>
> I pondered on that for half an hour. :-) But the amount of stuff you need to
> write to make it work in this particular case will make the implementation
> convoluted and bloated for no obvious gains IMO.
Yeah, I'd also thought about doing it as a "proper" iterator, and came
to the same conclusion. Although I was thinking of:
bit_field_def def (blah...);
for (bit_field_def::mode_iterator i = def.modes_begin();
i != def.modes_end();
++i)
{
enum machine_mode mode = *i;
...
}
Then presumably we'd need all the other iterator gloss too
(value_type, etc.).
I hadn't thought about an operator bool terminator. I agree that's
probably simpler, but do any libstdc++ classes have the same thing?
It doesn't feel any more standard than the "while (get_more)" idiom to me,
but that's probably just my ignorance of C++.
My problem with:
FOR_EACH_BITFIELD_MODE (mode, iter, bitsize, bitpos,
bitregion_start, bitregion_end,
align, volatilep)
is that there are too many parameters to make it obvious what we're
actually iterating over. I.e. these FOR_EACH_* macros tend to have a
combination of three things: inputs, "internal" iterator state that
nevertheless needs to be defined separately in order to avoid surprising
name clashes, and the iterator variable itself. When there's a lot of
parameters, it's not always obvious which is which.
Also, it makes it harder to implement loops like:
setup
if (get first mode)
if (want to search for more)
while (more modes...)
which is the structure used in patch 7.
Admittedly both "problems" apply to some of the existing FOR_EACH_*
macros too, so there's definitely a consistency argument in favour of
doing this anyway.
"next" was supposed to be "find and return another mode" rather than "++".
Did you think it was confusing because "next" sounded too much like the latter?
Or was it the whole "while (get more)" thing you didn't like?
Maybe it would be better to forget about using classes here and have
something like (with "bit_field_def def"):
mode = smallest_bit_field_mode (&def, params...);
while (mode != MAX_MACHINE_MODE)
{
...
mode = wider_bit_field_mode (&def, mode);
}
Richard
