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

[Mark Wielaard <mjw@redhat.com>]

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

On Fri, 2015-04-03 at 10:51 +0200, Petr Machata wrote:
> as requested by Mark, I made the iterator interfaces ABI stable, so that
> they can be safely used on API boundaries of third-party libraries.
> This patch should also include all the other points that we talked about
> (from the top of my head: returning reference from operator++, dropping
> of offset member function, change from cu_iterator to unit_iterator),
> except for exceptions, which are still std::runtime_error.

Thanks. I do think one of elfutils strength's is that it has a stable
ABI so people can just use it for years without worrying whether or not
to upgrade to get some new bug fixes or features. But if I am overdoing
it, because C++ and abi stability just don't mix, please do feel free to
push back a bit more.

> This adds a new library, libdwpp, which implements these interfaces.
> The interfaces themselves are fully pimpl'd.

pimpl'd? Aha. It is an actual term :)
https://en.wikipedia.org/wiki/Opaque_pointer
the "Pimpl idiom" (for "pointer to implementation idiom").

> Actual class layout
> etc. is fully internal detail--on the outside we publish one pointer
> field.  Due to this, the iterators are more heavyweight, involving a
> dynamic allocation for each instance, and PLT call for each operation,
> but on the other hand, virtually no ABI breakage is possible.

I like this because it makes things easy and clear on the ABI side.
I am not so concerned about the PLT call, but the new allocation and
delete in the destructor is indeed a little heavy-weight since I assume
iterators are often short lived so it will cause lots of small
new/deletes. I guess we just will have to live with that if we want ABI
stability. Do others also think this is a acceptable tradeoff?

> Symbols are versioned.  This ties us closely to the GCC ABI, but I think
> everybody uses Itanium ABI these days anyway.  This also means that if
> there should be a change in behavior that would constitute an ABI
> breakage, it can be mitigated the same way as with C symbols.
> 
> I hid many symbols as well.  Namely the private constructors are not
> exported at all, and I have also hidden the not-in-charge constructor
> and destructor variants, because those should only come into play when
> the iterators are inherited from, which they are not designed for.

I am happy we have an explicit ABI now. But I am somewhat concerned
about how libdw/libdwpp.map looks. Is this really the best we can do? It
feels somewhat unmaintainable if we have to hand edit this version
script. How did you generate this?

Still reading through the implementation, but it looks good to me.

Thanks,

Mark