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

Can't access CFString declaration #1548

Open
stuartmorgan opened this issue Sep 10, 2024 · 6 comments
Open

Can't access CFString declaration #1548

stuartmorgan opened this issue Sep 10, 2024 · 6 comments

Comments

@stuartmorgan
Copy link

The underlying type of CFStringRef is struct __CFString, and that's what ffigen is using as-is. Since it starts with _, it's private to the generated file, so it can't be used, even though it's part of the return type of some things it generates. E.g.:

ffi.Pointer<__CFString> get kCVPixelBufferPixelFormatTypeKey => _kCVPixelBufferPixelFormatTypeKey.value

Maybe that should be automatically changed to CFStringRef by the generator?

@liamappelbe
Copy link
Contributor

The usual fix for this is to add a rename rule:

structs:
  rename:
    # Removes prefix underscores
    # from all structures.
    '_(.*)': '$1'
  member-rename:
    '.*': # Matches any struct.
      # Removes prefix underscores
      # from members.
      '_(.*)': '$1'

I guess we could add a default rename rule for this, if none are specified. That might be more confusing for users though.

@stuartmorgan
Copy link
Author

stuartmorgan commented Sep 11, 2024

That might be more confusing for users though.

More confusing that generating public APIs that use file-internal types? That seems very unlikely to me.

Also, nobody writes __CFString. It's always CFStringRef. The internal details of what CFStringRef is a typedef for is something that nobody ever has to think about in normal usage. Generating APIs with the type name that all the docs, API references, declarations, etc. use would be much less confusing than using __CFString.

@HosseinYousefi
Copy link
Member

Related: #1551

I decided to prepend them with a dollar sign to make these identifiers public instead. It has the advantage of not colliding with an existing identifier.

@stuartmorgan
Copy link
Author

Even if there's some more generic solution, I still think we should special-case CFString as it's a pretty common type. There shouldn't be any risk of collision, because CFStringRef already has a definition on the native side, so no other type would be called that.

@liamappelbe
Copy link
Contributor

If you add CFStringRef to the typedefs.include config it should replace ffi.Pointer<__CFString>. But maybe it would it be worth adding CFStringRef to package:objective_c?

@stuartmorgan
Copy link
Author

But maybe it would it be worth adding CFStringRef to package:objective_c?

I didn't realize I hadn't replied to this; I would definitely suggest we include it in objective_c given how often it comes up with constants.

@liamappelbe liamappelbe added this to the ffigen 16.0.0 milestone Sep 29, 2024
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

3 participants