Revised the document with these suggestions, notably I clarified some of the language about memory safety, and specified that mutable overloads will only be added to UnsafeMutableBufferPointer and UnsafeMutableRawBufferPointer, not their plain variants. It also picks ` repeating:` as the argument label to replace `to:` in functions like ` initialize(to:count:)`.
<https://gist.github.com/kelvin13/1b8ae906be23dff22f7a7c4767f0c907> On Tue, Jul 18, 2017 at 4:41 PM, Michael Ilseman <milse...@apple.com> wrote: > > > 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@swift.org> wrote: > > > On Jul 17, 2017, at 10:06 PM, Taylor Swift via swift-evolution < > swift-evolution@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 *Dangerous*Pointers, 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> > > ~~~ > > > remove the capacity parameter from deallocate(capacity:) and > deallocate(bytes:alignedTo:) > > That's probably for the best. > > > add unsized memory methods to UnsafeMutableBufferPointer > > Yay! > > > add an assign(to:count:) method to UnsafeMutablePointer and an > assign(to:) method to UnsafeMutableBufferPointer > > Sure. > > > 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 > change. > > > 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 > > Yay! > > > 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 > > Yep. > > > 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. > > > -Andy > _______________________________________________ > swift-evolution mailing list > swift-evolution@swift.org > https://lists.swift.org/mailman/listinfo/swift-evolution > > >
_______________________________________________ swift-evolution mailing list swift-evolution@swift.org https://lists.swift.org/mailman/listinfo/swift-evolution