[ffigen] fix method filtering and renaming - #3590
Conversation
4824091 to
ceca878
Compare
| import 'util.dart'; | ||
|
|
||
| void main() { | ||
| group('cpp_filter_rename_test', () { |
There was a problem hiding this comment.
This is a bit weird. Most of the code in this test directly duplicates verify_bindings_test.dart. And if that duplication is removed, there's basically nothing left. I don't see any test that verifies the APIs are included/excluded/renamed correctly.
A better way to do this test would be to remove all the FfiGenerator stuff (since it's already generated in verify_bindings_test), and instead just try invoking the renamed APIs.
For the method and class that have been filtered out, you can read the bindings file into a string, and then verify that it doesn't contain the filtered API names.
| public: | ||
| // This method will be included (renamed to 'greet'). | ||
| void myMethod() {} | ||
| // This method will be filtered out via includeMember. |
There was a problem hiding this comment.
The filtering API isn't called includeMember anymore
| /// | ||
| /// Defaults to [originalName]. Can be changed via a visitor to rename the | ||
| /// method in the generated Dart class without affecting the glue symbol. | ||
| String dartName; |
There was a problem hiding this comment.
No, renames should be handled through Symbols. That way deduping (eg due to overloads) will be handled automatically by the Scope. You already have a Symbol name. Use that. Look at how ObjCMethods do this.
| if (method.isStatic) { | ||
| s.write('''\ | ||
| static $dartReturn ${method.dartName}($dartParams) { | ||
| static $dartReturn ${method.name}($dartParams) { |
There was a problem hiding this comment.
method.name.name. Or maybe rename Symbol name to Symbol symbol?
| /// A method or constructor belonging to a C++ class. | ||
| class CppMethod extends AstNode with HasLocalScope { | ||
| final Symbol name; | ||
| final String cGlueSymbol; |
There was a problem hiding this comment.
This should probably be a Symbol, and be added to a rootCppScope in Context (like rootObjCScope)
2 participants
fixes: #3552