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

Sat Sep 2 14:13:46 CDT 2017

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

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

>
> 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
>>
>>
>
> _______________________________________________
> 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/0b51caaf/attachment.html>