Brent, This is excellent feedback. I can tell that you paid close attention to the API details.
I’m in the middle of working on a revision to the proposal. I'll address your questions after I finish writing it up so that my answers make more sense. I’ll include most of your naming improvements in a separate revision that we can bikeshed on the list. -Andy > On Jul 2, 2016, at 8:10 PM, Brent Royal-Gordon via swift-evolution > <[email protected]> wrote: > >> * What is your evaluation of the proposal? > > I think this is basically a good design, but I have lots and lots of comments. > > * * * > > I have a pile of naming quibbles; rather than describe them all in prose > (which turned into a mess), I've annotated parts of the "Full > UnsafeRawPointer API" section in a gist: > <https://gist.github.com/brentdax/8f4ed4decafc1d18c4441092baa13cfe>. > > * * * > > More concrete issues: > > * Is there a reason there's a `load` that takes a byte offset, but not a > `storeRaw`? > > * I'm also a little nervous about the fact that `storeRaw` (and `load`?) is > documented to only work properly on "trivial types", but it doesn't have any > sort of constraints to ensure it's used correctly. (One could imagine, for > instance, the compiler automatically conforming trivial types to a `Trivial` > protocol.) > > * I don't think I understand `initialize(toContiguous:atIndex:with:)`. Does > it return a typed pointer to the whole buffer, or just the one instance it > initialized? In the `stringFromBytes` example, shouldn't we either subscript > the typed pointer from the previous `initialize(_:with:count:)` call, or call > `storeRaw(toContiguous:atIndex:with:)`, rather than initializing memory > twice? If this isn't a good use case for > `initialize(toContiguous:atIndex:with:)`, what would be? > > * * * > > I'm quite concerned by the "moveInitialize should be more elegant" section at > the bottom. > > Since the types are so close, `moveInitialize` could require mutating > arguments and actually swap the pointers. For instance: > > func grow(buffer: UnsafePointer<Int>, count: Int, toNewCapacity > capacity: Int) -> UnsafeBuffer<Int> { > var buffer = buffer > var uninitializedBuffer = UnsafeRawPointer.allocate(capacity: > capacity, of: Int.self) > > uninitializedBuffer.swapPointersAfterMoving(from: &buffer, > count: count) > // `buffer` now points to the new allocation, filled in with > the Ints. > // `uninitializedBuffer` now points to the old allocation, > which is deinitialized. > > uninitializedBuffer.deallocate() > return buffer > } > > This is *such* a strange semantic, however, that I'm not at all sure how to > name this function. > > `moveAssign(from:count:)` could do something much simpler, returning a raw > version of `from`: > > target.moveAssign(from: source).deallocate() > > `move()`, on the other hand, I don't see a good way to fix like this. > > One ridiculous thing we could do for `moveAssign(from:count:)` and perhaps > `move()` is to deliberately make `self` invalid by setting it to address 0. > If it were `Optional`, this would nil `self`. If it weren't...well, something > would probably fail eventually. > > * * * > > I notice that many APIs require type parameters merely to force the user to > explicitly state the types involved. I wonder if we could instead introduce > an attribute which you could place on a parameter or return type indicating > that there must be an explicit `as` cast specifying its type: > > func storeRaw<T>(_: @explicit T) > func load<T>() -> @explicit T > func cast<T>() -> @explicit UnsafePointer<T> > > rawPointer.storeRaw(3 as Int) > rawPointer.load() as Int > rawPointer.cast() as UnsafePointer<Int> > > This would also be useful on `unsafeBitCast`, and on user APIs which are > prone to type inference issues. > > * * * > > In the long run, however, I wonder if we might end up removing > `UnsafeRawPointer`. If `Never` becomes a subtype-of-all-types, then > `UnsafePointer<Never>` would gain the basic properties of an > `UnsafeRawPointer`: > > * Because `Never` is a subtype of all types, `UnsafePointer<Never>` could > alias any other pointer. > > * Accessing `pointee` would be inherently invalid (it would either take or > return a `Never`), and APIs which initialize or set `pointee` would be > inherently uncallable. > > * `Never` has no intrinsic size, so it could be treated as having a one-byte > size, allowing APIs which normally allocate, deallocate, or do pointer > arithmetic by instance size to automatically do so by byte size instead. > > * APIs for casting an `UnsafePointer<T>` to `UnsafePointer<supertype of T>` > or `<subtype of T>` would do the right thing with `UnsafePointer<Never>`. > > Thus, I could imagine `Unsafe[Mutable]RawPointer` becoming > `Unsafe[Mutable]Pointer<Never>` in the future, with some APIs being > generalized and moving to all `UnsafePointer`s while others are in extensions > on `UnsafePointer where Pointee == Never`. > > It might be worth taking a look at the current API designs and thinking about > how they would look in that world: > > * Is `nsStringPtr.casting(to: UnsafePointer<NSObject>)` how you would want to > write a pointee upcast? How about `UnsafePointer<NSString>(nsObjectPtr)` for > a pointee downcast? > > * Would you want `initialize<T>(_: T.Type, with: T, count: Int = 1) -> > UnsafeMutablePointer<T>` in the `Never` extension, or (with a > supertype-of-Pointee constraint on `T`) would it be something you'd put on > other `UnsafeMutablePointer`s too? What does that mean for > `UnsafeMutablePointer.initialize(with:)`? > > * Are `load` or `storeRaw` things that might make sense on any > `UnsafeMutablePointer` if they were constrained to supertypes only? > > * Are there APIs which are basically the same on `Unsafe[Mutable]Pointer`s > and their `Raw` equivalents, except that the `Raw` versions are "dumb" > because they don't know what type they're operating on? If so, should they be > given the same name? > >> * Is the problem being addressed significant enough to warrant a change >> to Swift? > > Yes. Even in the realm of Unsafe types, we don't want undefined behavior with > no way to define it. > >> * Does this proposal fit well with the feel and direction of Swift? > > Yes. Nailing things down is important. > >> * If you have used other languages or libraries with a similar feature, >> how do you feel that this proposal compares to those? > > Need I mention how bizarre and arbitrary C pointers are? > >> * How much effort did you put into your review? A glance, a quick >> reading, or an in-depth study? > > I've put in a fair few hours on this post, but even so, there are parts of > the proposal that I really haven't looked at very deeply; I will admit I > rarely work at such a low level and don't fully understand all of the > technicalities involved. > > -- > Brent Royal-Gordon > Architechies > > _______________________________________________ > swift-evolution mailing list > [email protected] > https://lists.swift.org/mailman/listinfo/swift-evolution _______________________________________________ swift-evolution mailing list [email protected] https://lists.swift.org/mailman/listinfo/swift-evolution
