[swift-evolution] SE-184 Improved Pointers

[Taylor Swift]

On Mon, Aug 21, 2017 at 11:09 PM, Andrew Trick <atrick at apple.com> wrote:

>
> 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>
>
> Important changes start here
> <https://github.com/kelvin13/swift-evolution/blob/patch-3/proposals/0184-improved-pointers.md#proposed-solution>
> .
>
>
> I have more feedback for you after discussing with Ben and Michael.
>
> Under the section "Background". The retaining/releasing counts are
> confusing because we normally talk about "+0/+1" with respect to a single
> value. Here you're summing the reference count effect across different
> source and destination values. I think you can drop the -1/+0/+1 from the
> cell labels and just label the headers as "Source-Copy(+1) vs.
> Source-Move(+0)" and "Dest-Initializing(+0)" vs. "Dest-Destroying(-1)".
>

I got rid of the total counts, but I used “retaining” and “releasing”
because the word “initialize” is already way overloaded in the document and
using it as a category in addition to referring to the actual method family
`initialize` and `initializeMemory` and Swift’s `init` initializers would
just get too confusing.


>
> Ben noticed that we still don't have `UnsafeMutableBufferPointer.deallocate()`.
> Please add that as well.
>

This is in the implementation and 2/3 of the API mocks; idk why it was lost
in the detailed changes section. That was a typo I just fixed it


> I think you should also add the default `alignedTo:Int =
> MemoryLayout<UInt>.alignment` to `UnsafeRawMutablePointer`. It is
> effectively part of Swift's memory model... [Background: We're providing a
> language level default guarantee of word-aligned storage. We don't want to
> expose the platform's default alignment. There is no such thing as a
> "maximal" alignment for all imported primitive types. We expect allocators
> to typically provide 16-byte alignment, but when developers are relying on
> that for correctness, we want it to be explicit. Developers usually rely on
> word alignment so there's no value in making that explicit.]
>

done and committed


>
> The source breaking changes can be marked deprecated for now, but can only
> be marked unavailable in Swift 5 mode or later.
>

done and committed


>
> We anticipate adding a ContiguouslyStored protocol "soon", which could be
> used to reduce the amount of overloading on the buffer
> `initialize`/`assign` APIs. But we need a lot more time to iterate on that
> design and it doesn't appear that migrating to it would be source breaking
> w.r.t. your proposal.
>
> I think we would be receptive to a "non-Optional baseAddress" proposal now
> if you or anyone else wants to pitch that. I know Dave Abrahams had a
> working implementation at some point. That would weaken some of the
> incentive to continue adding more convenience to buffer slices.
>

Really early versions of this proposal pitched that but it didn’t go so
well. also making baseAddress nonnillable means you don’t always have
access to `count` when `count == 0`


>
> 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

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()
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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-evolution/attachments/20170822/9675a29d/attachment.html>