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

[Andrew Trick via swift-evolution]

> On Sep 30, 2017, at 10:45 AM, Taylor Swift  wrote:
> 
> this function initializeMemory(as:from:) says it will be 
> removed in Swift 4.0. It is now Swift 4.0. can I remove it?

It looks safe to remove. However, the doc comments in the same file should be 
updated to refer to `initializeMemory(as:from:count:)`.

+cc Nate Cook

-Andy

> On Sat, Sep 30, 2017 at 2:15 AM, Taylor Swift  > wrote:
> 
> 
> On Thu, Sep 28, 2017 at 7:59 PM, Andrew Trick  > wrote:
> 
>> On Sep 6, 2017, at 10:15 PM, Taylor Swift > > wrote:
>> 
>> okay so I think so far what’s been agreed on is 
>> 
>> 1. rename “Bytes” to “Memory” in the raw pointer API. Note: this brings the 
>> `copyBytes(from:)` collection method into the scope of this proposal
>> 
>> 2. change raw offsets to be in terms of bytes, not typed strides. This 
>> argument will be called `atByteOffset:` and will only appear in 
>> UnsafeMutableRawBufferPointer. “at:” arguments are no longer needed in 
>> UnsafeMutableRawPointer, since we can just use pointer arithmetic now.
>> 
>> 
>> 3. move UnsafeMutableBufferPointer’s `at:` arguments to the front of the 
>> parameter list. mostly cause any pointer arithmetic happens in the front so 
>> structurally we want to mirror that.
>> 
>> 4. add dual (to:) single element initializers and assigners to 
>> UnsafeMutablePointer, but not UnsafeMutableRawPointer because it’s 
>> apparently not useful there. 
>> `UnsafeMutableRawPointer.initializeMemory(as:repeating:count:)` still 
>> loses its default count to prevent confusion with its buffer variant.
>> 
>> 5. memory deallocation on buffer pointers is clearly documented to only be 
>> defined behavior when the buffer matches a whole heap block. 
> 
> 
> Kelvin,
> 
> Attempting to limit the scope of this proposal backfired. I was hoping to 
> avoid discussing changes to the slice API, instead providing basic 
> functionality within the buffer API itself. However, Dave Abrahams has argued 
> that once the slice API is extended, the positional arguments are extraneous 
> and less clear.
> 
> Instead of
> 
>   buf.intialize(at: i, from: source)
> 
> We want to force a more obvious idiom:
> 
>   buf[i.. 
> I think this is a reasonable argument and convinced myself that it's possible 
> to extend the slice API. I'm also convinced now that we don't need overloads 
> to handle an UnsafeBufferPointer source, instead we can provide a single 
> generic entry point on both UnsafeMutableBufferPointer and its slice, 
> optimized within the implementation:
> 
>  `initialize(from: S) -> (S.Iterator, Index)
> 
> We can always drop down to the UnsafePointer API to get back to a direct 
> unsafe implementation as a temporary workaround for performance issues.
> 
> Let's set aside for now whether we support full or partial 
> initialization/assignment, how to handle moveInitialize, and whether we need 
> to return the Iterator/Index. This is going to require another iteration on 
> swift-evolution, which we should discuss in a separate thread. 
> 
> At this point, I suggest removing the controversial aspects of SE-0184 so 
> that we can put the other changes behind us and focus future discussion 
> around a smaller follow-up proposal.
> 
> Here I've summarized the changes that I think could be accepted as-is:
> https://gist.github.com/atrick/c1ed7afb598e5cc943bdac7683914e3e 
> 
> 
> If you amend SE-0184 accordingly and file a new PR, I think it can be quickly 
> approved.
> 
> -Andy
> 
> 
> Part one of SE-0184 is here as SE-0184 A 
> 
>  
> I’ll implement it tomorrow and then make the PR
> 

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

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

[Taylor Swift via swift-evolution]

On Thu, Sep 7, 2017 at 1:33 PM, Jordan Rose  wrote:

>
>
> On Sep 5, 2017, at 14:50, Andrew Trick via swift-evolution <
> swift-evolution@swift.org> wrote:
>
> We don't have a consensus, but I think the suggestion to distinguish
> between single value vs. multiple semantics was good. Otherwise,
> adding the default count could be very misleading. Normally, we try to
> minimize surface area, but adding two methods for the single-value case
> avoids ambiguity between the buffer and pointer semantics:
>
> UMP (pointer)
> --- func initialize(to:count:(=1))
> +++ func initialize(to:)
> +++ func initialize(repeating:count:) // no default count
> +++ func assign(to:)
> +++ func assign(repeating:count:) // no default count
>
> UMRP (raw pointer):
> --- func initializeMemory(as:at:(=0)count:(1)to:)
> +++ func initializeMemory(as:repeating:count:) // remove default count
>
>
> I am *mostly* in favor of this two-method approach, but 'to' may not be
> the right label. Today we have both initialize(to:…) and
> initialize(from:…), which are not opposites. I think we can live with that,
> but we *definitely* can't use assign(to:). "x.assign(to: y)" means some
> form of "y = x".
>
> That said, we don't actually *need* a single-element 'assign' variant,
> because you can also write it "x.pointee = y". But I agree that symmetry is
> nice, if we can get it.
>

assign(to:) was never part of the original proposal, now that you mention
it, it’s stupid and should be left out
___
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

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

[Andrew Trick via swift-evolution]

> On Sep 5, 2017, at 6:28 PM, Taylor Swift  wrote:
> 
>> 
>> UMRP (raw pointer):
>> --- func initializeMemory(as:at:(=0)count:(1)to:)
>> +++ func initializeMemory(as:repeating:count:) // remove default count 
>> 
>> still extremely suspicious of this but i’m willing to compromise. also there 
>> should be an `initializeMemory(at:to:)` to match the typed methods
>> 
>> Do you mean initializeMemory(as:to:)?
> 
> I don’t think it’s necessary since raw buffers are not normally used to hold 
> a single typed element. We don’t need symmetry between raw and typed pointers 
> and I wouldn’t want to add an API that will never be used. So, I would be ok 
> with it only if there’s some use case that I’m overlooking.
> 
> The only case i can think of is initializing a buffer header but then again i 
> rarely use raw pointers so i’m not sure how much this feature would be used. 
> However, this function is already 
> 
>  in the API (with a default count of 1 and offset of 0) so there’s that.

Yeah, but you’re deprecating the old API entry point anyway. The `at` argument 
was never used AFAICT. I doubt code is using the default `count` and it seems 
like a fixit could handle it.

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

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

[Taylor Swift via swift-evolution]

On Tue, Sep 5, 2017 at 5:36 PM, Andrew Trick  wrote:

>
>>> In the new raw initializeMemory methods, Xiaodi and I agreed to make
>>> it more clear that the offset is in terms of `self` rather than
>>> `from`, and to further reduce ambiguity by forcing manual stride
>>> computation and using an explicit "offset" label. The call site will
>>> be just as explicit as dropping down to `baseAddress` but without any
>>> pointer conversion boilerplate.
>>>
>>> UMRBP (raw buffer):
>>> +++ func initializeMemory(atByteOffset:as:from:)
>>> +++ func moveInitializeMemory(atByteOffset:as:from:)
>>>
>>>
>> Agree, but the label should just be `atByte:`, not `atByteOffset:`.
>> after all, we don’t use `atOffset:` in the strided case; its obvious
>> that it’s an offset
>>
>
> The existing APIs use the terminology "byte offset"--for example,
> URP.load(fromByteOffset:as:). The rationale is that "at" without a noun
> that follows implies, per Swift API naming guidelines, "at index." If you
> want to specify, as we do here, what the index _is_, then it's written out
> in full.
>
>
> Yes, it seems overly cumbersome, but I was following existing conventions
> which were intentionally very explicit.
>

it’s visually cumbersome because the “atByteOffset” overwhelms the other
two arguments but I guess i could live with it.


>
>>> We don't have a consensus, but I think the suggestion to distinguish
>>> between single value vs. multiple semantics was good. Otherwise,
>>> adding the default count could be very misleading. Normally, we try to
>>> minimize surface area, but adding two methods for the single-value case
>>> avoids ambiguity between the buffer and pointer semantics:
>>>
>>> UMP (pointer)
>>> --- func initialize(to:count:(=1))
>>> +++ func initialize(to:)
>>> +++ func initialize(repeating:count:) // no default count
>>> +++ func assign(to:)
>>> +++ func assign(repeating:count:) // no default count
>>>
>>> UMRP (raw pointer):
>>> --- func initializeMemory(as:at:(=0)count:(1)to:)
>>> +++ func initializeMemory(as:repeating:count:) // remove default
>>> count
>>>
>>
>> still extremely suspicious of this but i’m willing to compromise. also
>> there should be an `initializeMemory(at:to:)` to match the typed
>> methods
>>
>
> Do you mean initializeMemory(as:to:)?
>
>
> I don’t think it’s necessary since raw buffers are not normally used to
> hold a single typed element. We don’t need symmetry between raw and typed
> pointers and I wouldn’t want to add an API that will never be used. So, I
> would be ok with it only if there’s some use case that I’m overlooking.
>

The only case i can think of is initializing a buffer header but then again
i rarely use raw pointers so i’m not sure how much this feature would be
used. However, this function is already

in the API (with a default count of 1 and offset of 0) so there’s that.


>
> -Andy
>
>
>>> On Tue, Sep 5, 2017 at 11:31 AM, Andrew Trick  wrote:
>>>
 I think we’ve agreed to a few minor updates to this proposal. Since
 there hasn’t been any other feedback on the thread it may be worth posting
 an amended proposal so we all know what we’ve agreed on.

 -Andy

 On Sep 3, 2017, at 8:23 PM, Andrew Trick via swift-evolution <
 swift-evolution@swift.org> wrote:


 On Sep 3, 2017, at 8:05 PM, Xiaodi Wu  wrote:

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

 I agree that it seems quite sensible to remove the ‘at’ parameter
 altogether from the UMRP method.


 No code in tree or on github is using the `at` argument. I think it can
 be removed. A fixit should still be possible.

 Not convinced moving the at: argument to come before the as: argument
> is worth it in terms of source breakage.
>

 Since much of this proposal involves shuffling and relabeling
 arguments, I’d argue it’s better to break slight more source in one go for
 the optimal API than to break slightly less for a slightly less optimal
 API, no? (This is assuming there is agreement that ‘at:as:’ is less prone
 to misinterpretation than ‘as:at:’.)


 To be clear, we’re just talking about 
 UnsafeMutableRawBufferPointer.initializeMemory
 now, so this is purely additive.
 I think the label needs to be `atByteOffset`, and placing it before
 `as` makes a lot of sense because it no longer depends on the type’s
 stride.

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


>
___

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

[Taylor Swift via swift-evolution]

I’m fine with most of this

On Tue, Sep 5, 2017 at 4:49 PM, Andrew Trick  wrote:

>
> On Sep 5, 2017, at 9:53 AM, Taylor Swift  wrote:
>
> we agreed to deprecate the strided at:? Note that
> UnsafeMutableRawBufferPointer would still need a byteOffset: argument.
> I’m also still not comfortable with duplicating functionality for the sake
> of having two names
>
>
> I’ll summarize what I think happened in this thread...
>
> I don't think the suggested copyBytes rename was controversial:
>
> UMRP (raw pointer):
> --- copyBytes(from:count)
> +++ copyMemory(from:bytes:)
>
>
> UMRBP (raw buffer):
> --- copyBytes(from:)
> +++ copyMemory(atByteOffset:from:)
>
> --- copyBytes(from:)
> +++ copyMemory(from:)
>
>
no problem with this


>
> In the new raw initializeMemory methods, Xiaodi and I agreed to make
> it more clear that the offset is in terms of `self` rather than
> `from`, and to further reduce ambiguity by forcing manual stride
> computation and using an explicit "offset" label. The call site will
> be just as explicit as dropping down to `baseAddress` but without any
> pointer conversion boilerplate.
>
> UMRBP (raw buffer):
> +++ func initializeMemory(atByteOffset:as:from:)
> +++ func moveInitializeMemory(atByteOffset:as:from:)
>
>
Agree, but the label should just be `atByte:`, not `atByteOffset:`. after
all, we don’t use `atOffset:` in the strided case; its obvious that it’s an
offset


>
> Than, in the one existing initializeMemory method, just drop the strided
> index.
> It's never used, we want to be consistent in using a byte offset
> for the raw index, and that can be expressed just as easily as pointer
> arithmetic:
>
> UMRP (raw pointer):
> --- func initializeMemory(as:T.Type, at:Int = 0, count:Int = 1, to:T)
> +++ func initializeMemory(as:T.Type, repeating:T, count:Int = 1)
>
> Then you can simply leave these methods as-is:
> > func initializeMemory(as:T.Type, from:UnsafePointer, count:Int)
> > func moveInitializeMemory(as:T.Type, from:UnsafeMutablePointer,
> count:Int)
>
>
yes


> We don't have a consensus, but I think the suggestion to distinguish
> between single value vs. multiple semantics was good. Otherwise,
> adding the default count could be very misleading. Normally, we try to
> minimize surface area, but adding two methods for the single-value case
> avoids ambiguity between the buffer and pointer semantics:
>
> UMP (pointer)
> --- func initialize(to:count:(=1))
> +++ func initialize(to:)
> +++ func initialize(repeating:count:) // no default count
> +++ func assign(to:)
> +++ func assign(repeating:count:) // no default count
>
> UMRP (raw pointer):
> --- func initializeMemory(as:at:(=0)count:(1)to:)
> +++ func initializeMemory(as:repeating:count:) // remove default count
>

still extremely suspicious of this but i’m willing to compromise. also
there should be an `initializeMemory(at:to:)` to match the typed methods


>
> -Andy
>
> On Tue, Sep 5, 2017 at 11:31 AM, Andrew Trick  wrote:
>
>> I think we’ve agreed to a few minor updates to this proposal. Since there
>> hasn’t been any other feedback on the thread it may be worth posting an
>> amended proposal so we all know what we’ve agreed on.
>>
>> -Andy
>>
>> On Sep 3, 2017, at 8:23 PM, Andrew Trick via swift-evolution <
>> swift-evolution@swift.org> wrote:
>>
>>
>> On Sep 3, 2017, at 8:05 PM, Xiaodi Wu  wrote:
>>
>> 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).
>>>
>>
>> I agree that it seems quite sensible to remove the ‘at’ parameter
>> altogether from the UMRP method.
>>
>>
>> No code in tree or on github is using the `at` argument. I think it can
>> be removed. A fixit should still be possible.
>>
>> Not convinced moving the at: argument to come before the as: argument is
>>> worth it in terms of source breakage.
>>>
>>
>> Since much of this proposal involves shuffling and relabeling arguments,
>> I’d argue it’s better to break slight more source in one go for the optimal
>> API than to break slightly less for a slightly less optimal API, no? (This
>> is assuming there is agreement that ‘at:as:’ is less prone to
>> misinterpretation than ‘as:at:’.)
>>
>>
>> To be clear, we’re just talking about 
>> UnsafeMutableRawBufferPointer.initializeMemory
>> now, so this is purely additive.
>> I think the label needs to be `atByteOffset`, and placing it before `as`
>> makes a lot of sense because it no longer depends on the type’s stride.
>>
>> -Andy
>> ___
>> 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

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

[Andrew Trick via swift-evolution]

> On Sep 5, 2017, at 9:53 AM, Taylor Swift  wrote:
> 
> we agreed to deprecate the strided at:? Note that 
> UnsafeMutableRawBufferPointer would still need a byteOffset: argument. I’m 
> also still not comfortable with duplicating functionality for the sake of 
> having two names

I’ll summarize what I think happened in this thread...

I don't think the suggested copyBytes rename was controversial:

UMRP (raw pointer):
--- copyBytes(from:count)
+++ copyMemory(from:bytes:)


UMRBP (raw buffer):
--- copyBytes(from:)
+++ copyMemory(atByteOffset:from:)

--- copyBytes(from:)
+++ copyMemory(from:)


In the new raw initializeMemory methods, Xiaodi and I agreed to make
it more clear that the offset is in terms of `self` rather than
`from`, and to further reduce ambiguity by forcing manual stride
computation and using an explicit "offset" label. The call site will
be just as explicit as dropping down to `baseAddress` but without any
pointer conversion boilerplate.

UMRBP (raw buffer):
+++ func initializeMemory(atByteOffset:as:from:)
+++ func moveInitializeMemory(atByteOffset:as:from:)


Than, in the one existing initializeMemory method, just drop the strided index.
It's never used, we want to be consistent in using a byte offset
for the raw index, and that can be expressed just as easily as pointer
arithmetic:

UMRP (raw pointer):
--- func initializeMemory(as:T.Type, at:Int = 0, count:Int = 1, to:T)
+++ func initializeMemory(as:T.Type, repeating:T, count:Int = 1)

Then you can simply leave these methods as-is:
> func initializeMemory(as:T.Type, from:UnsafePointer, count:Int)
> func moveInitializeMemory(as:T.Type, from:UnsafeMutablePointer, 
> count:Int) 


We don't have a consensus, but I think the suggestion to distinguish
between single value vs. multiple semantics was good. Otherwise,
adding the default count could be very misleading. Normally, we try to
minimize surface area, but adding two methods for the single-value case
avoids ambiguity between the buffer and pointer semantics:

UMP (pointer)
--- func initialize(to:count:(=1))
+++ func initialize(to:)
+++ func initialize(repeating:count:) // no default count
+++ func assign(to:)
+++ func assign(repeating:count:) // no default count

UMRP (raw pointer):
--- func initializeMemory(as:at:(=0)count:(1)to:)
+++ func initializeMemory(as:repeating:count:) // remove default count

-Andy

> On Tue, Sep 5, 2017 at 11:31 AM, Andrew Trick  > wrote:
> I think we’ve agreed to a few minor updates to this proposal. Since there 
> hasn’t been any other feedback on the thread it may be worth posting an 
> amended proposal so we all know what we’ve agreed on.
> 
> -Andy
> 
>> On Sep 3, 2017, at 8:23 PM, Andrew Trick via swift-evolution 
>> > wrote:
>> 
>> 
>>> On Sep 3, 2017, at 8:05 PM, Xiaodi Wu >> > wrote:
>>> 
>>> 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).
>>> 
>>> I agree that it seems quite sensible to remove the ‘at’ parameter 
>>> altogether from the UMRP method.
>> 
>> No code in tree or on github is using the `at` argument. I think it can be 
>> removed. A fixit should still be possible.
>> 
>>> Not convinced moving the at: argument to come before the as: argument is 
>>> worth it in terms of source breakage.
>>> 
>>> Since much of this proposal involves shuffling and relabeling arguments, 
>>> I’d argue it’s better to break slight more source in one go for the optimal 
>>> API than to break slightly less for a slightly less optimal API, no? (This 
>>> is assuming there is agreement that ‘at:as:’ is less prone to 
>>> misinterpretation than ‘as:at:’.)
>> 
>> 
>> To be clear, we’re just talking about 
>> UnsafeMutableRawBufferPointer.initializeMemory now, so this is purely 
>> additive.
>> I think the label needs to be `atByteOffset`, and placing it before `as` 
>> makes a lot of sense because it no longer depends on the type’s stride. 
>> 
>> -Andy
>> ___
>> 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

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

[Michael Ilseman via swift-evolution]

> On Sep 1, 2017, at 10:27 PM, Douglas Gregor via swift-evolution 
>  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
>  
> 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.


> 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

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

[Taylor Swift via swift-evolution]

we agreed to deprecate the strided at:? Note that
UnsafeMutableRawBufferPointer would still need a byteOffset: argument. I’m
also still not comfortable with duplicating functionality for the sake of
having two names

On Tue, Sep 5, 2017 at 11:31 AM, Andrew Trick  wrote:

> I think we’ve agreed to a few minor updates to this proposal. Since there
> hasn’t been any other feedback on the thread it may be worth posting an
> amended proposal so we all know what we’ve agreed on.
>
> -Andy
>
> On Sep 3, 2017, at 8:23 PM, Andrew Trick via swift-evolution <
> swift-evolution@swift.org> wrote:
>
>
> On Sep 3, 2017, at 8:05 PM, Xiaodi Wu  wrote:
>
> 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).
>>
>
> I agree that it seems quite sensible to remove the ‘at’ parameter
> altogether from the UMRP method.
>
>
> No code in tree or on github is using the `at` argument. I think it can be
> removed. A fixit should still be possible.
>
> Not convinced moving the at: argument to come before the as: argument is
>> worth it in terms of source breakage.
>>
>
> Since much of this proposal involves shuffling and relabeling arguments,
> I’d argue it’s better to break slight more source in one go for the optimal
> API than to break slightly less for a slightly less optimal API, no? (This
> is assuming there is agreement that ‘at:as:’ is less prone to
> misinterpretation than ‘as:at:’.)
>
>
> To be clear, we’re just talking about 
> UnsafeMutableRawBufferPointer.initializeMemory
> now, so this is purely additive.
> I think the label needs to be `atByteOffset`, and placing it before `as`
> makes a lot of sense because it no longer depends on the type’s stride.
>
> -Andy
> ___
> 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

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

[Andrew Trick via swift-evolution]

I think we’ve agreed to a few minor updates to this proposal. Since there 
hasn’t been any other feedback on the thread it may be worth posting an amended 
proposal so we all know what we’ve agreed on.

-Andy

> On Sep 3, 2017, at 8:23 PM, Andrew Trick via swift-evolution 
>  wrote:
> 
> 
>> On Sep 3, 2017, at 8:05 PM, Xiaodi Wu > > wrote:
>> 
>> 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).
>> 
>> I agree that it seems quite sensible to remove the ‘at’ parameter altogether 
>> from the UMRP method.
> 
> No code in tree or on github is using the `at` argument. I think it can be 
> removed. A fixit should still be possible.
> 
>> Not convinced moving the at: argument to come before the as: argument is 
>> worth it in terms of source breakage.
>> 
>> Since much of this proposal involves shuffling and relabeling arguments, I’d 
>> argue it’s better to break slight more source in one go for the optimal API 
>> than to break slightly less for a slightly less optimal API, no? (This is 
>> assuming there is agreement that ‘at:as:’ is less prone to misinterpretation 
>> than ‘as:at:’.)
> 
> 
> To be clear, we’re just talking about 
> UnsafeMutableRawBufferPointer.initializeMemory now, so this is purely 
> additive.
> I think the label needs to be `atByteOffset`, and placing it before `as` 
> makes a lot of sense because it no longer depends on the type’s stride. 
> 
> -Andy
> ___
> 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

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

[Andrew Trick via swift-evolution]

> On Sep 3, 2017, at 8:38 PM, Taylor Swift  wrote:
> 
> what was the reasoning for making raw at: offset in strides and not bytes?

So, you’re talking about UnsafeMutableRawPointer(as:at:count:to:)…

The thinking was that it was typical for users to continue filling in the 
memory using the same type, and that manually performing address arithmetic is 
error prone.

Now that we're considering consistency with the raw buffer pointer API, which 
didn’t exist at the time, the thinking is that `at` can be ambiguous and that 
manual address arithmetic is actually less error prone and increases clarity at 
the call site.

Does that adequately sum of the last few messages in this thread?

-Andy


> On Sep 3, 2017, at 10:22 PM, Andrew Trick  > wrote:
> 
>> 
>>> On Sep 3, 2017, at 8:05 PM, Xiaodi Wu >> > wrote:
>>> 
>>> 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).
>>> 
>>> I agree that it seems quite sensible to remove the ‘at’ parameter 
>>> altogether from the UMRP method.
>> 
>> No code in tree or on github is using the `at` argument. I think it can be 
>> removed. A fixit should still be possible.
>> 
>>> Not convinced moving the at: argument to come before the as: argument is 
>>> worth it in terms of source breakage.
>>> 
>>> Since much of this proposal involves shuffling and relabeling arguments, 
>>> I’d argue it’s better to break slight more source in one go for the optimal 
>>> API than to break slightly less for a slightly less optimal API, no? (This 
>>> is assuming there is agreement that ‘at:as:’ is less prone to 
>>> misinterpretation than ‘as:at:’.)
>> 
>> 
>> To be clear, we’re just talking about 
>> UnsafeMutableRawBufferPointer.initializeMemory now, so this is purely 
>> additive.
>> I think the label needs to be `atByteOffset`, and placing it before `as` 
>> makes a lot of sense because it no longer depends on the type’s stride. 
>> 
>> -Andy

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

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

[Andrew Trick via swift-evolution]

> On Sep 3, 2017, at 8:05 PM, Xiaodi Wu  wrote:
> 
> 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).
> 
> I agree that it seems quite sensible to remove the ‘at’ parameter altogether 
> from the UMRP method.

No code in tree or on github is using the `at` argument. I think it can be 
removed. A fixit should still be possible.

> Not convinced moving the at: argument to come before the as: argument is 
> worth it in terms of source breakage.
> 
> Since much of this proposal involves shuffling and relabeling arguments, I’d 
> argue it’s better to break slight more source in one go for the optimal API 
> than to break slightly less for a slightly less optimal API, no? (This is 
> assuming there is agreement that ‘at:as:’ is less prone to misinterpretation 
> than ‘as:at:’.)


To be clear, we’re just talking about 
UnsafeMutableRawBufferPointer.initializeMemory now, so this is purely additive.
I think the label needs to be `atByteOffset`, and placing it before `as` makes 
a lot of sense because it no longer depends on the type’s stride. 

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

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

[Xiaodi Wu via swift-evolution]

On Sun, Sep 3, 2017 at 21:43 Taylor Swift  wrote:

> On Sat, Sep 2, 2017 at 7:53 PM, Andrew Trick  wrote:
>
>>
>> On Sep 2, 2017, at 5:34 PM, Xiaodi Wu  wrote:
>>
>> On Sat, Sep 2, 2017 at 4:41 PM, Andrew Trick  wrote:
>>
>>>
>>> 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.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.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.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).
>

I agree that it seems quite sensible to remove the ‘at’ parameter
altogether from the UMRP method.

Not convinced moving the at: argument to come before the as: argument is
> worth it in terms of source breakage.
>

Since much of this proposal involves shuffling and relabeling arguments,
I’d argue it’s better to break slight more source in one go for the optimal
API than to break slightly less for a slightly less optimal API, no? (This
is assuming there is agreement that ‘at:as:’ is less prone to
misinterpretation than ‘as:at:’.)

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

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

[Taylor Swift via swift-evolution]

On Sat, Sep 2, 2017 at 7:53 PM, Andrew Trick  wrote:

>
> On Sep 2, 2017, at 5:34 PM, Xiaodi Wu  wrote:
>
> On Sat, Sep 2, 2017 at 4:41 PM, Andrew Trick  wrote:
>
>>
>> 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.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.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.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.
___
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

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

[Andrew Trick via swift-evolution]

> On Sep 2, 2017, at 5:34 PM, Xiaodi Wu  wrote:
> 
> On Sat, Sep 2, 2017 at 4:41 PM, Andrew Trick  > wrote:
> 
>> On Sep 2, 2017, at 2:06 PM, Taylor Swift via swift-evolution 
>> > 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.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.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.stride, as:T.self, from: bufferOfT)
> ```

Sure, that probably makes sense if we decide to go with a byte offset vs. 
stride index.
-Andy___
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

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

[Xiaodi Wu via swift-evolution]

On Sat, Sep 2, 2017 at 4:41 PM, Andrew Trick  wrote:

>
> 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.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.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.stride, as:T.self, from: bufferOfT)
```
___
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

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

[Andrew Trick via swift-evolution]

> On Sep 2, 2017, at 2:06 PM, Taylor Swift via swift-evolution 
>  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.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.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___
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

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

[Xiaodi Wu via swift-evolution]

On Sat, Sep 2, 2017 at 4:06 PM, Taylor Swift  wrote:

>
>
> On Sat, Sep 2, 2017 at 3:39 PM, Xiaodi Wu  wrote:
>
>> On Sat, Sep 2, 2017 at 2:13 PM, Taylor Swift 
>> 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:

> 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
>
>
> 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(as:from:count:)`, which helps to reinforce
>> that fact.
>>
>>
>>> Concerns:

 

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

