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

objc_interface_type.h generates non-compiling code. #1954

Closed
emilio opened this issue Dec 23, 2020 · 6 comments · Fixed by #2176
Closed

objc_interface_type.h generates non-compiling code. #1954

emilio opened this issue Dec 23, 2020 · 6 comments · Fixed by #2176

Comments

@emilio
Copy link
Contributor

emilio commented Dec 23, 2020

It generates:

#[repr(transparent)]
#[derive(Clone)]
pub struct Foo(pub id);

#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct FooStruct {
    pub foo: Foo,
}

So either the interface pointer should derive Copy, Debug and co, or we should prevent structs like that from deriving them.

This particular case is a regression from 840b738 I think, cc @simlay. I'm commenting out the struct in #1953 to have macos CI again.

@emilio
Copy link
Contributor Author

emilio commented Dec 23, 2020

objc_whitelist has the same issue.

@emilio
Copy link
Contributor Author

emilio commented Dec 23, 2020

That last one was broken by 4562ef9 (should've reviewed the test update a bit more carefully)

@simlay
Copy link
Contributor

simlay commented Dec 23, 2020

Ah yes, the Copy derive removed from #1883 objective-c structs broke the derive Copy in the C struct. Come to think of it, I think RustAudio/coreaudio-sys#33 (comment) is this same issue. I'll admit that I've come to like how not deriving Copy for the objective-c stuff has caused me to write more ergonomic objective-c rust but if it produces broken bindings, it's not worth it.

I'm a little confused why we didn't catch it and why #1953 brings it to light though.

@emilio
Copy link
Contributor Author

emilio commented Dec 23, 2020

Another failure, from objc_template.h:

error[E0310]: the parameter type `ObjectType` may not live long enough                                                                                                                                                                   
  --> /Users/runner/work/rust-bindgen/rust-bindgen/tests/expectations/tests/libclang-4/objc_template.rs:34:9
   |
29 | pub trait IFoo<ObjectType>: Sized + std::ops::Deref {
   |                ---------- help: consider adding an explicit lifetime bound...: `ObjectType: 'static`
...
34 |         msg_send!(*self, get)
   |         ^^^^^^^^^^^^^^^^^^^^^ ...so that the type `*mut ObjectType` will meet its required lifetime bounds

These were covered up because we were running the test job on macos, but not the expectations job (which is basically cd tests/expectations && cargo test).

I noticed it, fixed it, and found all this fun :)

@simlay
Copy link
Contributor

simlay commented Dec 23, 2020

I noticed it, fixed it, and found all this fun :)

Well, I feel a little bad. I wish I would have known that we didn't run the expectations job in macos and would have added it to CI back in like February. I'm glad we're uncovering the issues though.

@emilio
Copy link
Contributor Author

emilio commented Dec 23, 2020

Right, right, this was a surprise to me as well, but glad to find these indeed :)

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

Successfully merging a pull request may close this issue.

2 participants