Skip to content

Added Bindgen names to objective-c pointer return types for #1835. #1847

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

Merged
merged 1 commit into from
Aug 15, 2020

Conversation

simlay
Copy link
Contributor

@simlay simlay commented Jul 27, 2020

This closes #1835.

This change seems too easy and really only works because the generated structs are #[repr(transparent)].

I tested it with UIKit in my iced branch, and this is the changeset.

I also added some more documentation to highlight this change. Rendered.

Also, I'd like to mention that this is definitely a breaking change to any crates that take advantage of the objective-c generated bindings. After a little bit of searching on crates.io, I can't really find any published crates that do this. Perhaps I'm the only one?

@highfive
Copy link

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!

@simlay simlay force-pushed the add_bindgen_names_for_objc_pointers branch from 0e88711 to 6ffbdef Compare July 27, 2020 22:17
Copy link
Contributor

@emilio emilio left a comment

Choose a reason for hiding this comment

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

Sorry for the lag reviewing this. This looks great, with one nit addressed. Though I guess I can also address it myself in a follow-up.

@emilio emilio force-pushed the add_bindgen_names_for_objc_pointers branch from 6ffbdef to 22fa828 Compare August 3, 2020 16:25
@emilio
Copy link
Contributor

emilio commented Aug 3, 2020

I rebased and addressed my nit, I'll merge once CI is back green. Thanks again for the fix!

@@ -3466,6 +3466,9 @@ impl TryToRustTy for Type {
inner.into_resolver().through_type_refs().resolve(ctx);
let inner_ty = inner.expect_type();

let is_objc_pointer =
matches!(inner_ty.kind(), TypeKind::ObjCInterface(..));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, d'oh, I thought that we were using the matches crate already... Sure, then let's make this just a plain match :)

@simlay simlay force-pushed the add_bindgen_names_for_objc_pointers branch from 22fa828 to 7ee6eb6 Compare August 6, 2020 21:44
* Took advantage of the repr transparent to use Bindgen return type
  names.
* Updated unit tests and book
@simlay simlay force-pushed the add_bindgen_names_for_objc_pointers branch from 834014f to 1431f2d Compare August 9, 2020 19:38
@simlay simlay requested a review from emilio August 14, 2020 23:57
Copy link
Contributor

@emilio emilio left a comment

Choose a reason for hiding this comment

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

I'll address my nit in a follow-up.

@@ -3466,6 +3466,14 @@ impl TryToRustTy for Type {
inner.into_resolver().through_type_refs().resolve(ctx);
let inner_ty = inner.expect_type();

let is_objc_pointer =
inner.kind().as_type().map_or(false, |ty| {
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use inner_ty here, which is just a type already, right?

@emilio emilio merged commit 4299255 into rust-lang:master Aug 15, 2020
emilio added a commit that referenced this pull request Aug 15, 2020
@simlay simlay mentioned this pull request Aug 18, 2020
@simlay simlay mentioned this pull request Sep 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use Bindgen names in Object-C pointer input and return types.
3 participants