[Review] SE-0184: Unsafe[Mutable][Raw][Buffer]Pointer: add missing methods, adjust existing labels for clarity, and remove deallocation size

Hello Swift community,

The review of SE-0184 "Unsafe[Mutable][Raw][Buffer]Pointer: add missing methods, adjust existing labels for clarity, and remove deallocation size" begins now and runs through September 7, 2017. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0184-unsafe-pointers-add-missing.md
Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at

https://lists.swift.org/mailman/listinfo/swift-evolution
or, if you would like to keep your feedback private, directly to the review manager. When replying, please try to keep the proposal link at the top of the message:

Proposal link:

https://github.com/apple/swift-evolution/blob/master/proposals/0184-unsafe-pointers-add-missing.md
Reply text
Other replies
<https://github.com/apple/swift-evolution/pulls#what-goes-into-a-review-1>What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

What is your evaluation of the proposal?
Is the problem being addressed significant enough to warrant a change to Swift?
Does this proposal fit well with the feel and direction of Swift?
If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
More information about the Swift evolution process is available at

https://github.com/apple/swift-evolution/blob/master/process.md
Thank you,

-Doug

Review Manager

Hello Swift community,

The review of SE-0184 "Unsafe[Mutable][Raw][Buffer]Pointer: add missing
methods, adjust existing labels for clarity, and remove deallocation size"
begins now and runs through September 7, 2017. The proposal is available
here:

https://github.com/apple/swift-evolution/blob/master/
proposals/0184-unsafe-pointers-add-missing.md

Reviews are an important part of the Swift evolution process. All reviews
should be sent to the swift-evolution mailing list at

https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the
review manager. When replying, please try to keep the proposal link at the
top of the message:

Proposal link:

https://github.com/apple/swift-evolution/blob/master/
proposals/0184-unsafe-pointers-add-missing.md

Reply text

Other replies

<https://github.com/apple/swift-evolution/pulls#what-goes-into-a-review-1>What
goes into a review?

The goal of the review process is to improve the proposal under review
through constructive criticism and, eventually, determine the direction of
Swift. When writing your review, here are some questions you might want to
answer in your review:

   - What is your evaluation of the proposal?

Overall, this is an improvement. However, I do have some questions and

concerns:

Questions:

## UnsafeMutableRawPointer

Regarding the options given for "whose `count` to use"--which option is
actually being proposed?

## UnsafeMutableBufferPointer

Please clarify: why are you proposing that the `at:` arguments in
`UnsafeMutableBufferPointer` and `UnsafeMutableRawBufferPointer` _should
not_ receive default values, but the `at:` arguments in
`UnsafeMutableRawPointer` _should_ receive a default value of `0`?

Concerns:

## UnsafeMutablePointer

