Re: [PATCH 2/2] Add C++ iterators

[Petr Machata <pmachata@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2638 bytes --]

Mark Wielaard <mjw@redhat.com> writes:

> But given the nice idea above of defining a "next iterator as end" for
> another. Would adding that functionality just be introducing a new
> constructor that takes a Dwarf_Die and a next_sibling () method that
> returns an iterator that one can use as above in a for loop to test
> whether one has walked a subtree?

Yes, that's how I'd do it.

> The only disadvantage of that seems to be that it would immediately
> introduce a v2 variant if we do it after a public release.

I don't even think it would.  Adding a new constructor doesn't break any
existing clients.  API-wise it would be stable as well, this adds use
cases, doesn't change any (not even by way of implicit conversions from,
say, Dwarf_Die * to unit_iterator).

As long as your iteration is limited to well-formed sub-tree (i.e. you
don't wan't to e.g. start at one leaf node and progress through half the
CU to another leaf node), the simple vector of offsets would, I think,
be still enough to keep all the context that you need.  There might be
code that assumes that iteration starts and ends at CU DIE's, but that
could be transparently fixed.

> I was thinking just doing it on the C++ level. But yes, it would be nice
> to also have that on the C level libdw interface. That was this
> discussion where you came up with a pretty nice plan:
> https://lists.fedorahosted.org/pipermail/elfutils-devel/2014-October/004204.html
>
> Again my worry is about whether retrofitting such a design into the
> current class is possible or not. Would it be as simple as adding a new
> constructor, a boolean field and using the new C functions (and updating
> the version) or would we need a whole new class/interface for it?

I'm not sure.  I don't think so, at least you would need a different way
of keeping track of context.  Personally I think adding a new iterator
that builds on the raw one is the best option.

That would mean that you couldn't pass a logical iterator to a function
that expects a raw one.  But you can't likewise pass, say, a child
iterator to such function--the genericity is just not there.  This sort
of thing would have to be achieved by making the function template.

One could also make the new iterator configurable, like Frank says in
his mail.  Then you would have a single tree iterator type that can
behave either way.

I still think exposing raw iterator is the right way forward, because it
is a logical building block.  A logical tree iterator can be built on
top of that even without C support, if that's what's deemed best.

Thanks,
Petr