On Wed, Dec 11, 2013 at 6:47 PM, Oleg Endo <oleg.e...@t-online.de> wrote:
> On Thu, 2013-11-21 at 00:04 +0100, Steven Bosscher wrote:
>> Declaring the edge_iterator inside the for() is not a good argument
>> against FOR_EACH_EDGE. Of course, brownie points are up for grabs for
>> the brave soul daring enough to make edge iterators be proper C++
>> iterators... ;-)
>
> So, I gave it a try -- see the attached patch.
> It allows edge iteration to look more like STL container iteration:
>
> for (basic_block::edge_iterator ei = bb->pred_edges ().begin ();
> ei != bb->pred_edges ().end (); ++ei)
> {
> basic_block pred_bb = (*ei)->src;
> ...
> }
>
> The idea is to first make class vec look more like an STL container.
> This would also allow iterating it with std::for_each and use C++11
> range based for loop (in a few years maybe). It would also make it
> easier to replace vec uses with STL containers if needed and vice versa.
>
> Then the
> typedef struct basic_block_def* basic_block;
>
> is replaced with a wrapper class 'basic_block', which is just a simple
> POD wrapper around a basic_block_def*. There should be no penalties
> compared to passing/storing raw pointers. Because of the union with
> constructor restriction of C++98 an additional wrapper class
> 'basic_block_in_union' is required, which doesn't have any constructors
> defined.
>
> Having 'basic_block' as a class allows putting typedefs for the edge
> iterator types in there (initially I tried putting the typedefs into
> struct basic_block_def, but gengtype would bail out).
> It would also be possible to have a free standing definition / typedef
> of edge_iterator, but it would conflict with the existing one and
> require too many changes at once. Moreover, the iterator type actually
> depends on the container type, which is vec<edge, ...>, and the
> container type is defined/selected by the basic_block class.
>
> The following
> basic_block pred_bb = (*ei)->src;
>
> can also be written as
> basic_block pred_bb = ei->src;
>
> after converting the edge typedef to a wrapper of edge_def*.
>
> The idea of the approach is to allow co-existence of the new
> edge_iterator and the old and thus be able to gradually convert code.
> The wrappers around raw pointers also helo encapsulating the underlying
> memory management issues. For example, it would be much easier to
> replace garbage collected objects with intrusive reference counting.
>
> Comments and feedback appreciated.
Apart from the issues raised by Trevor I want to question if we want
to transition
to STL-like iterators (yeah, I know, the gsi_ ones are already
similar). An obvious
cleanup to the existing FOR_EACH machinery is to declare the iterator inside
the FOR_EACH, thus make it an implementation detail (like I did with the loop
iterator).
Richard.
> Cheers,
> Oleg
