[swift-evolution] SE-184 Improved Pointers

[Andrew Trick]

> On Aug 21, 2017, at 10:59 PM, Taylor Swift <kelvin13ma at gmail.com> wrote:
> 
> Sorry to bring this up again, but I was not able to defend the addition of `UnsafeMutableBufferPointer.deinitialize()`. It is incorrect for the typical use case and doesn't appear to solve any important use case. The *only* fully initializing method is `initialize(repeating:)`, but that will usually be used for trivial values, which should not be deinitialized. It's preferable for the user to explicitly deinitialize just the segments that they know were initialized, which can be done on the base pointer. The only benefit in having a `deinitialize` on the buffer is to communicate to users who see the `initialize` API for the first time that it is their responsibility to deinitialize if the type requires it. To that end, we could add a `deinitialize(at:count:)` method, communicating the symmetry with `initialize(at:from:). Naturally `index + count <= self.count`.
> 
> -Andy
> 
> I don’t agree with this. If `deinitialize()` is a problem because it deinitializes the entire buffer, so are `moveAssign` and `moveInitialize`. They all assume the released buffer operand is fully initialized. `deinitialize()` has just as much use as the other full-buffer releasing methods. Just take the image buffer example there 

`moveAssign` and `moveInitialize` assume that the sub-buffer being moved from is fully initialized. That’s already obvious because the user is asking to move source.count elements. I don’t see any use cases where it would pose a problem. If the user is moving out of a partially initialized buffer, they have already to sliced (and unfortunately rebased) the buffer. OTOH `deinitialize` is incorrect for normal use cases. I don’t see any practical analogy between those APIs.
> let pixels:Int = scanlines.map{ $0.count }.reduce(0, +)
> var image = UnsafeMutableBufferPointer<Pixel>.allocate(capacity: pixels)
> 
> var filled:Int = 0
> for scanline:UnsafeMutableBufferPointer<Pixel> in scanlines 
> {
>     image.moveInitialize(at: filled, from: scanline)
>     filled += scanline.count
> }
> 
> image.deinitialize()
We don’t want developers to do this. Instead we want to see an explicitly named association between the number of items initialized and deinitialized:

image.deinitialize(at: 0, count: filled)

Flipping this around, it could be even more common to be writing into a larger than necessary buffer (pixels > filled). If we’re providing auto-slicing initializers, then deinitialization should follow the same approach, rather than:

UnsafeMutableBufferPointer(rebasing: image[0, filled]).deinitialize()
> image.deallocate()
> 
> and replace `Pixel` with a class type like `UIButton`.
> And `deinitialize(at:count:)` is bad because you’re asking for a count on a buffer method. `moveAssign` and `moveInitialize` can take range parameters because they each have a second operand that supplies the count number. `deinitialize` doesn’t. That means calls could end up looking like 
> 
> buffer.deinitialize(at: 0, count: buffer.count)
> 
> which is exactly what we were trying to avoid in the first place.

But there is no value in avoiding the `count` argument here. That’s not a valid motivation for introducing `deinitialize` on a buffer, and we’d be better off not introducing it at all.

The only valid motivation I can come up with for introducing `deinitialize` on buffer is to remind developers who are only looking at the buffer API (and not the plain pointer API) that it’s their responsibility to manually deinitialize (it doesn’t automatically happen on deallocation or destruction).

-Andy

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