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

Fix to Objective-C header parsing has broken certain method calls #2647

Closed
csmulhern opened this issue Sep 23, 2023 · 5 comments · Fixed by #2648
Closed

Fix to Objective-C header parsing has broken certain method calls #2647

csmulhern opened this issue Sep 23, 2023 · 5 comments · Fixed by #2648

Comments

@csmulhern
Copy link
Contributor

csmulhern commented Sep 23, 2023

Input C/C++ Header

@interface MyEvent

@property(nonatomic, retain) id type;

@end

Bindgen Invocation

$ bindgen input.h -- -x objective-c

Actual Results

/* automatically generated by rust-bindgen 0.68.1 */

use objc::{self, class, msg_send, sel, sel_impl};
#[allow(non_camel_case_types)]
pub type id = *mut objc::runtime::Object;
#[repr(transparent)]
#[derive(Debug, Copy, Clone)]
pub struct MyEvent(pub id);
impl std::ops::Deref for MyEvent {
    type Target = objc::runtime::Object;
    fn deref(&self) -> &Self::Target {
        unsafe { &*self.0 }
    }
}
unsafe impl objc::Message for MyEvent {}
impl MyEvent {
    pub fn alloc() -> Self {
        Self(unsafe { msg_send!(class!(MyEvent), alloc) })
    }
}
impl IMyEvent for MyEvent {}
pub trait IMyEvent: Sized + std::ops::Deref {
    unsafe fn type_(&self) -> id
    where
        <Self as std::ops::Deref>::Target: objc::Message + Sized,
    {
        msg_send!(*self, r#type)
    }
    unsafe fn setType_(&self, type_: id)
    where
        <Self as std::ops::Deref>::Target: objc::Message + Sized,
    {
        msg_send!(*self, setType: type_)
    }
}

Expected Results

type_ should be implemented as:

unsafe fn type_(&self) -> id
where
    <Self as std::ops::Deref>::Target: objc::Message + Sized,
{
    msg_send!(*self, type)
}

I believe this was caused by the changes in 97e29b4. While the fix in that commit fixes usage of format_method_call in parsing, format_method_call is also used in codegen, see:

let methods_and_args = method.format_method_call(&fn_args);

This change causes reserved words to escaped in the generated msg_send calls, which causes unrecognized selector errors at runtime (i.e. -[MyEvent r#type] is called instead of -[MyEvent type]).

@pvdrz, do you have any additional context here?

@pvdrz
Copy link
Contributor

pvdrz commented Sep 25, 2023

mmm... I checked out to 2df02c2 which is the parent of 97e29b4 and the emitted code is still

pub trait IMyEvent: Sized + std::ops::Deref {
    unsafe fn type_(&self) -> id
    where
        <Self as std::ops::Deref>::Target: objc::Message + Sized,
    {
        msg_send!(*self, r#type)
    }
    unsafe fn setType_(&self, type_: id)
    where
        <Self as std::ops::Deref>::Target: objc::Message + Sized,
    {
        msg_send ! (* self , setType : type_)
    }
}

so the culprit must be before that specific change.

Edit: after some bisecting, the culprit is 6e5a666

@pvdrz
Copy link
Contributor

pvdrz commented Sep 25, 2023

I added a fix in #2648, @csmulhern could you let me know if this fixes your issue?

@csmulhern
Copy link
Contributor Author

Ahh, sorry for the misleading attribution. #2648 does fix it! Thanks for the quick fix!

@pvdrz
Copy link
Contributor

pvdrz commented Sep 25, 2023

@csmulhern glad to hear! I haven't written any objective-C in the past so it is a bit hard for me to know what's right and what isn't 😅

I think it would make sense to add a small integration test for this to avoid future breakage so I'll try to do that on the same PR or at least open an issue so we don't forget about it.

@pvdrz
Copy link
Contributor

pvdrz commented Sep 27, 2023

I opened #2652 to track the integration tests for objective-c

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 a pull request may close this issue.

2 participants