native icon indicating copy to clipboard operation
native copied to clipboard

[ffigen] Restrict usage of `UniqueNamer`

Open liamappelbe opened this issue 1 year ago • 0 comments

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: https://github.com/dart-lang/native/issues/1529

liamappelbe avatar Oct 11 '24 01:10 liamappelbe