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

[Philip Pfaffe via Phabricator via llvm-commits]

philip.pfaffe added inline comments.


================
Comment at: unittests/Support/ISLTools.cpp:24
+  EXPECT_THAT(Sets, testing::UnorderedElementsAre(A, B));
+  isl_ctx_free(Ctx.release());
+}
----------------
Meinersbur wrote:
> philip.pfaffe wrote:
> > Meinersbur wrote:
> > > [suggestion] I quite like using RAII to free the `isl_ctx` as done in `IslTest.cpp`:
> > > ```
> > > std::unique_ptr<isl_ctx, decltype(&isl_ctx_free)> Ctx(isl_ctx_alloc(), &isl_ctx_free);
> > > ```
> > Why doesn't the C++ wrapper do this already?
> We could not decide what the right approach is. Freeing the isl_ctx would mean that `isl::ctx` owns the reference. But in most cases, you are not owning it: E.g. if returned by a get_ctx function to do something else.
Then why not just return the C object? Or decide ownership based on how the object is constructed?


Repository:
  rPLO Polly

https://reviews.llvm.org/D48136