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

Reply via email to