[Pitch] Add overrides with UnsafePointer sources to non-destructive copying methods on UnsafeMutablePointer


(Janosch Hildebrand) #1

Hi everyone!

I have a small change to `UnsafeMutablePointer` that I'd like to pitch here before turning it into a full proposal and I'd love to hear your feedback on it.

`UnsafeMutablePointer` offers the following methods to copy memory contents non-destructively:

func assignBackwardFrom(source: UnsafeMutablePointer<Pointee>, count: Int)
func assignFrom(source: UnsafeMutablePointer<Pointee>, count: Int)
func initializeFrom(source: UnsafeMutablePointer<Pointee>, count: Int)

Basically my proposal boils down to adding overloads for these methods to `UnsafeMutablePointer` that take `UnsafePointer` source arguments instead.

Currently it is necessary to cast an `UnsafePointer` to `UnsafeMutablePointer` before using it with one of these methods:

let source: UnsafePointer<Int> = ...
let destination: UnsafeMutablePointer<Int> = ...

destination.assignFrom(UnsafeMutablePointer(source), count: count)

I'd like to get rid of these casts. I would argue that they are not only technically unnecessary and add visual noise but that they could actually be a source of confusion.
A cast of an `UnsafePointer` to its mutable variant should throw up a red flag in most cases and even though this case is ultimately innocuous it still increases cognitive load on the reader.

Alternatives:

* The obvious alternative is to simply not add these methods. I'd argue that they provide enough benefit to justify their existence while only minimally increasing the stdlib surface area, especially by merit of being overloads.

* One way of working around the overloads would be to, for example, add a `PointerProtocol` to the standard library and make use of that in these methods. However, that would be a much more intrusive change with little immediate (if any) benefit and introduces more questions than it answers.

* Finally, the implicit conversions from `UnsafeMutablePointer<T>` to `UnsafePointer<T>` could be leveraged to work around the need for overloads by dropping the existing methods taking `UnsafeMutablePointer` source arguments. If I had to guess, I'd say that bit of compiler magic probably exists more in service of (Obj)-C interaction and I would be wary to depend on it here. It's also non-obvious from a documentation and auto-completion perspective.

Related:
I also have a few related items that might make sense to discuss in tandem while we're talking about `UnsafePointer` and friends.

* func distanceTo(x: Unsafe(Mutable)Pointer<Memory>) -> Int
This is the only other method on `UnsafePointer` and `UnsafeMutablePointer` for which similar overloads might make sense. However, I cannot really see enough of a use case to justify adding overloads here. Also the current methods match protocol requirements whereas the overloads would not.

* UnsafeBufferPointer.init()
One might argue that if we are not going to (a)buse implicit conversions that we should perhaps also add an explicitly overloaded initializer to `UnsafeBufferPointer` that takes an `UnsafeMutablePointer`. I'd agree that is a minor sore point but this is less problematic to begin with and given that one can already use the implicit conversion here I'm not particularly inclined to add this. Also I'd strongly argue against its symmetrical other (an initializer to `UnsafeMutableBufferPointer` taking an `UnsafePointer`).

* `Unsafe(Mutable)Pointer` conversion initializers.
The generic conversion initializers for `UnsafePointer` and `UnsafeMutablePointer` suffer from a minor type inference deficiency. For example:

let ptr: UnsafeMutablePointer<Int> = nil
let ptr2 = UnsafePointer(ptr) // Error: Cannot invoke initializer for type 'UnsafePointer<_>' with an argument list of type '(UnsafeMutablePointer<Int>)'

In this and similar cases, the compiler cannot currently infer the seemingly obvious choice of retaining the same generic parameter without some additional context.
Technically this could be solved by adding non-generic initializers but that seems like a clutch. This is probably a conscious design decision or at least known limitation with the type inference system and should be addressed there.

- Janosch


(Guillaume Lessard) #2

func assignBackwardFrom(source: UnsafeMutablePointer<Pointee>, count: Int)
func assignFrom(source: UnsafeMutablePointer<Pointee>, count: Int)
func initializeFrom(source: UnsafeMutablePointer<Pointee>, count: Int)

Basically my proposal boils down to adding overloads for these methods to `UnsafeMutablePointer` that take `UnsafePointer` source arguments instead.

I agree with these; I have run into this situation as well.

Related:
I also have a few related items that might make sense to discuss in tandem while we're talking about `UnsafePointer` and friends.

* func distanceTo(x: Unsafe(Mutable)Pointer<Memory>) -> Int
This is the only other method on `UnsafePointer` and `UnsafeMutablePointer` for which similar overloads might make sense. However, I cannot really see enough of a use case to justify adding overloads here. Also the current methods match protocol requirements whereas the overloads would not.

I don’t think this would makes sense. This method exists to support indexing within a single block of memory; overloading across types might encourage recklessness.

This being said, I’ve come to think the indexing operations should only exist on the Buffer wrappers to the UnsafePointer family.

Guillaume Lessard

···

On 2 févr. 2016, at 16:06, Janosch Hildebrand via swift-evolution <swift-evolution@swift.org> wrote:


(Félix Cloutier) #3

Looks reasonable to me.

Félix

···

Le 2 févr. 2016 à 19:50:50, Guillaume Lessard via swift-evolution <swift-evolution@swift.org> a écrit :

On 2 févr. 2016, at 16:06, Janosch Hildebrand via swift-evolution <swift-evolution@swift.org> wrote:

