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

Add CFRunLoop #2

Merged
merged 9 commits into from
Mar 19, 2017
Merged

Add CFRunLoop #2

merged 9 commits into from
Mar 19, 2017

Conversation

burtonageo
Copy link
Contributor

This also required defining some CFAllocator callback types, and some cf definitions from (CFDate), which I will add to in a future commit. Also, block parameters are passed as pointer, because the block crate (https://crates.io/crates/block) is not repr(C) safe.

Hope this is good to merge :-)

… types, and some cf definitions from (CFDate), which I will add to in a future commit. Also, block parameters are passed as pointer, because the `block` crate (https://crates.io/crates/block) is not repr(C) safe.
src/lib.rs Outdated
@@ -1,22 +1,28 @@
#![allow(non_upper_case_globals,non_snake_case)]
// TODO: remove allow(private_in_public), and fix compile errors
#![allow(non_upper_case_globals,non_snake_case,private_in_public)]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to add this allow statement to get an easy fix for rust-lang/rust#34537 because I didn't want to muddle this pr with a comprehensive fix.

Copy link
Owner

Choose a reason for hiding this comment

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

Is this still needed now that I merged #3?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, I've removed it now

@burtonageo
Copy link
Contributor Author

Maybe don't merge this until #3 has been merged, then I can base this commit on those fixes

@burtonageo
Copy link
Contributor Author

All done!

Cargo.toml Outdated
@@ -14,3 +14,4 @@ build = "build.rs"

[dependencies]
libc = "0.2"
mach = "0.0"
Copy link
Owner

Choose a reason for hiding this comment

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

mach_port_t was added in version 0.0.4, so we should restrict the mach version to prevent using versions older than that. 0.0.5 fixes private in public warnings, so I'd go with that version of mach.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All done, good catch :-)

Copy link
Owner

@dcuddeback dcuddeback left a comment

Choose a reason for hiding this comment

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

@burtonageo Thanks for submitting this. I double-checked the definitions against the header files in XCode. I found a few discrepancies that I pointed out as separate comments.

Out of curiosity, have you tried passing blocks from Rust to one of Apple's libraries? Based on reading the LLVM docs, it sounds like it will work correctly the way you defined the functions that take blocks in this PR. Just curious, because I've never tried it.

src/runloop.rs Outdated

pub type CFRunLoopRef = *mut __CFRunLoop;

pub type CFRunLoopResult = i32;
Copy link
Owner

@dcuddeback dcuddeback Oct 12, 2016

Choose a reason for hiding this comment

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

I see CFRunLoopRunResult in CFRunLoop.h (missing the second Run).

src/runloop.rs Outdated
pub fn CFRunLoopRemoveTimer(rl: CFRunLoopRef, timer: CFRunLoopTimerRef, mode: CFStringRef);
pub fn CFRunLoopContainsTimer(rl: CFRunLoopRef, timer: CFRunLoopTimerRef, mode: CFStringRef) -> Boolean;

pub fn CFRunLoopPerformBlock(rl: CFRunLoopRef, mode: CFStringRef, block: *mut c_void);
Copy link
Owner

Choose a reason for hiding this comment

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

I see CFTypeRef for the second parameter:

CF_EXPORT void CFRunLoopPerformBlock(CFRunLoopRef rl, CFTypeRef mode, void (^block)(void)) CF_AVAILABLE(10_6, 4_0);

src/runloop.rs Outdated
pub type CFRunLoopGetPortCallBack = extern "C" fn(info: *mut c_void) -> mach_port_t;
pub type CFRunLoopHashCallBack = extern "C" fn(info: *const c_void) -> CFHashCode;
pub type CFRunLoopMachPerformCallBack = extern "C" fn(msg: *mut c_void, size: CFIndex, allocator: CFAllocatorRef,
info: *mut c_void);
Copy link
Owner

Choose a reason for hiding this comment

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

This callback returns a void * in CFRunLoopSourceContext1:

void *  (*perform)(void *msg, CFIndex size, CFAllocatorRef allocator, void *info);

src/runloop.rs Outdated
pub retain: CFAllocatorRetainCallBack,
pub release: CFAllocatorReleaseCallBack,
pub copyDescription: CFAllocatorCopyDescriptionCallBack,
pub equal: CFRunLoopEqualCallBack,
Copy link
Owner

Choose a reason for hiding this comment

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

I don't see the CFRunLoop*Callback typedefs in CFRunLoop.h. Are those defined somewhere else?

src/runloop.rs Outdated
pub type CFRunLoopObserverCallBack = extern "C" fn(observer: CFRunLoopObserverRef, activity: CFRunLoopActivity,
info: *mut c_void);

pub type CFRunLoopActivity = u32;
Copy link
Owner

Choose a reason for hiding this comment

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

This is typedef'd as CFOptionFlags:

typedef CF_OPTIONS(CFOptionFlags, CFRunLoopActivity) { ... }

which is a typedef for an unsigned long (Rust definition here:

pub type CFOptionFlags = c_ulong;
). Since C doesn't guarantee that a long is 32 bits on all platforms, I think it's safest to keep the definitions as close as possible to how they're defined in C.

@burtonageo
Copy link
Contributor Author

All nits addressed, and private_in_public warnings have been addressed too.

I have successfully used the Block crate to pass blocks from Rust to ObjC.

@dcuddeback dcuddeback merged commit f358825 into dcuddeback:master Mar 19, 2017
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