> On Aug 21, 2017, at 10:59 PM, Taylor Swift <[email protected]> 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
_______________________________________________
swift-evolution mailing list
[email protected]
https://lists.swift.org/mailman/listinfo/swift-evolution
