[swift-evolution] SE-184 Improved Pointers

[Andrew Trick]

> On Aug 20, 2017, at 6:03 PM, Taylor Swift <kelvin13ma at gmail.com> 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 <kelvin13ma at gmail.com <mailto:kelvin13ma at gmail.com>> 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 <swift-evolution at swift.org <mailto:swift-evolution at swift.org>> wrote:
> 
>> 
>> 
>> On Sat, Aug 19, 2017 at 10:28 PM, Andrew Trick <atrick at apple.com <mailto:atrick at apple.com>> wrote:
>> 
>>> On Aug 19, 2017, at 6:42 PM, Taylor Swift <kelvin13ma at gmail.com <mailto:kelvin13ma at gmail.com>> wrote:
>>> 
>>> 
>>> 
>>> On Sat, Aug 19, 2017 at 9:31 PM, Andrew Trick <atrick at apple.com <mailto:atrick at apple.com>> wrote:
>>> 
>>>> On Aug 19, 2017, at 6:16 PM, Taylor Swift <kelvin13ma at gmail.com <mailto:kelvin13ma at gmail.com>> 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
>> swift-evolution at swift.org <mailto:swift-evolution at swift.org>
>> https://lists.swift.org/mailman/listinfo/swift-evolution <https://lists.swift.org/mailman/listinfo/swift-evolution>
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-evolution/attachments/20170821/93de5808/attachment.html>