native icon indicating copy to clipboard operation
native copied to clipboard

Filter categories

Open liamappelbe opened this issue 3 years ago • 2 comments

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 avatar Apr 29 '22 20:04 liamappelbe

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.

stuartmorgan-g avatar Aug 28 '24 19:08 stuartmorgan-g

Blocked on https://github.com/dart-lang/native/issues/1259. Once I've implemented transformers, I'll overhaul how categories are implemented.

liamappelbe avatar Sep 12 '24 00:09 liamappelbe

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 avatar Oct 31 '24 00:10 liamappelbe

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 avatar Nov 04 '24 01:11 liamappelbe

  • 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).

liamappelbe avatar Nov 04 '24 02:11 liamappelbe

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.

stuartmorgan-g avatar Nov 04 '24 15:11 stuartmorgan-g