[SR-1843] Libdispatch: DispatchData api inconsistency

[DevAndArtist]

Previous ID	SR-1843
Radar	None
Original Reporter	@DevAndArtist
Type	Bug

Environment

Swift 3 from first Xcode 8 beta.

Additional Detail from JIRA

Votes	0
Component/s	libdispatch
Labels	Bug, SDKOverlay
Assignee	mww (JIRA)
Priority	Medium

md5: 0a7dbaaa991f1ff9845f0d281525f77f

Issue Description:

Please check this diffs between `DispatchData` and `Data`, not all api parts are identical, where some of them should be. As an example different range types are used or labels and generic type `Result` vs. `ResultType` don't match.

Here is a formatted gist: https://gist.github.com/DevAndArtist/4f1cef7a0d58cc786e6ed1837a107a0c

Here is the raw diff:

- public struct Data : ReferenceConvertible, CustomStringConvertible, Equatable, Hashable, RandomAccessCollection, MutableCollection {
+ public struct DispatchData : RandomAccessCollection {

-   public typealias ReferenceType = NSData
-   public typealias ReadingOptions = NSData.ReadingOptions
-   public typealias WritingOptions = NSData.WritingOptions
-   public typealias SearchOptions = NSData.SearchOptions
-   public typealias Base64EncodingOptions = NSData.Base64EncodingOptions
-   public typealias Base64DecodingOptions = NSData.Base64DecodingOptions

+   public typealias Iterator = DispatchDataIterator

    public typealias Index = Int

-   public typealias Indices = DefaultRandomAccessIndices<Data>
+   public typealias Indices = DefaultRandomAccessIndices<DispatchData>

+   public static let empty: DispatchData
    
    public enum Deallocator {
-       case virtualMemory
-       case none
       
        case free
        case unmap

-       case custom((UnsafeMutablePointer<UInt8>, Int) -> Swift.Void)
+       case custom(DispatchQueue?, @convention(block) () -> Swift.Void)
    }
    
-   public init(bytes: UnsafePointer<UInt8>, count: Int)
-   public init(bytes: [UInt8])
-   public init(bytes: ArraySlice<UInt8>)
-   public init?(capacity: Int)
-   public init()
-   public init(contentsOf url: URL, options: ReadingOptions = default) throws
-   public init?(base64Encoded base64String: String, options: Base64EncodingOptions = default)
-   public init?(base64Encoded base64Data: Data, options: Base64DecodingOptions = default)

-   public init<SourceType>(buffer: UnsafeBufferPointer<SourceType>)
+   public init(bytes buffer: UnsafeBufferPointer<UInt8>)

-   public init(bytesNoCopy bytes: UnsafeMutablePointer<UInt8>, count: Int, deallocator: Data.Deallocator)
+   public init(bytesNoCopy bytes: UnsafeBufferPointer<UInt8>, deallocator: DispatchData.Deallocator = default)

-   public var count: Int
+   public var count: Int { get }

-   public func withUnsafeBytes<ResultType, ContentType>(_ body: @noescape (UnsafePointer<ContentType>) throws -> ResultType) rethrows -> ResultType
+   public func withUnsafeBytes<Result, ContentType>(body: @noescape (UnsafePointer<ContentType>) throws -> Result) rethrows -> Result

-   public mutating func withUnsafeMutableBytes<ResultType, ContentType>(_ body: @noescape (UnsafeMutablePointer<ContentType>) throws -> ResultType) rethrows -> ResultType

-   public func enumerateBytes(_ block: @noescape (buffer: UnsafeBufferPointer<UInt8>, byteIndex: Index, stop: inout Bool) -> Swift.Void)
+   public func enumerateBytes(block: @noescape (buffer: UnsafeBufferPointer<UInt8>, byteIndex: Int, stop: inout Bool) -> Swift.Void)

    public mutating func append(_ bytes: UnsafePointer<UInt8>, count: Int)

-   public mutating func append(_ other: Data)
+   public mutating func append(_ other: DispatchData)

    public mutating func append<SourceType>(_ buffer: UnsafeBufferPointer<SourceType>)
    
    public func copyBytes(to pointer: UnsafeMutablePointer<UInt8>, count: Int)

-   public func copyBytes(to pointer: UnsafeMutablePointer<UInt8>, from range: Range<Index>)
+   public func copyBytes(to pointer: UnsafeMutablePointer<UInt8>, from range: CountableRange<Index>)

-   public func copyBytes<DestinationType>(to buffer: UnsafeMutableBufferPointer<DestinationType>, from range: Range<Index>? = default) -> Int
+   public func copyBytes<DestinationType>(to buffer: UnsafeMutableBufferPointer<DestinationType>, from range: CountableRange<Index>? = default) -> Int

-   public subscript(index: Index) -> UInt8
+   public subscript(index: Index) -> UInt8 { get }

-   public subscript(bounds: Range<Int>) -> MutableRandomAccessSlice<Data>
+   public subscript(bounds: Range<Int>) -> RandomAccessSlice<DispatchData> { get }

-   public func subdata(in range: Range<Index>) -> Data
+   public func subdata(in range: CountableRange<Index>) -> DispatchData

+   public func region(location: Int) -> (data: DispatchData, offset: Int)

-   public func write(to url: URL, options: WritingOptions = default) throws
-   public func range(of dataToFind: Data, options: SearchOptions = default, in range: Range<Index>? = default) -> Range<Index>?
-   public mutating func resetBytes(in range: Range<Index>)
-   public mutating func replaceBytes(in range: Range<Index>, with data: Data)
-   public func base64EncodedString(_ options: Base64EncodingOptions = default) -> String
-   public func base64EncodedData(_ options: Base64EncodingOptions = default) -> Data
-   public var hashValue: Int { get }
-   public var description: String { get }
-   public var debugDescription: String { get }

    public var startIndex: Index { get }
    public var endIndex: Index { get }
    public func index(before i: Index) -> Index
    public func index(after i: Index) -> Index
    public func makeIterator() -> Iterator
}

[belkadan]

cc @phausler. It'd probably also be good to get Matt Wright in here, but it looks like he hasn't registered an account yet.

[DevAndArtist]

@belkadan talked to both via twitter already. (I missed `UnsafeBufferPointer` vs. `UnsafeMutablePointer` at first glance but I updated the post). There are still a few other small things to polish.

[swift-ci]

Comment by Daniel A. Steffen (JIRA)

it would be helpful if you could provide the specific adjustements that you would like to see rather than a generic diff, there are parts of the NSData API that don't make sense for DispatchData (and cannot be cheaply implemented in the Overlay which is a requirement for being backwards deployable to older versions of Darwin).

[swift-ci]

Comment by Matt Wright (JIRA)

Yeah, some of these we just can't match. Some I can match to Data (the initialiser was me attempting to match against an earlier API version, I think). Many of the mutable accessors cannot be implemented with DispatchData (and hence why its a separate type).

[DevAndArtist]

By api consistency, I'd wish types and labels to match with `Data` (it depends on how the Swift 3 api will look like when its not beta anymore). I'm fine with keeping both types.

Here are some wishes (its up to you to decide what to change):

1. Decide whether it's best to use `UnsafeBufferPointer` against `UnsafeMutablePointer` + `count` in both apis, so initializer and functions would match. Additionally we could have both version in both libdispatch and foundation.

2. Can `.custom` get visibility to immutable pointer of the data so one would be able to call for instance `free(pointer)`:

public enum Deallocator {
    case custom(DispatchQueue?, @convention(block) (UnsafePointer<UInt8>, Int) -> Swift.Void)
}

3. [Optional] Consider to add more initializer

public init(bytes: UnsafePointer<UInt8>, count: Int)
public init(bytes: [UInt8])
public init(bytes: ArraySlice<UInt8>)

4. [Optional] Can this initializer become generic, so we could store different types inside `DispatchData`?

-   public init(bytes buffer: UnsafeBufferPointer<UInt8>)
+   public init<SourceType>(buffer: UnsafeBufferPointer<SourceType>)

5. Match initializer or provide both versions to both libdispatch and foundation.

public init(bytesNoCopy bytes: UnsafeMutablePointer<UInt8>, count: Int, deallocator: Data.Deallocator)
public init(bytesNoCopy bytes: UnsafeBufferPointer<UInt8>, deallocator: DispatchData.Deallocator = default)

6. Match type names and labels.

public func withUnsafeBytes<ResultType, ContentType>(_ body: @noescape (UnsafePointer<ContentType>) throws -> ResultType) rethrows -> ResultType
public func withUnsafeBytes<Result, ContentType>(body: @noescape (UnsafePointer<ContentType>) throws -> Result) rethrows -> Result

public func enumerateBytes(_ block: @noescape (buffer: UnsafeBufferPointer<UInt8>, byteIndex: Index, stop: inout Bool) -> Swift.Void)
public func enumerateBytes(block: @noescape (buffer: UnsafeBufferPointer<UInt8>, byteIndex: Int, stop: inout Bool) -> Swift.Void)

7. [Optional] Here for instance we don't use `UnsafeBufferPointer`

public mutating func append(_ bytes: UnsafePointer<UInt8>, count: Int)

8. [Optional] Is there any technical reason for using different range types?

// CountableRange<Index>
public func copyBytes(to pointer: UnsafeMutablePointer<UInt8>, from range: CountableRange<Index>)
public func copyBytes<DestinationType>(to buffer: UnsafeMutableBufferPointer<DestinationType>, from range: CountableRange<Index>? = default) -> Int

// Range<Int>
public subscript(bounds: Range<Int>) -> RandomAccessSlice<DispatchData> { get }

9. Check `Index` vs. `Int`

10. [Optional but highly wished] Make `DispatchData` convertible to `Data` and vice versa (extension?).

11. [Optional] Consider to add these protocols:

• CustomDebugStringConvertible

• CustomStringConvertible

• Equatable

• Hashable

[belkadan]

From the Swift side, I'm in favor of Unsafe[Mutable]BufferPointer over Unsafe[Mutable]Pointer.

[karwa]

I was just about the file a bug about DispatchData not supporting RangeReplaceableCollection.

Since the two are toll-free bridged, theoretically you could get a DispatchData, hand it off to an Obj-C thunk which casts a dispatch_data_t to an NSData*, which you then bridge back to a Foundation Data. AFAIK, no copying happens in any of that, which means they are just two abstractions for the same underlying object. Surely they should both be able to support exactly the same interfaces?