objc: Implement class methods #558
Conversation
|
|
src/codegen/mod.rs
Outdated
| let sig = aster::AstBuilder::new() | ||
| let sig; | ||
|
|
||
| if method.is_class() { |
There was a problem hiding this comment.
nit:
let sig = if method.is_class() {
// ...
} else {
// ...
};There was a problem hiding this comment.
I'm still not used to this style, but I suppose its ok :)
src/codegen/mod.rs
Outdated
| @@ -2437,9 +2442,19 @@ impl CodeGenerator for ObjCInterface { | |||
|
|
|||
| let methods_and_args = | |||
| ctx.rust_ident(&method.format_method_call(&arg_names)); | |||
| let body = quote_stmt!(ctx.ext_cx(), | |||
| msg_send![self, $methods_and_args]) | |||
| let body; | |||
src/ir/objc.rs
Outdated
| let method = ObjCInstanceMethod::new(&name, signature); | ||
|
|
||
| interface.methods.push(method); | ||
| if c.kind() == CXCursor_ObjCInstanceMethodDecl { |
There was a problem hiding this comment.
nit: I think this may be cleaner with something like:
let is_class_method = c.kind() == CXCursor_ObjCInstanceMethodDecl;'
let method = ObjCMethod:new(&name, signature, is_class_method);
interface.add_method(method)where add_method checks the is_class_method flag and pushes it to the right list?
src/ir/objc.rs
Outdated
| name: name.to_owned(), | ||
| rust_name: rust_name.to_owned(), | ||
| signature: signature, | ||
| is_class: is_class, |
There was a problem hiding this comment.
How do you feel about calling this and the accessor is_class_method instead, so it's a bit more explicit?
src/codegen/mod.rs
Outdated
| .method_sig() | ||
| .unsafe_() | ||
| .fn_decl() | ||
| .self_() | ||
| .build(ast::SelfKind::Value(ast::Mutability::Immutable)) | ||
| .with_args(fn_args.clone()) | ||
| .build(fn_ret); | ||
| } | ||
|
|
||
| // Collect the actual used argument names | ||
| let arg_names: Vec<_> = fn_args.iter() |
There was a problem hiding this comment.
nit: Let's indent this to the right indentation now, given it's moved.
There was a problem hiding this comment.
Ehh, I guess I had ignore whitespaces on when adding chunks to the commit, indent should be better now
|
All good points, cleaned up version here |
| type Class = *mut objc::runtime::Class; | ||
| #[inline] | ||
| fn class(name: &str) -> Class { | ||
| unsafe { std::mem::transmute(objc::runtime::Class::get(name)) } |
There was a problem hiding this comment.
instead of using transmute, let's use something like:
objd::runtime::Class::get(name).map(|c| c as *const _ as * mut _).unwrap_or(ptr::null_mut())There was a problem hiding this comment.
But actually, it seems it's not needed, right? I mean, instead of having this function, why can't we just use something like the following in the methods?
unsafe fn method() {
msg_send![objc::runtime::Class::get("ClassName").expect("Couldn't find ClassName"), ...]
}There was a problem hiding this comment.
Well, yes, it can be inline, I'm mostly just imitating what cocoa-rs is doing for now.
The expect is a bit different, though might be better in this case.
Normally NSClassFromString can return nil, you either check for that or rely on the silent nil for it to do nothing interesting, but in this case knowing that the call doesn't do anything might be more useful, and have the dynamic availability checks be done other ways..
I'll do it like you suggested for now, but I'm pretty sure that the generated code will have various revisions after I get some real code running
|
☔ The latest upstream changes (presumably #544) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@bors-servo r+ |
|
📌 Commit cc6ab47 has been approved by |
objc: Implement class methods Ah yes, I was still missing class methods. They are handled pretty much identically with instance methods, except for in the codegen it needs to know the class name.
|
☀️ Test successful - status-travis |
Ah yes, I was still missing class methods.
They are handled pretty much identically with instance methods, except for in the codegen it needs to know the class name.