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

Add rc::Allocated #172

Merged
merged 1 commit into from
Jul 5, 2022
Merged

Add rc::Allocated #172

merged 1 commit into from
Jul 5, 2022

Conversation

madsmtm
Copy link
Owner

@madsmtm madsmtm commented Jun 15, 2022

Blocked on #120.

Add a new user facing opaque struct rc::Allocated<T>, which is intended to be used inside Id as Id<Allocated<T>, O>, and can be gotten from msg_send_id![cls, alloc]. This will help users differentiate between allocated and initialized objects.

See also #87 for more discussion about why we want this.

TODO:

  • Documentation
  • More functionality on Allocated<T>? Postponed
  • More functionality on Id<Allocated<T>, O>? Postponed
  • Should it be possible to retrieve ivars and initialize them? Postponed to Better class declaration #30
  • Consider Allocated<T>(*mut T)

@madsmtm madsmtm added enhancement New feature or request A-objc2 Affects the `objc2`, `objc2-exception-helper` and/or `objc2-encode` crates labels Jun 15, 2022
@madsmtm madsmtm added this to the objc2 v0.3 milestone Jun 15, 2022
This was referenced Jun 15, 2022
@madsmtm madsmtm force-pushed the smart-msg-send branch 2 times, most recently from 14a4fd2 to 3f1f138 Compare June 15, 2022 20:10
Base automatically changed from smart-msg-send to master June 15, 2022 21:06
@madsmtm
Copy link
Owner Author

madsmtm commented Jun 16, 2022

We may want to just make it Allocated<T: Message>(*const T), since:

  • The ownership doesn't really matter, you shouldn't deref it anyhow
  • Deallocation is less efficient if it has to unwrap an Option first (very slight issue)
  • Whether we should dealloc after alloc on panic may be runtime specific, and this would allow specialized behaviour for that. Nope, it doesn't differ between runtimes, at least not Apple (old+new) and GNUStep.
  • Easier to change afterwards (e.g. just add #[deprecated] type Allocated<T> = Option<Id<NewAllocated<T>, Unknown>>).

@madsmtm madsmtm force-pushed the allocated-struct branch 4 times, most recently from c1938be to baad092 Compare July 5, 2022 18:16
@madsmtm madsmtm marked this pull request as ready for review July 5, 2022 18:22
@madsmtm
Copy link
Owner Author

madsmtm commented Jul 5, 2022

I'll go with Id<Allocated<T>, O> over Allocated<T>(*mut T) simply because the former requires less code on our end - it's really not that important, both APIs would work fine.

@madsmtm madsmtm merged commit 79661f9 into master Jul 5, 2022
@madsmtm madsmtm deleted the allocated-struct branch July 5, 2022 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-objc2 Affects the `objc2`, `objc2-exception-helper` and/or `objc2-encode` crates enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant