Extend APINotes to support always importing C pointers as OpaquePointer

Development Compiler
,
1

In the Android workgroup we are currently looking at bringing the Android SDK minimum API level down from 28 to 24 or 23.

One of the "annoying" things about Android API 23, is specifically a difference in the stdio.h header file. The linked conditional import, makes FILE* import as UnsafeMutablePointer<FILE> on API 23 and OpaquePointer on API 24+.

This difference means that any libraries that explicitly type the FILE* pointer, like swift-log does, would not compile on API 23.

#if canImport(WASILibc) || os(Android) 
 internal typealias CFilePointer = OpaquePointer 
 #else 
 internal typealias CFilePointer = UnsafeMutablePointer<FILE> 
 #endif

There are solutions, like a C shim or using closures to hide the type. However, this is something that each library that needs the type FILE* has to do.

It would be nice if we could have some sort of unified solution in the compiler/stdlib to fix this issue. One of the ideas that has been brought up, is the use of APINotes.

However, I could not get the current features of APINotes to change the imported type of FILE*, but please correct me if I am wrong. The only possible way, is that we would have to redefine all the NDK function headers that use FILE* using APINotes.

Extending SwiftImportAs

My proposal is that we extend the SwiftImportAs API note with an opaque_pointer type, such that we can change the way we import any C structs. This would allow us to unify the imported type of FILE* on Android, like so:

     Tags:
       - Name: __sFILE
         SwiftImportAs: opaque_pointer

That would mean we would have to extend swiftlang/llvm-project to apply something like __attribute__((swift_attr("import_pointer_as_opaque"))) when that API note is present, and the ClangImporter in the compiler to respect this attribute.

Before I endeavor on implementing this, I just wanted to make sure people think this is the right direction to head or we should pursue another direction than extending API notes?

Thanks!

5 Likes
Android API minimum for the Swift SDK for Android
2

That is annoying. I feel like the root cause here is the fact that we don't have a good way to deal with pointers to incomplete types on the Swift side, so this kind of thing that's fine in C (so long as you're careful) is tricky to deal with. However, that's a bigger problem than you should have to tackle here.

As for your specific solution...

I was going to say that I'd rather not extend the API notes format further with Swift-specific things, and that I'd rather go through the swift_attr attribute. However, it looks like SwiftImportAs was already built for this, because it maps directly to swift_attr with the import_ prefix. The code is here.

I don't have any better ideas, so I'm okay with this direction.

Doug

6 Likes
3

If we had a Sizable marker protocol that was required for all operations dependent on a value's size (including initialization!) then we could offer first-class support for imported opaque C types like FILE.

My two cents.

4

I've submitted a PR :smiley:

1 Like
5

It's unfortunate that we'd lose some type safety by erasing any incomplete pointer type in just OpaquePointer. If I have a typedef struct Foo *FooP and a typedef struct Bar *BarP, the C compiler would yell at me if I try to use those interchangeably, but Swift would allow it.

Can we thread the needle a bit better? What if we treated OpaquePointer as a namespace and synthesized new types inside it? Something like:

extension OpaquePointer {
  public struct FILE {
    public let pointer: OpaquePointer
    public init(_ pointer: OpaquePointer) { self.pointer = pointer }
  }
}

Then functions like fopen import as returning an OpaquePointer.FILE, which would be distinct from any other OpaquePointers used in the codebase. There's an extra step of indirection if you need to get at the underlying pointer, but I would imagine that's not as common a need.

2 Likes
6

This is the same problem where pthread types could be imported to Swift as e.g. pthread_t or pthread_t? depending on whether the actual type is backed by a pointer or not. Honestly, I don't think the conditionals are the problem, it's that there isn't a natural Schelling point for these conditionals and this therefore ends up surfacing to the callsites.

I really should try and see if I can glue together something to try a proof of concept, I'm so busy these days...

7

We could also import opaque C structs as enum tag {} or enum typedef_name {}, and have all of the opaque pointers imported as Opaque<imported_c_type> where:

struct Opaque<Tag> {
  public let pointer: OpaquePointer
  public init(_ pointer: OpaquePointer) { self.pointer = pointer }
}

That way we only have to define the boilerplate wrapper once.

e.g. FILE* becomes:

// imported type
enum FILE {}

func fopen(_ path: String, _ mode: String) -> Opaque<FILE>
func fclose(_ stream: Opaque<FILE>) -> CInt
func fflush(_ stream: Opaque<FILE>) ->  CInt
// ...

This also keeps working if there's a platform where the imported type isn't opaque.

1 Like
8

Background on why we didn't just do that by default:

I know debugging across incompatibly-built contexts is a lot better these days than it used to be, but just be wary of painting into a corner.

2 Likes
9

Right, that's why I think a top-level synthesized tag type like enum FILE {} isn't viable, because it could clash with the top-level symbol of the complete type if it was imported elsewhere.

Synthesizing the incomplete type as a nested type inside OpaquePointer avoids those issues I believe, because OpaquePointer.FILE would be distinct from the complete FILE type referenced by UnsafeMutablePointer<FILE>.

10

What I'd like to see us do (I've ranted about this before I think) is have an implicit Sizable protocol like Copyable that most types conform to, and which is required for allocation, MemoryLayout, etc.

Then we could have the compiler import a C type like typedef struct __FILE FILE as struct __FILE: ~Sizable {}; typealias FILE = __FILE;.

If the type cannot be sized, then it is not necessarily uninhabited but you cannot create an instance of it in Swift nor pass it as an argument to a Swift function unless you do so as a pointer.

(This would presumably necessitate a language mode change.)

1 Like
11

Doug and I have talked about this before. IMHO, it seems like a good long-term direction; it’d probably be the case that most APIs other than Unsafe[Mutable]Pointer.Pointee and some of its members would require a complete type, but that actually seems like the correct behavior anyway.

That doesn’t mean the change proposed here is a bad one. It’s a good solution for current language modes.

3 Likes
12

I agree! Not suggesting we shouldn't move forward.