-
Notifications
You must be signed in to change notification settings - Fork 173
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
feat: don't manage blittable types #1407
Conversation
This commit adds an implementation of Boxes, memory manged heap allocated values. Boxes are implemented as C pointers, with no additional structure but are treated as structs in Carp. To facilitate this, we need to add them as a clause to our special type emissions (TypesToC) as they'd otherwise be emitted like other struct types. Co-authored-by: Veit Heller <[email protected]>
Make sure we free the box!
This reverts commit 70ec6d4.
Now that a builtin type named Box exists, the definitions in this file cause a conflict. I've renamed the "Box" type in the functor example to remove the conflict.
Box.peek allows users to transform a reference to a box into a a reference to the box's contained value. The returned reference will have the same lifetime as the box. This function allows callers to manipulate the value in a box without re-allocation, for example: ```clojure (deftype Num [val Int]) (let-do [box (Box.init (Num.init 0))] (Num.set-val! (Box.peek &box) 1) @(Num.val (Box.peek &box))) ``` This commit also includes tests for Box.peek. Co-authored-by: TimDeve <[email protected]>
The `blit` interface, when implemented, indicates that a type is freely copyable and does not require deletion. It is similar to Rust's Copy trait and indicates that a type should have copy instead of "move" semantics. This commit updates the memory management system to ignore such types, since we do not need to worry about deleting them. Thus, when using a blittable type, users can freely use a variable of the type in more than one place. This commit also fixes a small typo and adds a TODO to warn users when delete and blit are both implemented (as typically one shouldn't need delete if a type is truly blittable).
This commit updates memory.carp to test that types that implement blit are freely copyable (have copy instead of move semantics, are not borrow checked) and do not cause leaks. Of course, users can still cause leaks if they define blit on a type that isn't actually freely copyable (e.g. it owns some data that is heap allocated), but this test checks the basic behavior.
if this merges I'll open up an issue to produce a warning for types that implement both |
It would be useful to expose |
I think this is definitely worth doing. Related to this, perhaps we should add something about manual implementations being unsafe somewhere in the docs |
For sure, something like "This is a marker interface. DO NOT implement yourself" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty straight-forward!
@scolsen I'll merge this now if that's OK? |
Go for it! |
The
blit
interface, when implemented, indicates that a type is freelycopyable and does not require deletion. It is similar to Rust's Copy
trait and indicates that a type should have copy instead of "move"
semantics. This commit updates the memory management system to ignore
such types, since we do not need to worry about deleting them.
Thus, when using a blittable type, users can freely use a variable of
the type in more than one place.
This commit also fixes a small typo and adds a TODO to warn users when
delete and blit are both implemented (as typically one shouldn't need
delete if a type is truly blittable).
In other words, the borrow checker will no longer complain about the following:
That is, there's no need to borrow types that indicate blit. They are assumed to have copy semantics and not require deletion.
This PR also includes a test for this behavior.