> On Aug 20, 2017, at 6:03 PM, Taylor Swift <[email protected]> wrote:
> 
> New draft of the proposal is up here: 
> <https://github.com/kelvin13/swift-evolution/blob/patch-3/proposals/0184-improved-pointers.md
>  
> <https://github.com/kelvin13/swift-evolution/blob/patch-3/proposals/0184-improved-pointers.md>>
> 
> Important changes start here 
> <https://github.com/kelvin13/swift-evolution/blob/patch-3/proposals/0184-improved-pointers.md#proposed-solution>.

This should be brought to the attention of swift-evolution:

> The old `deallocate(capacity:)` method should be marked as `unavailable` 
> since it currently encourages dangerously incorrect code. This avoids 
> misleading future users, forces current users to address this potentially 
> catastrophic memory bug, and leaves the possibility open for us to add a 
> `deallocate(capacity:)` method in the future, or perhaps even a 
> `reallocate(toCapacity:)` method.

I can’t defend breaking existing source without having seen real code that was 
actually written incorrectly. I don’t see the downside of using the same 
deprecation strategy as the other changes. I expect code that was already 
written to be correct and future code to not call the deprecated API.

-Andy

> On Sun, Aug 20, 2017 at 1:40 PM, Kelvin Ma <[email protected] 
> <mailto:[email protected]>> wrote:
> actually never mind that, UnsafeMutablePointer should be the only type to not 
> support at: arguments since offsetting them is easy with +.
> 
> On Aug 20, 2017, at 12:12 AM, Taylor Swift via swift-evolution 
> <[email protected] <mailto:[email protected]>> wrote:
> 
>> 
>> 
>> On Sat, Aug 19, 2017 at 10:28 PM, Andrew Trick <[email protected] 
>> <mailto:[email protected]>> wrote:
>> 
>>> On Aug 19, 2017, at 6:42 PM, Taylor Swift <[email protected] 
>>> <mailto:[email protected]>> wrote:
>>> 
>>> 
>>> 
>>> On Sat, Aug 19, 2017 at 9:31 PM, Andrew Trick <[email protected] 
>>> <mailto:[email protected]>> wrote:
>>> 
>>>> On Aug 19, 2017, at 6:16 PM, Taylor Swift <[email protected] 
>>>> <mailto:[email protected]>> wrote:
>>>> 
>>>> What you’re describing is basically an earlier version of the proposal 
>>>> which had a slightly weaker precondition (source >= destination) than 
>>>> yours (source == destination). That one basically ignored the Sequence 
>>>> methods at the expense of greater API surface area.
>>> 
>>> The Sequence methods don’t provide the simpler, more convenient form of 
>>> initialization/deinitialization that I thought you wanted. I see two 
>>> reasonable options.
>>> 
>>> 1. Don’t provide any new buffer initialization/deinitialization 
>>> convenience. i.e. drop UsafeMutableBufferPointer moveInitialize, 
>>> moveAssign, and deinitialize from your proposal.
>>> 
>>> 2. Provide the full set of convenience methods: initialize, assign, 
>>> moveInitialize, and moveAssign assuming self.count==source.count. And 
>>> provide deinitialize() to be used only in conjunction with those new 
>>> initializers.
>>> 
>>> The question is really whether those new methods are going to significantly 
>>> simplify your code. If not, #1 is the conservative choice. Don't provide 
>>> convenience which could be misused. Put off solving that problem until we 
>>> can design a new move-only buffer type that tracks partially initialized 
>>> state.
>>> 
>>> -Andy 
>>> 
>>> 
>>> I’m not sure the answer is to just omit methods from 
>>> UnsafeMutableBufferPointer since most of the original complaints circulated 
>>> around having to un-nil baseAddress to do anything with them.
>> 
>> I know un-nil’ing baseAddress is horrible, but I don’t think working around 
>> that is an important goal yet. Eventually there will be a much safer, more 
>> convenient mechanism for manual allocation that doesn’t involve “pointers". 
>> I also considered adding API surface to UnsafeMutableBufferPointer.Slice, 
>> but that’s beyond what we should do now and may also become irrelevant when 
>> we have a more sophisticated buffer type. 
>> 
>>> What if only unary methods should be added to UnsafeMutableBufferPointer 
>>> without count:, meaning:
>>> 
>>> initialize(repeating:)
>> 
>> I actually have no problem with this one... except that it could be confused 
>> with UnsafeMutablePointer.initialize(repeating:), but I’ll ignore that since 
>> we already discussed it.
>> 
>>> assign(repeating:)
>>> deinitialize()
>> 
>> These are fine only if we have use cases that warrant them AND those use 
>> cases are expected to fully initialize the buffer, either via 
>> initialize(repeating:) or initialize(from: buffer) with 
>> precondition(source.count==self.count). They don’t really make sense for the 
>> use case that I’m familiar with. Without clear motivating code patterns, 
>> they aren’t worth the risk. “API Completeness” doesn’t have intrinsic value.
>> 
>> An example use for assign(repeating:) would be to zero out an image buffer.
>>  
>> 
>>> and the other methods should take both an offset parameter instead of a 
>>> count parameter:
>>> 
>>> initialize(from:at:)
>>> assign(from:at:)
>>> moveInitialize(from:at:)
>>> moveAssign(from:at:)
>>> 
>>> which provides maximum explicitness. This requires improvements to buffer 
>>> pointer slicing though. But I’m not a fan of the mission creep that’s 
>>> working into this proposal (i only originally wrote the thing to get 
>>> allocate(capacity:) and deallocate() into UnsafeMutableBufferPointer!)
>> 
>> I’m open to that, with source.count <= self.count + index. They are 
>> potentially ambiguous (the `at` could refer to a source index) but 
>> consistent with the idea that this API is for copying an entire source 
>> buffer into a slice of the destination buffer. Again, we need to find real 
>> code that benefits from this, but I expect the stdlib could use these.
>> 
>> -Andy
>> 
>> The more I think the more I believe using from:at: is the right approach. 
>> The only problem is that it would have to be written as a generic on 
>> Collection or Sequence to avoid having to provide up to 4 overloads for each 
>> operation, since we would want these to work well with buffer slices as well 
>> as buffers themselves. That puts them uncomfortably close to the turf of the 
>> existing buffer pointer Sequence API though.
>> 
>> Or we could make UnsafeMutableBufferPointer its own slice type. Right now 
>> MutableRandomAccessSlice<UnsafeMutableBufferPointer<Element>> takes up 4 
>> words of storage when it really only needs two.
>> _______________________________________________
>> swift-evolution mailing list
>> [email protected] <mailto:[email protected]>
>> https://lists.swift.org/mailman/listinfo/swift-evolution 
>> <https://lists.swift.org/mailman/listinfo/swift-evolution>
> 

_______________________________________________
swift-evolution mailing list
[email protected]
https://lists.swift.org/mailman/listinfo/swift-evolution

Reply via email to