Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow Refs as members in user defined types #1022

Open
scolsen opened this issue Nov 25, 2020 · 4 comments
Open

Allow Refs as members in user defined types #1022

scolsen opened this issue Nov 25, 2020 · 4 comments
Labels
memory Borrow checker and lifetimes nice-to-have under discussion Things we've not come to a conclusion about.

Comments

@scolsen
Copy link
Contributor

scolsen commented Nov 25, 2020

In some cases, our lack of support for Refs as members of user defined types can result in ownership problems. In particular, when mapping anonymous functions over the inhabitants of some type t these functions may need to take ownership when doing so is either not necessary or can lead to memory errors such as double frees. In particular, this can happen when currying lambdas, causing more than one lambda to capture the same value.

#997 (comment) has a concrete illustration of this issue.

Should we permit Refs in user defined types? In theory there should be no issue doing so for sumtypes. For product types, the getters become a bit hairy. The current signature of getters is as follows:

getter: my-type-member: (Fn [(Ref MyType)] (Ref t))

When t is a reference, the signature concretizes to:

(Fn [(Ref MyType)] (Ref (Ref t)))

So, if we do want to support this, we'd have to determine precisely what a Ref to a Ref means, what copying it looks like, how (if at all) that affects ownership, etc.

Note: There is already a workaround to this behavior using Ptr types. Ptrs behave similarly to Refs but are unmanaged, ensuring that problems arising from the anonymous function ownership described above can be circumvented--the user just needs to free these pointers manually in the capturing functions.

@scolsen scolsen added nice-to-have under discussion Things we've not come to a conclusion about. labels Nov 25, 2020
@scolsen
Copy link
Contributor Author

scolsen commented Nov 25, 2020

If #1012 is merged, Pointer.unsafe-alloc and Pointer.free may be used as workarounds for such cases. Instead of using a Ref as a type member, one uses a Ptr and frees it manually where needed.

@scolsen
Copy link
Contributor Author

scolsen commented Nov 25, 2020

Actually, I haven't tried this yet, but presumably one can also get around this by casting the captured argument of an outer lambda to a reference:

(fn [x] (fn [y] (~f &x y))

Then of course f needs to be typed (Fn [&a b] ...).

It's definitely more work than simply supporting refs in members, but it may not be needed. We should document some of these techniques though, if we wind up not implementing ref members.

@eriksvedang
Copy link
Collaborator

My hunch is that we should allow it (it will require struct types to carry a lifetime with them though).

I think the lifetime checker should catch the error even without that feature being implemented, though. So let's fix the bug so that the error is detected first (and add tests for it) and then we can start thinking about how to lift that same restriction.

Does that sound like a good plan?

@scolsen
Copy link
Contributor Author

scolsen commented Nov 25, 2020

Sounds great!

@eriksvedang eriksvedang added the memory Borrow checker and lifetimes label Jun 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
memory Borrow checker and lifetimes nice-to-have under discussion Things we've not come to a conclusion about.
Projects
None yet
Development

No branches or pull requests

2 participants