On Tue, Aug 22, 2017 at 2:35 AM, Andrew Trick <[email protected]> wrote:
> > 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 = 0for 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 > > I replaced UnsafeMutableBufferPointer.deinitialize() with UnsafeMutableBufferPointer.deinitialize(at:count:)
_______________________________________________ swift-evolution mailing list [email protected] https://lists.swift.org/mailman/listinfo/swift-evolution