[Taylor Swift via swift-evolution]

On Sat, Sep 2, 2017 at 3:39 PM, Xiaodi Wu  wrote:

> On Sat, Sep 2, 2017 at 2:13 PM, Taylor Swift  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:
>>>
 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


 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(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` 

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

[Xiaodi Wu via swift-evolution]

On Sat, Sep 2, 2017 at 3:36 PM, Andrew Trick  wrote:

> Thanks for the review as always…
>
> 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  evolut...@swift.org> wrote:
>
>> On Sat, Sep 2, 2017 at 12:27 AM, Douglas Gregor via swift-evolution <
>> swift-evolution@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
>>>
>>> ## 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.allocate(capacity :
> maxPixels)
>
> var filled:Int = 0
> for scanline: UnsafeMutableBufferPointer 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 

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

[Xiaodi Wu via swift-evolution]

On Sat, Sep 2, 2017 at 2:13 PM, Taylor Swift  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:
>>
>>> 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
>>>
>>>
>>> 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(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 

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

[Andrew Trick via swift-evolution]

Thanks for the review as always…

> On Sep 2, 2017, at 12:13 PM, Taylor Swift via swift-evolution 
>  wrote:
> 
> 
> 
> On Sat, Sep 2, 2017 at 10:03 AM, Xiaodi Wu via swift-evolution 
> > wrote:
> On Sat, Sep 2, 2017 at 12:27 AM, Douglas Gregor via swift-evolution 
> > 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
>  
> 
> ## 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.allocate(capacity : maxPixels)

var filled:Int = 0
for scanline: UnsafeMutableBufferPointer 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 

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

[Taylor Swift via swift-evolution]

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

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

[Xiaodi Wu via swift-evolution]

On Sat, Sep 2, 2017 at 12:27 AM, Douglas Gregor via swift-evolution <
swift-evolution@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
>
> 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 

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

[Douglas Gregor via 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
 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
___
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution