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

Make Sel an opaque struct like Ivar, Method, Class, Protocol and Object #105

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

madsmtm
Copy link

@madsmtm madsmtm commented Jun 16, 2021

The discrepancy between usage of Class/Object/... and Sel annoyed me, the others were used behind pointers / references, while Sel was consumed directly.

This changes the definition of Sel to be like the other types (so it has to be used as &Sel), and also opens up for the possiblity of non-static selectors (if that ever becomes a thing).

This is a breaking change, but fortunately most cases where Sel is used in user code (e.g. add_method) the error is caught at compile-time, with recommendations on how to fix it.

The discrepancy between usage of `Class`/`Object`/... and `Sel` annoyed me, the others were used behind pointers / references, while `Sel` was consumed directly.

This changes the definition of `Sel` to be like the other types (so it has to be used as `&Sel`), and also opens up for the possiblity of non-static selectors (if that ever becomes a thing).

This is a breaking change, but fortunately most cases where `Sel` is used in user code (e.g. `add_method`) the error is caught at compile-time, with recommendations on how to fix it.
@SSheldon
Copy link
Owner

SSheldon commented Aug 4, 2021

Selectors in objective-c are treated like they have copy semantics. Why does the discrepancy here annoy you? Is there a benefit to having a lifetime on selectors? I don't believe there will be a need for non-static selectors.

If anything, perhaps making Class/Method/etc references was the mistake, because those lifetimes are barely ever useful in practice. (idk if anything ever actually gets unregistered from the objc runtime)

@madsmtm
Copy link
Author

madsmtm commented Aug 6, 2021

Mostly it just confused me since all the other types worked the other way.

But I think it actually would make the workings of Sel more explicit: Since Sel::register would return a static reference, you'd be able to instantly see "hey, this is Copy and the Sel doesn't stop being valid when I drop it". Additionally Sel::name would automatically return &'static str (actually, maybe it should do that now too?).

But no, I don't think you can actually get a non-static selector.

Classes however can be destroyed with objc_disposeClassPair, and we might at some point want to make an API that allows control over this (right now we're just "leaking" classes). So I think working with Class behind references is appropriate for now.

@madsmtm madsmtm mentioned this pull request Sep 5, 2021
80 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants