[Review] SE-0107: UnsafeRawPointer API

From the proposal:

···

On Jul 5, 2016, at 5:26 AM, Anton Zhilin <antonyzhilin@gmail.com> wrote:

func initialize<T>(_: T.Type, to: T, count: Int = 1)
    -> UnsafeMutablePointer<T>

I wonder why the first parameter is needed. If one is passing literals, it's always more Swift'y to use 'as'.

---
The type parameter `T` passed to `initialize` is an explicit argument
because the user must reason about the type's size and alignment at
the point of initialization. Inferring the type from the value passed
to the second argument could result in miscompilation if the inferred
type ever deviates from the user's original expectations.

You can think of this as the “raw” API. With the latest proposal, it will be more common for users to initialize with a typed pointer anyway:

ptrToA.initialize(to: A())

-Andy

“as” is for types :)

Let’s say initialize(to:) is good and once we’ve bikeshedded all the other names we’ll see if there’s any potential for confusion…

Andy

···

On Jul 5, 2016, at 9:20 AM, Dave Abrahams <dabrahams@apple.com> wrote:

The use "as"?

Sent from my moss-covered three-handled family gradunza

On Jul 5, 2016, at 6:48 AM, Andrew Trick <atrick@apple.com <mailto:atrick@apple.com>> wrote:

On Jul 4, 2016, at 6:19 PM, Dave Abrahams via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

So Swift 3 users have already migrated to this “better” name.

I agree that initialize(to:) is consistent with the language we use
for assigning values. But grammatically, I think initialize(with:)
also makes perfect sense and is just as common.

“With” is a weak preposition with many possible interpretations, so we'd
like to avoid it. If I used “with” where “to” would have worked, I
regret it.

That looks like a +1 from DaveA.

So, my only objection is that I’m trying to establish a convention where “from:” reads from memory at a pointer and “to:” writes to memory at a pointer. Here “to” is backward because the object of the preposition is not being modified.

-Andy

I'm not sure about that. "Initialize backward from x, count y" is unambiguous as to how initialization starts and iterates (the first argument), and it is clear that `count` is an end condition dissociated from anything to do with how initialization starts and iterates.

By contrast, "Initialize from x, backward from y" associates the direction of movement with y instead of x. Thus, y becomes the start condition (the end condition being implicitly "to zero"), thus raising the question of what position x is in relative to the count y.

"Initialize backward from x" literally tells me that ‘x’ is the starting point, which is incorrect. Honestly, users will need to check the doc comments which are very precise.

-Andy

···

On Jul 5, 2016, at 11:27 AM, Xiaodi Wu <xiaodi.wu@gmail.com> wrote:

On Tue, Jul 5, 2016 at 11:10 Andrew Trick <atrick@apple.com <mailto:atrick@apple.com>> wrote:

On Jul 5, 2016, at 11:05 AM, Xiaodi Wu <xiaodi.wu@gmail.com <mailto:xiaodi.wu@gmail.com>> wrote:

I don't mind `initialize(from:forwardToCount:)`, but I do have trouble with Brent's suggestion of `initialize(from:backwardFromCount:)`. It adds ambiguity as to whether the pointer in the first argument points to the 0th element or the (count - 1)th element from which initializing is proceeding backward, a problem that does not exist with the currently proposed version `initializeBackward(from:count:)`. I don't find the symmetry wins compelling enough to overcome that additional ambiguity.

That’s a good point, but I think both forms are equally ambiguous.

-Andy

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:
Revise SE-0107: rename UnsafePointer & UnsafeRawPointer methods. by atrick · Pull Request #420 · apple/swift-evolution · GitHub

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:
[WIP] SE-0107: UnsafePointer and UnsafeRawPointer API changes. by atrick · Pull Request #3437 · apple/swift · GitHub

---
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&gt;

  Foundation.Data.copyBytes(to:count:)
  <https://developer.apple.com/reference/foundation/data/1780297-copybytes&gt;

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.

···

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

On Jul 11, 2016, at 12:50 AM, Ben Rimmington <me@benrimmington.com> wrote:

On 10 Jul 2016, at 14:41, Andrew Trick via swift-evolution <swift-evolution@swift.org> wrote:

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

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:
Revise SE-0107: rename UnsafePointer & UnsafeRawPointer methods. by atrick · Pull Request #420 · apple/swift-evolution · GitHub

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:
[WIP] SE-0107: UnsafePointer and UnsafeRawPointer API changes. by atrick · Pull Request #3437 · apple/swift · GitHub

---
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&gt;

  Foundation.Data.copyBytes(to:count:)
  <https://developer.apple.com/reference/foundation/data/1780297-copybytes&gt;

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.

Thanks. I’ll take that as a +1 for the current form.

We support implicit argument conversion from UnsafePointer<T> to UnsafeRawPointer primarily so that UnsafePointers can be passed as `void*` arguments.

-Andy

···

On Jul 11, 2016, at 10:22 AM, Dave Abrahams <dabrahams@apple.com> wrote:
on Mon Jul 11 2016, Andrew Trick <atrick-AT-apple.com <http://atrick-at-apple.com/&gt;&gt; wrote:

On Jul 11, 2016, at 12:50 AM, Ben Rimmington <me@benrimmington.com> wrote:

On 10 Jul 2016, at 14:41, Andrew Trick via swift-evolution <swift-evolution@swift.org> wrote:

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