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

[David Blaikie]

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.