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

Proof of concept for proc macros #413

Closed
wants to merge 2 commits into from
Closed

Proof of concept for proc macros #413

wants to merge 2 commits into from

Conversation

silvanshade
Copy link
Contributor

Following the discussion from #411

// string: &NSString,
// ) -> Id<Self, Shared>;
//
// #[method(sel = loadHTMLString:baseURL:, retain)] // retain without = defaults to `other`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could probably make specifying even retain optional in the typical case since we could just determine this by looking at the return type of the signature.

Copy link
Owner

@madsmtm madsmtm Jan 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason that I did it the @__retain_semantics extern_methods! originally is because of performance (otherwise it uses const stuff to figure out the retain method from the selector), but doing such work is possibly performant enough when running as a proc macro, so we might not need it at all.


// Examples:
//
// #[method(sel = initTextCell:, retain = init)]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to my other comment, we could allow eliding the retain info here as well (by examining the type of this).

@silvanshade
Copy link
Contributor Author

Thinking about this some more, it seems like we might be able to simplify things more than I anticipated.

What if we had something like this:

#[class(extern, super = NSActionCell, inherits = NSCell, NSObject)]
struct NSPathCell;

// `unsafe impl ClassType for NSPathCell` added automatically

#[class]
unsafe impl NSPathCell {
    // #[method] can often be skipped entirely
    pub unsafe fn pathStyle(&self) -> NSPathStyle;
    pub unsafe fn setPathStyle(&self, path_style: NSPathStyle);
    #[cfg(all(feature = "Foundation_NSArray", feature = "Foundation_NSString"))]
    pub unsafe fn allowedTypes(&self) -> Option<Id<NSArray<NSString>, Shared>>;
}

#[protocol] // `: NSObjectProtocol` can be added automatically
pub unsafe trait NSPathCellDelegate {
    #[cfg(all(feature = "AppKit_NSOpenPanel", feature = "AppKit_NSPathCell"))]
    #[method(optional)] // selector computed from name
    unsafe fn pathCell_willDisplayOpenPanel(
        &self,
        path_cell: &NSPathCell,
        open_panel: &NSOpenPanel,
    );
}

// `unsafe impl ProtocolType for dyn NSPathCellDelegate {}` added automatically

Anyway, I might try experimenting with a few different approaches to see what seems feasible.

@madsmtm
Copy link
Owner

madsmtm commented Jan 30, 2023

// #[method] can often be skipped entirely

Don't spend too much time on this; we'll want different names for methods anyhow: #284

#[class(extern, super = NSActionCell, inherits = NSCell, NSObject)]
struct NSPathCell;

I'm not sure about this one, it would allow writing something like #[class(super = NSString)] struct NSCell; without unsafe anywhere. We'd have to make sure that case was sound (if not still nonsensical).

@madsmtm
Copy link
Owner

madsmtm commented Jan 30, 2023

Also, I suspect I'd like to keep msg_send! and msg_send_id! as non proc-macros; that way, if users just want a better objc, they can opt out of the more advanced functionality, and just get the improvements that msg_send_id! provides.

@silvanshade silvanshade closed this by deleting the head repository Feb 6, 2023
@madsmtm madsmtm reopened this Feb 13, 2023
@madsmtm
Copy link
Owner

madsmtm commented Feb 19, 2023

Superseded by #425

@madsmtm madsmtm closed this Feb 19, 2023
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