search

Erk- / genfut

Generate bindings for Futhark
ISC License
37 stars 10 forks source link

Remove Copy from FutharkContext #20

Closed Erk- closed 4 years ago

Erk- commented 4 years ago

This is a breaking change that will make it possible to automatically drop the context. It also has some changes for the generated api.

supersedes #19 closes #15

Erk- commented 4 years ago

Thinking about it a bit more, the Context should not implement Clone as it is going to be broken if another context gets dropped. Though this could be remidied by internally wrapping the context in a reference counter so it is first deallocated when there are no references left. This would also make other things easier I think.

athas commented 4 years ago

Why does it need to implement Clone at all?

Erk- commented 4 years ago

It does not need to, I just forgot to remove it when refactoring it. It would only have it for a better userside API, but the user can wrap the context in a Arc themselves for that.

porcuquine commented 4 years ago

I have only had time to begin trying to integrate/test this change. After adding lifetimes to all my opaque pointers, I got hit by compiler errors flowing from that. This might require a lot of refactoring to address — enough so to be problematic from my perspective if true. I don't have time to fully investigate any further for now, but unless I eventually find this can be absorbed with relatively low impact, I would favor the status quo.

I see the value of automating freeing, but I already manage resource allocation so having to do it manually (which I never do, by design) isn't a big problem.

Erk- commented 4 years ago

I will close this for now as I don't think it is the best way to solve the issue. (for more info see #15)