[swift-evolution] Pitch: Improved Swift pointers
milseman at apple.com
Tue Jul 18 15:41:38 CDT 2017
I agree with all of Andy’s points. I really like this and think it’s a good time to start discussing its details and moving from a pitch to a proposal. Thank you for writing it!
Minor tweak: say “deprecate” instead of “remove” for APIs, which has a better connotation with respect to source compatibility. While I want to have the best APIs, it’s important to make migration smooth. For example, see https://github.com/apple/swift-evolution/blob/master/proposals/0180-string-index-overhaul.md#source-compatibility
The main thing that I think needs to be massaged before a formal proposal is the introduction and motivation section. It contains hyperbole that is distracting and misleading. Some examples:
> > Introduction
> > …
> > but the current API design is not very safe, consistent, or convenient.
This proposal does not address “safe” or unsafety. I think the proposal is very good and important for addressing consistency and convenience, which help encourage programmers to use APIs correctly, but “not very safe” is orthogonal to the proposal.
> > In some places, this design turns UnsafePointers into outright DangerousPointers, leading users to believe that they have allocated or freed memory when in fact, they have not.
I see nothing in this proposal that identifies, nor address UnsafePointers as being “DangerousPointers”. This proposal seeks to change idiomatic use to be more consistent and convenient, which is very important, but does not change what “Unsafe” means in Swift. Near as I can tell, the semantics and “dangerousness" of Unsafe*Pointers are unchanged by this proposal.
> > The current API suffers from inconsistent naming, poor usage of default argument values, missing methods, and excessive verbosity, and encourages excessively unsafe programming practices.
I agree with everything up until “excessively unsafe programming practices”. What do you mean by “unsafe”? What practice is that? It seems like you might have a very different definition of Unsafe than Swift does. If so, then this will be a very different sort of proposal and you should identify what you mean by “unsafe”.
> > This proposal seeks to iron out these inconsistencies, and offer a more convenient, more sensible, and less bug-prone API for Swift pointers.
100% agree and I’m super enthusiastic for this proposal for this reason! My main feedback is to align the motivation and pitch with the message, unless you really do have a different definition of “unsafe” that you’re wanting to pitch.
> > This results in an equally elegant API with about one-third less surface area.
> > Motivation:
> > Right now, UnsafeMutableBufferPointer is kind of a black box. To do anything with the memory block it represents, you have to extract baseAddresses and counts. This is unfortunate because UnsafeMutableBufferPointer provides a handy container for tracking the size of a memory buffer, but to actually make use of this information, the buffer pointer must be disassembled.
Note that Unsafe*BufferPointer conforms to RandomAccessCollection and thus gets all the same conveniences of anything else that is Array-like. This means that after it has been properly allocated, initialized, and pointer-casted, it is very convenient for consumers of Unsafe*BufferPointers. The main pain points, and this proposal is excellent at addressing, are on the producers of Unsafe*BufferPointers. For producers, there are a lot of rules and hoops to jump through and the APIs are not conveniently aligned with them. I do think you’ve done a great job of correctly identifying the pain points for people who need to produce Unsafe*BufferPointers.
The rest of the motivation section is excellent! I have done every single “idiom” you highlight and hated having to do it.
> On Jul 18, 2017, at 11:19 AM, Andrew Trick via swift-evolution <swift-evolution at swift.org> wrote:
>> On Jul 17, 2017, at 10:06 PM, Taylor Swift via swift-evolution <swift-evolution at swift.org <mailto:swift-evolution at swift.org>> wrote:
>> I’ve drafted a new version of the unsafe pointer proposal based on feedback I’ve gotten from this thread. You can read it here <https://gist.github.com/kelvin13/1b8ae906be23dff22f7a7c4767f0c907>.
>> Swift’s pointer types are an important interface for low-level memory manipulation, but the current API design is not very safe, consistent, or convenient. Many memory methods demand a capacity: or count: argument, forcing the user to manually track the size of the memory block, even though most of the time this is either unnecessary, or redundant as buffer pointers track this information natively. In some places, this design turns UnsafePointers into outright DangerousPointers, leading users to believe that they have allocated or freed memory when in fact, they have not.
>> The current API suffers from inconsistent naming, poor usage of default argument values, missing methods, and excessive verbosity, and encourages excessively unsafe programming practices. This proposal seeks to iron out these inconsistencies, and offer a more convenient, more sensible, and less bug-prone API for Swift pointers.
>> The previous draft <https://gist.github.com/kelvin13/a9c033193a28b1d4960a89b25fbffb06> of this proposal was relatively source-breaking, calling for a separation of functionality between singular pointer types and vector (buffer) pointer types. This proposal instead separates functionality between internally-tracked length pointer types and externally-tracked length pointer types. This results in an equally elegant API with about one-third less surface area.
>> <https://gist.github.com/kelvin13/1b8ae906be23dff22f7a7c4767f0c907 <https://gist.github.com/kelvin13/1b8ae906be23dff22f7a7c4767f0c907>>
> > remove the capacity parameter from deallocate(capacity:) and deallocate(bytes:alignedTo:)
> That's probably for the best.
> > add unsized memory methods to UnsafeMutableBufferPointer
> > add an assign(to:count:) method to UnsafeMutablePointer and an assign(to:) method to UnsafeMutableBufferPointer
> > add a default value of 1 to all size parameters on UnsafeMutablePointer and applicable
> > size parameters on UnsafeMutableRawPointer
> I'm not opposed to it.
> > rename copyBytes(from:count:) to copy(from:bytes:)
> LGTM in the interest of consistency. I should not have caved on this the first time around.
> > bytes refers to, well, a byte quantity that is not assumed to be initialized.
> > capacity refers to a strided quantity that is not assumed to be initialized.
> > count refers to a strided quantity that is assumed to be initialized.
> That's how I see it.
> > rename count in UnsafeMutableRawBufferPointer.allocate(count:) to bytes and add an
> > alignedTo parameter to make it UnsafeMutableRawBufferPointer.allocate(bytes:alignedTo:)
> Memory allocation is an issue unto itself. I generally prefer your
> proposed API. However...
> 1. Larger-than-pointer alignments aren't currently respected.
> 2. Users virtually never want to specify the alignment explicitly. They
> just want platform alignment. Unfortunately, there's no reasonable
> "maximal" alignment to use as a default. I think pointer-alignment
> is an excellent default guarantee.
> 3. The current allocation builtins seem to presume that
> allocation/deallocation can be made more efficient if the user code
> specifies alignment at deallocation. I don't think
> UnsafeRawBufferPointer should expose that to the user, so I agree
> with your proposal. In fact, I think aligned `free` should be
> handled within the Swift runtime.
> Resolving these issues requires changes to the Swift runtime API and
> implementation. This might be a good time to revisit that design, but
> it might slow down the rest of the proposal.
> > fix the ordering of the arguments in initializeMemory<Element>(as:at:count:to:)
> I think this ordering was an attempt to avoid confusion with binding
> memory where `to` refers to a type. However, it should be consistent
> with `UnsafePointer.initialize`, so we need to pick one of those to
> > add the sized memorystate functions withMemoryRebound<Element, Result>(to:count:_:) to
> > UnsafeMutableBufferPointer, and initializeMemory<Element>(as:at:to:count:),
> > initializeMemory<Element>(as:from:count:) moveInitializeMemory<Element>(as:from:count:),
> > and bindMemory<Element>(to:count:) to UnsafeMutableRawBufferPointer
> > add mutable overloads to non-vacating memorystate method arguments
> I'm not sure removing the need for implicit casts is a goal. I did
> that with the pointer `init` methods, but now I think that should be
> cleaned up to reduce API surface. I think smaller API surface wins in
> these cases. Is there a usability issue you're solving?
> > add a init(mutating:) initializer to UnsafeMutableBufferPointer
> Yes, finally.
> > remove initialize<C>(from:) from UnsafeMutablePointer
> > adding an initializer UnsafeMutableBufferPointer<Element>.init(allocatingCount:) instead > of a type method to UnsafeMutableBufferPointer
> For the record, I strongly prefer a type method for allocation for the reason you mention, it has important side effects beyond simply initializingn the pointer.
> swift-evolution mailing list
> swift-evolution at swift.org <mailto:swift-evolution at swift.org>
> https://lists.swift.org/mailman/listinfo/swift-evolution <https://lists.swift.org/mailman/listinfo/swift-evolution>
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the swift-evolution