From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: multipart/mixed; boundary="===============7881427596033650783==" MIME-Version: 1.0 From: Mark Wielaard To: elfutils-devel@lists.fedorahosted.org Subject: Re: [PATCH 2/2] Add C++ iterators Date: Mon, 20 Apr 2015 11:28:28 +0200 Message-ID: <1429522108.1938.28.camel@bordewijk.wildebeest.org> In-Reply-To: 87k2xalmpz.fsf@redhat.com --===============7881427596033650783== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Hi Petr, On Fri, 2015-04-17 at 18:54 +0200, Petr Machata wrote: > Mark Wielaard writes: > > On Thu, 2015-04-16 at 17:42 +0200, Petr Machata wrote: > >> Mark Wielaard writes: > >> 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 t= he > >> 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. > > > > So, this does concern me a little. We have to be absolutely sure that > > keeping the state as a vector of offsets is all we ever need. > = > The only problem that I see is what to do with the m_cuit. For > DIE-to-DIE iteration, it would probably be set to unit_iterator::end(). > That now represents an end-iterator. So end iterator would be > represented as m_die.addr =3D=3D NULL instead--no valid DIE will have a N= ULL > address. OK. I keep mixing issues. But indeed subtree DIE iteration is a separate from issue the logical/raw walks. > >> I'm not sure. I don't think so, at least you would need a different w= ay > >> of keeping track of context. > > > > Could we maybe abstract away the m_stack context (maybe pimpl it)? > = > Yes, if you decide to overload the iterator to do both logical and raw > iteration. I don't think that's the right approach. OK. > I meant the new iterator. Splitting the concerns into a raw iterator > and a high-level one that builds upon the raw one absolutely seems the > right approach to me. The raw iterator doesn't end up paying the > overhead for raw iteration, and the complexities of both raw and logical > iteration will be kept to their respective silos. > = > Just to make sure I'm not talking out of my ass, I put together a > logical_die_tree_iterator. It's on pmachata/iterators. Thanks. I'll work on the plain C implementation of "logical" DIE walks, then we can use that in the future. > >> 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. > > > > If none of the above convinced you otherwise then lets just go with the > > code you wrote now. But only on the condition that you come up with a > > non-stupid name for the future "logical_die_tree_iterator" :) > = > Note that the raw/logical distinction applies at least to child_iterator > as well Yes. If a DIE has a DW_TAG_imported_unit as child, the logical thing to do is to iterate over its children too. > , maybe also to unit_iterator. When using a unit_iterator you should be able to iterate over (combinations) of the different unit types. But I don't think there is a logical/raw distinction. And that is also the only thing I am still slightly confused about. Because the die_tree_iterator takes a Dwarf or unit_iterator to iterate over all the DIEs in the Dwarf or (remaining) units, I would like to indicate which units are interesting (e.g. I only want to iterate over DIEs in the compile_units and type_units, but not the partial_units). Should this depend on the type (raw or logical) of the die_tree_iterator? > I think something can be done to > make logical_die_tree_iterator::move configurable, so that it's useful > in child iterator as well. > = > (One could almost write a template for logical iteration that is > parametrized by iterator type, but not quite: for the tree iterator one > needs to skip the root that it's constructed from, but with child > iterator that's ignored implicitly. Besides, templates are ABI > nightmare.) If we end up with a template implementation, I think it would be better to just add a boolean flag to the constructors instead to indicate raw/logical walks. > Regarding the names, I don't have any good ideas. In dwgrep I ended up > with raw and cooked, which is in hindsight perhaps too cute. Maybe we > can rename the current bunch to raw_*, and add the logical ones without > a prefix. I think that would be best. Cheers, Mark --===============7881427596033650783==--