[PATCH] D47604: [Polly] Add isl C++ list iterators

Philip Pfaffe via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 4 02:50:18 PDT 2018


philip.pfaffe requested changes to this revision.
philip.pfaffe added a comment.
This revision now requires changes to proceed.

I love the gist of this, but there are a couple of implementation issues, some of which @Meinersbur pointed out already.

`list_iterator` is not an InputIterator, not even an Iterator. You're lacking various parts required by the concepts:

- It's not Swappable
- It's not CopyAssignable
- There's no `std::iterator_traits`
- There's no `operator->`
- There's no post-increment operator.

I very much don't like implementation as a template class, while outlining the specialized behavior. A much nicer way to build this would be to have an `index_iterator` CRTP iterator base for iteration via index, implemented on to of llvm::iterator_facade_base and scoped to an isl namespace (names subject to bikeshedding). Derive iterators from that to specialize the accessors. Similarly, have explicit overloads for {basic_,}{set,map} lists' begin() and end() functions. A low-tech solution is required for this, anything else will just come back to bite us.


Repository:
  rPLO Polly

https://reviews.llvm.org/D47604





More information about the llvm-commits mailing list