Skip to content

Filter categories #271

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

Closed
liamappelbe opened this issue Apr 29, 2022 · 6 comments · Fixed by #1690
Closed

Filter categories #271

liamappelbe opened this issue Apr 29, 2022 · 6 comments · Fixed by #1690
Labels
lang-objective_c Related to Objective C support package:ffigen

Comments

@liamappelbe
Copy link
Contributor

Currently filters are applied during parsing. When parsing part of the AST, we check if the node passes the config's filters. Transitive deps are handled using the ignoreFilters flag, because if A passes the filters, and depends on B, B should be included even if it doesn't pass the filters.

This works ok, but it's a bit complicated, and it means we need stuff like _CreateTypeFromCursorResult (see comments on that class). It's also problematic for categories, where the dependency arrow is backwards.

Categories are basically extension methods, so if A is an extension of B it should only be included if B is. But in the AST, A points to B. So the ignoreFilters approach won't work (if B is included later, how will we know we need to reprocess A?).

A more robust approach would be to just parse everything, and then apply the filters afterwards (though this might have a performance impact for huge imports).

@liamappelbe liamappelbe transferred this issue from dart-archive/ffigen Nov 15, 2023
HosseinYousefi added a commit that referenced this issue Nov 16, 2023
* make primitive types lowercased
* remove part statements
* add jset, jlist, jmap and jiterator
* add JNumber and JBoolean, JInteger and other boxed types
* increase coverage
HosseinYousefi added a commit that referenced this issue Nov 16, 2023
* make primitive types lowercased
* remove part statements
* add jset, jlist, jmap and jiterator
* add JNumber and JBoolean, JInteger and other boxed types
* increase coverage
@liamappelbe liamappelbe added the lang-objective_c Related to Objective C support label Jun 26, 2024
@liamappelbe liamappelbe added this to the ffigen 13.2.0 milestone Jul 10, 2024
@liamappelbe liamappelbe changed the title Cleanup: Refactor how filters are applied Fitler categories Aug 21, 2024
@liamappelbe liamappelbe changed the title Fitler categories Filter categories Aug 21, 2024
@stuartmorgan-g
Copy link

Putting some numbers on the issue raised in my duplicate issue, where having any reference to NSView brought in every single category on it and all transitive classes involved: I decided to restructure my native code slightly to firewall off NSView from the headers seen by my ffigen run, and it remove 120,000 lines of code I wasn't using.

@liamappelbe
Copy link
Contributor Author

Blocked on #1259. Once I've implemented transformers, I'll overhaul how categories are implemented.

@liamappelbe
Copy link
Contributor Author

Note to self:

  • Promote categories to an AST object (similar to ObjCProtocol), instead of just adding their methods to their interface
  • Code gen them as extension methods on the interface (test the case where the interface is defined in package:objective_c)
  • Add DeclarationFilters in the config for objc-categories
  • If everything goes well, there should be no code changes in the existing category test

@liamappelbe
Copy link
Contributor Author

liamappelbe commented Nov 4, 2024

One unfortunate thing I've noticed is that there are a bunch of methods that the Apple documentation treats as ordinary methods, but are actually methods on a category.

For example NSString only actually has a few core methods like length and characterAtIndex:. Most of the methods in the Apple documentation (eg searching, converting, encoding etc) are actually part of the NSStringExtensionMethods category, which isn't mentioned anywhere.

So when users are missing method, they're going to have to include: .* all categories, search for the method they want, find the category its part of, then include that category, which is pretty inconvenient. It's also going to be confusing for new users, as the Apple documentation lists a bunch of methods that will simply be missing from their class.

Maybe we need an include-transitive-categories option that defaults to true? This option would include all categories that extend an included class.

@liamappelbe
Copy link
Contributor Author

  • Code gen them as extension methods on the interface

This doesn't always work. If the method returns instancetype, then I can code gen it as an extension method that returns the interface, but then it will have the wrong return type when used on a subtype of the interface.

For example, NSData.dataWithBytes:length: is defined in the NSDataCreation category and returns instancetype. So when you invoke it on a NSMutableData it's supposed to return an NSMutableData. But if I code gen it as an extension method then it'll always return NSData.

I think the fix is to copy category methods that return instancetype to the interface (unless that interface is defined in package:objective_c).

@stuartmorgan-g
Copy link

stuartmorgan-g commented Nov 4, 2024

Maybe we need an include-transitive-categories option that defaults to true? This option would include all categories that extend an included class.

If we're changing Obj-C generation to not auto-include all types in methods, I think this would be fine. That would still avoid the problem of 'include any view'->'include NSView'->'include all NSView extensions'->'include every type, transitively, reachable from any method in any extension of NSView' that led to the crazy bloat I hit above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang-objective_c Related to Objective C support package:ffigen
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants