Added Objc category inheritance #1784

simlay · 2020-05-15T00:49:42Z

This closes #1779.

This is done by adding:

items_mut on BindgenContext which is just iter_mut on the items because a category needs to modify an item to add the name of a category, and the template names from that category. This method most certainly results in very very bad performance but I really don't know how to get a reference to the ObjCInterface using just a name. So :/
The category name and template names are added to the ObjCInterface asVec<(String, Vec<String>)>. I'm open to making this a struct, I just kinda don't know what to call it.

I originally had the categories field as a Vec<Box<ObjCInterface>> and then clone()ed the interface after it was finished being built. You can see what that's like here. It required deriving Clone on ObjCInterface, ObjCMethod, and FuncionSig. I think Vec<(String, Vec<String>)> is cleaner but still a bit gross.

I also noticed that there's a lot of the templates when generating bindings. Here's an example for the Foundation Framework. So, I added a bunch of tests for the different inheritance cases - categories, templates, and categories that are templates.

* Added items_mut to BindgenContext * Added Clone to ObjCInterface, ObjCMethod and FunctionSig :/ * Added references to the ObjCInterfaces for categories that extend a given ObjCInterface

* Added a bunch more tests to prevent future breakage.

highfive · 2020-05-15T00:49:47Z

Warning

These commits modify unsafe code. Please review it carefully!

emilio

I don't know enough about objective-c categories, but iterating all the items like that is not acceptable.

The way bindgen is designed to deal with this kind of stuff is via type references. Is there a way you can inspect the categories from the clang ast while parsing the original interface?

emilio · 2020-05-15T08:58:54Z

src/ir/objc.rs

@@ -37,6 +37,9 @@ pub struct ObjCInterface {
    /// The list of protocols that this interface conforms to.
    pub conforms_to: Vec<ItemId>,

+    /// The list of categories (and the template tags) that this interface is extended by.
+    pub categories: Vec<(String, Vec<String>)>,


Why shouldn't these be ItemIds?

These should probably be ItemIds. After seeing #1889, it looks like you can use categories to add more protocol conformance :/

simlay · 2020-05-15T23:47:38Z

Why shouldn't these be ItemIds?

I've tried a few things to use Vec<ItemId>s as the category identifiers rather than a Vec<String, Vec<String>>

let category_id : ItemId = Item::from_ty_or_ref(c.cur_type(), c, None, ctx).into()

it actually turns out to be the id of the class (the real_interface id that is) I want to modify which is neat. (More on that later).

If I do:

let category_id : ItemId = Item::from_ty_or_ref(cursor.cur_type(), *cursor, None, ctx).into()

it segfaults which is neat.

If I do:

let category_id : ItemId = Item::from_ty_or_ref(cursor.cur_type(), c, None, ctx).into()

it returns an opaque type for that id when I try to do generation.

So, I'm not sure how to get the ItemId for the category. Thoughts?

I don't know enough about objective-c categories, but iterating all the items like that is not acceptable.

let real_interface_id : ItemId = Item::from_ty_or_ref(c.cur_type(), c, None, ctx).into()

yields the ItemId to the interface that we actually want to modify which is pretty cool. I'm having trouble trying to borrow mutability for it though. Do you have advice on doing that?

emilio · 2020-05-16T16:18:35Z

it segfaults which is neat.

What's the stack for that? It might be that you're overflowing the stack.

yields the ItemId to the interface that we actually want to modify which is pretty cool. I'm having trouble trying to borrow mutability for it though. Do you have advice on doing that?

Why do you need to mutably borrow it?

simlay · 2020-05-16T20:45:56Z

What's the stack for that? It might be that you're overflowing the stack.

Yeah, after a adding some debug statements, it looks like that call ends up overflowing the stack. Though, it doesn't yield a stack trace nor a normal rust error like thread 'main' has overflowed its stack. I'm guessing because it's blowing the stack someplace in clang? I'm up to look more into this if it sounds like a bug.

In hindsight, I forgot to answer this question:

Is there a way you can inspect the categories from the clang ast while parsing the original interface?

Hmm. https://clang.llvm.org/doxygen/structclang_1_1serialization_1_1ObjCCategoriesInfo.html#details seems promising. I'm not sure how to use it though.

Why do you need to mutably borrow it?

If we can't get the list of categories while parsing the original class, the only way I think we can get the categories: Vec<ItemId> (or equivalent) is to modify the original class when parsing the category.

src/ir/objc.rs

…category-inheritance

simlay · 2020-08-06T07:27:25Z

@emilio Mind reviewing this PR again? I've been playing around with this branch combined with #1847 and the bindings generated are becoming kinda awesome. In my uikit-sys crate UIButton, all the trait implementations for with underscores are from categories and there are about 50 of them.

simlay · 2020-08-17T08:11:14Z

@scoopr are you available for a review? This PR adds a key feature to bindgen for objective-c support.

scoopr · 2020-08-17T09:17:31Z

I did a fairly quick look at the diff, and from what I gather it looks good to me! Can't really comment on the specifics of the implementation as I'm not too familiar with that anymore. The expectation test result look great to me :)

src/ir/objc.rs

emilio · 2020-08-17T09:36:55Z

src/codegen/mod.rs

@@ -3913,6 +3913,28 @@ impl CodeGenerator for ObjCInterface {
                        }
                    };
                    result.push(impl_trait);
+                    for (category_name, category_template_names) in


So here's a thought, is there any reason we can't just implement this when generating code for the ObjCInterface?

It feels weird that stuff that would be blacklisted would implement these traits and so on. It should also avoid the weird lookup, right?

As in, instead of keeping the category as a String in the interface, you would keep the ItemId. Then when doing code generation you can do the lookup just fine.

This is an interesting idea. I'm not a big fan of String for the type of category. I'll try this out.

…category-inheritance

…ndgen into objc-category-inheritance

src/ir/objc.rs

bors-servo · 2020-09-16T10:28:13Z

☔ The latest upstream changes (presumably 4f714ab) made this pull request unmergeable. Please resolve the merge conflicts.

…category-inheritance

simlay · 2020-09-19T19:47:01Z

So here's a thought, is there any reason we can't just implement this when generating code for the ObjCInterface?

It feels weird that stuff that would be blacklisted would implement these traits and so on. It should also avoid the weird lookup, right?

As in, instead of keeping the category as a String in the interface, you would keep the ItemId. Then when doing code generation you can do the lookup just fine.

After some messing around with trying to make categories be an ItemId, I'm not sure how to move forward. I've been trying to uselet item = Item::from_ty_or_ref(cursor.cur_type(), cursor, None, ctx); in ObjCInterface::from_ty which segfaults. I think this is because Item::from_ty_or_ref someplace down the line calls ObjCInterface::from_ty and so we end up with infinite recursion (I've added debug statements to back this up).

Anyway, @emilio thoughts on the way forward?

bors-servo · 2020-11-26T10:38:50Z

☔ The latest upstream changes (presumably 11ae350) made this pull request unmergeable. Please resolve the merge conflicts.

…category-inheritance

bors-servo · 2022-10-05T01:57:54Z

☔ The latest upstream changes (presumably #2284) made this pull request unmergeable. Please resolve the merge conflicts.

simlay added 3 commits May 14, 2020 17:21

Added Categories to objective-c inheritance.

0bedb67

* Added items_mut to BindgenContext * Added Clone to ObjCInterface, ObjCMethod and FunctionSig :/ * Added references to the ObjCInterfaces for categories that extend a given ObjCInterface

Removed derive Clones in favor of a tupple.

bcd3d7f

* Added a bunch more tests to prevent future breakage.

Removed unneeded methods in tests

1367006

highfive added the S-awaiting-review label May 15, 2020

emilio requested changes May 15, 2020

View reviewed changes

simlay added 3 commits May 19, 2020 08:41

A pretty gross implementation :/

079571d

Updated comment

3ad80d9

cargo fmt

70f38c6

simlay commented May 25, 2020

View reviewed changes

src/ir/objc.rs Outdated Show resolved Hide resolved

Added get_parent_ty to clean up the option branches

7a642c3

simlay requested a review from emilio May 27, 2020 00:22

simlay added 6 commits June 10, 2020 22:10

Merge branch 'master' of github.com:rust-lang/rust-bindgen into objc-…

a04e151

…category-inheritance

Merge branch 'master' of github.com:rust-lang/rust-bindgen into objc-…

5118d3e

…category-inheritance

Merge branch 'master' of github.com:rust-lang/rust-bindgen into objc-…

ade5901

…category-inheritance

Merge branch 'master' of github.com:rust-lang/rust-bindgen into objc-…

c162325

…category-inheritance

Merge branch 'master' of github.com:rust-lang/rust-bindgen into objc-…

670331c

…category-inheritance

Remove autogenerated comment

996af2e

simlay force-pushed the objc-category-inheritance branch from 43240cf to 996af2e Compare August 5, 2020 22:28

simlay mentioned this pull request Aug 6, 2020

Fixes iOS compiling. RustAudio/coreaudio-sys#33

Merged

emilio reviewed Aug 17, 2020

View reviewed changes

Merge branch 'master' of github.com:rust-lang/rust-bindgen into objc-…

924182b

…category-inheritance

This was referenced Aug 18, 2020

Added initial changes for iOS support. RustAudio/coreaudio-rs#69

Closed

Objective-C support #109

Open

simlay added 7 commits August 18, 2020 17:23

Merge branch 'master' of github.com:rust-lang/rust-bindgen into objc-…

5c93c5c

…category-inheritance

Added fixed stuff from minor nits

a2287a3

fix for rustfmt

b9615e6

Merge branch 'master' of github.com:rust-lang/rust-bindgen into objc-…

d19f52e

…category-inheritance

Merge branch 'objc-category-inheritance' of github.com:simlay/rust-bi…

999133e

…ndgen into objc-category-inheritance

Added loop for resolving typerefs.

84a3de9

Merge branch 'objc-category-inheritance' of github.com:simlay/rust-bi…

6a2c809

…ndgen into objc-category-inheritance

simlay commented Sep 1, 2020

View reviewed changes

src/ir/objc.rs Show resolved Hide resolved

simlay added 3 commits September 16, 2020 17:43

Merge branch 'master' of github.com:rust-lang/rust-bindgen into objc-…

a70adff

…category-inheritance

Fixes from merging master into feature branch

0ce8926

rustfmt and updated changelog

78bf091

simlay force-pushed the objc-category-inheritance branch from f540a82 to 78bf091 Compare September 17, 2020 05:03

simlay mentioned this pull request Sep 17, 2020

iOS Support bevyengine/bevy#87

Closed

highfive added the S-needs-rebase label Nov 26, 2020

simlay mentioned this pull request Dec 7, 2021

bindgen support madsmtm/objc2#85

Closed

simlay added 3 commits December 23, 2021 15:13

Merge branch 'master' of github.com:rust-lang/rust-bindgen into objc-…

6663c80

…category-inheritance

maybe fix cargo fmt

bd5f919

fix cargo fmt

4c00a97

simlay mentioned this pull request Dec 27, 2021

wrapper for AURenderEvent? RustAudio/coreaudio-sys#52

Open

simlay mentioned this pull request Jul 26, 2022

iOS support youknowone/apple-sys#1

Closed

pvdrz deleted the branch rust-lang:master November 2, 2023 17:50

pvdrz closed this Nov 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Objc category inheritance #1784

Added Objc category inheritance #1784

simlay commented May 15, 2020

highfive commented May 15, 2020

emilio left a comment

emilio May 15, 2020

simlay Sep 1, 2020

simlay commented May 15, 2020 •

edited

Loading

emilio commented May 16, 2020

simlay commented May 16, 2020

simlay commented Aug 6, 2020

simlay commented Aug 17, 2020

scoopr commented Aug 17, 2020

emilio Aug 17, 2020

simlay Aug 18, 2020

bors-servo commented Sep 16, 2020

simlay commented Sep 19, 2020

bors-servo commented Nov 26, 2020

bors-servo commented Oct 5, 2022

Added Objc category inheritance #1784

Added Objc category inheritance #1784

Conversation

simlay commented May 15, 2020

highfive commented May 15, 2020

emilio left a comment

Choose a reason for hiding this comment

emilio May 15, 2020

Choose a reason for hiding this comment

simlay Sep 1, 2020

Choose a reason for hiding this comment

simlay commented May 15, 2020 • edited Loading

emilio commented May 16, 2020

simlay commented May 16, 2020

simlay commented Aug 6, 2020

simlay commented Aug 17, 2020

scoopr commented Aug 17, 2020

emilio Aug 17, 2020

Choose a reason for hiding this comment

simlay Aug 18, 2020

Choose a reason for hiding this comment

bors-servo commented Sep 16, 2020

simlay commented Sep 19, 2020

bors-servo commented Nov 26, 2020

bors-servo commented Oct 5, 2022

simlay commented May 15, 2020 •

edited

Loading