Skip to content

Improve type safety in cocoa crate #200

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

Open
Tracked by #719
jrmuizel opened this issue Feb 19, 2018 · 10 comments
Open
Tracked by #719

Improve type safety in cocoa crate #200

jrmuizel opened this issue Feb 19, 2018 · 10 comments

Comments

@jrmuizel
Copy link
Collaborator

currently all of the interfaces are implemented as traits on Id. This means there's no type safety between objects. We should probably use something like the foreign_obj_type! macro from https://github.com/gfx-rs/metal-rs/blob/master/src/lib.rs to improve things here.

@jrmuizel
Copy link
Collaborator Author

https://github.com/SSheldon/rust-objc-foundation also contains some better infrastructure than we are currently using.

@vbo
Copy link
Contributor

vbo commented Apr 17, 2018

Could we just start by using newtype pattern around id for selected interfaces and incrementally move towards full type safety?

Also in this case I am wondering if we could make some methods safe. It seems if we can guarantee id can only be allocated through a typed constructor for the client there should be nothing unsafe about calling at least a subset of methods on it. Am I missing something?

E.g.

let mut window: NSWindow = NSWindow::init()
                                    .with_content_rect(rect)
                                    .with_style_mask(mask)
                                    .with_backing(backing)
                                    .with_defer(false)
                                    .build().unwrap();
let title: NSString = NSString::from_str("Hello world");
window.set_title(title)

should be safe.

@jrmuizel
Copy link
Collaborator Author

Yes, that sounds reasonable to me.

@jrmuizel
Copy link
Collaborator Author

https://github.com/gfx-rs/metal-rs/ is another project that has better type safety.

@hardiesoft
Copy link

Now that impl Trait has landed in stable rust, could using it for return types in place of -> id be an acceptable option?

This would be a breaking change. Does this crate have constraints that require it to work on earlier versions of rust?

I'd be happy to submit a PR if this seems like a good way forward. It seems simpler than newtyping all the things, but maybe there are some other type-safety benefits of going the newtype route that I am missing?

@jrmuizel
Copy link
Collaborator Author

jrmuizel commented May 10, 2018 via email

@hardiesoft
Copy link

I think it's a pretty straightforward transformation of ids into impl {Object} really, which has the nice fringe benefit of giving proper completion in IntelliJ Rust/rls.

In this example, note that CALayer and NSLayoutDimension are not currently defined, these could be left as opaque ids for now, and replaced if/when those types get defined.

pub trait NSView: Sized {
    unsafe fn alloc(_: Self) -> impl NSView {
        msg_send![class("NSView"), alloc]
    }

    unsafe fn init(self) -> impl NSView;
    unsafe fn initWithFrame_(self, frameRect: NSRect) -> impl NSView;
    unsafe fn bounds(self) -> NSRect;
    unsafe fn frame(self) -> NSRect;
    unsafe fn display_(self);
    unsafe fn setWantsBestResolutionOpenGLSurface_(self, flag: BOOL);
    unsafe fn convertPoint_fromView_(self, point: NSPoint, view: impl NSView) -> NSPoint;
    unsafe fn addSubview_(self, view: impl NSView);
    unsafe fn superview(self) -> impl NSView;
    unsafe fn removeFromSuperview(self);
    unsafe fn setAutoresizingMask_(self, autoresizingMask: NSAutoresizingMaskOptions);

    unsafe fn wantsLayer(self) -> BOOL;
    unsafe fn setWantsLayer(self, wantsLayer: BOOL);
    unsafe fn layer(self) -> impl CALayer;
    unsafe fn setLayer(self, layer: impl CALayer);

    unsafe fn widthAnchor(self) -> impl NSLayoutDimension;
    unsafe fn heightAnchor(self) -> impl NSLayoutDimension;
    unsafe fn convertRectToBacking(self, rect: NSRect) -> NSRect;
}

@hardiesoft
Copy link

hardiesoft commented May 10, 2018

Ugh, never mind, this totally doesn't work the way I thought it did, and is in fact invalid syntax.

It's possible this, or something like it might work in the future: https://github.com/rust-lang/rfcs/blob/master/text/1522-conservative-impl-trait.md#limitation-to-freeinherent-functions

@hardiesoft
Copy link

This does work however:

pub trait NSView: Sized {
    unsafe fn alloc(_: Self) -> id where id:NSView {
        msg_send![class("NSView"), alloc]
    }

    unsafe fn init(self) -> id where id:NSView;
    unsafe fn initWithFrame_(self, frameRect: NSRect) -> id where id:NSView;
    unsafe fn bounds(self) -> NSRect;
    unsafe fn frame(self) -> NSRect;
    unsafe fn display_(self);
    unsafe fn setWantsBestResolutionOpenGLSurface_(self, flag: BOOL);
    unsafe fn convertPoint_fromView_(self, point: NSPoint, view: impl NSView) -> NSPoint;
    unsafe fn addSubview_(self, view: impl NSView);
    unsafe fn superview(self) -> id where id:NSView;
    unsafe fn removeFromSuperview(self);
    unsafe fn setAutoresizingMask_(self, autoresizingMask: NSAutoresizingMaskOptions);

    unsafe fn wantsLayer(self) -> BOOL;
    unsafe fn setWantsLayer(self, wantsLayer: BOOL);
    unsafe fn layer(self) -> id where id:CALayer;
    unsafe fn setLayer(self, layer: impl CALayer);

    unsafe fn widthAnchor(self) -> id where id:NSLayoutDimension;
    unsafe fn heightAnchor(self) -> id where id:NSLayoutDimension;
    unsafe fn convertRectToBacking(self, rect: NSRect) -> NSRect;
}

@NeoLegends
Copy link

What's the state on this? Newtypes seem like a suitable solution, no?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants