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

Rename objc2::rc::Id to objc2::rc::Retained #617

Open
madsmtm opened this issue May 20, 2024 · 3 comments
Open

Rename objc2::rc::Id to objc2::rc::Retained #617

madsmtm opened this issue May 20, 2024 · 3 comments
Labels
A-objc2 Affects the `objc2`, `objc2-exception-helper` and/or `objc2-encode` crates documentation Improvements or additions to documentation enhancement New feature or request

Comments

@madsmtm
Copy link
Owner

madsmtm commented May 20, 2024

Id<T> is a leftover from objc_id, but that name doesn't actually convey any meaning to an outsider, and has only weak relevance to the concept in Objective-C (it somewhat matches id, but that's actually closer to AnyObject). Additionally, the shorthand "ID" is usually used to refer to an "Identity Document" or an "Identifier", both of which this most definitely is not.

Instead, we should use the name Retained<T>, which more clearly communicates that it is a retained smart pointer. (The concept of retain/release is one that I'd more expect previous and future Objective-C developers to understand).

Alternative names:

  • Arc<T>, would match well with the ARC i.e. Automatic Reference Counting concept in Objective-C, but conflicts with std::sync::Arc. Still, it's what fruity does.
  • R<T> as a shorthand. Also what cidre does.
  • Strong<T>.
  • Object<T>.
  • ArcObject<T>
  • ?

Migration plan:

  • objc2 v0.5.2: Introduce Retained<T> as an alias of Id<T>. EDIT: Done in b8adc2b.
  • Continuously: Convert internals to use Retained<T> instead of Id<T>. EDIT: Done in d097cd2.
  • objc2 v0.6.0: Deprecate the name Id<T>.
  • ?: Remove Id<T> (maybe never, the cost of keeping the alias is basically zero).

This also affects:

  • WeakId<T>, should be renamed to RetainedWeak<T>? Or just objc2::rc::Weak<T>, just like we have both std::sync::Weak<T> and std::rc::Weak<T>?
  • msg_send_id! / #[method_id(...))], I'm going to keep these names as id is still a concept in Objective-C that refers to "an object", and calling them _retain would be wrong (as whether or not to retain is). Maybe _object or _ would be better, but it's not as glaring a problem.
  • IsIdCloneable, should be renamed to IsRetainedCloneable? Or maybe don't do anything here, and instead wait for Consider removing (non-interior) mutability? #563.
  • DefaultId, IdFromIterator and IdIntoIterator should all be renamed.

CC @simlay @silvanshade, I'm making this change far too late in the project's lifetime, but I believe it to be the right thing to do, even considering the maintenance cost to dependencies. Do you have alternative naming suggestions, or other opinions?

@madsmtm madsmtm added documentation Improvements or additions to documentation enhancement New feature or request A-objc2 Affects the `objc2`, `objc2-exception-helper` and/or `objc2-encode` crates labels May 20, 2024
@madsmtm madsmtm added this to the objc2 v0.6 milestone May 20, 2024
@silvanshade
Copy link
Contributor

The only issue with Retained is that it's slightly long for a type that might show up a lot.

I'm not sure of a good alternative though. Maybe Ref? A lot of crates seem to use that rather generically for custom pointer-like types.

The R is certainly convenient but kind of has the same problem as Id in that it's really not informative, so I think maybe Ref would be better in that case.

You could also consider Obj as a shortened version for Object (which is also a bit long, but not bad otherwise).

I could probably live with Retained though. It's not much longer than Result anyway.

@simlay
Copy link
Collaborator

simlay commented May 21, 2024

I'll admit that I've strayed away from using Id<T> because I've been too lazy to re-learn the details each time I find myself near it. Part of that is me and part of that is because of the name collision with id and Id<T>. I'm almost certainly worse off for straying away due to being lazy.

If it were named Arc<T>, I would assume it acts pretty much like std::sync::Arc<T>. This could be both good and bad. I'll admit I don't know how Id<T> differs from Arc<T>. As "Id<T> can be thought of as kind of a weird combination of Arc and Box, this definitely could lead to a rough time.

I don't hate Retained<T>, it's descriptive about what it does with/to/for T without a thought. One could say "reference count" and "retained count" and it wouldn't mean something vastly different (though there'd be some questionable looks).

Throwing out some other (possibly bad) names:

  • NSArc<T> - Admittedly, this is just because of the NS in NSObject.
  • NSRc<T>

At first I thought that ArcObject<T> would be good (and was going to write it as a suggestion until I saw you already have it). I think what I'd say is that it's both longer than Retained<T> and mixing Object with Arc when it's more of an Arc than an Object to me.

Through the course of writing pros-cons and research, I've convinced myself that I like Retained<T> the most.

@madsmtm
Copy link
Owner Author

madsmtm commented May 21, 2024

Thanks for the input both of you, it's really valuable having someone to spar with! I think the rough conclusion (if any can be reached) is to go with Retained<T>, so that's what I'm gonna do.

As "Id can be thought of as kind of a weird combination of Arc and Box, this definitely could lead to a rough time.

After #563 it will just be Objectice-C's Arc.

madsmtm added a commit to rust-windowing/winit that referenced this issue May 21, 2024
`Id` is a soft-deprecated to `Retained`, that name better reflects what
it actually represents.

See also madsmtm/objc2#617.
madsmtm added a commit that referenced this issue Sep 17, 2024
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 documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants