Since Swift 5 just got opened up for proposals, SE-184 Improved Pointers is
ready for community review, and I encourage everyone to look it over and
provide feedback. Thank you!
<
https://github.com/apple/swift-evolution/blob/master/proposals/0184-improved-pointers.md
Since Swift 5 just got opened up for proposals, SE-184 Improved Pointers is ready for community review, and I encourage everyone to look it over and provide feedback. Thank you!
<https://github.com/apple/swift-evolution/blob/master/proposals/0184-improved-pointers.md>
Excellent. Thanks for patiently iterating on this. I know it's time consuming.
This differs from the previous draft of this proposal, in that this
expansion is more additive and less source-breaking, preserving the
much of the sized API present on UnsafeMutablePointer.
Yay!
For the binary operations assign(from:), moveAssign(from:),
moveInitialize(from:), and initialize(from:), it is assumed that the
other buffer pointer contains at least as many elements as self does.
Uh-oh! This should be consistent with
UnsafeRawBufferPointer.copy(from:bytes:) (the raw version of
assign/initialize(from:)). There we assume no data is dropped and
allow an uninitalized buffer tail. What was your rationale for the
opposite choice and how can we reconcile these APIs?
add a default value of 1 to all size parameters on
UnsafeMutablePointer and applicable size parameters on
UnsafeMutableRawPointer
I'm generally ok with this if you have seen the benefit of it in real
code. However, I do not think any `repeating:` methods should have a
default count.
avoids the contradictory and inconsistent use of count to represent a byte quantity
Background: Whether you consider an API consistent depends on how you
prioritize the guidelines. Here you've taken guidelines that I started
to use for new raw pointer APIs and given them higher priority than
other guidelines, in this case having the `count` initializer label match
the name of the public property being initialized. I think your change
is an improvement, but there was nothing accidental about the previous
API.
UnsafeMutableRawBufferPointer.allocate(bytes:alignedTo:)
Well, I think it's somewhat ridiculous for users to write this every time they allocate a buffer:
`UnsafeMutableRawBufferPointer.allocate(bytes: size, alignedTo: MemoryLayout<UInt>.alignment)`
If anyone reading the code is unsure about the Swift API's alignment
guarantee, it's trivial to check the API docs.
You could introduce a clearly documented default `alignedTo`
argument. The reason I didn't do that is that the runtime won't
respect it anyway. But I think it would be fair to go ahead with the
API and file a bug against the runtime.
and initializeMemory<Element>(as:at:repeating:count:),
initializeMemory<Element>(as:from:count:)
moveInitializeMemory<Element>(as:from:count:), and
bindMemory<Element>(to:count:) to UnsafeMutableRawBufferPointer
I think you should move the raw pointer changes to a separate bullet point.
Presumably the raw buffer capacity must match or exceed count * stride?
-Andy
···
On Aug 8, 2017, at 9:52 AM, Taylor Swift via swift-evolution <swift-evolution@swift.org> wrote:
I've only had a quick look, but I'm very positive about these changes! I've worked with some custom collections requiring pointers behind the scenes and completely agree that the current Swift API can be very confusing.
I'll try to take a more detailed look over the next few days, but I'd give a +1 just for having size tracking in mutable buffers alone, anything more is delicious icing 
···
On 8 Aug 2017, at 17:53, Taylor Swift via swift-evolution <swift-evolution@swift.org> wrote:
Since Swift 5 just got opened up for proposals, SE-184 Improved Pointers is ready for community review, and I encourage everyone to look it over and provide feedback. Thank you!
<https://github.com/apple/swift-evolution/blob/master/proposals/0184-improved-pointers.md>
This is an excellent document that demonstrates great care and attention.
It addresses pain points discovered through real-world use by a minimum of
well-justified API changes.
The motivating example is compelling, and my only suggestion there is to
show a before-and-after comparison of how the proposed changes will look.
The specific API changes have clearly been refined so many times that I’m
not sure I have much to add. Overall they improve consistency and represent
an improvement.
Bravo!
···
On Tue, Aug 8, 2017 at 11:53 Taylor Swift via swift-evolution < swift-evolution@swift.org> wrote:
Since Swift 5 just got opened up for proposals, SE-184 Improved Pointers
is ready for community review, and I encourage everyone to look it over and
provide feedback. Thank you!
<
https://github.com/apple/swift-evolution/blob/master/proposals/0184-improved-pointers.md
>_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution
I’ve revised the proposal based on what I learned from trying to implement
these changes. I think it’s worth tacking the existing methods that take
Sequences at the same time as this actually makes the design a bit simpler.
<https://gist.github.com/kelvin13/5edaf43dcd3d6d9ed24f303fc941214c>
*The previous version
<https://gist.github.com/kelvin13/1b8ae906be23dff22f7a7c4767f0c907> of this
document ignored the generic initialization methods on
UnsafeMutableBufferPointer and UnsafeMutableRawBufferPointer, leaving them
to be overhauled at a later date, in a separate proposal. Instead, this
version of the proposal leverages those existing methods to inform a more
compact API design which has less surface area, and is more future-proof
since it obviates the need to design and add another (redundant) set of
protocol-oriented pointer APIs later.*
···
On Tue, Aug 8, 2017 at 12:52 PM, Taylor Swift <kelvin13ma@gmail.com> wrote:
Since Swift 5 just got opened up for proposals, SE-184 Improved Pointers
is ready for community review, and I encourage everyone to look it over and
provide feedback. Thank you!
<GitHub - apple/swift-evolution: This maintains proposals for changes and user-visible enhancements to the Swift Programming Language.
proposals/0184-improved-pointers.md>
Since Swift 5 just got opened up for proposals, SE-184 Improved Pointers
is ready for community review, and I encourage everyone to look it over and
provide feedback. Thank you!
<GitHub - apple/swift-evolution: This maintains proposals for changes and user-visible enhancements to the Swift Programming Language.
proposals/0184-improved-pointers.md>Excellent. Thanks for patiently iterating on this. I know it's time
consuming.> This differs from the previous draft of this proposal, in that this
> expansion is more additive and less source-breaking, preserving the
> much of the sized API present on UnsafeMutablePointer.Yay!
> For the binary operations assign(from:), moveAssign(from:),
> moveInitialize(from:), and initialize(from:), it is assumed that the
> other buffer pointer contains at least as many elements as self does.Uh-oh! This should be consistent with
UnsafeRawBufferPointer.copy(from:bytes:) (the raw version of
assign/initialize(from:)). There we assume no data is dropped and
allow an uninitalized buffer tail. What was your rationale for the
opposite choice and how can we reconcile these APIs?
The rationale was simply that it’s not straightforward to “finish”
initializing a partially initialized buffer since we don’t have
UnsafeMutablePointer memory initializers that work on offsets from the base
address, so we might as well assume that anyone using it is initializing
the whole buffer in one go. This isn’t really that strong a rationale and
I’m open to switching it to be consistent with
UnsafeRawBufferPointer.copy(from:bytes:).
> add a default value of 1 to all size parameters on
> UnsafeMutablePointer and applicable size parameters on
> UnsafeMutableRawPointerI'm generally ok with this if you have seen the benefit of it in real
code. However, I do not think any `repeating:` methods should have a
default count.
Actually, i believe initialize(to:count:) is currently the one method that
already *has* a default count. That’s probably because the standard library
calls this method with a count argument of 1 more than any other
memorystate method. I don’t know if this decision was only made for the
sake of the stdlib or if it had an API justification.
> avoids the contradictory and inconsistent use of count to represent a
byte quantityBackground: Whether you consider an API consistent depends on how you
prioritize the guidelines. Here you've taken guidelines that I started
to use for new raw pointer APIs and given them higher priority than
other guidelines, in this case having the `count` initializer label match
the name of the public property being initialized. I think your change
is an improvement, but there was nothing accidental about the previous
API.> UnsafeMutableRawBufferPointer.allocate(bytes:alignedTo:)
Well, I think it's somewhat ridiculous for users to write this every time
they allocate a buffer:`UnsafeMutableRawBufferPointer.allocate(bytes: size, alignedTo:
MemoryLayout<UInt>.alignment)`If anyone reading the code is unsure about the Swift API's alignment
guarantee, it's trivial to check the API docs.You could introduce a clearly documented default `alignedTo`
argument. The reason I didn't do that is that the runtime won't
respect it anyway. But I think it would be fair to go ahead with the
API and file a bug against the runtime.
Default argument of MemoryLayout<Int>.alignment is the way to go but as you
said i don’t know if that is actually allowed/works. An alternative is to
have two allocate methods each, one that takes an alignment argument and
one that doesn’t (and aligns to pointer alignment) but that feels
inelegant. Default arguments would be better.
> and initializeMemory<Element>(as:at:repeating:count:),
> initializeMemory<Element>(as:from:count:)
> moveInitializeMemory<Element>(as:from:count:), and
> bindMemory<Element>(to:count:) to UnsafeMutableRawBufferPointerI think you should move the raw pointer changes to a separate bullet point.
Presumably the raw buffer capacity must match or exceed count * stride?
-Andy
The reason the raw buffer pointers don’t fill in their own size is the
destination type might not line up with the raw buffer size, and then
there’s questions of rounding and whatnot. bindMemory(to:count:) would also
have to perform integer division or some other defined behavior. Though if
people think computing the strided count inside the buffer pointer method
is the right way to go, I’m open to that.
···
On Tue, Aug 8, 2017 at 7:53 PM, Andrew Trick <atrick@apple.com> wrote:
On Aug 8, 2017, at 9:52 AM, Taylor Swift via swift-evolution < > swift-evolution@swift.org> wrote:
Implementation is here: [do not merge] [stdlib] Improved pointers by tayloraswift · Pull Request #11464 · apple/swift · GitHub
···
On Sat, Aug 12, 2017 at 8:23 PM, Taylor Swift <kelvin13ma@gmail.com> wrote:
I’ve revised the proposal based on what I learned from trying to implement
these changes. I think it’s worth tacking the existing methods that take
Sequences at the same time as this actually makes the design a bit
simpler.
<https://gist.github.com/kelvin13/5edaf43dcd3d6d9ed24f303fc941214c>*The previous version
<https://gist.github.com/kelvin13/1b8ae906be23dff22f7a7c4767f0c907> of this
document ignored the generic initialization methods on
UnsafeMutableBufferPointer and UnsafeMutableRawBufferPointer, leaving them
to be overhauled at a later date, in a separate proposal. Instead, this
version of the proposal leverages those existing methods to inform a more
compact API design which has less surface area, and is more future-proof
since it obviates the need to design and add another (redundant) set of
protocol-oriented pointer APIs later.*On Tue, Aug 8, 2017 at 12:52 PM, Taylor Swift <kelvin13ma@gmail.com> > wrote:
Since Swift 5 just got opened up for proposals, SE-184 Improved Pointers
is ready for community review, and I encourage everyone to look it over and
provide feedback. Thank you!
<https://github.com/apple/swift-evolution/blob/master/propos
als/0184-improved-pointers.md>
Since Swift 5 just got opened up for proposals, SE-184 Improved Pointers is ready for community review, and I encourage everyone to look it over and provide feedback. Thank you!
<https://github.com/apple/swift-evolution/blob/master/proposals/0184-improved-pointers.md>Excellent. Thanks for patiently iterating on this. I know it's time consuming.
> add a default value of 1 to all size parameters on
> UnsafeMutablePointer and applicable size parameters on
> UnsafeMutableRawPointerI'm generally ok with this if you have seen the benefit of it in real
code. However, I do not think any `repeating:` methods should have a
default count.Actually, i believe initialize(to:count:) is currently the one method that already has a default count. That’s probably because the standard library calls this method with a count argument of 1 more than any other memorystate method. I don’t know if this decision was only made for the sake of the stdlib or if it had an API justification.
Right you are. I had just noticed that none of the other `repeating` APIs had a default. But if this default argument simplifies real code patterns then I’m fine with it.
> UnsafeMutableRawBufferPointer.allocate(bytes:alignedTo:)
Well, I think it's somewhat ridiculous for users to write this every time they allocate a buffer:
`UnsafeMutableRawBufferPointer.allocate(bytes: size, alignedTo: MemoryLayout<UInt>.alignment)`
If anyone reading the code is unsure about the Swift API's alignment
guarantee, it's trivial to check the API docs.You could introduce a clearly documented default `alignedTo`
argument. The reason I didn't do that is that the runtime won't
respect it anyway. But I think it would be fair to go ahead with the
API and file a bug against the runtime.Default argument of MemoryLayout<Int>.alignment is the way to go but as you said i don’t know if that is actually allowed/works. An alternative is to have two allocate methods each, one that takes an alignment argument and one that doesn’t (and aligns to pointer alignment) but that feels inelegant. Default arguments would be better.
Default argument makes sense to me too. Then the raw buffer pointer and regular raw pointer APIs can be consistent with each other.
> and initializeMemory<Element>(as:at:repeating:count:),
> initializeMemory<Element>(as:from:count:)
> moveInitializeMemory<Element>(as:from:count:), and
> bindMemory<Element>(to:count:) to UnsafeMutableRawBufferPointerI think you should move the raw pointer changes to a separate bullet point.
Presumably the raw buffer capacity must match or exceed count * stride?
-Andy
The reason the raw buffer pointers don’t fill in their own size is the destination type might not line up with the raw buffer size, and then there’s questions of rounding and whatnot. bindMemory(to:count:) would also have to perform integer division or some other defined behavior. Though if people think computing the strided count inside the buffer pointer method is the right way to go, I’m open to that.
No, I think your API is fine. I just wanted to clarify that we will trap if the raw buffer is too small to fit the requested elements.
-Andy
···
On Aug 8, 2017, at 5:49 PM, Taylor Swift <kelvin13ma@gmail.com> wrote:
On Tue, Aug 8, 2017 at 7:53 PM, Andrew Trick <atrick@apple.com <mailto:atrick@apple.com>> wrote:On Aug 8, 2017, at 9:52 AM, Taylor Swift via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
yikes i was not aware of this. I don’t think it’s bad enough to warrant
dropping the argument like with deallocate(capacity:) but I can imagine bad
things happening to code that crams extra inhabitants into pointers.
···
On Tue, Aug 8, 2017 at 9:38 PM, Andrew Trick <atrick@apple.com> wrote:
> UnsafeMutableRawBufferPointer.allocate(bytes:alignedTo:)
Well, I think it's somewhat ridiculous for users to write this every time
they allocate a buffer:`UnsafeMutableRawBufferPointer.allocate(bytes: size, alignedTo:
MemoryLayout<UInt>.alignment)`If anyone reading the code is unsure about the Swift API's alignment
guarantee, it's trivial to check the API docs.You could introduce a clearly documented default `alignedTo`
argument. The reason I didn't do that is that the runtime won't
respect it anyway. But I think it would be fair to go ahead with the
API and file a bug against the runtime.Default argument of MemoryLayout<Int>.alignment is the way to go but as
you said i don’t know if that is actually allowed/works. An alternative is
to have two allocate methods each, one that takes an alignment argument and
one that doesn’t (and aligns to pointer alignment) but that feels
inelegant. Default arguments would be better.Default argument makes sense to me too. Then the raw buffer pointer and
regular raw pointer APIs can be consistent with each other.
Would you mind adding a deallocate method to (nonmutable) UnsafePointer/UnsafeBufferPointer to take care of
[SR-3309](https://bugs.swift.org/browse/SR-3309\)?
There’s simply nothing in the memory model that requires mutable memory for deallocation.
It fits right in with this proposal and hardly seems worth a separate one.
-Andy
···
On Aug 15, 2017, at 9:47 PM, Taylor Swift via swift-evolution <swift-evolution@swift.org> wrote:
Implementation is here: [do not merge] [stdlib] Improved pointers by tayloraswift · Pull Request #11464 · apple/swift · GitHub
On Sat, Aug 12, 2017 at 8:23 PM, Taylor Swift <kelvin13ma@gmail.com <mailto:kelvin13ma@gmail.com>> wrote:
I’ve revised the proposal based on what I learned from trying to implement these changes. I think it’s worth tacking the existing methods that take Sequences at the same time as this actually makes the design a bit simpler.
<https://gist.github.com/kelvin13/5edaf43dcd3d6d9ed24f303fc941214c>The previous version <https://gist.github.com/kelvin13/1b8ae906be23dff22f7a7c4767f0c907> of this document ignored the generic initialization methods on UnsafeMutableBufferPointer and UnsafeMutableRawBufferPointer, leaving them to be overhauled at a later date, in a separate proposal. Instead, this version of the proposal leverages those existing methods to inform a more compact API design which has less surface area, and is more future-proof since it obviates the need to design and add another (redundant) set of protocol-oriented pointer APIs later.
On Tue, Aug 8, 2017 at 12:52 PM, Taylor Swift <kelvin13ma@gmail.com <mailto:kelvin13ma@gmail.com>> wrote:
Since Swift 5 just got opened up for proposals, SE-184 Improved Pointers is ready for community review, and I encourage everyone to look it over and provide feedback. Thank you!
<https://github.com/apple/swift-evolution/blob/master/proposals/0184-improved-pointers.md>
Thanks for continuing to improve this proposal. It’s in great shape now.
Upon rereading it today I have to say I strongly object to the `count = 1` default in the following two cases:
+ UnsafeMutablePointer.withMemoryRebound(to: count: Int = 1)
+ UnsafeMutableRawPointer.bindMemory<T>(to:T.Type, count:Int = 1)
-> UnsafeMutablePointer<T>
To aid understanding, it needs to be clear at the call-site that binding memory only applies to the specified number of elements. It's a common mistake for users to think they can obtain a pointer to a different type, then use that pointer as a base to access other elements. These APIs are dangerous expert interfaces. We certainly don't want to make their usage more concise at the expense of clarity.
In general, I think there's very little value in the `count=1` default, and it creates potential confusion on the caller side between the `BufferPointer` API and the `Pointer` API. For example:
+ initialize(repeating:Pointee, count:Int = 1)
Seeing `p.initialize(repeating: x)`, the user may think `p` refers to the buffer instead of a pointer into the buffer and misunderstand the behavior.
+ UnsafeMutablePointer.deinitialize(count: Int = 1)
Again, `p.deinitialize()` looks to me like it might be deinitializing an entire buffer.
If the `count` label is always explicit, then there's a clear distinction between the low-level `pointer` APIs and the `buffer` APIs.
The pointer-to-single-element case never seemed interesting enough to me to worry about making convenient. If I'm wrong about that, is there some real-world code you can point to where the count=1 default significantly improves clarity?
-Andy
···
On Aug 15, 2017, at 9:47 PM, Taylor Swift via swift-evolution <swift-evolution@swift.org> wrote:
Implementation is here: [do not merge] [stdlib] Improved pointers by tayloraswift · Pull Request #11464 · apple/swift · GitHub
On Sat, Aug 12, 2017 at 8:23 PM, Taylor Swift <kelvin13ma@gmail.com <mailto:kelvin13ma@gmail.com>> wrote:
I’ve revised the proposal based on what I learned from trying to implement these changes. I think it’s worth tacking the existing methods that take Sequences at the same time as this actually makes the design a bit simpler.
<https://gist.github.com/kelvin13/5edaf43dcd3d6d9ed24f303fc941214c>The previous version <https://gist.github.com/kelvin13/1b8ae906be23dff22f7a7c4767f0c907> of this document ignored the generic initialization methods on UnsafeMutableBufferPointer and UnsafeMutableRawBufferPointer, leaving them to be overhauled at a later date, in a separate proposal. Instead, this version of the proposal leverages those existing methods to inform a more compact API design which has less surface area, and is more future-proof since it obviates the need to design and add another (redundant) set of protocol-oriented pointer APIs later.
On Tue, Aug 8, 2017 at 12:52 PM, Taylor Swift <kelvin13ma@gmail.com <mailto:kelvin13ma@gmail.com>> wrote:
Since Swift 5 just got opened up for proposals, SE-184 Improved Pointers is ready for community review, and I encourage everyone to look it over and provide feedback. Thank you!
<https://github.com/apple/swift-evolution/blob/master/proposals/0184-improved-pointers.md>
If we ever need to do pointer adjustment during deallocation to accommodate alignment, then I think the Swift runtime can track that. I see no reason to muddy the UnsafeRawPointer API with it. So, I agree with your proposed change to drop `alignedTo` there.
-Andy
···
On Aug 8, 2017, at 6:51 PM, Taylor Swift <kelvin13ma@gmail.com> wrote:
On Tue, Aug 8, 2017 at 9:38 PM, Andrew Trick <atrick@apple.com <mailto:atrick@apple.com>> wrote:
> UnsafeMutableRawBufferPointer.allocate(bytes:alignedTo:)
Well, I think it's somewhat ridiculous for users to write this every time they allocate a buffer:
`UnsafeMutableRawBufferPointer.allocate(bytes: size, alignedTo: MemoryLayout<UInt>.alignment)`
If anyone reading the code is unsure about the Swift API's alignment
guarantee, it's trivial to check the API docs.You could introduce a clearly documented default `alignedTo`
argument. The reason I didn't do that is that the runtime won't
respect it anyway. But I think it would be fair to go ahead with the
API and file a bug against the runtime.Default argument of MemoryLayout<Int>.alignment is the way to go but as you said i don’t know if that is actually allowed/works. An alternative is to have two allocate methods each, one that takes an alignment argument and one that doesn’t (and aligns to pointer alignment) but that feels inelegant. Default arguments would be better.
Default argument makes sense to me too. Then the raw buffer pointer and regular raw pointer APIs can be consistent with each other.
yikes i was not aware of this. I don’t think it’s bad enough to warrant dropping the argument like with deallocate(capacity:) but I can imagine bad things happening to code that crams extra inhabitants into pointers.
oh lol I was talking about assuming the pointer returned by
allocate(bytes:alignedTo:) is a multiple of alignedTo. Some code might be
relying on the last few bits of the pointer being zero; i.e. sticking bit
flags there like how some implementations store the red/black color
information in a red-black tree node.
···
On Tue, Aug 8, 2017 at 11:24 PM, Andrew Trick <atrick@apple.com> wrote:
On Aug 8, 2017, at 6:51 PM, Taylor Swift <kelvin13ma@gmail.com> wrote:
On Tue, Aug 8, 2017 at 9:38 PM, Andrew Trick <atrick@apple.com> wrote:
> UnsafeMutableRawBufferPointer.allocate(bytes:alignedTo:)
Well, I think it's somewhat ridiculous for users to write this every
time they allocate a buffer:`UnsafeMutableRawBufferPointer.allocate(bytes: size, alignedTo:
MemoryLayout<UInt>.alignment)`If anyone reading the code is unsure about the Swift API's alignment
guarantee, it's trivial to check the API docs.You could introduce a clearly documented default `alignedTo`
argument. The reason I didn't do that is that the runtime won't
respect it anyway. But I think it would be fair to go ahead with the
API and file a bug against the runtime.Default argument of MemoryLayout<Int>.alignment is the way to go but as
you said i don’t know if that is actually allowed/works. An alternative is
to have two allocate methods each, one that takes an alignment argument and
one that doesn’t (and aligns to pointer alignment) but that feels
inelegant. Default arguments would be better.Default argument makes sense to me too. Then the raw buffer pointer and
regular raw pointer APIs can be consistent with each other.yikes i was not aware of this. I don’t think it’s bad enough to warrant
dropping the argument like with deallocate(capacity:) but I can imagine
bad things happening to code that crams extra inhabitants into pointers.If we ever need to do pointer adjustment during deallocation to
accommodate alignment, then I think the Swift runtime can track that. I see
no reason to muddy the UnsafeRawPointer API with it. So, I agree with your
proposed change to drop `alignedTo` there.-Andy
Should the immutable buffer pointer types also get deallocate()?
···
On Fri, Aug 18, 2017 at 7:55 PM, Andrew Trick <atrick@apple.com> wrote:
On Aug 15, 2017, at 9:47 PM, Taylor Swift via swift-evolution < > swift-evolution@swift.org> wrote:
Implementation is here: [do not merge] [stdlib] Improved pointers by tayloraswift · Pull Request #11464 · apple/swift · GitHub
On Sat, Aug 12, 2017 at 8:23 PM, Taylor Swift <kelvin13ma@gmail.com> > wrote:
I’ve revised the proposal based on what I learned from trying to
implement these changes. I think it’s worth tacking the existing methods
that take Sequences at the same time as this actually makes the design a
bit simpler.
<https://gist.github.com/kelvin13/5edaf43dcd3d6d9ed24f303fc941214c>*The previous version
<https://gist.github.com/kelvin13/1b8ae906be23dff22f7a7c4767f0c907> of this
document ignored the generic initialization methods on
UnsafeMutableBufferPointer and UnsafeMutableRawBufferPointer, leaving them
to be overhauled at a later date, in a separate proposal. Instead, this
version of the proposal leverages those existing methods to inform a more
compact API design which has less surface area, and is more future-proof
since it obviates the need to design and add another (redundant) set of
protocol-oriented pointer APIs later.*On Tue, Aug 8, 2017 at 12:52 PM, Taylor Swift <kelvin13ma@gmail.com> >> wrote:
Since Swift 5 just got opened up for proposals, SE-184 Improved Pointers
is ready for community review, and I encourage everyone to look it over and
provide feedback. Thank you!
<https://github.com/apple/swift-evolution/blob/master/propos
als/0184-improved-pointers.md>Would you mind adding a deallocate method to (nonmutable) UnsafePointer/UnsafeBufferPointer
to take care of
[SR-3309](https://bugs.swift.org/browse/SR-3309\)?There’s simply nothing in the memory model that requires mutable memory
for deallocation.It fits right in with this proposal and hardly seems worth a separate one.
-Andy
I agree it’s probably a bad idea to add the default arg to those two
functions. However, the default argument in initialize(repeating:count:) is
there for backwards compatibility since it already had it before and
there’s like a hundred places in the stdlib that use this default value.
···
On Sat, Aug 19, 2017 at 6:02 PM, Andrew Trick <atrick@apple.com> wrote:
On Aug 15, 2017, at 9:47 PM, Taylor Swift via swift-evolution < > swift-evolution@swift.org> wrote:
Implementation is here: [do not merge] [stdlib] Improved pointers by tayloraswift · Pull Request #11464 · apple/swift · GitHub
On Sat, Aug 12, 2017 at 8:23 PM, Taylor Swift <kelvin13ma@gmail.com> > wrote:
I’ve revised the proposal based on what I learned from trying to
implement these changes. I think it’s worth tacking the existing methods
that take Sequences at the same time as this actually makes the design a
bit simpler.
<https://gist.github.com/kelvin13/5edaf43dcd3d6d9ed24f303fc941214c>*The previous version
<https://gist.github.com/kelvin13/1b8ae906be23dff22f7a7c4767f0c907> of this
document ignored the generic initialization methods on
UnsafeMutableBufferPointer and UnsafeMutableRawBufferPointer, leaving them
to be overhauled at a later date, in a separate proposal. Instead, this
version of the proposal leverages those existing methods to inform a more
compact API design which has less surface area, and is more future-proof
since it obviates the need to design and add another (redundant) set of
protocol-oriented pointer APIs later.*On Tue, Aug 8, 2017 at 12:52 PM, Taylor Swift <kelvin13ma@gmail.com> >> wrote:
Since Swift 5 just got opened up for proposals, SE-184 Improved Pointers
is ready for community review, and I encourage everyone to look it over and
provide feedback. Thank you!
<https://github.com/apple/swift-evolution/blob/master/propos
als/0184-improved-pointers.md>Thanks for continuing to improve this proposal. It’s in great shape now.
Upon rereading it today I have to say I strongly object to the `count = 1`
default in the following two cases:+ UnsafeMutablePointer.withMemoryRebound(to: count: Int = 1)
+ UnsafeMutableRawPointer.bindMemory<T>(to:T.Type, count:Int = 1)
-> UnsafeMutablePointer<T>To aid understanding, it needs to be clear at the call-site that binding
memory only applies to the specified number of elements. It's a common
mistake for users to think they can obtain a pointer to a different type,
then use that pointer as a base to access other elements. These APIs are
dangerous expert interfaces. We certainly don't want to make their usage
more concise at the expense of clarity.In general, I think there's very little value in the `count=1` default,
and it creates potential confusion on the caller side between the
`BufferPointer` API and the `Pointer` API. For example:+ initialize(repeating:Pointee, count:Int = 1)
Seeing `p.initialize(repeating: x)`, the user may think `p` refers to the
buffer instead of a pointer into the buffer and misunderstand the behavior.+ UnsafeMutablePointer.deinitialize(count: Int = 1)
Again, `p.deinitialize()` looks to me like it might be deinitializing an
entire buffer.If the `count` label is always explicit, then there's a clear distinction
between the low-level `pointer` APIs and the `buffer` APIs.The pointer-to-single-element case never seemed interesting enough to me
to worry about making convenient. If I'm wrong about that, is there some
real-world code you can point to where the count=1 default significantly
improves clarity?-Andy
Oh, sure. But I think it will be easy to fix the runtime. We could probably do it before the proposal is accepted if necessary.
-Andy
···
On Aug 8, 2017, at 8:29 PM, Taylor Swift <kelvin13ma@gmail.com> wrote:
On Tue, Aug 8, 2017 at 11:24 PM, Andrew Trick <atrick@apple.com <mailto:atrick@apple.com>> wrote:
On Aug 8, 2017, at 6:51 PM, Taylor Swift <kelvin13ma@gmail.com <mailto:kelvin13ma@gmail.com>> wrote:
On Tue, Aug 8, 2017 at 9:38 PM, Andrew Trick <atrick@apple.com <mailto:atrick@apple.com>> wrote:
> UnsafeMutableRawBufferPointer.allocate(bytes:alignedTo:)
Well, I think it's somewhat ridiculous for users to write this every time they allocate a buffer:
`UnsafeMutableRawBufferPointer.allocate(bytes: size, alignedTo: MemoryLayout<UInt>.alignment)`
If anyone reading the code is unsure about the Swift API's alignment
guarantee, it's trivial to check the API docs.You could introduce a clearly documented default `alignedTo`
argument. The reason I didn't do that is that the runtime won't
respect it anyway. But I think it would be fair to go ahead with the
API and file a bug against the runtime.Default argument of MemoryLayout<Int>.alignment is the way to go but as you said i don’t know if that is actually allowed/works. An alternative is to have two allocate methods each, one that takes an alignment argument and one that doesn’t (and aligns to pointer alignment) but that feels inelegant. Default arguments would be better.
Default argument makes sense to me too. Then the raw buffer pointer and regular raw pointer APIs can be consistent with each other.
yikes i was not aware of this. I don’t think it’s bad enough to warrant dropping the argument like with deallocate(capacity:) but I can imagine bad things happening to code that crams extra inhabitants into pointers.
If we ever need to do pointer adjustment during deallocation to accommodate alignment, then I think the Swift runtime can track that. I see no reason to muddy the UnsafeRawPointer API with it. So, I agree with your proposed change to drop `alignedTo` there.
-Andy
oh lol I was talking about assuming the pointer returned by allocate(bytes:alignedTo:) is a multiple of alignedTo. Some code might be relying on the last few bits of the pointer being zero; i.e. sticking bit flags there like how some implementations store the red/black color information in a red-black tree node.
cool,, as for UnsafeMutableRawBufferPointer.copy(from:bytes:), I cannot
find such a function anywhere in the API. There is copyBytes(from:)
<https://developer.apple.com/documentation/swift/unsafemutablerawbufferpointer/2635415-copybytes>,
but the documentation is messed up and mentions a nonexistent count:
argument over and over again. The documentation also doesn’t mention what
happens if there is a length mismatch, so users are effectively relying on
an implementation detail. I don’t know how to best resolve this.
···
On Tue, Aug 8, 2017 at 11:33 PM, Andrew Trick <atrick@apple.com> wrote:
On Aug 8, 2017, at 8:29 PM, Taylor Swift <kelvin13ma@gmail.com> wrote:
On Tue, Aug 8, 2017 at 11:24 PM, Andrew Trick <atrick@apple.com> wrote:
On Aug 8, 2017, at 6:51 PM, Taylor Swift <kelvin13ma@gmail.com> wrote:
On Tue, Aug 8, 2017 at 9:38 PM, Andrew Trick <atrick@apple.com> wrote:
> UnsafeMutableRawBufferPointer.allocate(bytes:alignedTo:)
Well, I think it's somewhat ridiculous for users to write this every
time they allocate a buffer:`UnsafeMutableRawBufferPointer.allocate(bytes: size, alignedTo:
MemoryLayout<UInt>.alignment)`If anyone reading the code is unsure about the Swift API's alignment
guarantee, it's trivial to check the API docs.You could introduce a clearly documented default `alignedTo`
argument. The reason I didn't do that is that the runtime won't
respect it anyway. But I think it would be fair to go ahead with the
API and file a bug against the runtime.Default argument of MemoryLayout<Int>.alignment is the way to go but as
you said i don’t know if that is actually allowed/works. An alternative is
to have two allocate methods each, one that takes an alignment argument and
one that doesn’t (and aligns to pointer alignment) but that feels
inelegant. Default arguments would be better.Default argument makes sense to me too. Then the raw buffer pointer and
regular raw pointer APIs can be consistent with each other.yikes i was not aware of this. I don’t think it’s bad enough to warrant
dropping the argument like with deallocate(capacity:) but I can imagine
bad things happening to code that crams extra inhabitants into pointers.If we ever need to do pointer adjustment during deallocation to
accommodate alignment, then I think the Swift runtime can track that. I see
no reason to muddy the UnsafeRawPointer API with it. So, I agree with your
proposed change to drop `alignedTo` there.-Andy
oh lol I was talking about assuming the pointer returned by
allocate(bytes:alignedTo:) is a multiple of alignedTo. Some code might be
relying on the last few bits of the pointer being zero; i.e. sticking bit
flags there like how some implementations store the red/black color
information in a red-black tree node.Oh, sure. But I think it will be easy to fix the runtime. We could
probably do it before the proposal is accepted if necessary.
-Andy
Should the immutable buffer pointer types also get deallocate()?
Both UnsafePointer and UnsafeBufferPointer should get deallocate. The Raw API already has those methods.
-Andy
···
On Aug 18, 2017, at 5:36 PM, Taylor Swift <kelvin13ma@gmail.com> wrote:
On Fri, Aug 18, 2017 at 7:55 PM, Andrew Trick <atrick@apple.com <mailto:atrick@apple.com>> wrote:
On Aug 15, 2017, at 9:47 PM, Taylor Swift via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
Implementation is here: [do not merge] [stdlib] Improved pointers by tayloraswift · Pull Request #11464 · apple/swift · GitHub
On Sat, Aug 12, 2017 at 8:23 PM, Taylor Swift <kelvin13ma@gmail.com <mailto:kelvin13ma@gmail.com>> wrote:
I’ve revised the proposal based on what I learned from trying to implement these changes. I think it’s worth tacking the existing methods that take Sequences at the same time as this actually makes the design a bit simpler.
<https://gist.github.com/kelvin13/5edaf43dcd3d6d9ed24f303fc941214c>The previous version <https://gist.github.com/kelvin13/1b8ae906be23dff22f7a7c4767f0c907> of this document ignored the generic initialization methods on UnsafeMutableBufferPointer and UnsafeMutableRawBufferPointer, leaving them to be overhauled at a later date, in a separate proposal. Instead, this version of the proposal leverages those existing methods to inform a more compact API design which has less surface area, and is more future-proof since it obviates the need to design and add another (redundant) set of protocol-oriented pointer APIs later.
On Tue, Aug 8, 2017 at 12:52 PM, Taylor Swift <kelvin13ma@gmail.com <mailto:kelvin13ma@gmail.com>> wrote:
Since Swift 5 just got opened up for proposals, SE-184 Improved Pointers is ready for community review, and I encourage everyone to look it over and provide feedback. Thank you!
<https://github.com/apple/swift-evolution/blob/master/proposals/0184-improved-pointers.md>Would you mind adding a deallocate method to (nonmutable) UnsafePointer/UnsafeBufferPointer to take care of
[SR-3309](https://bugs.swift.org/browse/SR-3309\)?There’s simply nothing in the memory model that requires mutable memory for deallocation.
It fits right in with this proposal and hardly seems worth a separate one.
-Andy
I agree it’s probably a bad idea to add the default arg to those two functions. However, the default argument in initialize(repeating:count:) is there for backwards compatibility since it already had it before and there’s like a hundred places in the stdlib that use this default value.
Alright, I could agree to that if no one else wants to weigh in. As long as you remove the default from the memory binding API.
-Andy
···
On Aug 19, 2017, at 5:33 PM, Taylor Swift <kelvin13ma@gmail.com> wrote:
On Sat, Aug 19, 2017 at 6:02 PM, Andrew Trick <atrick@apple.com <mailto:atrick@apple.com>> wrote:
On Aug 15, 2017, at 9:47 PM, Taylor Swift via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
Implementation is here: [do not merge] [stdlib] Improved pointers by tayloraswift · Pull Request #11464 · apple/swift · GitHub
On Sat, Aug 12, 2017 at 8:23 PM, Taylor Swift <kelvin13ma@gmail.com <mailto:kelvin13ma@gmail.com>> wrote:
I’ve revised the proposal based on what I learned from trying to implement these changes. I think it’s worth tacking the existing methods that take Sequences at the same time as this actually makes the design a bit simpler.
<https://gist.github.com/kelvin13/5edaf43dcd3d6d9ed24f303fc941214c>The previous version <https://gist.github.com/kelvin13/1b8ae906be23dff22f7a7c4767f0c907> of this document ignored the generic initialization methods on UnsafeMutableBufferPointer and UnsafeMutableRawBufferPointer, leaving them to be overhauled at a later date, in a separate proposal. Instead, this version of the proposal leverages those existing methods to inform a more compact API design which has less surface area, and is more future-proof since it obviates the need to design and add another (redundant) set of protocol-oriented pointer APIs later.
On Tue, Aug 8, 2017 at 12:52 PM, Taylor Swift <kelvin13ma@gmail.com <mailto:kelvin13ma@gmail.com>> wrote:
Since Swift 5 just got opened up for proposals, SE-184 Improved Pointers is ready for community review, and I encourage everyone to look it over and provide feedback. Thank you!
<https://github.com/apple/swift-evolution/blob/master/proposals/0184-improved-pointers.md>Thanks for continuing to improve this proposal. It’s in great shape now.
Upon rereading it today I have to say I strongly object to the `count = 1` default in the following two cases:
+ UnsafeMutablePointer.withMemoryRebound(to: count: Int = 1)
+ UnsafeMutableRawPointer.bindMemory<T>(to:T.Type, count:Int = 1)
-> UnsafeMutablePointer<T>To aid understanding, it needs to be clear at the call-site that binding memory only applies to the specified number of elements. It's a common mistake for users to think they can obtain a pointer to a different type, then use that pointer as a base to access other elements. These APIs are dangerous expert interfaces. We certainly don't want to make their usage more concise at the expense of clarity.
In general, I think there's very little value in the `count=1` default, and it creates potential confusion on the caller side between the `BufferPointer` API and the `Pointer` API. For example:
+ initialize(repeating:Pointee, count:Int = 1)
Seeing `p.initialize(repeating: x)`, the user may think `p` refers to the buffer instead of a pointer into the buffer and misunderstand the behavior.
+ UnsafeMutablePointer.deinitialize(count: Int = 1)
Again, `p.deinitialize()` looks to me like it might be deinitializing an entire buffer.
If the `count` label is always explicit, then there's a clear distinction between the low-level `pointer` APIs and the `buffer` APIs.
The pointer-to-single-element case never seemed interesting enough to me to worry about making convenient. If I'm wrong about that, is there some real-world code you can point to where the count=1 default significantly improves clarity?
-Andy
We currently have `UnsafeMutableRawBufferPointer.copyBytes(from:)`. I don’t think your proposal changes that. The current docs refer to the `source` parameter, which is correct. Docs refer to the parameter name, not the label name. So `source.count` is the size of the input. I was pointing out that it has the semantics: `debugAssert(source.count <= self.count)`.
Your proposal changes `UnsafeRawPointer.copyBytes(from:count:)` to `UnsafeRawPointer.copy(from:bytes:)`. Originally we wanted to those API names to match, but I’m fine with your change. What is more important is that the semantics are the same as `copyBytes(from:)`. Furthermore, any new methods that you add that copy into a raw buffer (e.g. initializeMemory(as:from:count:)) should have similar behavior.
···
On Aug 8, 2017, at 8:44 PM, Taylor Swift <kelvin13ma@gmail.com> wrote:
cool,, as for UnsafeMutableRawBufferPointer.copy(from:bytes:), I cannot find such a function anywhere in the API. There is copyBytes(from:) <https://developer.apple.com/documentation/swift/unsafemutablerawbufferpointer/2635415-copybytes>, but the documentation is messed up and mentions a nonexistent count: argument over and over again. The documentation also doesn’t mention what happens if there is a length mismatch, so users are effectively relying on an implementation detail. I don’t know how to best resolve this.
—
Another thing. The initialization methods that you’re adding to `UnsafeRawPointer` and `UnsafeRawBufferPointer` should return typed `UnsafePointer<Element>` and `UnsafeBufferPointer<Element>` respectively.
Thanks,
-Andy
On Tue, Aug 8, 2017 at 11:33 PM, Andrew Trick <atrick@apple.com <mailto:atrick@apple.com>> wrote:
On Aug 8, 2017, at 8:29 PM, Taylor Swift <kelvin13ma@gmail.com <mailto:kelvin13ma@gmail.com>> wrote:
On Tue, Aug 8, 2017 at 11:24 PM, Andrew Trick <atrick@apple.com <mailto:atrick@apple.com>> wrote:
On Aug 8, 2017, at 6:51 PM, Taylor Swift <kelvin13ma@gmail.com <mailto:kelvin13ma@gmail.com>> wrote:
On Tue, Aug 8, 2017 at 9:38 PM, Andrew Trick <atrick@apple.com <mailto:atrick@apple.com>> wrote:
> UnsafeMutableRawBufferPointer.allocate(bytes:alignedTo:)
Well, I think it's somewhat ridiculous for users to write this every time they allocate a buffer:
`UnsafeMutableRawBufferPointer.allocate(bytes: size, alignedTo: MemoryLayout<UInt>.alignment)`
If anyone reading the code is unsure about the Swift API's alignment
guarantee, it's trivial to check the API docs.You could introduce a clearly documented default `alignedTo`
argument. The reason I didn't do that is that the runtime won't
respect it anyway. But I think it would be fair to go ahead with the
API and file a bug against the runtime.Default argument of MemoryLayout<Int>.alignment is the way to go but as you said i don’t know if that is actually allowed/works. An alternative is to have two allocate methods each, one that takes an alignment argument and one that doesn’t (and aligns to pointer alignment) but that feels inelegant. Default arguments would be better.
Default argument makes sense to me too. Then the raw buffer pointer and regular raw pointer APIs can be consistent with each other.
yikes i was not aware of this. I don’t think it’s bad enough to warrant dropping the argument like with deallocate(capacity:) but I can imagine bad things happening to code that crams extra inhabitants into pointers.
If we ever need to do pointer adjustment during deallocation to accommodate alignment, then I think the Swift runtime can track that. I see no reason to muddy the UnsafeRawPointer API with it. So, I agree with your proposed change to drop `alignedTo` there.
-Andy
oh lol I was talking about assuming the pointer returned by allocate(bytes:alignedTo:) is a multiple of alignedTo. Some code might be relying on the last few bits of the pointer being zero; i.e. sticking bit flags there like how some implementations store the red/black color information in a red-black tree node.
Oh, sure. But I think it will be easy to fix the runtime. We could probably do it before the proposal is accepted if necessary.
-Andy