[swift-evolution] Pitch: Improved Swift pointers

[Taylor Swift]

On Thu, Jul 13, 2017 at 6:56 PM, Andrew Trick <atrick at apple.com> wrote:

>
> On Jul 12, 2017, at 12:16 PM, Taylor Swift via swift-evolution <
> swift-evolution at swift.org> wrote:
>
> Hi all, I’ve written up a proposal to modify the unsafe pointer API for
> greater consistency, safety, and ease of use.
>
> ~~~
>
> Swift currently offers two sets of pointer types — singular pointers such
> as UnsafeMutablePointer, and vector (buffer) pointers such as
> UnsafeMutable*Buffer*Pointer. This implies a natural separation of tasks
> the two kinds of pointers are meant to do. For example, buffer pointers
> implement Collection conformance, while singular pointers do not.
>
> However, some aspects of the pointer design contradict these implied
> roles. It is possible to allocate an arbitrary number of instances from a
> type method on a singular pointer, but not from a buffer pointer. The
> result of such an operation returns a singular pointer, even though a
> buffer pointer would be more appropriate to capture the information about
> the *number* of instances allocated. It’s possible to subscript into a
> singular pointer, even though they are not real Collections. Some parts
> of the current design turn UnsafePointers into downright *Dangerous*Pointers,
> leading users to believe that they have allocated or freed memory when in
> fact, they have not.
>
> This proposal seeks to iron out these inconsistencies, and offer a more
> convenient, more sensible, and less bug-prone API for Swift pointers.
>
> <https://gist.github.com/kelvin13/a9c033193a28b1d4960a89b25fbffb06>
>
> ~~~
>
>
> Thanks for taking time to write this up.
>
> General comments:
>
> UnsafeBufferPointer is an API layer on top of UnsafePointer. The role
> of UnsafeBufferPointer is direct memory access sans lifetime
> management with Collection semantics. The role of UnsafePointer is
> primarily C interop. Those C APIs should be wrapped in Swift APIs that
> take UnsafeBufferPointer whenever the pointer represents a C array. I
> suppose making UnsafePointer less convenient would push developers
> toward UnsafeBufferPointer. I don't think that's worth outright
> breaking source, but gradual deprecation of convenience methods, like
> `susbscript` might be acceptable.
>

Gradual deprecation is exactly what I am proposing. As the document states
<https://gist.github.com/kelvin13/a9c033193a28b1d4960a89b25fbffb06#proposed-solution>,
the only methods which should be marked immediately as unavailable are the `
deallocate(capacity:)` methods, for safety and source compatibility
reasons. Removing `deallocate(capacity:)` now and forcing a loud compiler
error prevents catastrophic *silent* source breakage in the future, or
worse, from having to *support our own bug*.

>
>
> I have mixed feelings about stripping UnsafePointer of basic
> functionality. Besides breaking source, doing that would be
> inconsistent with its role as a lower API layer. The advantage would
> just be descreasing API surface area and forcing developers to use a
> higher-level API.
>

UnsafePointer is as much a high level API as UnsafeBufferPointer is. You
wouldn’t create a buffer pointer of length 1 just so you can “stick with
the high level API”. UnsafePointer and UnsafeBufferPointer are two tools
that do related but different things and they can exist at whatever
abstract level you need them at. After all, UnsafeBufferPointer is nothing
but an UnsafePointer? with a length value attached to it. If you’re
allocating more than one instance of memory, you almost certainly need to
track the length of the buffer anyway.


> The additive changes you propose are fairly obvious. See [SR-3088]
> UnsafeMutableBufferPointer doesn't have an allocating init.
>
> I haven't wanted to waste review cycles on small additive
> changes. It may make sense to batch them up into one coherent
> proposal. Here are a few more to consider.
>
> - [SR-3929] UnsafeBufferPointer should have init from mutable
> - [SR-4340] UnsafeBufferPointer needs a withMemoryRebound method
> - [SR-3087] No way to arbitrarily initialise an Array's storage
>

The feature requests you mention are all very valuable, however with
Michael’s point about fixing the memorystate API’s, the size of this
proposal has already grown to encompass dozens of methods in five types. I
think this says a lot about just how broken the current system is, but I
think it’s better to try to fix one class of problems at a time, and save
the less closely-related issues for separate proposals.


