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

Improve #[derive(...)] in extern_class! and declare_class! #267

Open
madsmtm opened this issue Sep 8, 2022 · 5 comments
Open

Improve #[derive(...)] in extern_class! and declare_class! #267

madsmtm opened this issue Sep 8, 2022 · 5 comments
Labels
A-objc2 Affects the `objc2`, `objc-sys` and/or `objc2-encode` crates enhancement New feature or request good first issue Good for newcomers

Comments

@madsmtm
Copy link
Owner

madsmtm commented Sep 8, 2022

Deriving PartialEq, Eq and Hash automatically in extern_class! works because it delegates to a msg_send! call to the relevant selector.

#[derive(Debug)], however, prints out the internal variable name as well that ensures that the type has the correct auto traits, which is really ugly, so we should use some macro hacks to extract and implement it ourselves as a direct delegation to the superclass' Debug instead.

@madsmtm madsmtm added enhancement New feature or request A-objc2 Affects the `objc2`, `objc-sys` and/or `objc2-encode` crates good first issue Good for newcomers labels Sep 8, 2022
@madsmtm madsmtm added this to the Polish icrate milestone Sep 11, 2023
@madsmtm
Copy link
Owner Author

madsmtm commented Sep 30, 2023

We also need to consider how things should work in declare_class!? Because when deriving, we could actually use that syntax for also implementing relevant protocols on the class!

Interesting std derive macros Relevant protocols/methods to implement
PartialEq + Eq -[NSObjectProtocol isEqual:]
Hash -[NSObjectProtocol hash]
Clone -[NSCopying copyWithZone]
Debug -[NSObjectProtocol description/debugDescription]
DefaultId -[NSObjectProtocol init] (+ ensure no new)
PartialOrd + Ord compare:

However I'm unsure how these implementations should actually work?

In the case of types that inherit NSObject, you'd think the behaviour to be fairly straightforward, we compare the ivars for equality - but even then, we probably don't want things that inherit NSObject, but don't provide any ivars, to compare equal, so maybe it's not so clear cut?

@madsmtm
Copy link
Owner Author

madsmtm commented Sep 30, 2023

Maybe the actually useful thing in declare_class! would be to allow #[derive(IdDefault, NSCopying, NSObjectProtocol)]?

IdDefault would create:

unsafe impl MyObject {
    #[method_id(init)]
    fn init(this: Allocated<Self>) -> Option<Id<Self>> {
        let this = this.set_ivars(Self::Ivars::default()); // Require `Ivars: Default`
        // SAFETY: Because of `Self::Super: IdDefault`, it is sound to call the `init` method on this
        // `IdDefault` would need to be an `unsafe` trait, so that we can ensure the super class is `init`-able
        unsafe { msg_send_id![super(this), init] }
    }
}

// SAFETY: `init` is implemented and valid for this object
unsafe impl IdDefault for MyObject {}

NSCopying would create (unsure which implementation would actually be the most correct? copy works differently from init, since objects are responsible for allocating the object themselves too):

unsafe impl NSCopying for MyObject {
    #[method_id(copyWithZone:)]
    fn copyWithZone(&self, zone: *const NSZone) -> Id<Self> {
        // Implementation 1
        // Require `T::Super: NSCopying`
        let new = unsafe { msg_send_id![super(self), copyWithZone: zone] };
        // Require `Self::Ivars: Clone`
        new.set_ivars(self.ivars().clone())

        // Implementation 2
        // Require `Self::Ivars: Clone`
        let new = Self::alloc().set_ivars(self.ivars().clone());
        // Require `Self::Super: IdDefault`
        unsafe { msg_send_id![super(self), init] }
    }
}

@madsmtm
Copy link
Owner Author

madsmtm commented Sep 30, 2023

I think for NSObjectProtocol, it would perhaps be possible to use autoref specialization to check if the user implemented PartialEq, Hash or Debug for their class, and then generate proper implementations of NSObjectProtocol:

unsafe impl NSObjectProtocol for MyObject {
    #[method(isEqual:)]
    #[cfg(Self implements PartialEq)]
    fn isEqual(&self, other: &Self) -> bool {
        <Self as PartialEq>::partial_eq(self, other)
    }

    #[method(hash)]
    #[cfg(Self implements Hash)]
    fn hash(&self) -> NSUInteger {
        let mut hasher = DefaultHasher::new();
        <Self as Hash>::hash(self, &mut hasher);
        hasher.finish() as NSUInteger
    }

    #[method_id(description)]
    #[cfg(Self implements Debug)] // Maybe `Display`?
    fn description(&self) -> Id<NSString> {
        let s = format!("{self:?}");
        NSString::from_str(&s)
    }

    #[method_id(debugDescription)]
    #[cfg(Self implements Debug)]
    fn debugDescription(&self) -> Id<NSString> {
        let s = format!("{self:#?}"); // Note the `#:?` formatting
        NSString::from_str(&s)
    }
}

@madsmtm
Copy link
Owner Author

madsmtm commented Sep 30, 2023

The difficult part here is that in general, you could want two things when deriving PartialEq: Referential equality or structural equality. Usually you want the latter, though that is really difficult to generate an implementation for in general (since NSObject, and hence all other objects by default, compare referentially. It's something you have to know when implementing PartialEq, whether you should just defer to the superclass, compare your own ivars, or do both).

So that's why making NSObjectProtocol defer to a custom implementation of PartialEq and Hash instead is somewhat nice.

@madsmtm madsmtm changed the title Improve Debug in extern_class! Improve #[derive(...)] in extern_class! Oct 26, 2023
@madsmtm madsmtm changed the title Improve #[derive(...)] in extern_class! Improve #[derive(...)] in extern_class! and declare_class! Oct 26, 2023
@madsmtm
Copy link
Owner Author

madsmtm commented Dec 3, 2023

Swift's Equatable protocol is automatically implemented for types inheriting NSObject, and implements reference equality by default.

If the user wants anything else, they're supposed to override the isEqual: method, and there's a lint that protects users from accidentally overriding == instead.

Similarly for Hashable/hash.

So maybe we should also just do referential equality by default?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-objc2 Affects the `objc2`, `objc-sys` and/or `objc2-encode` crates enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

1 participant