[swift2objc] Filtering Support #1730
Conversation
pkgs/swift2objc/lib/src/parser/parsers/declaration_parsers/parse_variable_declaration.dart
Outdated
Show resolved
Hide resolved
| return matches.map((match) => match.group(0)!).toSet(); | ||
| } | ||
|
|
||
| // TODO: Type restrictions have not yet been implemented in system |
There was a problem hiding this comment.
TODOs should link to a bug. If there isn't one, file one first, then link to the bug like TODO(https://github.com/dart-lang/native/issues/...): Type restrictions...
Same below
| (previous, element) => | ||
| previous.union(dependencyVisitor.visitDeclaration(element))); | ||
| final depDecls = | ||
| (allDecls ?? decls).where((d) => deps.contains(d.name)); |
There was a problem hiding this comment.
Names aren't unique (eg two nested types, nested in different parent types, can have the same name). Use the declaration id instead.
Better yet, why not just have the dependency visitor construct a Set<Declaration> directly, rather than constructing a Set<String> and then looking up the declaration from the string?
There was a problem hiding this comment.
Most of the types do not reference declarations directly (like function parameters, variable types), which therefore means that declarations need to be looked up eventually.
The lookup is done afterwards to prevent multiple lookups and just have a single lookup per visitation.
There was a problem hiding this comment.
You shouldn't ever need to look up types by name or id, after parsing is complete. The AST already has references to the declarations. Check if the ReferredType is a DeclaredType.
| var _d = decls; | ||
|
|
||
| while (true) { | ||
| final deps = _d.fold<Set<String>>( |
There was a problem hiding this comment.
This algorithm is pretty hard to follow. A simpler approach would be to give DependencyVisitor a member Set that it outputs all the dependencies into. generateDependencies would just construct a DependencyVisitor, then iterate over every element of decls and call dependencyVisitor.visitDeclaration, then return the Set that dependencyVisitor constructed.
DependencyVisitor.visitDeclaration would just DFS from the given element to any child element not in that Set. This would also mean you don't have to pass around the context variable through all those visit methods in DependencyVisitor, because the context would be replaced by this member Set.
There was a problem hiding this comment.
The reason for recursiveness is because of at the moment, the algorithm generates bindings for all parts of all dependencies of filtered types, and since these parts (e.g methods and properties) may also require types as well, those are generated as well recursively.
What can be done, is to pass the declarations to DependencyVisitor, allow it to perform the recursive visiting, and then return the output.
There was a problem hiding this comment.
yeah, all that recursion should happen inside the visitor
|
|
||
| // since we are making such visitations on normal declarations in a file, | ||
| // we do not need to filter out primitives at the moment | ||
| cont.addAll(_getTypeNames(type.swiftType)); |
There was a problem hiding this comment.
You shouldn't have to re-parse these types from strings. We did all the parsing already, and now have a nice clean AST to work with. In this case you can just check if the type is a DeclaredType, and get its id or declaration or whatever. Also need to handle the OptionalType case.
This Pull Request is to add filtering support to the swiftgen tool.
ConfigMore information can be found here: #1394