>
> Point by point:
>
> > drop the capacity parameter from UnsafeMutablePointer.allocate() and
> deallocate().
>
> I do not agree with removing the capacity parameter and adding a
> single-instance allocation API. UnsafePointer was not designed for
> single instances, it was primarily designed for C-style arrays. I
> don't see the value in providing a different unsafe API for single
> vs. multiple values.
>

Although it’s common to *receive* Unsafe__Pointers from C API’s, it’s rare
to *create* them from the Swift side. 95% of the time your Swift data lives
in a Swift Array, and you use withUnsafePointer(_:) to send them to the C
API, or just pass them directly with Array bridging.

The only example I can think of where I had to allocate memory from the
Swift side to pass to a C API is when I was using the Cairo C library and I
wanted the Swift code to own the image buffer backing the Cairo C structs
and I wanted to manage the memory manually to prevent the buffer backing
from getting deallocated prematurely. I think I ended up using
UnsafeMutableBufferPointer and extracting baseAddresses to manage the
memory. This proposal tries to mitigate that pain of extracting
baseAddresses by giving buffer pointers their own memory management methods.

As for the UnsafePointers you get from C APIs, they almost always come with
a size (or you specify it beforehand with a parameter) so you’re probably
going to be turning them into UnsafeBufferPointers anyway.

I also have to say it’s not common to deallocate something in Swift that
you didn’t previously allocate in Swift.


> I agree the primary allocation API should be
> UnsafeMutableBufferPointer.allocate(capacity:). There is an argument
> to be made for removing UnsafeMutablePointer.allocate(capacity:)
> entirely. But, as Michael Ilseman pointed out, that would involve
> reevaluating several other members of the UnsafePointer API. I think
> it's reasonable for UnsafePointer to retain all its functionality as a
> lower level API.
>
>
I think duplication of functionality is something to be avoided if possible.


> I don't understand what is misleading about
> UnsafePointer.deallocate(capacity:). It *is* inconvenienent for the
> user to keep track of memory capacity. Presumably that was done so
> either the implementation can move away from malloc/free or some sort
> of memory tracking can be implemented on the standard library
> side. Obviously, UnsafeBufferPointer.deallocate() would be cleaner in
> most cases.
>

It’s misleading because it plain doesn’t deallocate `capacity` instances.
It deletes the whole memory block regardless of what you pass in the
capacity argument. If the implementation is ever “fixed” so that it
actually deallocates `capacity` instances, suddenly every source that uses
`deallocate(capacity:)` will break, and *no one will know* until their app
starts mysteriously crashing. If the method is not removed, we will have to
support this behavior to avoid breaking sources, and basically say “yes the
argument label says it deallocates a capacity, but what it *really* does is
free the whole block and we can’t fix it because existing code assumes this
behavior”.


>
> > add an allocate(count:) type method to UnsafeMutableBufferPointer
>
> `capacity` should be used for allocating uninitialized memory not
> `count`. `count` should only refer to a number of initialized objects!
>

We can decide on what the correct term should be, but the current state of
Swift pointers is that *neither* convention is being followed. Just look at
the API for UnsafeMutableRawPointer. It’s a mess. This proposal at the
minimum establishes a consistent convention. It can be revised if you feel
`capacity` is more appropriate than `count`. If what you mean is that it’s
important to maintain the distinction between “initialized counts” and
“uninitialized counts”, well that can be revised in too.


> > add a deallocate() instance method to UnsafeMutableBufferPointer
>
> Yes, of course! I added a mention of that in SR-3088.
>
> > remove subscripts from UnsafePointer and UnsafeMutablePointer
>
> It's often more clear to perform arithmetic on C array indices rather
> than pointers. That said, I'm happy to push developers to use
> UnsafeBufferPointer whenever that have a known capacity. To me, this
> is a question of whether the benefit of making a dangerous thing less
> convenient is worth breaking source compatibility.
>

Again, I think this is more about what the real use patterns are. If you
are subscripting into a C array with integers, then UnsafeBufferPointer is
the tool for the job, since it give you Collection conformance. If you
can’t make an UnsafeBufferPointer, it’s probably because you don’t know the
length of the array, and so you’re probably iterating through it one
element at a time. UnsafeMutablePointer.successor() is perfect for this
job. If you want to extract or set fields at fixed but irregular offsets,
UnsafeRawPointer is the tool for the job. But I’m hard-pressed to think of
a use case for random access into a singular typed pointer.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-evolution/attachments/20170713/bf1159b6/attachment.html>