[cfe-dev] specific/filtered_decl_iterator aren't iterators

[David Blaikie]

Bump.

On Wed, Apr 4, 2012 at 2:28 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
> Resending to the intended (dev, rather than commits) mailing list.
>
> On Fri, Mar 30, 2012 at 12:54 PM, David Blaikie <dblaikie at gmail.com>
> wrote:
> > These filtering iterators ( include/clang/AST/DeclBase.h:1304, 1226 (&
> > even decl_iterator at 1172 and redecl_iterator at 689)) aren't
> > actually implementing the iterator concept correctly. Most noticeably
> > their value, pointer, and reference types are all the same.
> >
> > The reason I'm interested in this is that I'd like to pull out some of
> > this functionality (filtering iterators*) so it can be reused by my
> > work improving the CFG to improve the accuracy of -Wunreachable-code.
> >
> > There seem to be two possible solutions to this:
> >
> > 1) value_type is SpecificDecl*
> > Not a perfect fit, because we don't have a SpecificDecl* available to
> > refer to in the case of the specific/filter iterators (since the real
> > pointer is a Decl*, not a SpecificDecl*). We could just keep a
> > temporary SpecificDecl* as a member of the iterator (I've a simple
> > prototype of this). This does mean removing the operator-> from these
> > iterators, since they don't iterate over struct types. Some code seems
> > to have been written with this in mind already ("(*iter)->foo") but
> > other parts (lots of uses of specific_decl_iterator, few of
> > filtered_decl_iterator) do use "iter->foo" - I guess the op-> was
> > added as an afterthought.
> >
> > 2) value_type is SpecificDecl
> > Also not a perfect fit, since we don't have concrete SpecificDecls -
> > we have pointers to possibly-derived instances of SpecificDecl, so if
> > anyone attempted "value_type v = *iter;" they would slice the object
> > (but I think we suppress the copy constructors anyway - so this is
> > perhaps not an issue). Other than that this seems to make the most
> > sense to me, since the Decl pointers cannot be null, so it's just
> > convenient to use '->' (or op* to get a SpecificDecl& directly, rather
> > than a SpecificDecl* that's always non-null anyway).
> > Actually this option may require/benefit from the use of adapting
> > iterators at the same time - to make it easy to copy the sequence of
> > pointers into a SmallVector which is done in a couple of places in
> > SemaLambda.
> >
> > I'd like to do (2) to at least filter/specific_decl_iterator (and
> > happy to fix up the other iterators with this inconsistency as well) &
> > then continue on to refactor out the functionality in to some general
> > purpose iterator adaptors for reuse in the CFG.
> >
> > Does that sound reasonable?
> >
> > * the filtering/specific iterators in DeclBase.h are a bit more than
> > the classic filter_iterator since they modify the value_type whereas a
> > filter_iterator (ala boost's implementation, for example) is only
> > about selecting on a condition. We'd need an adapting_iterator too, or
> > a single device combining both features of adapting and filtering.