func assignBackwardFrom(source: UnsafeMutablePointer<Pointee>, count: Int)
func assignFrom(source: UnsafeMutablePointer<Pointee>, count: Int)
func initializeFrom(source: UnsafeMutablePointer<Pointee>, count: Int)

Basically my proposal boils down to adding overloads for these methods to `UnsafeMutablePointer` that take `UnsafePointer` source arguments instead.

I agree with these; I have run into this situation as well.

Related:
I also have a few related items that might make sense to discuss in tandem while we're talking about `UnsafePointer` and friends.

* func distanceTo(x: Unsafe(Mutable)Pointer<Memory>) -> Int
This is the only other method on `UnsafePointer` and `UnsafeMutablePointer` for which similar overloads might make sense. However, I cannot really see enough of a use case to justify adding overloads here. Also the current methods match protocol requirements whereas the overloads would not.

I don’t think this would makes sense. This method exists to support indexing within a single block of memory; overloading across types might encourage recklessness.

This being said, I’ve come to think the indexing operations should only exist on the Buffer wrappers to the UnsafePointer family.

Guillaume Lessard

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Janosch Hildebrand) #4

A proposal draft can be found here:

https://github.com/Jnosh/swift-evolution/blob/copying-to-unsafe-mutable-pointer-with-unsafe-pointer-source/proposals/0000-copying-to-unsafe-mutable-pointer-with-unsafe-pointer-source.md

- Janosch

···

On 03 Feb 2016, at 21:52, Félix Cloutier <felixcca@yahoo.ca> wrote:

Looks reasonable to me.

Félix

Le 2 févr. 2016 à 19:50:50, Guillaume Lessard via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> a écrit :

On 2 févr. 2016, at 16:06, Janosch Hildebrand via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

func assignBackwardFrom(source: UnsafeMutablePointer<Pointee>, count: Int)
func assignFrom(source: UnsafeMutablePointer<Pointee>, count: Int)
func initializeFrom(source: UnsafeMutablePointer<Pointee>, count: Int)

Basically my proposal boils down to adding overloads for these methods to `UnsafeMutablePointer` that take `UnsafePointer` source arguments instead.

I agree with these; I have run into this situation as well.

Related:
I also have a few related items that might make sense to discuss in tandem while we're talking about `UnsafePointer` and friends.

* func distanceTo(x: Unsafe(Mutable)Pointer<Memory>) -> Int
This is the only other method on `UnsafePointer` and `UnsafeMutablePointer` for which similar overloads might make sense. However, I cannot really see enough of a use case to justify adding overloads here. Also the current methods match protocol requirements whereas the overloads would not.

I don’t think this would makes sense. This method exists to support indexing within a single block of memory; overloading across types might encourage recklessness.

This being said, I’ve come to think the indexing operations should only exist on the Buffer wrappers to the UnsafePointer family.

Guillaume Lessard

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution


(Janosch Hildebrand) #5

Great question.

I think it might be useful for a case where you don't know if you have overlapping ranges and use `assignBackwardFrom()` defensively?

- Janosch

···

On 09 Feb 2016, at 17:40, Guillaume Lessard <glessard@tffenterprises.com> wrote:

Nice.
Question: does assignBackwardFrom() need an UnsafePointer overload? Its use case is for moving a block into an overlapping range, which implies both would have to be UnsafeMutablePointer, unless typecasting has already happened; in that case another typecasting operation is not a problem…

Cheers,
Guillaume Lessard


(Rob Mayoff) #6

You cannot use assignBackwardFrom "defensively" if you don't know whether
you have overlapping ranges. If the ranges overlap, and the source range
starts after the destination range, then assignBackwardFrom copies
destructively. (It overwrites some prefix of the source range with some
suffix of the source range, before copying the original prefix to the
destination.)

It would be better to just have assignFrom behave like memmove (always
non-destructively) and not have assignBackwardFrom at all… but that's not
the point of this proposal.

···

On Tue, Feb 9, 2016 at 10:53 AM, Janosch Hildebrand via swift-evolution < swift-evolution@swift.org> wrote:

Great question.

I think it might be useful for a case where you don't know if you have
overlapping ranges and use `assignBackwardFrom()` defensively?


(Janosch Hildebrand) #7

Great question.

I think it might be useful for a case where you don't know if you have overlapping ranges and use `assignBackwardFrom()` defensively?

You cannot use assignBackwardFrom "defensively" if you don't know whether you have overlapping ranges. If the ranges overlap, and the source range starts after the destination range, then assignBackwardFrom copies destructively. (It overwrites some prefix of the source range with some suffix of the source range, before copying the original prefix to the destination.)

Thanks for catching that, I didn't think that through properly.

Although I think my point still stands. You can have two pointers for which you cannot guarantee that they don't overlap so you need to test and select `assignFrom` or `assignBackwardFrom` appropriately so there is a use case for the `assignBackwardFrom` overload.

It would be better to just have assignFrom behave like memmove (always non-destructively) and not have assignBackwardFrom at all… but that's not the point of this proposal.

I guess performance was considered to be more important than convenience in this case, especially since (potentially) overlapping memory ranges should be a relatively limited occurrence in Swift. Although a point could probably be made for adding a convenience method à la memmove; as you said though, different proposal :slight_smile:

- Janosch

···

On 09 Feb 2016, at 22:55, Rob Mayoff via swift-evolution <swift-evolution@swift.org> wrote:
On Tue, Feb 9, 2016 at 10:53 AM, Janosch Hildebrand via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote: