Page MenuHomePhabricator

[Support] Add casting iterator adapters
Needs RevisionPublic

Authored by hamzasood on Jul 30 2018, 3:36 PM.



This patch adds a simple type that lets you create a functor from a function at compile time.

While working on Clang, I needed a way to create an llvm::filter_iterator that filters based on the dynamic type as determined by llvm::isa. As far as I'm aware, the best way to do this currently (without lambdas as they'd make the iterator type unutterable) is:

llvm::make_filter_range(MyRange, static_cast<bool(*)(Base *)>(&isa<Derived>))

That's problematic because the iterator object ends up storing a function pointer; if the compiler can't follow the iterator's path from creation to use then it probably won't be able to get round the unnecessary indirection. This new type solves that by storing the function address as part of the object type, so there's no runtime overhead.

Diff Detail

Event Timeline

hamzasood created this revision.Jul 30 2018, 3:36 PM

I'd probably still vote in favor of a lambda, even if it made the iterator
type unutterable - do you have an example where that makes things
especially difficult to read?

There’re a lot of places where you need a type that you can write down. E.g. function return type.

Is there a real life example that justifies the optimization?

hamzasood updated this revision to Diff 158526.Aug 1 2018, 6:51 AM
hamzasood retitled this revision from [ADT] Add a static function pointer type to [Support] Add casting iterator adapters.

The previous revision was an unnecessarily general solution to a specific problem.

I think there's a need for a special kind of filter_iterator that filters and casts based on the dynamic type of the elements, as determined by llvm::dyn_cast. In Clang there're at least 4 custom-written, entirely separate iterators that do just this: specific_decl_itererator, filtered_decl_iterator, specific_clause_iterator, specific_attr_iterator. And I'm at the point where I need to add yet another one.

This patch contains a generic iterator that yields Derived * from Base * by dyn_casting each element and filtering out those element whose dynamic type doesn't match.

dblaikie added inline comments.Aug 1 2018, 11:57 AM

I'd probably skip this extra generality of providing an extra predicate on top of the isa/dyn_cast filter. Users can implement that with another filtering iterator if needed.

hamzasood updated this revision to Diff 158789.Aug 2 2018, 9:57 AM

Removed the extra predicate customisation point.

fhahn requested changes to this revision.Dec 23 2019, 6:05 AM

@hamzasood are you still looking into pushing this patch? It would be helpful if you could show a few cases where this new utility helps with existing code in LLVM/Clang, preferably as a separate patch linked to this one.

Marking as requested changes to clear up review queue.

This revision now requires changes to proceed.Dec 23 2019, 6:05 AM