It's alarming that omitting `count` in `initialize(repeating:count:)` (and
assign, etc.) means initialize _one_ element, but elsewhere (such as
`UnsafeMutableBufferPointer` means initialize _all_ elements. The behavior
of the proposed API also contradicts its own spelling on its face:
`initialize(repeating: foo)` means *do not repeat* `foo`.

Yes, I understand the argument that `*BufferPointer` types have an
intrinsic count, etc., but in the context of code where types are inferred,
`let foo = T.allocate(capacity: 100); foo.initialize(repeating: bar)`
should not mean one thing for `*BufferPointer` types and a totally
different thing for plain `*Pointer` types--particularly when both can be
allocated with a certain capacity greater than one.

Either `count` should always be required, or for convenience there should
be a separate overload `initialize(pointee:)` that does not require `count`.

## UnsafeMutableRawBufferPointer

In `copyBytes`, the use of `Bytes` to emphasize that it's the memory that's
being copied is thoughtful, but it is inconsistent with the other method
names that use the terminology `Memory` for the same purpose (e.g.,
`moveInitializeMemory` to clarify the meaning of `moveInitialize`).

For better consistency--and since you're proposing to rename
`copyBytes(from:count:)` on `UnsafeMutableRawPointer` to
`copy(from:bytes:)`--this particular API on `UnsafeMutableRawBufferPointer`
should be named `copyMemory(from:)` and not `copyBytes(from:)`.

Although, actually, looking at the APIs on `UnsafeMutableRawPointer`
itself, that particular method too might best be written as
`copyMemory(from:bytes:)` instead of merely `copy(from:bytes:)` for better
consistency with the rest of the methods on that type as well.

## General comment

Many `at:` arguments, especially such as in the case of
`copyBytes(at:from:)`, make sense only when read in a list with all other
methods. Standing alone, `at` is ambiguous as to whether it's referring to
the _source_ or the _destination_. Why do these APIs on `*BufferPointer`
types not take advantage of subscripts? That is, why not:

  `foo[x...].copyMemory(from: bar)`

instead of

  `foo.copyBytes(at: x, from: bar)`

The first seems dramatically clearer as to its meaning. The same feedback
applies to nearly all uses of `at` on `*BufferPointer` types: they would
seem to be greatly clarified (in terms of the "what does `at` mean"
question) by the use of a subscript spelling.

I notice that you comment that you feel there are ergonomic issues with
buffer pointer slicing; can you please comment on what is "wasteful"
currently? Is there a performance hit to slicing a `*BufferPointer` type?
If so, we should fix that, as the whole rationale of having these types (as
I understand it) is to improve the ergonomics of working with pointers to
multiple elements by conforming them to `*Collection` APIs.

It seems deeply unsatisfactory to invent new methods that use `at:`
arguments _on a type whose purpose is to expose `*Collection` APIs_ if we
agree that the slicing notation is demonstrably clearer. I do not have the
same concerns about adding `at:` arguments to `UnsafeMutableRawPointer`
methods, which are quite appropriate.

   - Is the problem being addressed significant enough to warrant a
   change to Swift?

Yes.

   - Does this proposal fit well with the feel and direction of Swift?

In parts, yes. In others (see above), it could be improved to fit better

with the feel and direction of other Swift APIs.

   - If you have used other languages or libraries with a similar
   feature, how do you feel that this proposal compares to those?

There is much more friction to using pointers in Swift than in C. However,

making Swift pointers like C pointers is clearly a non-goal. This proposal
appropriate addresses major pain points to Swift-specific usages of
pointers.

   - How much effort did you put into your review? A glance, a quick
   reading, or an in-depth study?

A moderately thorough reading, drawing on some experience using pointer

APIs in Swift, and based on prior readings of previous iterations of this
proposal and the on-list discussion.

···

On Sat, Sep 2, 2017 at 12:27 AM, Douglas Gregor via swift-evolution < swift-evolution@swift.org> wrote:

More information about the Swift evolution process is available at

https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

-Doug

Review Manager

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

Hello Swift community,

The review of SE-0184 "Unsafe[Mutable][Raw][Buffer]Pointer: add missing methods, adjust existing labels for clarity, and remove deallocation size" begins now and runs through September 7, 2017. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0184-unsafe-pointers-add-missing.md
Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at

https://lists.swift.org/mailman/listinfo/swift-evolution
or, if you would like to keep your feedback private, directly to the review manager. When replying, please try to keep the proposal link at the top of the message:

Proposal link:

https://github.com/apple/swift-evolution/blob/master/proposals/0184-unsafe-pointers-add-missing.md
Reply text
Other replies
<https://github.com/apple/swift-evolution/pulls#what-goes-into-a-review-1>What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

What is your evaluation of the proposal?

+1.

Is the problem being addressed significant enough to warrant a change to Swift?

I use the affected constructs every day, and without this proposal using them correctly and safely requires working around a lot of missing API surface area. This will allow me to clean up my code significantly.

Does this proposal fit well with the feel and direction of Swift?

It is a logical extension of the UnsafePointer and UnsafeBufferPointer model, allowing many existing operations on UnsafePointer to be performed directly on UnsafeBufferPointer.

If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

N/A

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

I did a in-depth review of an earlier draft. I did a quick read of this final one.

···

On Sep 1, 2017, at 10:27 PM, Douglas Gregor via swift-evolution <swift-evolution@swift.org> wrote:

More information about the Swift evolution process is available at

https://github.com/apple/swift-evolution/blob/master/process.md
Thank you,

-Doug

Review Manager

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

Currently, memory is deallocated by an instance method on UnsafeMutablePointer, deallocate(count:). Like much of the Swift pointer API, performing this operation on a buffer pointer requires extracting baseAddress! and count. It is very common for the allocation code above to be immediately followed by:

defer

{
    buffer.
baseAddress?.deallocate(capacity: buffer.count
)
}

This method is extremely problematic because nearly all users, on first seeing the signature of deallocate(capacity:), will naturally conclude from the capacity label that deallocate(capacity:) is equivalent to some kind of realloc()that can only shrink the buffer. However this is not the actual behavior — deallocate(capacity:) actually ignores the capacity argument and just calls free() on self. The current API is not only awkward and suboptimal, it is misleading. You can write perfectly legal Swift code that shouldn’t segfault, but still can, for example

var ptr = UnsafeMutablePointer<UInt8>.allocate(capacity: 1000000
)
ptr.
initialize(to: 13, count: 1000000
)
ptr.
deallocate(capacity: 500000) // deallocate the second half of the memory block
ptr[0] // segmentation fault
where the first 500000 addresses should still be valid if the documentation is to be read literally.

The fact that the Swift runtime currently uses malloc/free is an implementation detail. Tracking deallocation size is a very useful optimization for better allocator backends, and C++ underwent an ABI break to make it so that sized delete can be supported. Maybe we can change the name to `deallocate(allocatedCapacity:)` to make it clear that it isn't resizing the memory, and/or make the capacity argument optional so that you can pay for the overhead of the allocator deriving the size if it's inconvenient for the calling code to carry the size around, but we shouldn't remove the functionality altogether.

-Joe

Hello Swift community,

The review of SE-0184 "Unsafe[Mutable][Raw][Buffer]Pointer: add missing
methods, adjust existing labels for clarity, and remove deallocation size"
begins now and runs through September 7, 2017. The proposal is available
here:

https://github.com/apple/swift-evolution/blob/master/proposa
ls/0184-unsafe-pointers-add-missing.md

Reviews are an important part of the Swift evolution process. All reviews
should be sent to the swift-evolution mailing list at

https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the
review manager. When replying, please try to keep the proposal link at the
top of the message:

Proposal link:

https://github.com/apple/swift-evolution/blob/master/proposa
ls/0184-unsafe-pointers-add-missing.md

Reply text

Other replies

<https://github.com/apple/swift-evolution/pulls#what-goes-into-a-review-1>What
goes into a review?

The goal of the review process is to improve the proposal under review
through constructive criticism and, eventually, determine the direction of
Swift. When writing your review, here are some questions you might want to
answer in your review:

   - What is your evaluation of the proposal?

Overall, this is an improvement. However, I do have some questions and

concerns:

Questions:

## UnsafeMutableRawPointer

Regarding the options given for "whose `count` to use"--which option is
actually being proposed?

I don’t understand the question, `UnsafeMutableRawPointer` takes an
explicit `count:`. the “whose count to use” option is irrelevant.

## UnsafeMutableBufferPointer

Please clarify: why are you proposing that the `at:` arguments in
`UnsafeMutableBufferPointer` and `UnsafeMutableRawBufferPointer` _should
not_ receive default values, but the `at:` arguments in
`UnsafeMutableRawPointer` _should_ receive a default value of `0`?

The extant API for `UnsafeMutableRawPointer` already included these
default arguments which seem to be widely used in the stdlib, the proposal
tries to avoid these kinds of source breakages wherever possible. We avoid
providing the default arguments on buffer pointers because we want the fact
that it takes a *start–length* segment pair to be obvious at the call site.

Concerns:

## UnsafeMutablePointer

It's alarming that omitting `count` in `initialize(repeating:count:)` (and
assign, etc.) means initialize _one_ element, but elsewhere (such as
`UnsafeMutableBufferPointer` means initialize _all_ elements. The behavior
of the proposed API also contradicts its own spelling on its face:
`initialize(repeating: foo)` means *do not repeat* `foo`.

Yes, I understand the argument that `*BufferPointer` types have an
intrinsic count, etc., but in the context of code where types are inferred,
`let foo = T.allocate(capacity: 100); foo.initialize(repeating: bar)`
should not mean one thing for `*BufferPointer` types and a totally
different thing for plain `*Pointer` types--particularly when both can be
allocated with a certain capacity greater than one.

Either `count` should always be required, or for convenience there should
be a separate overload `initialize(pointee:)` that does not require `count`.

I understand the naming is not optimal, but reams of discussion on this
list have concluded that it’s the least bad alternative available. We can’t
just get rid of the default value for `count:` because usage in real code
bases shows that this default argument is actually extremely useful. I
believe well over 90% of the calls to these methods in the standard library
currently rely on the default argument value. Renaming the `repeating:`
argument to `to:` would make the API inconsistent and hide the fact that
plain pointers are still capable of operating on many elements in sequence
— “`to:count:`” makes no grammatical sense to read — “to” is a singular
preposition.

## UnsafeMutableRawBufferPointer

In `copyBytes`, the use of `Bytes` to emphasize that it's the memory
that's being copied is thoughtful, but it is inconsistent with the other
method names that use the terminology `Memory` for the same purpose (e.g.,
`moveInitializeMemory` to clarify the meaning of `moveInitialize`).

For better consistency--and since you're proposing to rename
`copyBytes(from:count:)` on `UnsafeMutableRawPointer` to
`copy(from:bytes:)`--this particular API on `UnsafeMutableRawBufferPointer`
should be named `copyMemory(from:)` and not `copyBytes(from:)`.

Although, actually, looking at the APIs on `UnsafeMutableRawPointer`
itself, that particular method too might best be written as
`copyMemory(from:bytes:)` instead of merely `copy(from:bytes:)` for better
consistency with the rest of the methods on that type as well.

“Memory” methods are distinct from “Bytes” methods in that they assume
typed memory. “Bytes” on the other hand does not care about types.

## General comment

Many `at:` arguments, especially such as in the case of
`copyBytes(at:from:)`, make sense only when read in a list with all other
methods. Standing alone, `at` is ambiguous as to whether it's referring to
the _source_ or the _destination_. Why do these APIs on `*BufferPointer`
types not take advantage of subscripts? That is, why not:

  `foo[x...].copyMemory(from: bar)`

instead of

  `foo.copyBytes(at: x, from: bar)`

The first seems dramatically clearer as to its meaning. The same feedback
applies to nearly all uses of `at` on `*BufferPointer` types: they would
seem to be greatly clarified (in terms of the "what does `at` mean"
question) by the use of a subscript spelling.

This idea sounds elegant on the surface but it’s deeply problematic. `
foo[x...]` suggests that whatever happens to it, will happen to the entire
rest of the buffer slice as well, which is not always the case. It would
have to be written as `foo[x ... x + count].copyMemory(from: bar)` or `foo[x
... x + bar.count].copyMemory(from: bar)` which seems *less* clear. Having
to write `foo[0...]` to operate with no offset also seems silly. It also
means the API would have to be written on `
MutableRandomAccessSlice<UnsafeMutable*BufferPointer>`. Finally, what
happens when you subscript a raw pointer? the subscript doesn’t know about
the stride that you want to use. If you want to get rid of the `at:`
ambiguity, you have to get rid of it everywhere, or users will just wind up
having to remember two ways (one ambiguous and one less ambiguous) of doing
the same thing, instead of one (ambiguous) way of doing it.

I notice that you comment that you feel there are ergonomic issues with
buffer pointer slicing; can you please comment on what is "wasteful"
currently? Is there a performance hit to slicing a `*BufferPointer` type?
If so, we should fix that, as the whole rationale of having these types (as
I understand it) is to improve the ergonomics of working with pointers to
multiple elements by conforming them to `*Collection` APIs.

Slices are not the right abstraction for this because of Swift’s indexing
semantics. A buffer pointer requires two words of information (beginning
and end address), while a buffer pointer slice occupies four words of
information (beginning, end, start-index, end-index) to preserve the index
semantics. Creating a way to “slice” a buffer pointer into another buffer
pointer without going through `init(rebasing:)` is also problematic because
you can’t `deallocate()` a debased buffer pointer, so we should make that
operation explicit.

···

On Sat, Sep 2, 2017 at 10:03 AM, Xiaodi Wu via swift-evolution < swift-evolution@swift.org> wrote:

On Sat, Sep 2, 2017 at 12:27 AM, Douglas Gregor via swift-evolution < > swift-evolution@swift.org> wrote:

It seems deeply unsatisfactory to invent new methods that use `at:`
arguments _on a type whose purpose is to expose `*Collection` APIs_ if we
agree that the slicing notation is demonstrably clearer. I do not have the
same concerns about adding `at:` arguments to `UnsafeMutableRawPointer`
methods, which are quite appropriate.

   - Is the problem being addressed significant enough to warrant a
   change to Swift?

Yes.

   - Does this proposal fit well with the feel and direction of Swift?

In parts, yes. In others (see above), it could be improved to fit better

with the feel and direction of other Swift APIs.

   - If you have used other languages or libraries with a similar
   feature, how do you feel that this proposal compares to those?

There is much more friction to using pointers in Swift than in C.

However, making Swift pointers like C pointers is clearly a non-goal. This
proposal appropriate addresses major pain points to Swift-specific usages
of pointers.

   - How much effort did you put into your review? A glance, a quick
   reading, or an in-depth study?

A moderately thorough reading, drawing on some experience using pointer

APIs in Swift, and based on prior readings of previous iterations of this
proposal and the on-list discussion.

More information about the Swift evolution process is available at

https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

-Doug

Review Manager

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

The idea is to get the house in order by removing all parameters from
deallocate(), since that’s what it really does right now. Then, in the
future, if Swift gets a more sophisticated allocator backend, a new method
like deallocate(capacity:) or reallocate(toCapacity:) could be added
without conflicting with the currently named deallocate(capacity:).
However, using the function signature to pretend that it does something it
can’t actually do right now is extremely dangerous.

···

On Wed, Sep 6, 2017 at 12:41 PM, Joe Groff via swift-evolution < swift-evolution@swift.org> wrote:

> Currently, memory is deallocated by an instance method on
UnsafeMutablePointer, deallocate(count:). Like much of the Swift pointer
API, performing this operation on a buffer pointer requires extracting
baseAddress! and count. It is very common for the allocation code above to
be immediately followed by:
>
> defer
>
> {
> buffer.
> baseAddress?.deallocate(capacity: buffer.count
> )
> }
>
> This method is extremely problematic because nearly all users, on first
seeing the signature of deallocate(capacity:), will naturally conclude from
the capacity label that deallocate(capacity:) is equivalent to some kind of
realloc()that can only shrink the buffer. However this is not the actual
behavior — deallocate(capacity:) actually ignores the capacity argument and
just calls free() on self. The current API is not only awkward and
suboptimal, it is misleading. You can write perfectly legal Swift code that
shouldn’t segfault, but still can, for example
>
> var ptr = UnsafeMutablePointer<UInt8>.allocate(capacity: 1000000
> )
> ptr.
> initialize(to: 13, count: 1000000
> )
> ptr.
> deallocate(capacity: 500000) // deallocate the second half of the memory
block
> ptr[0] // segmentation fault
> where the first 500000 addresses should still be valid if the
documentation is to be read literally.

The fact that the Swift runtime currently uses malloc/free is an
implementation detail. Tracking deallocation size is a very useful
optimization for better allocator backends, and C++ underwent an ABI break
to make it so that sized delete can be supported. Maybe we can change the
name to `deallocate(allocatedCapacity:)` to make it clear that it isn't
resizing the memory, and/or make the capacity argument optional so that you
can pay for the overhead of the allocator deriving the size if it's
inconvenient for the calling code to carry the size around, but we
shouldn't remove the functionality altogether.

-Joe
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

Thanks for the review as always…

Hello Swift community,

The review of SE-0184 "Unsafe[Mutable][Raw][Buffer]Pointer: add missing methods, adjust existing labels for clarity, and remove deallocation size" begins now and runs through September 7, 2017. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0184-unsafe-pointers-add-missing.md
## UnsafeMutableBufferPointer

Please clarify: why are you proposing that the `at:` arguments in `UnsafeMutableBufferPointer` and `UnsafeMutableRawBufferPointer` _should not_ receive default values, but the `at:` arguments in `UnsafeMutableRawPointer` _should_ receive a default value of `0`?

The extant API for `UnsafeMutableRawPointer` already included these default arguments which seem to be widely used in the stdlib,, the proposal tries to avoid these kinds of source breakages wherever possible. We avoid providing the default arguments on buffer pointers because we want the fact that it takes a start–length segment pair to be obvious at the call site.

That’s right, the buffer pointer API is just one level above pointers. It primarily offers the safety of tracking its capacity and bounds checking in debug mode. It does not help you safely manage fully initialized buffer slices (we *do* want to provide that convenience later as a higher-level non-pointer type). Instead, it exposes segments within the buffer for initialization, assignment, deinitialization. It needs to be obvious in the client code, at every call site, that the caller is responsible for tracking those segments. Earlier iterations of this proposal attempted to hide this as a convenience, but that led to dangerous scenarios.

Taking at Kelvin’s example:

var image = UnsafeMutableBufferPointer<Pixel>.allocate(capacity : maxPixels)

var filled:Int = 0
for scanline: UnsafeMutableBufferPointer<Pixel> in scanlines {
    image.moveInitialize(at: filled, from: scanline)
    filled += scanline.count
}

// We don’t want to allow any defaults here.
// It’s important for the client code to explicitly correlate the range
// that it initialized with the range that it deinitialized.
image.deinitialize(at: 0, count: filled)

Concerns:

## UnsafeMutablePointer

It's alarming that omitting `count` in `initialize(repeating:count:)` (and assign, etc.) means initialize _one_ element, but elsewhere (such as `UnsafeMutableBufferPointer` means initialize _all_ elements. The behavior of the proposed API also contradicts its own spelling on its face: `initialize(repeating: foo)` means *do not repeat* `foo`.

Yes, I understand the argument that `*BufferPointer` types have an intrinsic count, etc., but in the context of code where types are inferred, `let foo = T.allocate(capacity: 100); foo.initialize(repeating: bar)` should not mean one thing for `*BufferPointer` types and a totally different thing for plain `*Pointer` types--particularly when both can be allocated with a certain capacity greater than one.

Either `count` should always be required, or for convenience there should be a separate overload `initialize(pointee:)` that does not require `count`.

I understand the naming is not optimal, but reams of discussion on this list have concluded that it’s the least bad alternative available. We can’t just get rid of the default value for `count:` because usage in real code bases shows that this default argument is actually extremely useful. I believe well over 90% of the calls to these methods in the standard library currently rely on the default argument value. Renaming the `repeating:` argument to `to:` would make the API inconsistent and hide the fact that plain pointers are still capable of operating on many elements in sequence — “`to:count:`” makes no grammatical sense to read — “to” is a singular preposition.

The *only* thing I really don’t like about the proposed API is the potential confusion between UnsafeMutablePointer calls with a default count and UnsafeMutableBufferPointer calls using the implied buffer count. But this is where we ended up because the default count turns out to be useful. I’m willing to live with it since I don’t see a great alternative.

## UnsafeMutableRawBufferPointer

In `copyBytes`, the use of `Bytes` to emphasize that it's the memory that's being copied is thoughtful, but it is inconsistent with the other method names that use the terminology `Memory` for the same purpose (e.g., `moveInitializeMemory` to clarify the meaning of `moveInitialize`).

For better consistency--and since you're proposing to rename `copyBytes(from:count:)` on `UnsafeMutableRawPointer` to `copy(from:bytes:)`--this particular API on `UnsafeMutableRawBufferPointer` should be named `copyMemory(from:)` and not `copyBytes(from:)`.

Although, actually, looking at the APIs on `UnsafeMutableRawPointer` itself, that particular method too might best be written as `copyMemory(from:bytes:)` instead of merely `copy(from:bytes:)` for better consistency with the rest of the methods on that type as well.

Xiaodi has a fair point. I’m a moderate +1 on his suggestion. I can’t remember why it wasn’t originally called “copyMemory”. Someone may have taken issue with the fact that it’s the contents of memory being copied, but that’s a silly distinction.

“Memory” methods are distinct from “Bytes” methods in that they assume typed memory. “Bytes” on the other hand does not care about types.

The other Memory methods take a type parameter out of necessity, but that doesn’t need to be a rule. The “Memory” suffix is there because the semantics of operating on untyped memory is slightly different. I think copyMemory would fit with that convention. After all, it’s really meant to be Swift's ‘memcpy’.

## General comment

Many `at:` arguments, especially such as in the case of `copyBytes(at:from:)`, make sense only when read in a list with all other methods. Standing alone, `at` is ambiguous as to whether it's referring to the _source_ or the _destination_. Why do these APIs on `*BufferPointer` types not take advantage of subscripts? That is, why not:

  `foo[x...].copyMemory(from: bar)`

instead of

  `foo.copyBytes(at: x, from: bar)`

The first seems dramatically clearer as to its meaning. The same feedback applies to nearly all uses of `at` on `*BufferPointer` types: they would seem to be greatly clarified (in terms of the "what does `at` mean" question) by the use of a subscript spelling.

In the proposed API, we have a consistent convention that some segment within potentially larger buffer is being replaced with the contents of a smaller buffer.

Ultimately, we want buffer slices to work the way you describe, but first we need to introduce new kind of buffer slice. That won’t happen in Swift 5. Our current buffer slices are fundamentally dangerous, and I don’t think it’s right to build convenience around danger.

This idea sounds elegant on the surface but it’s deeply problematic. `foo[x...]` suggests that whatever happens to it, will happen to the entire rest of the buffer slice as well, which is not always the case. It would have to be written as `foo[x ... x + count].copyMemory(from: bar)` or `foo[x ... x + bar.count].copyMemory(from: bar)` which seems less clear. Having to write `foo[0...]` to operate with no offset also seems silly. It also means the API would have to be written on `MutableRandomAccessSlice<UnsafeMutable*BufferPointer>`. Finally, what happens when you subscript a raw pointer? the subscript doesn’t know about the stride that you want to use. If you want to get rid of the `at:` ambiguity, you have to get rid of it everywhere, or users will just wind up having to remember two ways (one ambiguous and one less ambiguous) of doing the same thing, instead of one (ambiguous) way of doing it.

Yes, Xiaodi is reopening this can of worms again. This is simply out of scope since the proposed API isn’t making any of these problems harder to solve in the future.

I notice that you comment that you feel there are ergonomic issues with buffer pointer slicing; can you please comment on what is "wasteful" currently? Is there a performance hit to slicing a `*BufferPointer` type? If so, we should fix that, as the whole rationale of having these types (as I understand it) is to improve the ergonomics of working with pointers to multiple elements by conforming them to `*Collection` APIs.

Slices are not the right abstraction for this because of Swift’s indexing semantics. A buffer pointer requires two words of information (beginning and end address), while a buffer pointer slice occupies four words of information (beginning, end, start-index, end-index) to preserve the index semantics. Creating a way to “slice” a buffer pointer into another buffer pointer without going through `init(rebasing:)` is also problematic because you can’t `deallocate()` a debased buffer pointer, so we should make that operation explicit.

Today’s buffer slices can certainly be useful when you want to expose a slice as a Sequence. They correctly refer back to the parent buffer and are exactly as efficient as they should be. You probably wouldn’t want to persistently store slices though. You’re better off rebasing the slice as a new buffer. When you do that it’s reasonably obvious that those rebased buffers don’t own their underlying memory. We can definitely come up with a better higher-level API, but there are bigger fish to fry.

It seems deeply unsatisfactory to invent new methods that use `at:` arguments _on a type whose purpose is to expose `*Collection` APIs_ if we agree that the slicing notation is demonstrably clearer. I do not have the same concerns about adding `at:` arguments to `UnsafeMutableRawPointer` methods, which are quite appropriate.

The proposed new methods provide reasonable functionality on UnsafeBufferPointers when you’re *not* using collection APIs. A major motivation for this proposal is that to do anything low-level with an UnsafeBufferPointer, you currently need to drop down to an UnsafePointer first, which is hideous, and you lose bounds checking.

-Andy

···

On Sep 2, 2017, at 12:13 PM, Taylor Swift via swift-evolution <swift-evolution@swift.org> wrote:
On Sat, Sep 2, 2017 at 10:03 AM, Xiaodi Wu via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
On Sat, Sep 2, 2017 at 12:27 AM, Douglas Gregor via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

Is the problem being addressed significant enough to warrant a change to Swift?
Yes.

Does this proposal fit well with the feel and direction of Swift?
In parts, yes. In others (see above), it could be improved to fit better with the feel and direction of other Swift APIs.

If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
There is much more friction to using pointers in Swift than in C. However, making Swift pointers like C pointers is clearly a non-goal. This proposal appropriate addresses major pain points to Swift-specific usages of pointers.

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
A moderately thorough reading, drawing on some experience using pointer APIs in Swift, and based on prior readings of previous iterations of this proposal and the on-list discussion.

More information about the Swift evolution process is available at

https://github.com/apple/swift-evolution/blob/master/process.md
Thank you,

-Doug

Review Manager

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution

Hello Swift community,

The review of SE-0184 "Unsafe[Mutable][Raw][Buffer]Pointer: add missing
methods, adjust existing labels for clarity, and remove deallocation size"
begins now and runs through September 7, 2017. The proposal is available
here:

https://github.com/apple/swift-evolution/blob/master/proposa
ls/0184-unsafe-pointers-add-missing.md

Reviews are an important part of the Swift evolution process. All
reviews should be sent to the swift-evolution mailing list at

https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the
review manager. When replying, please try to keep the proposal link at the
top of the message:

Proposal link:

https://github.com/apple/swift-evolution/blob/master/proposa
ls/0184-unsafe-pointers-add-missing.md

Reply text

Other replies

<https://github.com/apple/swift-evolution/pulls#what-goes-into-a-review-1>What
goes into a review?

The goal of the review process is to improve the proposal under review
through constructive criticism and, eventually, determine the direction of
Swift. When writing your review, here are some questions you might want to
answer in your review:

   - What is your evaluation of the proposal?

Overall, this is an improvement. However, I do have some questions and

concerns:

Questions:

## UnsafeMutableRawPointer

Regarding the options given for "whose `count` to use"--which option is
actually being proposed?

I don’t understand the question, `UnsafeMutableRawPointer` takes an
explicit `count:`. the “whose count to use” option is irrelevant.

In "Proposed Solution," under subheading "UnsafeMutableRawPointer," you
write "the question of whose `count` to use becomes important." You then
outline "[o]ne option" as well as "[a] better option." Which of these
options are you actually proposing? For clarity, could you please excise
the non-proposed option from the "Proposed Solution" section and move it to
the "Alternatives Considered" section?

## UnsafeMutableBufferPointer

Please clarify: why are you proposing that the `at:` arguments in
`UnsafeMutableBufferPointer` and `UnsafeMutableRawBufferPointer` _should
not_ receive default values, but the `at:` arguments in
`UnsafeMutableRawPointer` _should_ receive a default value of `0`?

The extant API for `UnsafeMutableRawPointer` already included these
default arguments which seem to be widely used in the stdlib, the proposal
tries to avoid these kinds of source breakages wherever possible. We avoid
providing the default arguments on buffer pointers because we want the fact
that it takes a *start–length* segment pair to be obvious at the call
site.

Thanks for the clarification; that would be helpful information to put into
the proposal text. It is not an intuitive start-length pair, since the `at`
refers to an offset of the destination buffer but `count` refers to a
length of the source buffer. I appreciate how you separated the proposed
new argument `at` and the existing argument `count` in what is currently
named `initializeMemory<T>(as:from:count:)`, which helps to reinforce that
fact.

Concerns:

## UnsafeMutablePointer

It's alarming that omitting `count` in `initialize(repeating:count:)`
(and assign, etc.) means initialize _one_ element, but elsewhere (such as
`UnsafeMutableBufferPointer` means initialize _all_ elements. The behavior
of the proposed API also contradicts its own spelling on its face:
`initialize(repeating: foo)` means *do not repeat* `foo`.

Yes, I understand the argument that `*BufferPointer` types have an
intrinsic count, etc., but in the context of code where types are inferred,
`let foo = T.allocate(capacity: 100); foo.initialize(repeating: bar)`
should not mean one thing for `*BufferPointer` types and a totally
different thing for plain `*Pointer` types--particularly when both can be
allocated with a certain capacity greater than one.

Either `count` should always be required, or for convenience there should
be a separate overload `initialize(pointee:)` that does not require `count`.

I understand the naming is not optimal, but reams of discussion on this
list have concluded that it’s the least bad alternative available. We can’t
just get rid of the default value for `count:` because usage in real code
bases shows that this default argument is actually extremely useful. I
believe well over 90% of the calls to these methods in the standard library
currently rely on the default argument value. Renaming the `repeating:`
argument to `to:` would make the API inconsistent and hide the fact that
plain pointers are still capable of operating on many elements in sequence
— “`to:count:`” makes no grammatical sense to read — “to” is a singular
preposition.

Let me clarify my concern here. This is not intended to be a bikeshedding
exercise and I agree with your choice of `repeating` (as I did in the
original conversations). For the sake of clarity, however, I'll proceed
with this discussion as though the argument were named `xxxx`. My point
here is that, regardless of what `xxxx` is called, we have a problem here
as follows:

let foo = UnsafeMutablePointer<Int>.allocate(capacity: 21)
foo.initialize(xxxx: 42) // initializes 1 value

let bar = UnsafeMutableBufferPointer<Int>.allocate(capacity: 21)
bar.initialize(xxxx: 42) // initializes 21 values

The same spelling, `initialize(xxxx:)`, does two *different* things under
your proposal depending on whether it's invoked on a UMP or a UMBP. Even
though it's admirable that your proposal is filling in the gaps of Swift's
pointer design and increasing consistency by making similar things have
similar names, different things need to have different names; otherwise, we
are actively creating footguns.

Therefore, while I agree with your choice for `xxxx`, and while I also
agree that it is very useful to have a method that initializes a single
value on a UMP, we need to have a different label `yyyy` for that method.
My suggestion is `pointee`, but I would be equally happy with `value`,
`to`, or whatever else you may choose.

## UnsafeMutableRawBufferPointer

In `copyBytes`, the use of `Bytes` to emphasize that it's the memory
that's being copied is thoughtful, but it is inconsistent with the other
method names that use the terminology `Memory` for the same purpose (e.g.,
`moveInitializeMemory` to clarify the meaning of `moveInitialize`).

For better consistency--and since you're proposing to rename
`copyBytes(from:count:)` on `UnsafeMutableRawPointer` to
`copy(from:bytes:)`--this particular API on `UnsafeMutableRawBufferPointer`
should be named `copyMemory(from:)` and not `copyBytes(from:)`.

Although, actually, looking at the APIs on `UnsafeMutableRawPointer`
itself, that particular method too might best be written as
`copyMemory(from:bytes:)` instead of merely `copy(from:bytes:)` for better
consistency with the rest of the methods on that type as well.

“Memory” methods are distinct from “Bytes” methods in that they assume
typed memory. “Bytes” on the other hand does not care about types.

It's unclear to me that this distinction is either consistently observed or
helpful. For instance, by that standard, `initializeMemory(as:from:)`
should be named `initializeBytes(as:fromMemory:)`, since the memory being
initialized is raw until after initialization. This strikes me as not at
all necessary for user comprehension of the APIs. (On the other hand, if it
_is_ necessary for comprehension, then `copyBytes` should never be
shortened to `copy`, since it would be necessary to emphasize that both the
source and destination are treated as "bytes" rather than "memory.")

## General comment

Many `at:` arguments, especially such as in the case of
`copyBytes(at:from:)`, make sense only when read in a list with all other
methods. Standing alone, `at` is ambiguous as to whether it's referring to
the _source_ or the _destination_. Why do these APIs on `*BufferPointer`
types not take advantage of subscripts? That is, why not:

  `foo[x...].copyMemory(from: bar)`

instead of

  `foo.copyBytes(at: x, from: bar)`

The first seems dramatically clearer as to its meaning. The same feedback
applies to nearly all uses of `at` on `*BufferPointer` types: they would
seem to be greatly clarified (in terms of the "what does `at` mean"
question) by the use of a subscript spelling.

This idea sounds elegant on the surface but it’s deeply problematic. `
foo[x...]` suggests that whatever happens to it, will happen to the
entire rest of the buffer slice as well, which is not always the case.

No more so than `foo.copyBytes(at:from:)` suggests the same?

It would have to be written as `foo[x ... x + count].copyMemory(from: bar)`
or `foo[x ... x + bar.count].copyMemory(from: bar)` which seems *less*
clear. Having to write `foo[0...]` to operate with no offset also seems
silly.

It is unclear to me why one would have to write `foo[0...]`.

It also means the API would have to be written on `
MutableRandomAccessSlice<UnsafeMutable*BufferPointer>`.

Not necessarily; it's unclear to me that
`MutableRandomAccessSlice<UnsafeMutable*BufferPointer>` is the right slice
type for `UnsafeMutable*BufferPointer` in the first place (see below).

Finally, what happens when you subscript a raw pointer?

As you know, only buffer pointers conform to `Collection`, so it's unclear
to me what your question is here.

the subscript doesn’t know about the stride that you want to use. If you
want to get rid of the `at:` ambiguity, you have to get rid of it
everywhere, or users will just wind up having to remember two ways (one
ambiguous and one less ambiguous) of doing the same thing, instead of one
(ambiguous) way of doing it.

Certainly, that's a good point. On rethink and another re-reading of the
proposal, it's unclear to me that the addition of `at` arguments to
UnsafeMutablePointer is sufficiently justified by the proposal text. Is it
merely that it's shorter than writing `foo + MemoryLayout<T>.stride *
offset`? With the ambiguity of `at`, it seems that the current way of
writing it, though longer, is certainly less ambiguous.

I notice that you comment that you feel there are ergonomic issues with

buffer pointer slicing; can you please comment on what is "wasteful"
currently? Is there a performance hit to slicing a `*BufferPointer` type?
If so, we should fix that, as the whole rationale of having these types (as
I understand it) is to improve the ergonomics of working with pointers to
multiple elements by conforming them to `*Collection` APIs.

Slices are not the right abstraction for this because of Swift’s indexing
semantics.

This is an alarming statement if true, in that it would seem to undermine
the basic premise of `*BufferPointer` types conforming to `Collection`,
would it not?

A buffer pointer requires two words of information (beginning and end
address), while a buffer pointer slice occupies four words of information
(beginning, end, start-index, end-index) to preserve the index semantics.
Creating a way to “slice” a buffer pointer into another buffer pointer
without going through `init(rebasing:)` is also problematic because you
can’t `deallocate()` a debased buffer pointer, so we should make that
operation explicit.

It would seem, then, that to properly support slicing--which collection
types should do--we will need a custom slice type a la `Substring`. This
slice type would clearly not support deallocation, but it would conform to
a protocol (a la `StringProtocol`) which would require all the operations
such as `copyMemory` that makes sense for both buffer pointers and their
slices.

It seems deeply unsatisfactory to invent new methods that use `at:`

···

On Sat, Sep 2, 2017 at 2:13 PM, Taylor Swift <kelvin13ma@gmail.com> wrote:

On Sat, Sep 2, 2017 at 10:03 AM, Xiaodi Wu via swift-evolution < > swift-evolution@swift.org> wrote:

On Sat, Sep 2, 2017 at 12:27 AM, Douglas Gregor via swift-evolution < >> swift-evolution@swift.org> wrote:
arguments _on a type whose purpose is to expose `*Collection` APIs_ if we
agree that the slicing notation is demonstrably clearer. I do not have the
same concerns about adding `at:` arguments to `UnsafeMutableRawPointer`
methods, which are quite appropriate.

   - Is the problem being addressed significant enough to warrant a
   change to Swift?

Yes.

   - Does this proposal fit well with the feel and direction of Swift?

In parts, yes. In others (see above), it could be improved to fit better

with the feel and direction of other Swift APIs.

   - If you have used other languages or libraries with a similar
   feature, how do you feel that this proposal compares to those?

There is much more friction to using pointers in Swift than in C.

However, making Swift pointers like C pointers is clearly a non-goal. This
proposal appropriate addresses major pain points to Swift-specific usages
of pointers.

   - How much effort did you put into your review? A glance, a quick
   reading, or an in-depth study?

A moderately thorough reading, drawing on some experience using pointer

APIs in Swift, and based on prior readings of previous iterations of this
proposal and the on-list discussion.

More information about the Swift evolution process is available at

https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

-Doug

Review Manager

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

Thanks for the review as always…

Hello Swift community,

The review of SE-0184 "Unsafe[Mutable][Raw][Buffer]Pointer: add missing
methods, adjust existing labels for clarity, and remove deallocation size"
begins now and runs through September 7, 2017. The proposal is available
here:

https://github.com/apple/swift-evolution/blob/master/proposa
ls/0184-unsafe-pointers-add-missing.md

## UnsafeMutableBufferPointer

Please clarify: why are you proposing that the `at:` arguments in
`UnsafeMutableBufferPointer` and `UnsafeMutableRawBufferPointer` _should
not_ receive default values, but the `at:` arguments in
`UnsafeMutableRawPointer` _should_ receive a default value of `0`?

The extant API for `UnsafeMutableRawPointer` already included these
default arguments which seem to be widely used in the stdlib, the proposal
tries to avoid these kinds of source breakages wherever possible. We avoid
providing the default arguments on buffer pointers because we want the fact
that it takes a *start–length* segment pair to be obvious at the call
site.

That’s right, the buffer pointer API is just one level above pointers. It
primarily offers the safety of tracking its capacity and bounds checking in
debug mode. It does not help you safely manage fully initialized buffer
slices (we *do* want to provide that convenience later as a higher-level
non-pointer type). Instead, it exposes segments within the buffer for
initialization, assignment, deinitialization. It needs to be obvious in the
client code, at every call site, that the caller is responsible for
tracking those segments. Earlier iterations of this proposal attempted to
hide this as a convenience, but that led to dangerous scenarios.

Taking at Kelvin’s example:

var image = UnsafeMutableBufferPointer<Pixel>.allocate(capacity :
maxPixels)

var filled:Int = 0
for scanline: UnsafeMutableBufferPointer<Pixel> in scanlines {
    image.moveInitialize(at: filled, from: scanline)
    filled += scanline.count
}

// We don’t want to allow any defaults here.
// It’s important for the client code to explicitly correlate the range
// that it initialized with the range that it deinitialized.
image.deinitialize(at: 0, count: filled)

Concerns:

## UnsafeMutablePointer

It's alarming that omitting `count` in `initialize(repeating:count:)`
(and assign, etc.) means initialize _one_ element, but elsewhere (such as
`UnsafeMutableBufferPointer` means initialize _all_ elements. The behavior
of the proposed API also contradicts its own spelling on its face:
`initialize(repeating: foo)` means *do not repeat* `foo`.

Yes, I understand the argument that `*BufferPointer` types have an
intrinsic count, etc., but in the context of code where types are inferred,
`let foo = T.allocate(capacity: 100); foo.initialize(repeating: bar)`
should not mean one thing for `*BufferPointer` types and a totally
different thing for plain `*Pointer` types--particularly when both can be
allocated with a certain capacity greater than one.

Either `count` should always be required, or for convenience there should
be a separate overload `initialize(pointee:)` that does not require `count`.

I understand the naming is not optimal, but reams of discussion on this
list have concluded that it’s the least bad alternative available. We can’t
just get rid of the default value for `count:` because usage in real code
bases shows that this default argument is actually extremely useful. I
believe well over 90% of the calls to these methods in the standard library
currently rely on the default argument value. Renaming the `repeating:`
argument to `to:` would make the API inconsistent and hide the fact that
plain pointers are still capable of operating on many elements in sequence
— “`to:count:`” makes no grammatical sense to read — “to” is a singular
preposition.

The *only* thing I really don’t like about the proposed API is the
potential confusion between UnsafeMutablePointer calls with a default count
and UnsafeMutableBufferPointer calls using the implied buffer count. But
this is where we ended up because the default count turns out to be useful.
I’m willing to live with it since I don’t see a great alternative.

This only thing is also the major thing that causes me concern. I have no
problem with the name itself. In my view, it is important to provide the
very useful function with a default count of 1 using a different name, as
I've written in the other reply.

## UnsafeMutableRawBufferPointer

In `copyBytes`, the use of `Bytes` to emphasize that it's the memory
that's being copied is thoughtful, but it is inconsistent with the other
method names that use the terminology `Memory` for the same purpose (e.g.,
`moveInitializeMemory` to clarify the meaning of `moveInitialize`).

For better consistency--and since you're proposing to rename
`copyBytes(from:count:)` on `UnsafeMutableRawPointer` to
`copy(from:bytes:)`--this particular API on `UnsafeMutableRawBufferPointer`
should be named `copyMemory(from:)` and not `copyBytes(from:)`.

Although, actually, looking at the APIs on `UnsafeMutableRawPointer`
itself, that particular method too might best be written as
`copyMemory(from:bytes:)` instead of merely `copy(from:bytes:)` for better
consistency with the rest of the methods on that type as well.

Xiaodi has a fair point. I’m a moderate +1 on his suggestion. I can’t
remember why it wasn’t originally called “copyMemory”. Someone may have
taken issue with the fact that it’s the contents of memory being copied,
but that’s a silly distinction.

“Memory” methods are distinct from “Bytes” methods in that they assume
typed memory. “Bytes” on the other hand does not care about types.

The other Memory methods take a type parameter out of necessity, but that
doesn’t need to be a rule. The “Memory” suffix is there because the
semantics of operating on untyped memory is slightly different. I think
copyMemory would fit with that convention. After all, it’s really meant to
be Swift's ‘memcpy’.

## General comment

Many `at:` arguments, especially such as in the case of
`copyBytes(at:from:)`, make sense only when read in a list with all other
methods. Standing alone, `at` is ambiguous as to whether it's referring to
the _source_ or the _destination_. Why do these APIs on `*BufferPointer`
types not take advantage of subscripts? That is, why not:

  `foo[x...].copyMemory(from: bar)`

instead of

  `foo.copyBytes(at: x, from: bar)`

The first seems dramatically clearer as to its meaning. The same feedback
applies to nearly all uses of `at` on `*BufferPointer` types: they would
seem to be greatly clarified (in terms of the "what does `at` mean"
question) by the use of a subscript spelling.

In the proposed API, we have a consistent convention that some segment
within potentially larger buffer is being replaced with the contents of a
smaller buffer.

Ultimately, we want buffer slices to work the way you describe, but first
we need to introduce new kind of buffer slice. That won’t happen in Swift
5. Our current buffer slices are fundamentally dangerous, and I don’t think
it’s right to build convenience around danger.

Got it. I think we've circled back to the same conclusion here about the
need for a new kind of buffer slice.

This idea sounds elegant on the surface but it’s deeply problematic. `
foo[x...]` suggests that whatever happens to it, will happen to the
entire rest of the buffer slice as well, which is not always the case. It
would have to be written as `foo[x ... x + count].copyMemory(from: bar)`
or `foo[x ... x + bar.count].copyMemory(from: bar)` which seems *less* clear.
Having to write `foo[0...]` to operate with no offset also seems silly.
It also means the API would have to be written on `
MutableRandomAccessSlice<UnsafeMutable*BufferPointer>`. Finally, what
happens when you subscript a raw pointer? the subscript doesn’t know about
the stride that you want to use. If you want to get rid of the `at:`
ambiguity, you have to get rid of it everywhere, or users will just wind up
having to remember two ways (one ambiguous and one less ambiguous) of doing
the same thing, instead of one (ambiguous) way of doing it.

Yes, Xiaodi is reopening this can of worms again. This is simply out of
scope since the proposed API isn’t making any of these problems harder to
solve in the future.

It isn't making future solutions harder, but my concern is that the
ambiguity of `at` and its use with different meanings (`at` in typed
pointers referring to an offset that is a multiple of the *destination*
pointee type, but `at` in raw pointers referring to an offset that is a
multiple of the *source* pointee type instead of raw bytes) is confusing
enough that designing a whole set of new methods to increase this usage is
not clearly an improvement. At least, as compared to the status quo of
tolerating the less ergonomic, but also unambiguous, `foo +
MemoryLayout<T>.stride * offset`. Especially given that, as you say, these
problems do need to be solved some other way in the future. Unless I am
missing another major argument for incorporating additional uses of `at`,
it would seem (to me) preferable to eliminate all or most of these uses
entirely even if new slice types are out of scope for Swift 5.

···

On Sat, Sep 2, 2017 at 3:36 PM, Andrew Trick <atrick@apple.com> wrote:

On Sep 2, 2017, at 12:13 PM, Taylor Swift via swift-evolution < > swift-evolution@swift.org> wrote:
On Sat, Sep 2, 2017 at 10:03 AM, Xiaodi Wu via swift-evolution <swift- > evolution@swift.org> wrote:

On Sat, Sep 2, 2017 at 12:27 AM, Douglas Gregor via swift-evolution < >> swift-evolution@swift.org> wrote:

I notice that you comment that you feel there are ergonomic issues with

buffer pointer slicing; can you please comment on what is "wasteful"
currently? Is there a performance hit to slicing a `*BufferPointer` type?
If so, we should fix that, as the whole rationale of having these types (as
I understand it) is to improve the ergonomics of working with pointers to
multiple elements by conforming them to `*Collection` APIs.

Slices are not the right abstraction for this because of Swift’s indexing
semantics. A buffer pointer requires two words of information (beginning
and end address), while a buffer pointer slice occupies four words of
information (beginning, end, start-index, end-index) to preserve the index
semantics. Creating a way to “slice” a buffer pointer into another buffer
pointer without going through `init(rebasing:)` is also problematic
because you can’t `deallocate()` a debased buffer pointer, so we should
make that operation explicit.

Today’s buffer slices can certainly be useful when you want to expose a
slice as a Sequence. They correctly refer back to the parent buffer and are
exactly as efficient as they should be. You probably wouldn’t want to
persistently store slices though. You’re better off rebasing the slice as a
new buffer. When you do that it’s reasonably obvious that those rebased
buffers don’t own their underlying memory. We can definitely come up with a
better higher-level API, but there are bigger fish to fry.

It seems deeply unsatisfactory to invent new methods that use `at:`

arguments _on a type whose purpose is to expose `*Collection` APIs_ if we
agree that the slicing notation is demonstrably clearer. I do not have the
same concerns about adding `at:` arguments to `UnsafeMutableRawPointer`
methods, which are quite appropriate.

The proposed new methods provide reasonable functionality on
UnsafeBufferPointers when you’re *not* using collection APIs. A major
motivation for this proposal is that to do anything low-level with an
UnsafeBufferPointer, you currently need to drop down to an UnsafePointer
first, which is hideous, and you lose bounds checking.

-Andy

   - Is the problem being addressed significant enough to warrant a
   change to Swift?

Yes.

   - Does this proposal fit well with the feel and direction of Swift?

In parts, yes. In others (see above), it could be improved to fit better

with the feel and direction of other Swift APIs.

   - If you have used other languages or libraries with a similar
   feature, how do you feel that this proposal compares to those?

There is much more friction to using pointers in Swift than in C.

However, making Swift pointers like C pointers is clearly a non-goal. This
proposal appropriate addresses major pain points to Swift-specific usages
of pointers.

   - How much effort did you put into your review? A glance, a quick
   reading, or an in-depth study?

A moderately thorough reading, drawing on some experience using pointer

APIs in Swift, and based on prior readings of previous iterations of this
proposal and the on-list discussion.

More information about the Swift evolution process is available at

https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

-Doug

Review Manager

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

I don't think that's a good idea in this case, because it's not unlikely we would explore an optimized allocator soon after ABI stability, and retrofitting these interfaces in a future version of Swift would put a deployment target limit on when they can be used, and mean that a lot of user code would need to be retrofitted to carry allocated capacities where needed to see any benefit.

-Joe

···

On Sep 6, 2017, at 1:06 PM, Taylor Swift <kelvin13ma@gmail.com> wrote:

On Wed, Sep 6, 2017 at 12:41 PM, Joe Groff via swift-evolution <swift-evolution@swift.org> wrote:
> Currently, memory is deallocated by an instance method on UnsafeMutablePointer, deallocate(count:). Like much of the Swift pointer API, performing this operation on a buffer pointer requires extracting baseAddress! and count. It is very common for the allocation code above to be immediately followed by:
>
> defer
>
> {
> buffer.
> baseAddress?.deallocate(capacity: buffer.count
> )
> }
>
> This method is extremely problematic because nearly all users, on first seeing the signature of deallocate(capacity:), will naturally conclude from the capacity label that deallocate(capacity:) is equivalent to some kind of realloc()that can only shrink the buffer. However this is not the actual behavior — deallocate(capacity:) actually ignores the capacity argument and just calls free() on self. The current API is not only awkward and suboptimal, it is misleading. You can write perfectly legal Swift code that shouldn’t segfault, but still can, for example
>
> var ptr = UnsafeMutablePointer<UInt8>.allocate(capacity: 1000000
> )
> ptr.
> initialize(to: 13, count: 1000000
> )
> ptr.
> deallocate(capacity: 500000) // deallocate the second half of the memory block
> ptr[0] // segmentation fault
> where the first 500000 addresses should still be valid if the documentation is to be read literally.

The fact that the Swift runtime currently uses malloc/free is an implementation detail. Tracking deallocation size is a very useful optimization for better allocator backends, and C++ underwent an ABI break to make it so that sized delete can be supported. Maybe we can change the name to `deallocate(allocatedCapacity:)` to make it clear that it isn't resizing the memory, and/or make the capacity argument optional so that you can pay for the overhead of the allocator deriving the size if it's inconvenient for the calling code to carry the size around, but we shouldn't remove the functionality altogether.

-Joe
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

The idea is to get the house in order by removing all parameters from deallocate(), since that’s what it really does right now. Then, in the future, if Swift gets a more sophisticated allocator backend, a new method like deallocate(capacity:) or reallocate(toCapacity:) could be added without conflicting with the currently named deallocate(capacity:). However, using the function signature to pretend that it does something it can’t actually do right now is extremely dangerous.

The fact that we’re using malloc and free is already part of the ABI because old libraries need to be able to deallocate memory allocated by newer libraries.

Within the standard library we could make use of some new deallocation fast path in the future without worrying about backward deployment.

Outside of the standard library, clients will get the benefits of whatever allocator is available on their deployed platform because we now encourage them to use UnsafeBufferPointer.deallocate(). We can change the implementation inside UnsafeBufferPointer all we want, as long as it’s still malloc-compatible.

I’m sure we’ll want to provide a better allocation/deallocation API in the future for systems programmers based on move-only types. That will already be deployment-limited.

Absolute worst case, we introduce a sized UnsafePointer.deallocate in the future. Any new code outside the stdlib that wants the performance advantage would either need to
- trivially wrap deallocation using UnsafeBufferPointer
- create a trivial UnsafePointer.deallocate thunk under an availability flag

-Andy

···

On Sep 6, 2017, at 1:12 PM, Joe Groff via swift-evolution <swift-evolution@swift.org> wrote:

On Sep 6, 2017, at 1:06 PM, Taylor Swift <kelvin13ma@gmail.com> wrote:

On Wed, Sep 6, 2017 at 12:41 PM, Joe Groff via swift-evolution <swift-evolution@swift.org> wrote:

Currently, memory is deallocated by an instance method on UnsafeMutablePointer, deallocate(count:). Like much of the Swift pointer API, performing this operation on a buffer pointer requires extracting baseAddress! and count. It is very common for the allocation code above to be immediately followed by:

defer

{
   buffer.
baseAddress?.deallocate(capacity: buffer.count
)
}

This method is extremely problematic because nearly all users, on first seeing the signature of deallocate(capacity:), will naturally conclude from the capacity label that deallocate(capacity:) is equivalent to some kind of realloc()that can only shrink the buffer. However this is not the actual behavior — deallocate(capacity:) actually ignores the capacity argument and just calls free() on self. The current API is not only awkward and suboptimal, it is misleading. You can write perfectly legal Swift code that shouldn’t segfault, but still can, for example

var ptr = UnsafeMutablePointer<UInt8>.allocate(capacity: 1000000
)
ptr.
initialize(to: 13, count: 1000000
)
ptr.
deallocate(capacity: 500000) // deallocate the second half of the memory block
ptr[0] // segmentation fault
where the first 500000 addresses should still be valid if the documentation is to be read literally.

The fact that the Swift runtime currently uses malloc/free is an implementation detail. Tracking deallocation size is a very useful optimization for better allocator backends, and C++ underwent an ABI break to make it so that sized delete can be supported. Maybe we can change the name to `deallocate(allocatedCapacity:)` to make it clear that it isn't resizing the memory, and/or make the capacity argument optional so that you can pay for the overhead of the allocator deriving the size if it's inconvenient for the calling code to carry the size around, but we shouldn't remove the functionality altogether.

-Joe
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

The idea is to get the house in order by removing all parameters from deallocate(), since that’s what it really does right now. Then, in the future, if Swift gets a more sophisticated allocator backend, a new method like deallocate(capacity:) or reallocate(toCapacity:) could be added without conflicting with the currently named deallocate(capacity:). However, using the function signature to pretend that it does something it can’t actually do right now is extremely dangerous.

I don't think that's a good idea in this case, because it's not unlikely we would explore an optimized allocator soon after ABI stability, and retrofitting these interfaces in a future version of Swift would put a deployment target limit on when they can be used, and mean that a lot of user code would need to be retrofitted to carry allocated capacities where needed to see any benefit.

-Joe

Currently, memory is deallocated by an instance method on UnsafeMutablePointer, deallocate(count:). Like much of the Swift pointer API, performing this operation on a buffer pointer requires extracting baseAddress! and count. It is very common for the allocation code above to be immediately followed by:

defer

{
   buffer.
baseAddress?.deallocate(capacity: buffer.count
)
}

This method is extremely problematic because nearly all users, on first seeing the signature of deallocate(capacity:), will naturally conclude from the capacity label that deallocate(capacity:) is equivalent to some kind of realloc()that can only shrink the buffer. However this is not the actual behavior — deallocate(capacity:) actually ignores the capacity argument and just calls free() on self. The current API is not only awkward and suboptimal, it is misleading. You can write perfectly legal Swift code that shouldn’t segfault, but still can, for example

var ptr = UnsafeMutablePointer<UInt8>.allocate(capacity: 1000000
)
ptr.
initialize(to: 13, count: 1000000
)
ptr.
deallocate(capacity: 500000) // deallocate the second half of the memory block
ptr[0] // segmentation fault
where the first 500000 addresses should still be valid if the documentation is to be read literally.

The fact that the Swift runtime currently uses malloc/free is an implementation detail. Tracking deallocation size is a very useful optimization for better allocator backends, and C++ underwent an ABI break to make it so that sized delete can be supported. Maybe we can change the name to `deallocate(allocatedCapacity:)` to make it clear that it isn't resizing the memory, and/or make the capacity argument optional so that you can pay for the overhead of the allocator deriving the size if it's inconvenient for the calling code to carry the size around, but we shouldn't remove the functionality altogether.

-Joe
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

The idea is to get the house in order by removing all parameters from deallocate(), since that’s what it really does right now. Then, in the future, if Swift gets a more sophisticated allocator backend, a new method like deallocate(capacity:) or reallocate(toCapacity:) could be added without conflicting with the currently named deallocate(capacity:). However, using the function signature to pretend that it does something it can’t actually do right now is extremely dangerous.

I don't think that's a good idea in this case, because it's not unlikely we would explore an optimized allocator soon after ABI stability, and retrofitting these interfaces in a future version of Swift would put a deployment target limit on when they can be used, and mean that a lot of user code would need to be retrofitted to carry allocated capacities where needed to see any benefit.

-Joe

The fact that we’re using malloc and free is already part of the ABI because old libraries need to be able to deallocate memory allocated by newer libraries.

The compiler doesn't ever generate calls directly to malloc and free, and the runtime entry points we do use already take size and alignment on both allocation and deallocation.

Within the standard library we could make use of some new deallocation fast path in the future without worrying about backward deployment.

Outside of the standard library, clients will get the benefits of whatever allocator is available on their deployed platform because we now encourage them to use UnsafeBufferPointer.deallocate(). We can change the implementation inside UnsafeBufferPointer all we want, as long as it’s still malloc-compatible.

I’m sure we’ll want to provide a better allocation/deallocation API in the future for systems programmers based on move-only types. That will already be deployment-limited.

Absolute worst case, we introduce a sized UnsafePointer.deallocate in the future. Any new code outside the stdlib that wants the performance advantage would either need to
- trivially wrap deallocation using UnsafeBufferPointer
- create a trivial UnsafePointer.deallocate thunk under an availability flag

Since we already have sized deallocate, why would we take it away? If the name is misleading, we can change the name.

-Joe

···

On Sep 6, 2017, at 2:28 PM, Andrew Trick <atrick@apple.com> wrote:

On Sep 6, 2017, at 1:12 PM, Joe Groff via swift-evolution <swift-evolution@swift.org> wrote:

On Sep 6, 2017, at 1:06 PM, Taylor Swift <kelvin13ma@gmail.com> wrote:
On Wed, Sep 6, 2017 at 12:41 PM, Joe Groff via swift-evolution <swift-evolution@swift.org> wrote:

The fact that we’re using malloc and free is already part of the ABI because old libraries need to be able to deallocate memory allocated by newer libraries.

The compiler doesn't ever generate calls directly to malloc and free, and the runtime entry points we do use already take size and alignment on both allocation and deallocation.

True, we’ve never said that UnsafePointer deallocation is compatible with C, and I we don't currently expose malloc_size functionality in any API AFAIK.

Within the standard library we could make use of some new deallocation fast path in the future without worrying about backward deployment.

Outside of the standard library, clients will get the benefits of whatever allocator is available on their deployed platform because we now encourage them to use UnsafeBufferPointer.deallocate(). We can change the implementation inside UnsafeBufferPointer all we want, as long as it’s still malloc-compatible.

I’m sure we’ll want to provide a better allocation/deallocation API in the future for systems programmers based on move-only types. That will already be deployment-limited.

Absolute worst case, we introduce a sized UnsafePointer.deallocate in the future. Any new code outside the stdlib that wants the performance advantage would either need to
- trivially wrap deallocation using UnsafeBufferPointer
- create a trivial UnsafePointer.deallocate thunk under an availability flag

Since we already have sized deallocate, why would we take it away? If the name is misleading, we can change the name.

I don't think leaving it as an optional argument is worthwhile, as I explained above. Deallocation capacity is either required or we drop it completely. If we keep it, then `allocatedCapacity` is the right name.

The reason for taking it away, beside being misleading, is that it exposes another level of unsafety.

My thinking has been that this is not the allocation fast path of the future, and the stdlib itself could track the size of unsafely-allocated blocks if it ever used a different underlying allocator.

Now I realize this isn't really about fast/slow deallocation paths. Removing `capacity` or even making it optional forces all future Swift implementations to provide malloc_size functionality for any piece of memory that is compatible with the Unsafe API.

I'm actually ok with that, because I think it's generally desirable for application memory to reside in either in malloc-compatible blocks or fixed size pools. i.e. I think malloc_size is something the platform needs. However, you seem to think this would be too restrictive in the future. How is this a known problem for C and what's your confidence level it will be a problem for Swift?

-Andy

···

On Sep 6, 2017, at 2:31 PM, Joe Groff <jgroff@apple.com> wrote:

Exactly, we are changing the name to `deallocate()`. As for the old `deallocate(capacity:)` method that *needs* to be removed because it is actively harmful. As I’ve explained it’s not enough to just drop in a sized backend later as an “implementation detail” because it’s not an implementation detail, you’re changing the fundamental behavior of that method.

···

On Sep 6, 2017, at 4:31 PM, Joe Groff <jgroff@apple.com> wrote:

On Sep 6, 2017, at 2:28 PM, Andrew Trick <atrick@apple.com> wrote:

On Sep 6, 2017, at 1:12 PM, Joe Groff via swift-evolution <swift-evolution@swift.org> wrote:

On Sep 6, 2017, at 1:06 PM, Taylor Swift <kelvin13ma@gmail.com> wrote:

On Wed, Sep 6, 2017 at 12:41 PM, Joe Groff via swift-evolution <swift-evolution@swift.org> wrote:
Currently, memory is deallocated by an instance method on UnsafeMutablePointer, deallocate(count:). Like much of the Swift pointer API, performing this operation on a buffer pointer requires extracting baseAddress! and count. It is very common for the allocation code above to be immediately followed by:

defer

{
  buffer.
baseAddress?.deallocate(capacity: buffer.count
)
}

This method is extremely problematic because nearly all users, on first seeing the signature of deallocate(capacity:), will naturally conclude from the capacity label that deallocate(capacity:) is equivalent to some kind of realloc()that can only shrink the buffer. However this is not the actual behavior — deallocate(capacity:) actually ignores the capacity argument and just calls free() on self. The current API is not only awkward and suboptimal, it is misleading. You can write perfectly legal Swift code that shouldn’t segfault, but still can, for example

var ptr = UnsafeMutablePointer<UInt8>.allocate(capacity: 1000000
)
ptr.
initialize(to: 13, count: 1000000
)
ptr.
deallocate(capacity: 500000) // deallocate the second half of the memory block
ptr[0] // segmentation fault
where the first 500000 addresses should still be valid if the documentation is to be read literally.

The fact that the Swift runtime currently uses malloc/free is an implementation detail. Tracking deallocation size is a very useful optimization for better allocator backends, and C++ underwent an ABI break to make it so that sized delete can be supported. Maybe we can change the name to `deallocate(allocatedCapacity:)` to make it clear that it isn't resizing the memory, and/or make the capacity argument optional so that you can pay for the overhead of the allocator deriving the size if it's inconvenient for the calling code to carry the size around, but we shouldn't remove the functionality altogether.

-Joe
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

The idea is to get the house in order by removing all parameters from deallocate(), since that’s what it really does right now. Then, in the future, if Swift gets a more sophisticated allocator backend, a new method like deallocate(capacity:) or reallocate(toCapacity:) could be added without conflicting with the currently named deallocate(capacity:). However, using the function signature to pretend that it does something it can’t actually do right now is extremely dangerous.

I don't think that's a good idea in this case, because it's not unlikely we would explore an optimized allocator soon after ABI stability, and retrofitting these interfaces in a future version of Swift would put a deployment target limit on when they can be used, and mean that a lot of user code would need to be retrofitted to carry allocated capacities where needed to see any benefit.

-Joe

The fact that we’re using malloc and free is already part of the ABI because old libraries need to be able to deallocate memory allocated by newer libraries.

The compiler doesn't ever generate calls directly to malloc and free, and the runtime entry points we do use already take size and alignment on both allocation and deallocation.

Within the standard library we could make use of some new deallocation fast path in the future without worrying about backward deployment.

Outside of the standard library, clients will get the benefits of whatever allocator is available on their deployed platform because we now encourage them to use UnsafeBufferPointer.deallocate(). We can change the implementation inside UnsafeBufferPointer all we want, as long as it’s still malloc-compatible.

I’m sure we’ll want to provide a better allocation/deallocation API in the future for systems programmers based on move-only types. That will already be deployment-limited.

Absolute worst case, we introduce a sized UnsafePointer.deallocate in the future. Any new code outside the stdlib that wants the performance advantage would either need to
- trivially wrap deallocation using UnsafeBufferPointer
- create a trivial UnsafePointer.deallocate thunk under an availability flag

Since we already have sized deallocate, why would we take it away? If the name is misleading, we can change the name.

-Joe

Hello Swift community,

The review of SE-0184 "Unsafe[Mutable][Raw][Buffer]Pointer: add
missing methods, adjust existing labels for clarity, and remove
deallocation size" begins now and runs through September 7, 2017. The
proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposa
ls/0184-unsafe-pointers-add-missing.md

Reviews are an important part of the Swift evolution process. All
reviews should be sent to the swift-evolution mailing list at

https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the
review manager. When replying, please try to keep the proposal link at the
top of the message:

Proposal link:

https://github.com/apple/swift-evolution/blob/master/proposa
ls/0184-unsafe-pointers-add-missing.md

Reply text

Other replies

<https://github.com/apple/swift-evolution/pulls#what-goes-into-a-review-1>What
goes into a review?

The goal of the review process is to improve the proposal under review
through constructive criticism and, eventually, determine the direction of
Swift. When writing your review, here are some questions you might want to
answer in your review:

   - What is your evaluation of the proposal?

Overall, this is an improvement. However, I do have some questions and

concerns:

Questions:

## UnsafeMutableRawPointer

Regarding the options given for "whose `count` to use"--which option is
actually being proposed?

I don’t understand the question, `UnsafeMutableRawPointer` takes an
explicit `count:`. the “whose count to use” option is irrelevant.

In "Proposed Solution," under subheading "UnsafeMutableRawPointer," you
write "the question of whose `count` to use becomes important." You then
outline "[o]ne option" as well as "[a] better option." Which of these
options are you actually proposing? For clarity, could you please excise
the non-proposed option from the "Proposed Solution" section and move it to
the "Alternatives Considered" section?

The *Proposed solution* section is divided into two parts, one dealing with
plain pointers and one dealing with buffer pointers. The sections are
separated by the horizontal rule above “*Buffer pointers are conceptually
similar…*”. I don’t know why we’re arguing over typographic formatting but
I am glad this proposal is noncontroversial enough to induce debate over
its horizontal rules. As for the two options, the first is a strawman to
explain why we are going with the second option.

## UnsafeMutableBufferPointer

Please clarify: why are you proposing that the `at:` arguments in
`UnsafeMutableBufferPointer` and `UnsafeMutableRawBufferPointer`
_should not_ receive default values, but the `at:` arguments in
`UnsafeMutableRawPointer` _should_ receive a default value of `0`?

The extant API for `UnsafeMutableRawPointer` already included these
default arguments which seem to be widely used in the stdlib, the proposal
tries to avoid these kinds of source breakages wherever possible. We avoid
providing the default arguments on buffer pointers because we want the fact
that it takes a *start–length* segment pair to be obvious at the call
site.

Thanks for the clarification; that would be helpful information to put
into the proposal text. It is not an intuitive start-length pair, since the
`at` refers to an offset of the destination buffer but `count` refers to a
length of the source buffer. I appreciate how you separated the proposed
new argument `at` and the existing argument `count` in what is currently
named `initializeMemory<T>(as:from:count:)`, which helps to reinforce
that fact.

Concerns:

## UnsafeMutablePointer

It's alarming that omitting `count` in `initialize(repeating:count:)`
(and assign, etc.) means initialize _one_ element, but elsewhere (such as
`UnsafeMutableBufferPointer` means initialize _all_ elements. The behavior
of the proposed API also contradicts its own spelling on its face:
`initialize(repeating: foo)` means *do not repeat* `foo`.

Yes, I understand the argument that `*BufferPointer` types have an
intrinsic count, etc., but in the context of code where types are inferred,
`let foo = T.allocate(capacity: 100); foo.initialize(repeating: bar)`
should not mean one thing for `*BufferPointer` types and a totally
different thing for plain `*Pointer` types--particularly when both can be
allocated with a certain capacity greater than one.

Either `count` should always be required, or for convenience there
should be a separate overload `initialize(pointee:)` that does not require
`count`.

I understand the naming is not optimal, but reams of discussion on this
list have concluded that it’s the least bad alternative available. We can’t
just get rid of the default value for `count:` because usage in real
code bases shows that this default argument is actually extremely useful. I
believe well over 90% of the calls to these methods in the standard library
currently rely on the default argument value. Renaming the `repeating:`
argument to `to:` would make the API inconsistent and hide the fact that
plain pointers are still capable of operating on many elements in sequence
— “`to:count:`” makes no grammatical sense to read — “to” is a singular
preposition.

Let me clarify my concern here. This is not intended to be a bikeshedding
exercise and I agree with your choice of `repeating` (as I did in the
original conversations). For the sake of clarity, however, I'll proceed
with this discussion as though the argument were named `xxxx`. My point
here is that, regardless of what `xxxx` is called, we have a problem here
as follows:

let foo = UnsafeMutablePointer<Int>.allocate(capacity: 21)
foo.initialize(xxxx: 42) // initializes 1 value

let bar = UnsafeMutableBufferPointer<Int>.allocate(capacity: 21)
bar.initialize(xxxx: 42) // initializes 21 values

The same spelling, `initialize(xxxx:)`, does two *different* things under
your proposal depending on whether it's invoked on a UMP or a UMBP. Even
though it's admirable that your proposal is filling in the gaps of Swift's
pointer design and increasing consistency by making similar things have
similar names, different things need to have different names; otherwise, we
are actively creating footguns.

Therefore, while I agree with your choice for `xxxx`, and while I also
agree that it is very useful to have a method that initializes a single
value on a UMP, we need to have a different label `yyyy` for that method.
My suggestion is `pointee`, but I would be equally happy with `value`,
`to`, or whatever else you may choose.

yes this is a problem, but your solution is to add a second set of methods
that are functionally identical, except for the names of the argument
labels. This strikes me as silly and wasteful of API surface.

## UnsafeMutableRawBufferPointer

In `copyBytes`, the use of `Bytes` to emphasize that it's the memory
that's being copied is thoughtful, but it is inconsistent with the other
method names that use the terminology `Memory` for the same purpose (e.g.,
`moveInitializeMemory` to clarify the meaning of `moveInitialize`).

For better consistency--and since you're proposing to rename
`copyBytes(from:count:)` on `UnsafeMutableRawPointer` to
`copy(from:bytes:)`--this particular API on `UnsafeMutableRawBufferPointer`
should be named `copyMemory(from:)` and not `copyBytes(from:)`.

Although, actually, looking at the APIs on `UnsafeMutableRawPointer`
itself, that particular method too might best be written as
`copyMemory(from:bytes:)` instead of merely `copy(from:bytes:)` for better
consistency with the rest of the methods on that type as well.

“Memory” methods are distinct from “Bytes” methods in that they assume
typed memory. “Bytes” on the other hand does not care about types.

It's unclear to me that this distinction is either consistently observed
or helpful. For instance, by that standard, `initializeMemory(as:from:)`
should be named `initializeBytes(as:fromMemory:)`, since the memory being
initialized is raw until after initialization. This strikes me as not at
all necessary for user comprehension of the APIs. (On the other hand, if it
_is_ necessary for comprehension, then `copyBytes` should never be
shortened to `copy`, since it would be necessary to emphasize that both the
source and destination are treated as "bytes" rather than "memory.")

`copyMemory` would probably be better and I am not opposed to adding it to
the proposal. at the same time I don’t think it’s so bad a problem that it
really *needs* to be renamed.

## General comment

Many `at:` arguments, especially such as in the case of
`copyBytes(at:from:)`, make sense only when read in a list with all other
methods. Standing alone, `at` is ambiguous as to whether it's referring to
the _source_ or the _destination_. Why do these APIs on `*BufferPointer`
types not take advantage of subscripts? That is, why not:

  `foo[x...].copyMemory(from: bar)`

instead of

  `foo.copyBytes(at: x, from: bar)`

The first seems dramatically clearer as to its meaning. The same
feedback applies to nearly all uses of `at` on `*BufferPointer` types: they
would seem to be greatly clarified (in terms of the "what does `at` mean"
question) by the use of a subscript spelling.

This idea sounds elegant on the surface but it’s deeply problematic. `
foo[x...]` suggests that whatever happens to it, will happen to the
entire rest of the buffer slice as well, which is not always the case.

No more so than `foo.copyBytes(at:from:)` suggests the same?

`at:` suggests an unbounded starting point. `x...` suggests a concrete
range of the buffer since it’s really shorthand for `x ... endIndex`.

It would have to be written as `foo[x ... x + count].copyMemory(from:
bar)` or `foo[x ... x + bar.count].copyMemory(from: bar)` which seems
*less* clear. Having to write `foo[0...]` to operate with no offset
also seems silly.

It is unclear to me why one would have to write `foo[0...]`.

How else would you operate at offset 0?

It also means the API would have to be written on `
MutableRandomAccessSlice<UnsafeMutable*BufferPointer>`.

Not necessarily; it's unclear to me that `MutableRandomAccessSlice<UnsafeMutable*BufferPointer>`
is the right slice type for `UnsafeMutable*BufferPointer` in the first
place (see below).

Finally, what happens when you subscript a raw pointer?

As you know, only buffer pointers conform to `Collection`, so it's unclear
to me what your question is here.

I meant a raw *buffer* pointer.

the subscript doesn’t know about the stride that you want to use. If you
want to get rid of the `at:` ambiguity, you have to get rid of it
everywhere, or users will just wind up having to remember two ways (one
ambiguous and one less ambiguous) of doing the same thing, instead of one
(ambiguous) way of doing it.

Certainly, that's a good point. On rethink and another re-reading of the
proposal, it's unclear to me that the addition of `at` arguments to
UnsafeMutablePointer is sufficiently justified by the proposal text. Is it
merely that it's shorter than writing `foo + MemoryLayout<T>.stride *
offset`? With the ambiguity of `at`, it seems that the current way of
writing it, though longer, is certainly less ambiguous.

Please reread it; UnsafeMutablePointer’s methods do *not* use `at:`.

I notice that you comment that you feel there are ergonomic issues with

buffer pointer slicing; can you please comment on what is "wasteful"
currently? Is there a performance hit to slicing a `*BufferPointer` type?
If so, we should fix that, as the whole rationale of having these types (as
I understand it) is to improve the ergonomics of working with pointers to
multiple elements by conforming them to `*Collection` APIs.

Slices are not the right abstraction for this because of Swift’s indexing
semantics.

This is an alarming statement if true, in that it would seem to undermine
the basic premise of `*BufferPointer` types conforming to `Collection`,
would it not?

I think Andy’s message has a good explanation for this.

A buffer pointer requires two words of information (beginning and end
address), while a buffer pointer slice occupies four words of information
(beginning, end, start-index, end-index) to preserve the index semantics.
Creating a way to “slice” a buffer pointer into another buffer pointer
without going through `init(rebasing:)` is also problematic because you
can’t `deallocate()` a debased buffer pointer, so we should make that
operation explicit.

It would seem, then, that to properly support slicing--which collection
types should do--we will need a custom slice type a la `Substring`. This
slice type would clearly not support deallocation, but it would conform to
a protocol (a la `StringProtocol`) which would require all the operations
such as `copyMemory` that makes sense for both buffer pointers and their
slices.

You’re basically proposing Protocol Oriented Pointers. Probably a good
eventual goal, but we need to take things step by step.

···

On Sat, Sep 2, 2017 at 3:39 PM, Xiaodi Wu <xiaodi.wu@gmail.com> wrote:

On Sat, Sep 2, 2017 at 2:13 PM, Taylor Swift <kelvin13ma@gmail.com> wrote:

On Sat, Sep 2, 2017 at 10:03 AM, Xiaodi Wu via swift-evolution < >> swift-evolution@swift.org> wrote:

On Sat, Sep 2, 2017 at 12:27 AM, Douglas Gregor via swift-evolution < >>> swift-evolution@swift.org> wrote:

It seems deeply unsatisfactory to invent new methods that use `at:`

arguments _on a type whose purpose is to expose `*Collection` APIs_ if we
agree that the slicing notation is demonstrably clearer. I do not have the
same concerns about adding `at:` arguments to `UnsafeMutableRawPointer`
methods, which are quite appropriate.

   - Is the problem being addressed significant enough to warrant a
   change to Swift?

Yes.

   - Does this proposal fit well with the feel and direction of Swift?

In parts, yes. In others (see above), it could be improved to fit

better with the feel and direction of other Swift APIs.

   - If you have used other languages or libraries with a similar
   feature, how do you feel that this proposal compares to those?

There is much more friction to using pointers in Swift than in C.

However, making Swift pointers like C pointers is clearly a non-goal. This
proposal appropriate addresses major pain points to Swift-specific usages
of pointers.

   - How much effort did you put into your review? A glance, a quick
   reading, or an in-depth study?

A moderately thorough reading, drawing on some experience using pointer

APIs in Swift, and based on prior readings of previous iterations of this
proposal and the on-list discussion.

More information about the Swift evolution process is available at

https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

-Doug

Review Manager

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

It would remove a mode of "holding it wrong", but the specified behavior will not change; it has and will always fully deallocate the object being referenced in the cases where the call has defined behavior. A sized implementation may fail more obviously when you violate the contract in the future. Not having sized deallocation is a known deficiency of the C model we've been fairly diligent about avoiding in Swift's allocation interfaces, and it would be extremely unfortunate for us to backpedal from it.

-Joe

···

On Sep 6, 2017, at 3:07 PM, Taylor Swift <kelvin13ma@gmail.com> wrote:

On Sep 6, 2017, at 4:31 PM, Joe Groff <jgroff@apple.com> wrote:

On Sep 6, 2017, at 2:28 PM, Andrew Trick <atrick@apple.com> wrote:

On Sep 6, 2017, at 1:12 PM, Joe Groff via swift-evolution <swift-evolution@swift.org> wrote:

On Sep 6, 2017, at 1:06 PM, Taylor Swift <kelvin13ma@gmail.com> wrote:

On Wed, Sep 6, 2017 at 12:41 PM, Joe Groff via swift-evolution <swift-evolution@swift.org> wrote:
Currently, memory is deallocated by an instance method on UnsafeMutablePointer, deallocate(count:). Like much of the Swift pointer API, performing this operation on a buffer pointer requires extracting baseAddress! and count. It is very common for the allocation code above to be immediately followed by:

defer

{
buffer.
baseAddress?.deallocate(capacity: buffer.count
)
}

This method is extremely problematic because nearly all users, on first seeing the signature of deallocate(capacity:), will naturally conclude from the capacity label that deallocate(capacity:) is equivalent to some kind of realloc()that can only shrink the buffer. However this is not the actual behavior — deallocate(capacity:) actually ignores the capacity argument and just calls free() on self. The current API is not only awkward and suboptimal, it is misleading. You can write perfectly legal Swift code that shouldn’t segfault, but still can, for example

var ptr = UnsafeMutablePointer<UInt8>.allocate(capacity: 1000000
)
ptr.
initialize(to: 13, count: 1000000
)
ptr.
deallocate(capacity: 500000) // deallocate the second half of the memory block
ptr[0] // segmentation fault
where the first 500000 addresses should still be valid if the documentation is to be read literally.

The fact that the Swift runtime currently uses malloc/free is an implementation detail. Tracking deallocation size is a very useful optimization for better allocator backends, and C++ underwent an ABI break to make it so that sized delete can be supported. Maybe we can change the name to `deallocate(allocatedCapacity:)` to make it clear that it isn't resizing the memory, and/or make the capacity argument optional so that you can pay for the overhead of the allocator deriving the size if it's inconvenient for the calling code to carry the size around, but we shouldn't remove the functionality altogether.

-Joe
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

The idea is to get the house in order by removing all parameters from deallocate(), since that’s what it really does right now. Then, in the future, if Swift gets a more sophisticated allocator backend, a new method like deallocate(capacity:) or reallocate(toCapacity:) could be added without conflicting with the currently named deallocate(capacity:). However, using the function signature to pretend that it does something it can’t actually do right now is extremely dangerous.

I don't think that's a good idea in this case, because it's not unlikely we would explore an optimized allocator soon after ABI stability, and retrofitting these interfaces in a future version of Swift would put a deployment target limit on when they can be used, and mean that a lot of user code would need to be retrofitted to carry allocated capacities where needed to see any benefit.

-Joe

The fact that we’re using malloc and free is already part of the ABI because old libraries need to be able to deallocate memory allocated by newer libraries.

The compiler doesn't ever generate calls directly to malloc and free, and the runtime entry points we do use already take size and alignment on both allocation and deallocation.

Within the standard library we could make use of some new deallocation fast path in the future without worrying about backward deployment.

Outside of the standard library, clients will get the benefits of whatever allocator is available on their deployed platform because we now encourage them to use UnsafeBufferPointer.deallocate(). We can change the implementation inside UnsafeBufferPointer all we want, as long as it’s still malloc-compatible.

I’m sure we’ll want to provide a better allocation/deallocation API in the future for systems programmers based on move-only types. That will already be deployment-limited.

Absolute worst case, we introduce a sized UnsafePointer.deallocate in the future. Any new code outside the stdlib that wants the performance advantage would either need to
- trivially wrap deallocation using UnsafeBufferPointer
- create a trivial UnsafePointer.deallocate thunk under an availability flag

Since we already have sized deallocate, why would we take it away? If the name is misleading, we can change the name.

-Joe

Exactly, we are changing the name to `deallocate()`. As for the old `deallocate(capacity:)` method that *needs* to be removed because it is actively harmful. As I’ve explained it’s not enough to just drop in a sized backend later as an “implementation detail” because it’s not an implementation detail, you’re changing the fundamental behavior of that method.

No, I agree that being malloc-compatible is a reasonable goal; on Apple platforms, being able to register any custom allocator we come up with as a malloc zone would mean that the platform's existing memory profiling and debugging tools can still work. Even if we have a scheme where the allocator directly reaches into per-thread fixed-sized pools, it seems to me like it'd be hard to make malloc_size impossible to implement, though it might be slow, asking each pool for each thread whether an address is inside it. Strongly encouraging, if not requiring, user code to provide deallocation sizes seems to me like a prerequisite to making that sort of design a net win over plain malloc/free.

-Joe

···

On Sep 6, 2017, at 3:26 PM, Andrew Trick <atrick@apple.com> wrote:

On Sep 6, 2017, at 2:31 PM, Joe Groff <jgroff@apple.com> wrote:

The fact that we’re using malloc and free is already part of the ABI because old libraries need to be able to deallocate memory allocated by newer libraries.

The compiler doesn't ever generate calls directly to malloc and free, and the runtime entry points we do use already take size and alignment on both allocation and deallocation.

True, we’ve never said that UnsafePointer deallocation is compatible with C, and I we don't currently expose malloc_size functionality in any API AFAIK.

Within the standard library we could make use of some new deallocation fast path in the future without worrying about backward deployment.

Outside of the standard library, clients will get the benefits of whatever allocator is available on their deployed platform because we now encourage them to use UnsafeBufferPointer.deallocate(). We can change the implementation inside UnsafeBufferPointer all we want, as long as it’s still malloc-compatible.

I’m sure we’ll want to provide a better allocation/deallocation API in the future for systems programmers based on move-only types. That will already be deployment-limited.

Absolute worst case, we introduce a sized UnsafePointer.deallocate in the future. Any new code outside the stdlib that wants the performance advantage would either need to
- trivially wrap deallocation using UnsafeBufferPointer
- create a trivial UnsafePointer.deallocate thunk under an availability flag

Since we already have sized deallocate, why would we take it away? If the name is misleading, we can change the name.

I don't think leaving it as an optional argument is worthwhile, as I explained above. Deallocation capacity is either required or we drop it completely. If we keep it, then `allocatedCapacity` is the right name.

The reason for taking it away, beside being misleading, is that it exposes another level of unsafety.

My thinking has been that this is not the allocation fast path of the future, and the stdlib itself could track the size of unsafely-allocated blocks if it ever used a different underlying allocator.

Now I realize this isn't really about fast/slow deallocation paths. Removing `capacity` or even making it optional forces all future Swift implementations to provide malloc_size functionality for any piece of memory that is compatible with the Unsafe API.

I'm actually ok with that, because I think it's generally desirable for application memory to reside in either in malloc-compatible blocks or fixed size pools. i.e. I think malloc_size is something the platform needs. However, you seem to think this would be too restrictive in the future. How is this a known problem for C and what's your confidence level it will be a problem for Swift?

Currently, memory is deallocated by an instance method on UnsafeMutablePointer, deallocate(count:). Like much of the Swift pointer API, performing this operation on a buffer pointer requires extracting baseAddress! and count. It is very common for the allocation code above to be immediately followed by:

defer

{
buffer.
baseAddress?.deallocate(capacity: buffer.count
)
}

This method is extremely problematic because nearly all users, on first seeing the signature of deallocate(capacity:), will naturally conclude from the capacity label that deallocate(capacity:) is equivalent to some kind of realloc()that can only shrink the buffer. However this is not the actual behavior — deallocate(capacity:) actually ignores the capacity argument and just calls free() on self. The current API is not only awkward and suboptimal, it is misleading. You can write perfectly legal Swift code that shouldn’t segfault, but still can, for example

var ptr = UnsafeMutablePointer<UInt8>.allocate(capacity: 1000000
)
ptr.
initialize(to: 13, count: 1000000
)
ptr.
deallocate(capacity: 500000) // deallocate the second half of the memory block
ptr[0] // segmentation fault
where the first 500000 addresses should still be valid if the documentation is to be read literally.

The fact that the Swift runtime currently uses malloc/free is an implementation detail. Tracking deallocation size is a very useful optimization for better allocator backends, and C++ underwent an ABI break to make it so that sized delete can be supported. Maybe we can change the name to `deallocate(allocatedCapacity:)` to make it clear that it isn't resizing the memory, and/or make the capacity argument optional so that you can pay for the overhead of the allocator deriving the size if it's inconvenient for the calling code to carry the size around, but we shouldn't remove the functionality altogether.

-Joe
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

The idea is to get the house in order by removing all parameters from deallocate(), since that’s what it really does right now. Then, in the future, if Swift gets a more sophisticated allocator backend, a new method like deallocate(capacity:) or reallocate(toCapacity:) could be added without conflicting with the currently named deallocate(capacity:). However, using the function signature to pretend that it does something it can’t actually do right now is extremely dangerous.

I don't think that's a good idea in this case, because it's not unlikely we would explore an optimized allocator soon after ABI stability, and retrofitting these interfaces in a future version of Swift would put a deployment target limit on when they can be used, and mean that a lot of user code would need to be retrofitted to carry allocated capacities where needed to see any benefit.

-Joe

The fact that we’re using malloc and free is already part of the ABI because old libraries need to be able to deallocate memory allocated by newer libraries.

The compiler doesn't ever generate calls directly to malloc and free, and the runtime entry points we do use already take size and alignment on both allocation and deallocation.

Within the standard library we could make use of some new deallocation fast path in the future without worrying about backward deployment.

Outside of the standard library, clients will get the benefits of whatever allocator is available on their deployed platform because we now encourage them to use UnsafeBufferPointer.deallocate(). We can change the implementation inside UnsafeBufferPointer all we want, as long as it’s still malloc-compatible.

I’m sure we’ll want to provide a better allocation/deallocation API in the future for systems programmers based on move-only types. That will already be deployment-limited.

Absolute worst case, we introduce a sized UnsafePointer.deallocate in the future. Any new code outside the stdlib that wants the performance advantage would either need to
- trivially wrap deallocation using UnsafeBufferPointer
- create a trivial UnsafePointer.deallocate thunk under an availability flag

Since we already have sized deallocate, why would we take it away? If the name is misleading, we can change the name.

-Joe

Exactly, we are changing the name to `deallocate()`. As for the old `deallocate(capacity:)` method that *needs* to be removed because it is actively harmful. As I’ve explained it’s not enough to just drop in a sized backend later as an “implementation detail” because it’s not an implementation detail, you’re changing the fundamental behavior of that method.

It would remove a mode of "holding it wrong", but the specified behavior will not change; it has and will always fully deallocate the object being referenced in the cases where the call has defined behavior.

idk what you mean by it holding it wrong, but while the specified behavior won’t change the actual behavior *will* change, because right now the two don’t agree. We can’t just treat it as a bug to fix because it’s been around since the beginning of Swift and some people treat it as part of expected behavior, using the function like `free()`. This isn’t using it incorrectly, in fact in terms of behavior, passing a meaningful capacity argument to `deallocate(capacity:)` is *incorrect usage*, as demonstrated in the segfaulting example in the proposal.

“fixing” this bug will cause programs that once operated on previously valid assumptions of “free()” semantics to behave differently, without any warnings ever being generated. Conversely incorrect code will suddenly become “correct” though this is less of a problem.

A sized implementation may fail more obviously when you violate the contract in the future. Not having sized deallocation is a known deficiency of the C model we've been fairly diligent about avoiding in Swift's allocation interfaces, and it would be extremely unfortunate for us to backpedal from it.

-Joe

Which is worse, an active gaping hole in Swift’s memory system, or potentially discouraging users from using a hypothetical future allocation API?

Making sure the existing allocation API is working properly is a prerequisite to introducing a future more advanced allocation API.

···

On Sep 6, 2017, at 5:12 PM, Joe Groff <jgroff@apple.com> wrote:

On Sep 6, 2017, at 3:07 PM, Taylor Swift <kelvin13ma@gmail.com> wrote:

On Sep 6, 2017, at 4:31 PM, Joe Groff <jgroff@apple.com> wrote:

On Sep 6, 2017, at 2:28 PM, Andrew Trick <atrick@apple.com> wrote:

On Sep 6, 2017, at 1:12 PM, Joe Groff via swift-evolution <swift-evolution@swift.org> wrote:
On Sep 6, 2017, at 1:06 PM, Taylor Swift <kelvin13ma@gmail.com> wrote:

On Wed, Sep 6, 2017 at 12:41 PM, Joe Groff via swift-evolution <swift-evolution@swift.org> wrote:

Regarding the typed buffer pointers, I think it is clear by convention, and will be well documented, that the `at` label refers to a position in `self`.

The raw buffer pointer API isn’t so obvious. Since the `at` refers to `self` it might more logically be a byte offset. Note that `at` as a label name always refers to a strided index.

This would be a bit more explicit:
UnsafeRawBufferPointer.initializeMemory(as:T.self, atByteOffset: position * MemoryLayout<T>.stride, from: bufferOfT)

But possibly less convenient… Since that `at` label currently on UnsafeRawPointer.initializeMemory is almost never used, I don’t think we need to worry too much about convenience.

That existing `at` label on UnsafeRawPointer.initializeMemory, would also need to be renamed, which is fine.

-Andy

···

On Sep 2, 2017, at 2:06 PM, Taylor Swift via swift-evolution <swift-evolution@swift.org> wrote:

the subscript doesn’t know about the stride that you want to use. If you want to get rid of the `at:` ambiguity, you have to get rid of it everywhere, or users will just wind up having to remember two ways (one ambiguous and one less ambiguous) of doing the same thing, instead of one (ambiguous) way of doing it.

Certainly, that's a good point. On rethink and another re-reading of the proposal, it's unclear to me that the addition of `at` arguments to UnsafeMutablePointer is sufficiently justified by the proposal text. Is it merely that it's shorter than writing `foo + MemoryLayout<T>.stride * offset`? With the ambiguity of `at`, it seems that the current way of writing it, though longer, is certainly less ambiguous.

Please reread it; UnsafeMutablePointer’s methods do not use `at:`.

Hello Swift community,

The review of SE-0184 "Unsafe[Mutable][Raw][Buffer]Pointer: add
missing methods, adjust existing labels for clarity, and remove
deallocation size" begins now and runs through September 7, 2017. The
proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposa
ls/0184-unsafe-pointers-add-missing.md

Reviews are an important part of the Swift evolution process. All
reviews should be sent to the swift-evolution mailing list at

https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the
review manager. When replying, please try to keep the proposal link at the
top of the message:

Proposal link:

https://github.com/apple/swift-evolution/blob/master/proposa
ls/0184-unsafe-pointers-add-missing.md

Reply text

Other replies

<https://github.com/apple/swift-evolution/pulls#what-goes-into-a-review-1>What
goes into a review?

The goal of the review process is to improve the proposal under review
through constructive criticism and, eventually, determine the direction of
Swift. When writing your review, here are some questions you might want to
answer in your review:

   - What is your evaluation of the proposal?

Overall, this is an improvement. However, I do have some questions and

concerns:

Questions:

## UnsafeMutableRawPointer

Regarding the options given for "whose `count` to use"--which option is
actually being proposed?

I don’t understand the question, `UnsafeMutableRawPointer` takes an
explicit `count:`. the “whose count to use” option is irrelevant.

In "Proposed Solution," under subheading "UnsafeMutableRawPointer," you
write "the question of whose `count` to use becomes important." You then
outline "[o]ne option" as well as "[a] better option." Which of these
options are you actually proposing? For clarity, could you please excise
the non-proposed option from the "Proposed Solution" section and move it to
the "Alternatives Considered" section?

The *Proposed solution* section is divided into two parts, one dealing
with plain pointers and one dealing with buffer pointers. The sections are
separated by the horizontal rule above “*Buffer pointers are conceptually
similar…*”. I don’t know why we’re arguing over typographic formatting
but I am glad this proposal is noncontroversial enough to induce debate
over its horizontal rules. As for the two options, the first is a strawman
to explain why we are going with the second option.

I am not arguing over formatting. I'm only asking for clarification as to
which of two options, listed in contiguous paragraphs without any
intervening or surrounding horizontal rules, is actually being proposed. Am
I correct in understanding that you are proposing that there be a
precondition that `offset + source.count <= destination.count`? I do agree
that this is the superior design. In that case, the text could use some
clarification as to which is the strawman.

## UnsafeMutableBufferPointer

Please clarify: why are you proposing that the `at:` arguments in
`UnsafeMutableBufferPointer` and `UnsafeMutableRawBufferPointer`
_should not_ receive default values, but the `at:` arguments in
`UnsafeMutableRawPointer` _should_ receive a default value of `0`?

The extant API for `UnsafeMutableRawPointer` already included these
default arguments which seem to be widely used in the stdlib, the proposal
tries to avoid these kinds of source breakages wherever possible. We avoid
providing the default arguments on buffer pointers because we want the fact
that it takes a *start–length* segment pair to be obvious at the call
site.

Thanks for the clarification; that would be helpful information to put
into the proposal text. It is not an intuitive start-length pair, since the
`at` refers to an offset of the destination buffer but `count` refers to a
length of the source buffer. I appreciate how you separated the proposed
new argument `at` and the existing argument `count` in what is currently
named `initializeMemory<T>(as:from:count:)`, which helps to reinforce
that fact.

Concerns:

## UnsafeMutablePointer

It's alarming that omitting `count` in `initialize(repeating:count:)`
(and assign, etc.) means initialize _one_ element, but elsewhere (such as
`UnsafeMutableBufferPointer` means initialize _all_ elements. The behavior
of the proposed API also contradicts its own spelling on its face:
`initialize(repeating: foo)` means *do not repeat* `foo`.

Yes, I understand the argument that `*BufferPointer` types have an
intrinsic count, etc., but in the context of code where types are inferred,
`let foo = T.allocate(capacity: 100); foo.initialize(repeating: bar)`
should not mean one thing for `*BufferPointer` types and a totally
different thing for plain `*Pointer` types--particularly when both can be
allocated with a certain capacity greater than one.

Either `count` should always be required, or for convenience there
should be a separate overload `initialize(pointee:)` that does not require
`count`.

I understand the naming is not optimal, but reams of discussion on this
list have concluded that it’s the least bad alternative available. We can’t
just get rid of the default value for `count:` because usage in real
code bases shows that this default argument is actually extremely useful. I
believe well over 90% of the calls to these methods in the standard library
currently rely on the default argument value. Renaming the `repeating:`
argument to `to:` would make the API inconsistent and hide the fact that
plain pointers are still capable of operating on many elements in sequence
— “`to:count:`” makes no grammatical sense to read — “to” is a singular
preposition.

Let me clarify my concern here. This is not intended to be a bikeshedding
exercise and I agree with your choice of `repeating` (as I did in the
original conversations). For the sake of clarity, however, I'll proceed
with this discussion as though the argument were named `xxxx`. My point
here is that, regardless of what `xxxx` is called, we have a problem here
as follows:

let foo = UnsafeMutablePointer<Int>.allocate(capacity: 21)
foo.initialize(xxxx: 42) // initializes 1 value

let bar = UnsafeMutableBufferPointer<Int>.allocate(capacity: 21)
bar.initialize(xxxx: 42) // initializes 21 values

The same spelling, `initialize(xxxx:)`, does two *different* things under
your proposal depending on whether it's invoked on a UMP or a UMBP. Even
though it's admirable that your proposal is filling in the gaps of Swift's
pointer design and increasing consistency by making similar things have
similar names, different things need to have different names; otherwise, we
are actively creating footguns.

Therefore, while I agree with your choice for `xxxx`, and while I also
agree that it is very useful to have a method that initializes a single
value on a UMP, we need to have a different label `yyyy` for that method.
My suggestion is `pointee`, but I would be equally happy with `value`,
`to`, or whatever else you may choose.

yes this is a problem, but your solution is to add a second set of methods
that are functionally identical, except for the names of the argument
labels. This strikes me as silly and wasteful of API surface.

Disagree strongly. If the default value of 1 is important enough to merit
inclusion and even important enough that you are willing to risk the
problem of causing an identical spelling to do two different things, then
it certainly merits its own function. That is an appropriate and necessary
increase in the API surface and not at all wasteful.

## UnsafeMutableRawBufferPointer

In `copyBytes`, the use of `Bytes` to emphasize that it's the memory
that's being copied is thoughtful, but it is inconsistent with the other
method names that use the terminology `Memory` for the same purpose (e.g.,
`moveInitializeMemory` to clarify the meaning of `moveInitialize`).

For better consistency--and since you're proposing to rename
`copyBytes(from:count:)` on `UnsafeMutableRawPointer` to
`copy(from:bytes:)`--this particular API on `UnsafeMutableRawBufferPointer`
should be named `copyMemory(from:)` and not `copyBytes(from:)`.

Although, actually, looking at the APIs on `UnsafeMutableRawPointer`
itself, that particular method too might best be written as
`copyMemory(from:bytes:)` instead of merely `copy(from:bytes:)` for better
consistency with the rest of the methods on that type as well.

“Memory” methods are distinct from “Bytes” methods in that they assume
typed memory. “Bytes” on the other hand does not care about types.

It's unclear to me that this distinction is either consistently observed
or helpful. For instance, by that standard, `initializeMemory(as:from:)`
should be named `initializeBytes(as:fromMemory:)`, since the memory
being initialized is raw until after initialization. This strikes me as not
at all necessary for user comprehension of the APIs. (On the other hand, if
it _is_ necessary for comprehension, then `copyBytes` should never be
shortened to `copy`, since it would be necessary to emphasize that both the
source and destination are treated as "bytes" rather than "memory.")

`copyMemory` would probably be better and I am not opposed to adding it to
the proposal. at the same time I don’t think it’s so bad a problem that it
really *needs* to be renamed.

## General comment

Many `at:` arguments, especially such as in the case of
`copyBytes(at:from:)`, make sense only when read in a list with all other
methods. Standing alone, `at` is ambiguous as to whether it's referring to
the _source_ or the _destination_. Why do these APIs on `*BufferPointer`
types not take advantage of subscripts? That is, why not:

  `foo[x...].copyMemory(from: bar)`

instead of

  `foo.copyBytes(at: x, from: bar)`

The first seems dramatically clearer as to its meaning. The same
feedback applies to nearly all uses of `at` on `*BufferPointer` types: they
would seem to be greatly clarified (in terms of the "what does `at` mean"
question) by the use of a subscript spelling.

This idea sounds elegant on the surface but it’s deeply problematic. `
foo[x...]` suggests that whatever happens to it, will happen to the
entire rest of the buffer slice as well, which is not always the case.

No more so than `foo.copyBytes(at:from:)` suggests the same?

`at:` suggests an unbounded starting point. `x...` suggests a concrete
range of the buffer since it’s really shorthand for `x ... endIndex`.

It would have to be written as `foo[x ... x + count].copyMemory(from:
bar)` or `foo[x ... x + bar.count].copyMemory(from: bar)` which seems
*less* clear. Having to write `foo[0...]` to operate with no offset
also seems silly.

It is unclear to me why one would have to write `foo[0...]`.

How else would you operate at offset 0?

As I wrote earlier, `copyMemory` would be available on a protocol (a la
`StringProtocol`) to which both buffer pointers and their slices would
conform.

It also means the API would have to be written on `
MutableRandomAccessSlice<UnsafeMutable*BufferPointer>`.

Not necessarily; it's unclear to me that `MutableRandomAccessSlice<UnsafeMutable*BufferPointer>`
is the right slice type for `UnsafeMutable*BufferPointer` in the first
place (see below).

Finally, what happens when you subscript a raw pointer?

As you know, only buffer pointers conform to `Collection`, so it's
unclear to me what your question is here.

I meant a raw *buffer* pointer.

Those would be, just as they are now, subscripted by bytes. Although of
course, with generic subscripts, we could permit additional conveniences by
specifying, as a second parameter, the type whose stride is to be used
(strawman syntax: `umrbp[42, stride: Int.self]`).

the subscript doesn’t know about the stride that you want to use. If you

want to get rid of the `at:` ambiguity, you have to get rid of it
everywhere, or users will just wind up having to remember two ways (one
ambiguous and one less ambiguous) of doing the same thing, instead of one
(ambiguous) way of doing it.

Certainly, that's a good point. On rethink and another re-reading of the
proposal, it's unclear to me that the addition of `at` arguments to
UnsafeMutablePointer is sufficiently justified by the proposal text. Is it
merely that it's shorter than writing `foo + MemoryLayout<T>.stride *
offset`? With the ambiguity of `at`, it seems that the current way of
writing it, though longer, is certainly less ambiguous.

Please reread it; UnsafeMutablePointer’s methods do *not* use `at:`.

Apologies; I meant UnsafeMutableRawPointer (as opposed to buffer pointers).

I notice that you comment that you feel there are ergonomic issues with

buffer pointer slicing; can you please comment on what is "wasteful"
currently? Is there a performance hit to slicing a `*BufferPointer` type?
If so, we should fix that, as the whole rationale of having these types (as
I understand it) is to improve the ergonomics of working with pointers to
multiple elements by conforming them to `*Collection` APIs.

Slices are not the right abstraction for this because of Swift’s
indexing semantics.

This is an alarming statement if true, in that it would seem to undermine
the basic premise of `*BufferPointer` types conforming to `Collection`,
would it not?

I think Andy’s message has a good explanation for this.

Agreed. My takeaway from that message is that, indeed, `*BufferPointer`
types do not currently implement all the functionality necessary to
interact with them as bona fide collections in all respects, despite their
stated conformance. This is indeed problematic; I hope that it is agreed
that, at some later point, they either *ought to*, given the stated
conformance, or they ought to drop the conformance.

A buffer pointer requires two words of information (beginning and end

address), while a buffer pointer slice occupies four words of information
(beginning, end, start-index, end-index) to preserve the index semantics.
Creating a way to “slice” a buffer pointer into another buffer pointer
without going through `init(rebasing:)` is also problematic because you
can’t `deallocate()` a debased buffer pointer, so we should make that
operation explicit.

It would seem, then, that to properly support slicing--which collection
types should do--we will need a custom slice type a la `Substring`. This
slice type would clearly not support deallocation, but it would conform to
a protocol (a la `StringProtocol`) which would require all the operations
such as `copyMemory` that makes sense for both buffer pointers and their
slices.

You’re basically proposing Protocol Oriented Pointers. Probably a good
eventual goal, but we need to take things step by step.

I like the sound of it, and I'm fine with this being put off as an eventual
goal. My concern is that the `at:` arguments may not be a step forward or
even orthogonal to the eventual goal, in that there seems to be (at least,
to me) added rather than reduced ambiguity in APIs that use this argument.

The reasoning is in part, as I've outlined in other messages, the
inconsistent meaning of the `at:` argument (for example, `at: x` on methods
in UMBP<T> would correspond to `umbp[x]`, but `at: y` on methods in UMRBP
would *not* correspond to `umrbp[y]`), particularly given that the chief
ambiguity of these terms (`at`, `count`, etc.), has to do with the question
of "which one--source index/count or destination index/count?". This is
especially the case if, as Andy says, the eventual goal is to create slice
types so that subscripting becomes the preferred idiom.

···

On Sat, Sep 2, 2017 at 4:06 PM, Taylor Swift <kelvin13ma@gmail.com> wrote:

On Sat, Sep 2, 2017 at 3:39 PM, Xiaodi Wu <xiaodi.wu@gmail.com> wrote:

On Sat, Sep 2, 2017 at 2:13 PM, Taylor Swift <kelvin13ma@gmail.com> >> wrote:

On Sat, Sep 2, 2017 at 10:03 AM, Xiaodi Wu via swift-evolution < >>> swift-evolution@swift.org> wrote:

On Sat, Sep 2, 2017 at 12:27 AM, Douglas Gregor via swift-evolution < >>>> swift-evolution@swift.org> wrote:

It seems deeply unsatisfactory to invent new methods that use `at:`

arguments _on a type whose purpose is to expose `*Collection` APIs_ if we
agree that the slicing notation is demonstrably clearer. I do not have the
same concerns about adding `at:` arguments to `UnsafeMutableRawPointer`
methods, which are quite appropriate.

   - Is the problem being addressed significant enough to warrant a
   change to Swift?

Yes.

   - Does this proposal fit well with the feel and direction of Swift?

In parts, yes. In others (see above), it could be improved to fit

better with the feel and direction of other Swift APIs.

   - If you have used other languages or libraries with a similar
   feature, how do you feel that this proposal compares to those?

There is much more friction to using pointers in Swift than in C.

However, making Swift pointers like C pointers is clearly a non-goal. This
proposal appropriate addresses major pain points to Swift-specific usages
of pointers.

   - How much effort did you put into your review? A glance, a quick
   reading, or an in-depth study?

A moderately thorough reading, drawing on some experience using

pointer APIs in Swift, and based on prior readings of previous iterations
of this proposal and the on-list discussion.

More information about the Swift evolution process is available at

https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

-Doug

Review Manager

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution