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

Consider moving objc2_foundation into the objc2 crate #39

Closed
madsmtm opened this issue Oct 5, 2021 · 4 comments
Closed

Consider moving objc2_foundation into the objc2 crate #39

madsmtm opened this issue Oct 5, 2021 · 4 comments
Labels
question Further information is requested

Comments

@madsmtm
Copy link
Owner

madsmtm commented Oct 5, 2021

There's a lot of places in objc2_foundation that would benefit from being able to implement From for specific Ids:

  • NSArray::from_vec
  • NSArray::into_vec
  • NSData::from_vec
  • NSDictionary::from_keys_and_objects
  • NSString::from_str
  • And so on...

Same goes for traits like Default.

But unfortunately, we can't create these implementations because Id is defined in another crate, see RFC 2451. I tried creating a helper trait FromId for this purpose, but that doesn't work either:

pub trait FromId<T, O: Ownership> {
    fn from_id(obj: Id<T, O>) -> Self;
}

impl<T, O: Ownership, X: FromId<T, O>> From<Id<T, O>> for X {
    fn from(obj: Id<T, O>) -> Self {
        Self::from_id(obj)
    }
}

// Errors with:
// note: conflicting implementation in crate `core`:
// - impl<T> From<T> for T;
// note: downstream crates may implement trait `FromId<_, _>` for type `Id<_, _>`

Though doing it for Default would:

// In objc2:

pub trait IdDefault: Sized {
    type Ownership: Ownership;
    fn id_default() -> Id<Self, Self::Ownership>;
}

impl<T: IdDefault> Default for Id<T, T::Ownership> {
    fn default() -> Self {
        T::id_default()
    }
}

// In objc2_foundation:

impl IdDefault for NSString {
    type Ownership = <NSString as INSObject>::Ownership;
    fn id_default() -> Id<Self, Self::Ownership> {
        <NSString as INSObject>::new()
    }
}

So possible solutions:

  • Move objc2_foundation to objc2::foundation (under a feature flag)
  • Move Id to objc2_foundation?
@madsmtm madsmtm added the question Further information is requested label Oct 5, 2021
@madsmtm
Copy link
Owner Author

madsmtm commented Oct 6, 2021

Might also be relevant to move most of the implementation of objc2_exception into objc2.

It currently uses an Exception type that doesn't implement Encode and Message, and it links manually to libobjc. Of course, these could be alleviated by introducing objc2_encode and objc2_sys as dependencies of it.

But it also returns *mut Exception instead of the correct Id<Exception, Shared>, and the only way to fix that is by moving the implementation to objc2.

@madsmtm
Copy link
Owner Author

madsmtm commented Oct 15, 2021

I think we may move the objc2_exception implementation at some point, but the objc2_foundation separation should stay; it is fundamentally separate from the objc2 crate, and having these limitations in our own crate encourages us to create helper traits (like DefaultId and such, see #41) that can be used by all downstream crates.

@madsmtm madsmtm mentioned this issue Oct 15, 2021
9 tasks
@madsmtm
Copy link
Owner Author

madsmtm commented Oct 15, 2021

I asked on the forum whether this is possible, we'll see if someone comes up with something; otherwise people will just have to import FromId/IntoId and use from_id/into_id manually

@madsmtm
Copy link
Owner Author

madsmtm commented Oct 31, 2021

I'm not gonna move objc2_foundation, so I'll close this issue. Whether to use the approach in #48 or #50 can be determined in the PRs themselves.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

1 participant