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

[SR-1843] Libdispatch: DispatchData api inconsistency #742

Open
DevAndArtist mannequin opened this issue Jun 21, 2016 · 7 comments
Open

[SR-1843] Libdispatch: DispatchData api inconsistency #742

DevAndArtist mannequin opened this issue Jun 21, 2016 · 7 comments

Comments

@DevAndArtist
Copy link
Mannequin

DevAndArtist mannequin commented Jun 21, 2016

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
Copy link

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
Copy link
Mannequin Author

DevAndArtist mannequin commented Jun 21, 2016

@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
Copy link

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
Copy link

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
Copy link
Mannequin Author

DevAndArtist mannequin commented Jun 21, 2016

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
Copy link

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

@karwa
Copy link

karwa commented Dec 19, 2016

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?

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@shahmishal shahmishal transferred this issue from apple/swift May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants