Skip to content
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

[ffigen] Restrict usage of UniqueNamer #1646

Open
liamappelbe opened this issue Oct 11, 2024 · 0 comments
Open

[ffigen] Restrict usage of UniqueNamer #1646

liamappelbe opened this issue Oct 11, 2024 · 0 comments

Comments

@liamappelbe
Copy link
Contributor

To avoid API updates changing method numbering, we should tighten up how we're using UniqueNamer. We've been using it as a crutch to solve the few edge cases where renaming is necessary, but it should only be used for purely internal things like private variables.

I know of 3 problematic cases currently, but there may be more:

  1. Method renaming like renaming new to new1 will break if the target API adds a method named new1. In this case new will now be renamed to new2, breaking existing code. We should use jnigen's approach of inserting a $.
  2. Block wrappers get names based on the signature of the block, like ObjCBlock_Int32_Int32. But that name doesn't 100% uniquely identify the signature, so we also renumber them if they collide. So if an API update adds a new block type, it could renumber existing block types depending on the order they're parsed. We could fix this by making sure the name does uniquely identify the signature, or by using hash naming like we do for msgSend. Either approach is a bit awkward.
  3. In ObjC, a method can have the same name as a type, but in Dart we have to renumber that method (eg if foo.Bar() collides with class Bar, we change it to foo.Bar1()). So if you have an existing foo.Bar() method, and an API update adds a Bar type, then the method would be renamed to foo.Bar1(). There's no good way of fixing this edge case without something like the JSON file being proposed for jnigen: [jnigen] Don't break method names #1529
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Todo
Development

No branches or pull requests

1 participant