[PATCH] D35731: [Polly] Introduce isl_conv:: isl++ classes that automatically convert to old C classes

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added a comment.

[Opinion] I don't see the problem with converting the C-style interface one-by-one to isl++. There is a mechanical conversion for legacy users, e.g.
`getDomain()` to `getDomain().release()`. For the parts already primarily using isl++, `give(getDomain().release())` collapses to `getDomain()`.

With isl_conv we still need to go through all users to eventually get rid of it again. Until then, we just have increased the complexity and chances for memory errors significantly, something the C++ interface was trying to improve.

If you really need this class, please don't take too long to remove it again. Can you give a time frame for when it will be gone?



================
Comment at: include/polly/Support/ISLConversion.h:23
+
+template<class ISL_CLASS, typename ISL_TYPE>
+class IslConversion : public ISL_CLASS {
----------------
[Style] UPPER_CASE is usually reserved for macros. Template arguments in LLVM usually are CamelCase.


================
Comment at: lib/Transform/Simplify.cpp:175-176
 
-      AccRel = AccRel.intersect_domain(give(Acc->getStatement()->getDomain()));
+      AccRel = AccRel.intersect_domain(Acc->getStatement()->getDomain());
       AccRel = AccRel.intersect_params(give(S->getContext()));
       auto CommonElt = Targets.intersect(AccRel);
----------------
`give` removed  for `getDomain()`, but not for `getContext()`?

If you need to touch a significant amount of users for this patch anyway, I don't see the purpose.


https://reviews.llvm.org/D35731