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

[Taylor Swift]

On Sat, Sep 2, 2017 at 7:53 PM, Andrew Trick <atrick at apple.com> wrote:

>
> On Sep 2, 2017, at 5:34 PM, Xiaodi Wu <xiaodi.wu at gmail.com> wrote:
>
> On Sat, Sep 2, 2017 at 4:41 PM, Andrew Trick <atrick at apple.com> wrote:
>
>>
>> On Sep 2, 2017, at 2:06 PM, Taylor Swift via swift-evolution <
>> swift-evolution at 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:`.
>>
>>
>> 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.
>>
>
> If I may suggest one more incremental improvement in clarity, it would be
> to move `at[ByteOffset]` to be the first argument; this eliminates the
> possible reading that we are offsetting the source buffer:
>
> ```
> UnsafeRawBufferPointer.initializeMemory(atByteOffset: position *
> MemoryLayout<T>.stride, as:T.self, from: bufferOfT)
> ```
>
>
> Sure, that probably makes sense if we decide to go with a byte offset vs.
> stride index.
> -Andy
>

If we use byte offset, then the at parameter in UnsafeMutableRawPointer
should be removed, since pointer arithmetic can be used instead (just like
with UnsafeMutablePointer). Not convinced moving the at: argument to come
before the as: argument is worth it in terms of source breakage.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-evolution/attachments/20170903/eb2f2ce4/attachment.html>