On 18/06/20 00:59 +0100, Jonathan Wakely wrote:
On 18/06/20 00:55 +0100, Jonathan Wakely wrote:
On 16/06/20 15:14 +0100, Richard Sandiford wrote:
Martin Li?ka <mli...@suse.cz> writes:
On 6/16/20 10:50 AM, Richard Sandiford wrote:
Hmm, sounds like a nice abstraction but I see 2 problems with that:
1) How can I define a constructor of the iterator_pair when I need something
like:
iterator_pair<const_iterator> (region_begin, region_end)?
Not sure if I'm answering the right question, sorry, but I meant
something along the lines of:
template<typename T>
struct iterator_pair
{
public:
iterator_pair (const T &begin, const T &end)
: m_begin (begin), m_end (end) {}
T begin () const { return m_begin; }
T end () const { return m_end; }
private:
T m_begin;
T m_end;
};
You could also add an "object generator" for that type:
template<typename T>
iterator_range<T>
make_iterator_range(T begin, T end)
{
return iterator_range<T>(begin, end);
}
This deduces the type T from the function arguments, so will allow you
to write:
return make_iterator_range(x, y);
instead of:
return iterator_range<const_iterator>(x, y);
i.e. you don't need to say <const_iterator> because the compiler can
deduce it from the arguments.
Does your reverse iterator work correctly? It looks to me like it will
fail to visit the region_begin statement, because this loop will
terminate when the reverse iterator reaches the end() value, and will
not actually process that gsi:
- gimple_stmt_iterator si = bb_vinfo->region_end;
- gimple *stmt;
- do
+ for (gimple *stmt : bb_vinfo->reverse_region_stmts ())
Adding const_iterator::operator--() and using
std::reverse_iterator<const_iterator> would fix that problem. The
std::reverse_iterator utility already solves the problem of getting
the begin and end of a reversed range correct.
You might need to add a couple of things to your const_iterator to
make it more like a conventional STL iterator in order to use
std::reverse_iterator, but IMHO it would be worth it. I can look at
that in the morning.
One more thing I noticed: is there a reason your operator++ returns a
const reference? Conventionally it should be a non-const reference.
i.e.
const_iterator& operator++();
And if you want to make it bidirectional:
const_iterator& operator--();
And another thing...
Why const_iterator and not just iterator? It returns a gimple* not a
const gimple* which means that it can be used to change the contents
of the range. And even if it can't, if you don't plan to add a
(non-const) iterator later that is for mutating things, then it's just
noise to make everybody type const_iterator.
It's fine for an 'iterator' type to disallow modifications, e.g. see
std::set<T>::iterator which is the same type as
std::set<T>::const_iterator, because modifying std::set elements is
not permitted (it would invalidate the ordering).
And why reverse_const_iterator not const_reverse_iterator like every
container in the standard library? (Although that question doesn't
need to be answered if you rename them as iterator and
reverse_iterator).
