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

[Xiaodi Wu]

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

> 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.


> 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 at swift.org
> https://lists.swift.org/mailman/listinfo/swift-evolution
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-evolution/attachments/20170902/e55c6e81/attachment.html>