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

[David Blaikie]

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.