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

Evolution Announcements
41

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<C>(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<T>(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.

42

I don’t see any source for this claim in the documentation <https://developer.apple.com/documentation/swift/unsafemutablepointer/2295090-deallocate&gt;, or the source code <https://github.com/apple/swift/blob/master/stdlib/public/core/UnsafePointer.swift.gyb#L432&gt;\. As far as I can tell the expected behavior is that partial deallocation “should” work.

The segfaulting example is an incorrect usage. The only valid parameters to deallocate(capacity:) are the base address of an allocation, and the original capacity passed into allocate(); it has never been intended to support partial deallocation of allocated blocks. It seems to me like this proposal is based on a misunderstanding of how the API works. The documentation and/or name should be clarified.

-Joe

> “fixing” this bug will cause programs that once operated on previously valid assumptions of “free()” semantics to behave differently, without any warnings ever being generated. Conversely incorrect code will suddenly become “correct” though this is less of a problem.
>
>> A sized implementation may fail more obviously when you violate the contract in the future. Not having sized deallocation is a known deficiency of the C model we've been fairly diligent about avoiding in Swift's allocation interfaces, and it would be extremely unfortunate for us to backpedal from it.
>>
>> -Joe

This discussion needs to be grounded by reiterating role of the API. UnsafePointer specifies the memory model without extraneous functionality or convenience.

The UnsafePointer.deallocate() API *is not*:

- a common, expected, or encouraged way to deallocate

- the simplest, safest, or most convenient way to deallocate

- necessarilly the most optimal path for deallocation

I don't think this is correct. UnsafePointer.deallocate is the API you must use to deallocate memory allocated with UnsafePointer.allocate. My question is whether it's acceptable to break all the people who didn't know this and are using it to deallocate memory allocated with malloc or new on Apple platforms. It sounds like the answer to that is "no, we want to be malloc-compatible", and therefore the 'capacity' parameter isn't currently serving a purpose today. We will always need to check if the memory is actually in the Swift pool before even believing the 'capacity' parameter.

(It is definitely true that the intent was for this to be the allocation capacity, and I'm surprised you interpreted it as supporting partial deallocation. But we probably can't fix that at this point.)

Jordan

···

On Sep 7, 2017, at 10:31, Andrew Trick via swift-evolution <swift-evolution@swift.org> wrote:

On Sep 7, 2017, at 8:06 AM, Taylor Swift <kelvin13ma@gmail.com <mailto:kelvin13ma@gmail.com>> wrote:
On Thu, Sep 7, 2017 at 8:59 AM, Joe Groff <jgroff@apple.com <mailto:jgroff@apple.com>> wrote:

There is only one decision that needs to be made here. Does the Swift runtime track allocation size for manually allocated blocks? I think the answer should be "yes", or at least haven't heard a strong argument against it. UnsafePointer.deallocate() needs to direcly reflect that model by making `allocatedCapacity` an *optional* argument.

Discussion about whether this API is unsafe, misleading, suboptimal or incorrectly implemented are secondary. Those are all deficiencies in the current documentation, current implementation, and availability of higher-level APIs.

Note that yesterday I argued that an optional argument wasn't worth the potential for confusion. That's true from a practical perspective, but I had lost sight of need to clearly specify the memory model. We want the Swift runtime to both have the functionality for tracking block size and also allow user code to track it more efficiently. Both those intentions need to be reflected in this API.

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

43

idk how the swift heap is planned to be implemented, but why is passing the
capacity to deallocate considered the fast path anyway? i thought the block
size was stored in a header right before the block pointer

···

On Thu, Sep 7, 2017 at 12:31 PM, Andrew Trick <atrick@apple.com> wrote:

On Sep 7, 2017, at 8:06 AM, Taylor Swift <kelvin13ma@gmail.com> wrote:

I don’t see any source for this claim in the documentation
<https://developer.apple.com/documentation/swift/unsafemutablepointer/2295090-deallocate&gt;,
or the source code
<https://github.com/apple/swift/blob/master/stdlib/public/core/UnsafePointer.swift.gyb#L432&gt;\.
As far as I can tell the expected behavior is that partial deallocation
“should” work.

On Thu, Sep 7, 2017 at 8:59 AM, Joe Groff <jgroff@apple.com> wrote:

The segfaulting example is an incorrect usage. The only valid parameters
to deallocate(capacity:) are the base address of an allocation, and the
original capacity passed into allocate(); it has never been intended to
support partial deallocation of allocated blocks. It seems to me like this
proposal is based on a misunderstanding of how the API works. The
documentation and/or name should be clarified.

-Joe

> “fixing” this bug will cause programs that once operated on previously
valid assumptions of “free()” semantics to behave differently, without any
warnings ever being generated. Conversely incorrect code will suddenly
become “correct” though this is less of a problem.
>
>> A sized implementation may fail more obviously when you violate the
contract in the future. Not having sized deallocation is a known deficiency
of the C model we've been fairly diligent about avoiding in Swift's
allocation interfaces, and it would be extremely unfortunate for us to
backpedal from it.
>>
>> -Joe

This discussion needs to be grounded by reiterating role of the API.
UnsafePointer specifies the memory model without extraneous functionality
or convenience.

The UnsafePointer.deallocate() API *is not*:

- a common, expected, or encouraged way to deallocate

- the simplest, safest, or most convenient way to deallocate

- necessarilly the most optimal path for deallocation

There is only one decision that needs to be made here. Does the Swift
runtime track allocation size for manually allocated blocks? I think the
answer should be "yes", or at least haven't heard a strong argument against
it. UnsafePointer.deallocate() needs to direcly reflect that model by
making `allocatedCapacity` an *optional* argument.

Discussion about whether this API is unsafe, misleading, suboptimal or
incorrectly implemented are secondary. Those are all deficiencies in the
current documentation, current implementation, and availability of
higher-level APIs.

Note that yesterday I argued that an optional argument wasn't worth the
potential for confusion. That's true from a practical perspective, but I
had lost sight of need to clearly specify the memory model. We want the
Swift runtime to both have the functionality for tracking block size and
also allow user code to track it more efficiently. Both those intentions
need to be reflected in this API.

-Andy

44

Would it be sufficient to assert that malloc_good_size(passedCapacity) == malloc_size(base) ? It wouldn't be perfect but could still catch a lot of misuses.

-Joe

···

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

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

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

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

The semantics of buffer.deallocate() needs to be: free `buffer.count` bytes of memory at `buffer.baseAddress`. So, that will always be the fast path!
Kelvin, do you agree with that?

this could be problematic if you have multiple contiguous buffers carved out of the same heap block. i agree that this is the best semantics for buffer pointers but we need the sized backend in Swift before this is possible else we will end up in the same boat we’re in right now with `deallocate(capacity:)` where we would have to make buffer deallocate heap block-based for now and then pull the rug out from underneath users later in order to switch to the improved semantics

If I understand your proposal, it’s only valid to deallocate a buffer that was allocated with the same capacity. Anything else should assert.
-Andy

the proposal isn’t specific enough there and that’s my fault but this seems like a good solution. in the future if we get a sized backend we can loosen the assertions and make the partial heap block buffer case defined behavior.

The important thing is that the UnsafeBufferPointer API is clearly documented. We do not want users to think it’s ok to deallocate a smaller buffer than they allocated.

Unfortunately, there’s actually no way to assert this in the runtime because malloc_size could be larger than the allocated capacity. Incorrect code could happen to work and we can live with that.

45

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 <atrick@apple.com> 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 <xiaodi.wu@gmail.com> 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

46

Yay for all 5.
-Andy

···

On Sep 6, 2017, at 10:15 PM, Taylor Swift <kelvin13ma@gmail.com> 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<C>(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<T>(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.

47

This discussion needs to be grounded by reiterating role of the API. UnsafePointer specifies the memory model without extraneous functionality or convenience.

The UnsafePointer.deallocate() API *is not*:

- a common, expected, or encouraged way to deallocate

- the simplest, safest, or most convenient way to deallocate

- necessarilly the most optimal path for deallocation

I don't think this is correct. UnsafePointer.deallocate is the API you must use to deallocate memory allocated with UnsafePointer.allocate.

No, all of Swift’s APIs for manual allocation/deallocation need to be compatible. UnsafeBufferPointer is highly preferable to UnsafePointer today. In the future we will have a safe API for manual allocation, still based on the underlying model specified by UnsafePointer.

UnsafePointer.allocate() is *not*

- a common, expected, or encouraged way to allocate

- the simplest, safest, or most convenient way to allocate

- necessarily the most optimal path for allocation

Even though high-level APIs are specified in terms of this model, they can be implemented via their own fast-paths.

Myquestion is whether it's acceptable to break all the people who didn't know this and are using it to deallocate memory allocated with malloc or new on Apple platforms. It sounds like the answer to that is "no, we want to be malloc-compatible", and therefore the 'capacity' parameter isn't currently serving a purpose today. We will always need to check if the memory is actually in the Swift pool before even believing the 'capacity' parameter.

We don’t need to claim that manually allocated Swift memory is malloc compatible on every platform that happens to have malloc. We do want the Swift memory model to be consistent with the reality that on most platforms, we need the runtime to track block size.

(It is definitely true that the intent was for this to be the allocation capacity, and I'm surprised you interpreted it as supporting partial deallocation. But we probably can't fix that at this point.)

I never misinterpreted the meaning of the API, but apparently multiple people on the evolution list did. Regardless, that is not valid reason for changing the API. It’s only a valid reason for improving documentation and encouraging the use of safer APIs.

If our memory model states that the runtime tracks capacity of manually allocated blocks, then the deallocation capacity should be optional to reflect that.

Still waiting to hear any arguments that something about that memory model is bad.

-Andy

···

On Sep 7, 2017, at 11:39 AM, Jordan Rose <jordan_rose@apple.com> wrote:

48

It doesn't have to be. When all allocation and deallocation calls carry matching capacity values, then that overhead can be eliminated when allocating out of a heterogeneous heap. Optimized allocators also generally have per-thread pools for common allocation sizes, and if you have the capacity value on hand, it can be quickly matched to the right pool size, and constant allocation sizes can potentially be recognized by the compiler and turned into allocator calls that directly allocate out of a specific pool.

-Joe

···

On Sep 7, 2017, at 11:55 AM, Taylor Swift <kelvin13ma@gmail.com> wrote:

On Thu, Sep 7, 2017 at 12:31 PM, Andrew Trick <atrick@apple.com <mailto:atrick@apple.com>> wrote:

On Sep 7, 2017, at 8:06 AM, Taylor Swift <kelvin13ma@gmail.com <mailto:kelvin13ma@gmail.com>> wrote:

I don’t see any source for this claim in the documentation <https://developer.apple.com/documentation/swift/unsafemutablepointer/2295090-deallocate&gt;, or the source code <https://github.com/apple/swift/blob/master/stdlib/public/core/UnsafePointer.swift.gyb#L432&gt;\. As far as I can tell the expected behavior is that partial deallocation “should” work.

On Thu, Sep 7, 2017 at 8:59 AM, Joe Groff <jgroff@apple.com <mailto:jgroff@apple.com>> wrote:

The segfaulting example is an incorrect usage. The only valid parameters to deallocate(capacity:) are the base address of an allocation, and the original capacity passed into allocate(); it has never been intended to support partial deallocation of allocated blocks. It seems to me like this proposal is based on a misunderstanding of how the API works. The documentation and/or name should be clarified.

-Joe

> “fixing” this bug will cause programs that once operated on previously valid assumptions of “free()” semantics to behave differently, without any warnings ever being generated. Conversely incorrect code will suddenly become “correct” though this is less of a problem.
>
>> A sized implementation may fail more obviously when you violate the contract in the future. Not having sized deallocation is a known deficiency of the C model we've been fairly diligent about avoiding in Swift's allocation interfaces, and it would be extremely unfortunate for us to backpedal from it.
>>
>> -Joe

This discussion needs to be grounded by reiterating role of the API. UnsafePointer specifies the memory model without extraneous functionality or convenience.

The UnsafePointer.deallocate() API *is not*:

- a common, expected, or encouraged way to deallocate

- the simplest, safest, or most convenient way to deallocate

- necessarilly the most optimal path for deallocation

There is only one decision that needs to be made here. Does the Swift runtime track allocation size for manually allocated blocks? I think the answer should be "yes", or at least haven't heard a strong argument against it. UnsafePointer.deallocate() needs to direcly reflect that model by making `allocatedCapacity` an *optional* argument.

Discussion about whether this API is unsafe, misleading, suboptimal or incorrectly implemented are secondary. Those are all deficiencies in the current documentation, current implementation, and availability of higher-level APIs.

Note that yesterday I argued that an optional argument wasn't worth the potential for confusion. That's true from a practical perspective, but I had lost sight of need to clearly specify the memory model. We want the Swift runtime to both have the functionality for tracking block size and also allow user code to track it more efficiently. Both those intentions need to be reflected in this API.

-Andy

idk how the swift heap is planned to be implemented, but why is passing the capacity to deallocate considered the fast path anyway? i thought the block size was stored in a header right before the block pointer

49

I mean I would have thought it’s reasonable to expect that if the method
asks for a capacity parameter, it will actually use it

···

On Thu, Sep 7, 2017 at 1:39 PM, Jordan Rose <jordan_rose@apple.com> wrote:

On Sep 7, 2017, at 10:31, Andrew Trick via swift-evolution < > swift-evolution@swift.org> wrote:

On Sep 7, 2017, at 8:06 AM, Taylor Swift <kelvin13ma@gmail.com> wrote:

I don’t see any source for this claim in the documentation
<https://developer.apple.com/documentation/swift/unsafemutablepointer/2295090-deallocate&gt;,
or the source code
<https://github.com/apple/swift/blob/master/stdlib/public/core/UnsafePointer.swift.gyb#L432&gt;\.
As far as I can tell the expected behavior is that partial deallocation
“should” work.

On Thu, Sep 7, 2017 at 8:59 AM, Joe Groff <jgroff@apple.com> wrote:

The segfaulting example is an incorrect usage. The only valid parameters
to deallocate(capacity:) are the base address of an allocation, and the
original capacity passed into allocate(); it has never been intended to
support partial deallocation of allocated blocks. It seems to me like this
proposal is based on a misunderstanding of how the API works. The
documentation and/or name should be clarified.

-Joe

> “fixing” this bug will cause programs that once operated on previously
valid assumptions of “free()” semantics to behave differently, without any
warnings ever being generated. Conversely incorrect code will suddenly
become “correct” though this is less of a problem.
>
>> A sized implementation may fail more obviously when you violate the
contract in the future. Not having sized deallocation is a known deficiency
of the C model we've been fairly diligent about avoiding in Swift's
allocation interfaces, and it would be extremely unfortunate for us to
backpedal from it.
>>
>> -Joe

This discussion needs to be grounded by reiterating role of the API.
UnsafePointer specifies the memory model without extraneous functionality
or convenience.

The UnsafePointer.deallocate() API *is not*:

- a common, expected, or encouraged way to deallocate

- the simplest, safest, or most convenient way to deallocate

- necessarilly the most optimal path for deallocation

I don't think this is correct. UnsafePointer.deallocate is the API you
must use to deallocate memory allocated with UnsafePointer.allocate. *My* question
is whether it's acceptable to break all the people who *didn't* know this
and are using it to deallocate memory allocated with malloc or new on Apple
platforms. It sounds like the answer to that is "no, we want to be
malloc-compatible", and therefore the 'capacity' parameter isn't currently
serving a purpose today. We will *always* need to check if the memory is
actually in the Swift pool before even believing the 'capacity' parameter.

(It is definitely true that the *intent* was for this to be the
allocation capacity, and I'm surprised you interpreted it as supporting
partial deallocation. But we probably can't fix that at this point.)

Jordan

There is only one decision that needs to be made here. Does the Swift
runtime track allocation size for manually allocated blocks? I think the
answer should be "yes", or at least haven't heard a strong argument against
it. UnsafePointer.deallocate() needs to direcly reflect that model by
making `allocatedCapacity` an *optional* argument.

Discussion about whether this API is unsafe, misleading, suboptimal or
incorrectly implemented are secondary. Those are all deficiencies in the
current documentation, current implementation, and availability of
higher-level APIs.

Note that yesterday I argued that an optional argument wasn't worth the
potential for confusion. That's true from a practical perspective, but I
had lost sight of need to clearly specify the memory model. We want the
Swift runtime to both have the functionality for tracking block size and
also allow user code to track it more efficiently. Both those intentions
need to be reflected in this API.

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

50

That theory does hold up for a million random values, but I don’t know if we can rely on malloc_size never being larger than roundUp(sz, 16). Greg?

-Andy

···

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

The important thing is that the UnsafeBufferPointer API is clearly documented. We do not want users to think it’s ok to deallocate a smaller buffer than they allocated.

Unfortunately, there’s actually no way to assert this in the runtime because malloc_size could be larger than the allocated capacity. Incorrect code could happen to work and we can live with that.

Would it be sufficient to assert that malloc_good_size(passedCapacity) == malloc_size(base) ? It wouldn't be perfect but could still catch a lot of misuses.

51

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..<n].intialize(from: source)

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<S : Sequence>(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:

If you amend SE-0184 accordingly and file a new PR, I think it can be quickly approved.

-Andy

···

On Sep 6, 2017, at 10:15 PM, Taylor Swift <kelvin13ma@gmail.com> 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<C>(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<T>(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.

52

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<C>(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<T>(atByteOffset:as:from:)
+++ func moveInitializeMemory<T>(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<T>(as:T.Type, at:Int = 0, count:Int = 1, to:T)
+++ func initializeMemory<T>(as:T.Type, repeating:T, count:Int = 1)

Then you can simply leave these methods as-is:

func initializeMemory<T>(as:T.Type, from:UnsafePointer<T>, count:Int)
func moveInitializeMemory<T>(as:T.Type, from:UnsafeMutablePointer<T>, 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<T>(as:at:(=0)count:(1)to:)
+++ func initializeMemory<T>(as:repeating:count:) // remove default count

-Andy

···

On Sep 5, 2017, at 9:53 AM, Taylor Swift <kelvin13ma@gmail.com> wrote:

On Tue, Sep 5, 2017 at 11:31 AM, Andrew Trick <atrick@apple.com <mailto:atrick@apple.com>> 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 <mailto:swift-evolution@swift.org>> wrote:

On Sep 3, 2017, at 8:05 PM, Xiaodi Wu <xiaodi.wu@gmail.com <mailto:xiaodi.wu@gmail.com>> 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 <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution

53

It will.

-Joe

···

On Sep 7, 2017, at 11:57 AM, Taylor Swift <kelvin13ma@gmail.com> wrote:

On Thu, Sep 7, 2017 at 1:39 PM, Jordan Rose <jordan_rose@apple.com <mailto:jordan_rose@apple.com>> wrote:

On Sep 7, 2017, at 10:31, Andrew Trick via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

On Sep 7, 2017, at 8:06 AM, Taylor Swift <kelvin13ma@gmail.com <mailto:kelvin13ma@gmail.com>> wrote:

I don’t see any source for this claim in the documentation <https://developer.apple.com/documentation/swift/unsafemutablepointer/2295090-deallocate&gt;, or the source code <https://github.com/apple/swift/blob/master/stdlib/public/core/UnsafePointer.swift.gyb#L432&gt;\. As far as I can tell the expected behavior is that partial deallocation “should” work.

On Thu, Sep 7, 2017 at 8:59 AM, Joe Groff <jgroff@apple.com <mailto:jgroff@apple.com>> wrote:

The segfaulting example is an incorrect usage. The only valid parameters to deallocate(capacity:) are the base address of an allocation, and the original capacity passed into allocate(); it has never been intended to support partial deallocation of allocated blocks. It seems to me like this proposal is based on a misunderstanding of how the API works. The documentation and/or name should be clarified.

-Joe

> “fixing” this bug will cause programs that once operated on previously valid assumptions of “free()” semantics to behave differently, without any warnings ever being generated. Conversely incorrect code will suddenly become “correct” though this is less of a problem.
>
>> A sized implementation may fail more obviously when you violate the contract in the future. Not having sized deallocation is a known deficiency of the C model we've been fairly diligent about avoiding in Swift's allocation interfaces, and it would be extremely unfortunate for us to backpedal from it.
>>
>> -Joe

This discussion needs to be grounded by reiterating role of the API. UnsafePointer specifies the memory model without extraneous functionality or convenience.

The UnsafePointer.deallocate() API *is not*:

- a common, expected, or encouraged way to deallocate

- the simplest, safest, or most convenient way to deallocate

- necessarilly the most optimal path for deallocation

I don't think this is correct. UnsafePointer.deallocate is the API you must use to deallocate memory allocated with UnsafePointer.allocate. My question is whether it's acceptable to break all the people who didn't know this and are using it to deallocate memory allocated with malloc or new on Apple platforms. It sounds like the answer to that is "no, we want to be malloc-compatible", and therefore the 'capacity' parameter isn't currently serving a purpose today. We will always need to check if the memory is actually in the Swift pool before even believing the 'capacity' parameter.

(It is definitely true that the intent was for this to be the allocation capacity, and I'm surprised you interpreted it as supporting partial deallocation. But we probably can't fix that at this point.)

Jordan

There is only one decision that needs to be made here. Does the Swift runtime track allocation size for manually allocated blocks? I think the answer should be "yes", or at least haven't heard a strong argument against it. UnsafePointer.deallocate() needs to direcly reflect that model by making `allocatedCapacity` an *optional* argument.

Discussion about whether this API is unsafe, misleading, suboptimal or incorrectly implemented are secondary. Those are all deficiencies in the current documentation, current implementation, and availability of higher-level APIs.

Note that yesterday I argued that an optional argument wasn't worth the potential for confusion. That's true from a practical perspective, but I had lost sight of need to clearly specify the memory model. We want the Swift runtime to both have the functionality for tracking block size and also allow user code to track it more efficiently. Both those intentions need to be reflected in this API.

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

I mean I would have thought it’s reasonable to expect that if the method asks for a capacity parameter, it will actually use it

54

This discussion needs to be grounded by reiterating role of the API. UnsafePointer specifies the memory model without extraneous functionality or convenience.

The UnsafePointer.deallocate() API *is not*:

- a common, expected, or encouraged way to deallocate

- the simplest, safest, or most convenient way to deallocate

- necessarilly the most optimal path for deallocation

I don't think this is correct. UnsafePointer.deallocate is the API you must use to deallocate memory allocated with UnsafePointer.allocate.

No, all of Swift’s APIs for manual allocation/deallocation need to be compatible. UnsafeBufferPointer is highly preferable to UnsafePointer today. In the future we will have a safe API for manual allocation, still based on the underlying model specified by UnsafePointer.

UnsafePointer.allocate() is *not*

- a common, expected, or encouraged way to allocate

- the simplest, safest, or most convenient way to allocate

- necessarily the most optimal path for allocation

Even though high-level APIs are specified in terms of this model, they can be implemented via their own fast-paths.

That's all fine; I'll rephrase to say that if I allocated with UnsafeMutablePointer.allocate, I'll probably deallocate with Unsafe[Mutable]Pointer.deallocate. They're not independent.

Myquestion is whether it's acceptable to break all the people who didn't know this and are using it to deallocate memory allocated with malloc or new on Apple platforms. It sounds like the answer to that is "no, we want to be malloc-compatible", and therefore the 'capacity' parameter isn't currently serving a purpose today. We will always need to check if the memory is actually in the Swift pool before even believing the 'capacity' parameter.

We don’t need to claim that manually allocated Swift memory is malloc compatible on every platform that happens to have malloc.

I suspect that we cannot revoke that from existing platforms.

We do want the Swift memory model to be consistent with the reality that on most platforms, we need the runtime to track block size.

I don't know where this comes from. If you don't need to be malloc-compatible, you don't strictly "need" this information. (It would break tools like 'leaks', though.)

(It is definitely true that the intent was for this to be the allocation capacity, and I'm surprised you interpreted it as supporting partial deallocation. But we probably can't fix that at this point.)

I never misinterpreted the meaning of the API, but apparently multiple people on the evolution list did. Regardless, that is not valid reason for changing the API. It’s only a valid reason for improving documentation and encouraging the use of safer APIs.

That "you" was addressed to Kelvin (Taylor), but if we really think people are mixing malloc/free and UnsafePointer APIs today, we should not break that on our existing platforms. It's unfortunate, but we're on Swift 4 already. It is not worth the cost.

If our memory model states that the runtime tracks capacity of manually allocated blocks, then the deallocation capacity should be optional to reflect that.

Still waiting to hear any arguments that something about that memory model is bad.

I don't see an advantage to having the parameter if we're going to promise that it's always present. It is additional complexity for very little win in a generic interface. If someone wants to write a special allocation entry point that actually makes use of this, they can do so, but it shouldn't live on UnsafePointer.

(We also have nothing that prevents you from doing `(ptr+1).deallocate()`, but, well, "unsafe".)

Jordan

···

On Sep 7, 2017, at 13:02, Andrew Trick <atrick@apple.com> wrote:

On Sep 7, 2017, at 11:39 AM, Jordan Rose <jordan_rose@apple.com <mailto:jordan_rose@apple.com>> wrote:

55

but it doesn’t

···

On Sep 7, 2017, at 3:10 PM, Joe Groff <jgroff@apple.com> wrote:

On Sep 7, 2017, at 11:57 AM, Taylor Swift <kelvin13ma@gmail.com> wrote:

On Thu, Sep 7, 2017 at 1:39 PM, Jordan Rose <jordan_rose@apple.com> wrote:

On Sep 7, 2017, at 10:31, Andrew Trick via swift-evolution <swift-evolution@swift.org> wrote:

On Sep 7, 2017, at 8:06 AM, Taylor Swift <kelvin13ma@gmail.com> wrote:

I don’t see any source for this claim in the documentation, or the source code. As far as I can tell the expected behavior is that partial deallocation “should” work.

On Thu, Sep 7, 2017 at 8:59 AM, Joe Groff <jgroff@apple.com> wrote:

The segfaulting example is an incorrect usage. The only valid parameters to deallocate(capacity:) are the base address of an allocation, and the original capacity passed into allocate(); it has never been intended to support partial deallocation of allocated blocks. It seems to me like this proposal is based on a misunderstanding of how the API works. The documentation and/or name should be clarified.

-Joe

> “fixing” this bug will cause programs that once operated on previously valid assumptions of “free()” semantics to behave differently, without any warnings ever being generated. Conversely incorrect code will suddenly become “correct” though this is less of a problem.
>
>> A sized implementation may fail more obviously when you violate the contract in the future. Not having sized deallocation is a known deficiency of the C model we've been fairly diligent about avoiding in Swift's allocation interfaces, and it would be extremely unfortunate for us to backpedal from it.
>>
>> -Joe

This discussion needs to be grounded by reiterating role of the API. UnsafePointer specifies the memory model without extraneous functionality or convenience.

The UnsafePointer.deallocate() API *is not*:

- a common, expected, or encouraged way to deallocate

- the simplest, safest, or most convenient way to deallocate

- necessarilly the most optimal path for deallocation

I don't think this is correct. UnsafePointer.deallocate is the API you must use to deallocate memory allocated with UnsafePointer.allocate. My question is whether it's acceptable to break all the people who didn't know this and are using it to deallocate memory allocated with malloc or new on Apple platforms. It sounds like the answer to that is "no, we want to be malloc-compatible", and therefore the 'capacity' parameter isn't currently serving a purpose today. We will always need to check if the memory is actually in the Swift pool before even believing the 'capacity' parameter.

(It is definitely true that the intent was for this to be the allocation capacity, and I'm surprised you interpreted it as supporting partial deallocation. But we probably can't fix that at this point.)

Jordan

There is only one decision that needs to be made here. Does the Swift runtime track allocation size for manually allocated blocks? I think the answer should be "yes", or at least haven't heard a strong argument against it. UnsafePointer.deallocate() needs to direcly reflect that model by making `allocatedCapacity` an *optional* argument.

Discussion about whether this API is unsafe, misleading, suboptimal or incorrectly implemented are secondary. Those are all deficiencies in the current documentation, current implementation, and availability of higher-level APIs.

Note that yesterday I argued that an optional argument wasn't worth the potential for confusion. That's true from a practical perspective, but I had lost sight of need to clearly specify the memory model. We want the Swift runtime to both have the functionality for tracking block size and also allow user code to track it more efficiently. Both those intentions need to be reflected in this API.

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

I mean I would have thought it’s reasonable to expect that if the method asks for a capacity parameter, it will actually use it

It will.

-Joe

56

You can’t. This may be true while alloc size if less than a page, but a quick test show that:

malloc_size(malloc(4097)) = 4608

···

Le 8 sept. 2017 à 03:03, Andrew Trick via swift-evolution <swift-evolution@swift.org> a écrit :

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

The important thing is that the UnsafeBufferPointer API is clearly documented. We do not want users to think it’s ok to deallocate a smaller buffer than they allocated.

Unfortunately, there’s actually no way to assert this in the runtime because malloc_size could be larger than the allocated capacity. Incorrect code could happen to work and we can live with that.

Would it be sufficient to assert that malloc_good_size(passedCapacity) == malloc_size(base) ? It wouldn't be perfect but could still catch a lot of misuses.

That theory does hold up for a million random values, but I don’t know if we can rely on malloc_size never being larger than roundUp(sz, 16). Greg?

57

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<C>(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.initi
alizeMemory<T>(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..<n].intialize(from: source)

The problem with subscript notation is we currently get the n argument from
the source argument. So what would really have to be written is

buf[i ..< i + source.count].initialize(from: source)

which is a lot more ugly and redundant. One option could be to decouple the
count parameter from the length of the source buffer, but that opens up the
whole can of worms in which length do we use? What happens if n - i is less
than or longer than source.count? If we enforce the precondition that
source.count
== n - i, then this syntax seems horribly redundant.

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<S : Sequence>(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.

Using Sequences throws out a whole host of assumptions we’ve been taking
advantage of. We can’t check for source.count anymore since that requires
traversing the entire Sequence. And if the performance is so bad relative
to the UnsafePointer API, we have to ask what the purpose of such a buffer
API would even be. The whole purpose of the buffer API was to make it
easier to do things with pointers without having to keep track of buffer
lengths externally. It should be built around the UnsafePointer API, not
the much higher level Sequence API.

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:
SE-0184-part1.md · GitHub

If you amend SE-0184 accordingly and file a new PR, I think it can be
quickly approved.

-Andy

fine with me i guess

···

On Thu, Sep 28, 2017 at 7:59 PM, Andrew Trick <atrick@apple.com> wrote:

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

58

Part one of SE-0184 is here as SE-0184 A
<https://github.com/kelvin13/swift-evolution/blob/improved-pointers/proposals/0184a-unsafe-pointers-part-1.md&gt;
I’ll implement it tomorrow and then make the PR

···

On Thu, Sep 28, 2017 at 7:59 PM, Andrew Trick <atrick@apple.com> wrote:

On Sep 6, 2017, at 10:15 PM, Taylor Swift <kelvin13ma@gmail.com> 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<C>(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.initi
alizeMemory<T>(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..<n].intialize(from: source)

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<S : Sequence>(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:
SE-0184-part1.md · GitHub

If you amend SE-0184 accordingly and file a new PR, I think it can be
quickly approved.

-Andy

59

So I understand that partial deallocation was never really supported (even if the old API allowed you to write it) but, given that, would it be possible to add reallocation to re-size an already allocated buffer? That’s a pretty glaring hole in the Swift raw-memory API.

- Karl

···

On 29. Sep 2017, at 02:59, Andrew Trick via swift-evolution <swift-evolution@swift.org> wrote:

On Sep 6, 2017, at 10:15 PM, Taylor Swift <kelvin13ma@gmail.com <mailto:kelvin13ma@gmail.com>> 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<C>(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<T>(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..<n].intialize(from: source)

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<S : Sequence>(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:
SE-0184-part1.md · GitHub

If you amend SE-0184 accordingly and file a new PR, I think it can be quickly approved.

-Andy

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

60

I’m fine with most of this

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<C>(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<T>(atByteOffset:as:from:)
+++ func moveInitializeMemory<T>(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<T>(as:T.Type, at:Int = 0, count:Int = 1, to:T)
+++ func initializeMemory<T>(as:T.Type, repeating:T, count:Int = 1)

Then you can simply leave these methods as-is:
> func initializeMemory<T>(as:T.Type, from:UnsafePointer<T>, count:Int)
> func moveInitializeMemory<T>(as:T.Type, from:UnsafeMutablePointer<T>,
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<T>(as:at:(=0)count:(1)to:)
+++ func initializeMemory<T>(as:repeating:count:) // remove default count

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

···

On Tue, Sep 5, 2017 at 4:49 PM, Andrew Trick <atrick@apple.com> wrote:

On Sep 5, 2017, at 9:53 AM, Taylor Swift <kelvin13ma@gmail.com> wrote:

-Andy

On Tue, Sep 5, 2017 at 11:31 AM, Andrew Trick <atrick@apple.com> 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 <xiaodi.wu@gmail.com> 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