[swift-evolution] [Review] SE-0107: UnsafeRawPointer API (binding memory to type)

[Dave Abrahams]

on Mon Jul 11 2016, Andrew Trick <atrick-AT-apple.com> wrote:

>> On Jul 11, 2016, at 12:50 AM, Ben Rimmington <me at benrimmington.com> wrote:
>> 
>> 
>>> On 10 Jul 2016, at 14:41, Andrew Trick via swift-evolution <swift-evolution at swift.org> wrote:
>>> 
>
>>> I'm revising this proposal based on last week's feedback. A few of the
>>> additive APIs are removed and a number of UnsafePointer and
>>> UnsafeRawPointer methods are renamed.
>>> 
>>> Here is a PR for the revision. Note that the examples in the proposal
>>> text still need to be updated:
>>> https://github.com/apple/swift-evolution/pull/420
>>> 
>>> I updated the short-form summary of the API:
>>> https://github.com/atrick/swift-evolution/blob/3122ace9d2fb55072ebd7395c7353fcbf497318a/proposals/0107-unsaferawpointer.md#full-unsaferawpointer-api
>>> 
>>> The full UnsafeRawPointer API with doc comments is here:
>>> https://github.com/atrick/swift/blob/22e3a2885e4236888ec447a7148acf633d8544f5/stdlib/public/core/UnsafeRawPointer.swift.gyb
>>> 
>>> The UnsafePointer and UnsafeRawPointer changes are on this branch:
>>> https://github.com/atrick/swift/commits/rawptr
>>> 
>>> If you wish to comment line-by-line on the detailed docs or
>>> implementation, you can do so here:
>>> https://github.com/apple/swift/pull/3437
>>> 
>>> ---
>>> The only concern I have about this version of the proposal is this method name: 
>>> 
>>>  func copyBytes(from: UnsafeRawPointer, count: Int)
>>> 
>>> because `count` usually refers to a number of values. I think it should be:
>>> 
>>>  func copy(bytes: Int, from: UnsafeRawPointer)
>> 
>> Using `bytes` to label the count / length / size would be inconsistent with:
>> 
>> 	Foundation.Data.init(bytes:count:)
>> 	<https://developer.apple.com/reference/foundation/data/1780158-init>
>> 
>> 	Foundation.Data.copyBytes(to:count:)
>> 	<https://developer.apple.com/reference/foundation/data/1780297-copybytes>
>> 
>> UnsafeMutableRawPointer could use a `size` or `sizeInBytes` label.
>> (This also applies to the `allocate` and `deallocate` methods).
>> 
>> — Ben
>
> Thanks for pointing that out.
>
> My concern is code like:
>
>   let ptrToInt: UnsafePointer<Int32> = …
>   rawPtr.copyBytes(from: ptrToInt, count: 4)
>
> which looks a lot like 4 Int32s will be copied when only 1 Int32 will actually be copied.
>
> Anyone care to vote on this?

I think you're being overly fussy.  The name clearly says we're copying
bytes.  count says how many.  But if you want to avoid any possibility
of confusion, only support source pointers that are UnsafeRawPointer.

>
>
> Current:
>
>   let rawPtr = UnsafeMutableRawPointer.allocate(bytes: 24)
>   rawPtr.copyBytes(from: ptrToInt, count: 24)
>   rawPtr.deallocate(bytes: 24)
>
> Proposed:
>
>   let rawPtr = UnsafeMutableRawPointer.allocate(sizeInBytes: 24)
>   rawPtr.copyBytes(from: ptrToInt, sizeInBytes: 24)
>   rawPtr.deallocate(sizeInBytes: 24)
>
> -Andy

-- 
Dave