[PATCH] D48136: [Polly] Implement an iterator for isl maps, basic_maps, sets, basic_sets

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added a comment.

Thanks for the test case

In https://reviews.llvm.org/D48136#1132114, @philip.pfaffe wrote:

> Looks like I forgot to add pollydev as a subscriber. Shall I resubmit the patch?


Don't think it's necessary.



================
Comment at: include/polly/Support/ISLTools.h:50
+protected:
+  ListT *List;
+  int Position = 0;
----------------
philip.pfaffe wrote:
> Meinersbur wrote:
> >  Should this hold a reference to the list? That is, the underlying object is not freed until all iterators are destroyed.
> It needs a reference to do the iteration.
What I mean is that the iterator could increase isl's ref counter to ensure that it is not free'd as long as one iterator exists. That is, is the following code valid?
```
basic_set_iterator I, E;
{
  auto List = S.get_basic_set_list();
  I = begin(List); E = end(List);
} // List out-of-scope here
for (; I != E; ++I) { ... }
```


================
Comment at: unittests/Support/ISLTools.cpp:12
+TEST(Support, isl_iterator) {
+  isl::ctx Ctx(isl_ctx_alloc());
+  isl::basic_set A(
----------------
`Ctx` no free'd?


Repository:
  rPLO Polly

https://reviews.llvm.org/D48136