cpp: isl::ctx implicit conversion to isl_ctx*

[nicolasvasilache]

Memory management and lifetime of isl_ctx objects seems quite different from the ones that islpp automatically extracts. Let's not pretend the API should be the same (i.e. get, release etc), it feels more confusing than anything else.
From skimming through uses of isl::ctx it seems it is alway used with .release().
How about dropping all these raw pointer accessors and just implicitly convert to isl_ctx* ?

[tobiasgrosser]

I like the idea. For backwards compatibility reasons (and consistency) I would likely not drop the existing interface, but adding an implicit conversion for isl_ctx * seems a straightforward and well justified change that certainly makes mixed C and C++ code a lot more readable.

Care to propose a patch?

[aisoard]

I prefer to be (overly) cautious about implicit conversions as this create surprising behaviors that will come back to bite us. That being said, the ctx object is almost never used in any expression, we rarely check it is non-null, and that is a lossless conversion. Which makes me think that is probably ok.

In any case, I agree that having get/release in the interface make little sense as isl::ctx does not actually manage the underlying isl_ctx.

Anyway, the isl_ctx is the black sheep of the family, I don't see any proper way of managing it as it is a transverse data-structure that isl does not handle. I would almost be ok with having a thread_local global isl_ctx, but I assume that would displease a lot of people. :-) (and that is an other unrelated discussion)

[ftynse]

On 04/08/17 16:35, Alexandre Isoard wrote: I prefer to be (overly) cautious about implicit conversions as this create surprising behaviors that will come back to bite us. That being said, the ctx object is almost never used in any expression, we rarely check it is non-null, and that is a lossless conversion. Which makes me think that is probably ok. In any case, I agree that having get/release in the interface make little sense as |isl::ctx| does not actually manage the underlying |isl_ctx|. Anyway, the isl_ctx is the black sheep of the family, I don't see any proper way of managing it as it is a transverse data-structure that isl does not handle. I would almost be ok with having a thread_local global isl_ctx, but I assume that would displease a lot of people. :-) (and that is an other unrelated discussion)

Interestingly, thread_local context popped in our internal discussion a couple of days ago :)

…

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#12 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABcTa7hI1NKB7CUguXsXpyOqFTZN07Llks5sUyw8gaJpZM4Otlu7>.

[tobiasgrosser]

Albert also mentioned that a ctx free (or thread-local only) use of the isl bindings would be great. It might make sense to make this a separate issue. (Seems very useful for scripting-style usage of these bindings)

[ftynse]

Now it's #14