This is an excellent, thoroughly thought out, and well written proposal! I’m eager to see these improvements land.
> On Aug 22, 2017, at 11:33 AM, Taylor Swift <[email protected]> wrote: > > > > On Tue, Aug 22, 2017 at 2:35 AM, Andrew Trick <[email protected] > <mailto:[email protected]>> wrote: > >> On Aug 21, 2017, at 10:59 PM, Taylor Swift <[email protected] >> <mailto:[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 > > > I replaced UnsafeMutableBufferPointer.deinitialize() with > UnsafeMutableBufferPointer.deinitialize(at:count:)
_______________________________________________ swift-evolution mailing list [email protected] https://lists.swift.org/mailman/listinfo/swift-evolution
