Review #2 of SE-0237: Introduce withUnsafe Mutable BufferPointerIfSupported methods


(Douglas Gregor) #1

Hello Swift Community,

Based on feedback from the Core Team and the first review, SE-0237 has been revised. We will have a second review of the revised SE-0237 "Introduce withUnsafe{Mutable}BufferPointerIfSupported methods" that starts now and goes through December 3, 2018.

Reviews are an important part of the Swift evolution process. All review feedback should be either on this forum thread or, if you would like to keep your feedback private, directly to the review manager (via direct message in the Swift forums).

What goes into a review of a proposal?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift.

When reviewing a proposal, here are some questions to consider:

  • What is your evaluation of the proposal?
  • Is the problem being addressed significant enough to warrant a change to Swift?
  • Does this proposal fit well with the feel and direction of Swift?
  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Thanks!
Doug Gregor
Review Manager


(Jordan Rose) #2

Looks like there's a stray "with a default implementation when the collection conforms to MutableContiguousCollection" which should just be dropped. (And a handful of references to the old names without "IfAvailable".)


(Nobody1707) #3

I believe this is also left over from the previous version of the proposal:

The choice to refine RandomAccessCollection is based on the assumption that there are no reasonable contiguous collections that wouldn't also be random-access.


(Cory Benfield) #4

+1. Some notes:

  • There's a typo in the first paragraph, which reads "These methods will allow generic code on to make use of the thos respective"; "thos" should be "those".
  • The first sentence is weirdly ungrammatical: "This proposal introduces two new methods, on Sequence , on MutableCollection". That may want to be revised.

Otherwise I think this is good.

Yes. Particularly when writing high performance networking code, which spends a lot of time shuttling bytes around, it is helpful to be able to expose the bytes in a wide range of different types without paying too much cost for indirection. This proposal makes doing this kind of thing much easier.

Yes.

A read of this proposal and the related proposal for contiguous collections.


(Johannes Weiss) #5

Yes, please! This would allow SwiftNIO to have much better implementations of writing various standard data-types to ByteBuffers and will generally allow things to avoid extra copies where they're really unnecessary.

Yes, minimal change to Swift for great benefit.

Yes!

Read the proposal and struggled without having it before :slight_smile:


(Ben Cohen) #6

Sorry about the leftovers of the protocol in the updated version. Should be fixed now on master. Diff here.