swiftcore-foundation

Is Swift 5.10 warning about "likely incorrect" actually incorrect for optional `CFType`s?


I have some code that started producing a warning about my use of the & operator with Swift 5.10 (and maybe earlier).

Forming 'UnsafeMutableRawPointer' to a variable of type 'Optional'; this is likely incorrect because 'Optional' may contain an object reference.

public func bad(audioObject: AudioObjectID) {
    var propAddr = AudioObjectPropertyAddress()
    
    var x: CFString!
    var size = UInt32(MemoryLayout.size(ofValue: x))
    
    let _ = AudioObjectGetPropertyData(audioObject, &propAddr, 0, nil, &size, &x)
    //                                                                        ^
    // Forming 'UnsafeMutableRawPointer' to a variable of type 'Optional<CFString>'; this is likely incorrect because 'Optional<CFString>' may contain an object reference.

}

So I mechanically transformed the code to not produce that warning, but I don't understand what the difference is nor if the original version was indeed hiding a bug.

func good(audioObject: AudioObjectID) {
    var propAddr = AudioObjectPropertyAddress()

    var x: CFString!
    var size = UInt32(MemoryLayout.size(ofValue: x))
    
    let _ = withUnsafeMutablePointer(to: &x) { pointerToX in
        return AudioObjectGetPropertyData(audioObject, &propAddr, 0, nil, &size, pointerToX)
    }
}

So what was likely wrong with the "bad" version and why is that not a problem with the "good" one?


Solution

  • This warning is added by this commit. This warning is emitted when you implicitly convert a non-trivial type to a raw pointer. The commit message says:

    For example, supporting this conversion turns out to be bad:

    void read_void(const void *input);
    
    func foo(data: inout Data) {
        read_void(&data)
    }
    

    People understandably expect Foundation.Data to have the same sort of implicit conversion as Array. But it does something very wrong instead.

    The idea is that people often misunderstand what this & conversion does, and would use it to call functions that would read data directly from the pointer with a non-trivial type. For example, in the example above, read_void(&data) would not pass a pointer to the actual buffer (in the heap) that Data holds. The pointer formed this way will only point to the parameter on the stack. The Data struct itself contains a pointer to the actual buffer, and one should have used Data.withUnsafeBytes to get that pointer and pass it to read_void instead.

    This is why the error message mentions "may contain an object reference". It's essentially saying "there might be a further layer of indirection to the data you actually want to pass".

    Of course, with the CFString example, you know what you are doing, and AudioObjectGetPropertyData knows what it's doing. You know that the raw pointer will not be read like a buffer, and that the function will simply write a CFString to it.

    By using withUnsafeMutablePointer, you express this intention more clearly. You are explicitly saying you want a pointer to the CFString value on the stack, not its contents.