dart-lang / native

Dart packages related to FFI and native assets bundling.
BSD 3-Clause "New" or "Revised" License
157 stars 44 forks source link

Can't access `CFString` declaration #1548

Open stuartmorgan opened 2 months ago

stuartmorgan commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

Related: https://github.com/dart-lang/native/pull/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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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.