[Review] SE-0076: Add overrides taking an UnsafePointer source to non-destructive copying methods on UnsafeMutablePointer

Chris_Lattner · 2016-05-04T03:56:36.000Z

Hello Swift community,

The review of "SE-0076: Add overrides taking an UnsafePointer source to non-destructive copying methods on UnsafeMutablePointer" begins now and runs through May 9. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0076-copying-to-unsafe-mutable-pointer-with-unsafe-pointer-source.md

Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at

https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and contribute to the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

  * 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?

More information about the Swift evolution process is available at

swift-evolution/process.md at master · apple/swift-evolution · GitHub

Thank you,

-Chris Lattner
Review Manager

Andrew_Trick · 2016-05-04T04:39:00.000Z

The new methods are needed, but they don’t need to be overloads. I have no idea why the argument type was originally declared Mutable.

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

FWIW: I made precisely this change a while back on an experimental branch while experimenting with UnsafePointer conversion. I don’t see a problem with it.

Implicit argument conversion from UnsafeMutablePointer<Pointee> to UnsafePointer<Pointee> is normal and extremely obvious.

-Andy

···

On May 3, 2016, at 8:56 PM, Chris Lattner <clattner@apple.com> wrote:

Hello Swift community,

The review of "SE-0076: Add overrides taking an UnsafePointer source to non-destructive copying methods on UnsafeMutablePointer" begins now and runs through May 9. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0076-copying-to-unsafe-mutable-pointer-with-unsafe-pointer-source.md

* What is your evaluation of the proposal?

glessard · 2016-05-04T16:32:30.000Z

* What is your evaluation of the proposal?

It makes sense to have `assignFrom` and `initializeFrom` defined for `UnsafePointer` sources.

I would much rather see them defined simply with `UnsafePointer` rather than having overloads. The ability to use UnsafeMutablePointer with UnsafePointer parameters is not so much compiler magic as it is an acknowledgement of how memory works.

Defining `assignBackwardFrom` for an `UnsafePointer` source is unneeded, because its purpose is copying between overlapping ranges of memory — that’s destructive. The source range of `assignBackwardFrom` *must* be an `UnsafeMutablePointer`, by definition. This being said, C’s memmove has a `const void*` source, even though the ranges may overlap.

I’d prefer having all three than having none, but IMO the best outcome is to change only the first 2.

* Is the problem being addressed significant enough to warrant a change to Swift?

Yes. This has always seemed like a mistake to me.

* Does this proposal fit well with the feel and direction of Swift?

Yes. I like how non-magical these methods are.

* If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

There’s no need to remember which of memmove() or memcpy() allows overlapping ranges!

* How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Decades of using pointers informed my thoughts.

Cheers,
Guillaume Lessard

moiseev · 2016-05-05T22:40:59.000Z

Hi all,

This email is sent on behalf of the standard library team.

Since the implicit conversion is available from `UnsafeMutablePointer` to `UnsafePointer`, the functions listed in the proposal do not have to be overloads, they can simply replace existing ones instead.
If/when the implicit conversion will be removed from the language, overloads accepting `UnsafeMutablePointer` would have to be introduced.

We also suggest extending the proposal to include other instances of the same pattern, if there are any elsewhere in the standard library.

thanks,
max

···

On May 3, 2016, at 8:56 PM, Chris Lattner <clattner@apple.com> wrote:

Hello Swift community,

The review of "SE-0076: Add overrides taking an UnsafePointer source to non-destructive copying methods on UnsafeMutablePointer" begins now and runs through May 9. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0076-copying-to-unsafe-mutable-pointer-with-unsafe-pointer-source.md

Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at

  https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and contribute to the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

  * 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?

More information about the Swift evolution process is available at

  https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

-Chris Lattner
Review Manager
_______________________________________________
swift-evolution-announce mailing list
swift-evolution-announce@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution-announce

Joe_Groff · 2016-05-04T16:18:14.000Z

Yeah, Andy's approach seems cleaner than overloading.

-Joe

···

On May 3, 2016, at 9:39 PM, Andrew Trick via swift-evolution <swift-evolution@swift.org> wrote:

On May 3, 2016, at 8:56 PM, Chris Lattner <clattner@apple.com> wrote:

Hello Swift community,

The review of "SE-0076: Add overrides taking an UnsafePointer source to non-destructive copying methods on UnsafeMutablePointer" begins now and runs through May 9. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0076-copying-to-unsafe-mutable-pointer-with-unsafe-pointer-source.md

* What is your evaluation of the proposal?

The new methods are needed, but they don’t need to be overloads. I have no idea why the argument type was originally declared Mutable.

func assignBackwardFrom(source: UnsafePointer<Pointee>, count: Int
)

func assignFrom(source: UnsafePointer<Pointee>, count: Int
)

func initializeFrom(source: UnsafePointer<Pointee>, count: Int)

FWIW: I made precisely this change a while back on an experimental branch while experimenting with UnsafePointer conversion. I don’t see a problem with it.

Implicit argument conversion from UnsafeMutablePointer<Pointee> to UnsafePointer<Pointee> is normal and extremely obvious.

jrose · 2016-05-04T16:40:42.000Z

:-( …but it’s an implicit conversion. Which we’re trying to expunge from the language. (Sort of.)

Jordan

···

On May 4, 2016, at 09:18, Joe Groff via swift-evolution <swift-evolution@swift.org> wrote:

On May 3, 2016, at 9:39 PM, Andrew Trick via swift-evolution <swift-evolution@swift.org> wrote:

On May 3, 2016, at 8:56 PM, Chris Lattner <clattner@apple.com> wrote:

Hello Swift community,

The review of "SE-0076: Add overrides taking an UnsafePointer source to non-destructive copying methods on UnsafeMutablePointer" begins now and runs through May 9. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0076-copying-to-unsafe-mutable-pointer-with-unsafe-pointer-source.md

* What is your evaluation of the proposal?

The new methods are needed, but they don’t need to be overloads. I have no idea why the argument type was originally declared Mutable.

func assignBackwardFrom(source: UnsafePointer<Pointee>, count: Int
)

func assignFrom(source: UnsafePointer<Pointee>, count: Int
)

func initializeFrom(source: UnsafePointer<Pointee>, count: Int)

FWIW: I made precisely this change a while back on an experimental branch while experimenting with UnsafePointer conversion. I don’t see a problem with it.

Implicit argument conversion from UnsafeMutablePointer<Pointee> to UnsafePointer<Pointee> is normal and extremely obvious.

Yeah, Andy's approach seems cleaner than overloading.

Andrew_Trick · 2016-05-04T17:07:56.000Z

I’ve heard exactly the opposite argument recently. Namely that explicit UnsafePointer construction indicates an “unsafe” cast (I personally don’t agree with that argument though).

Tangential: Which of our current implicit conversions are considered bad? I can’t think of a good alternative, particularly for String/Array to UnsafePointer.

-Andy

···

On May 4, 2016, at 9:40 AM, Jordan Rose <jordan_rose@apple.com> wrote:

On May 4, 2016, at 09:18, Joe Groff via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

On May 3, 2016, at 9:39 PM, Andrew Trick via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

On May 3, 2016, at 8:56 PM, Chris Lattner <clattner@apple.com <mailto:clattner@apple.com>> wrote:

Hello Swift community,

The review of "SE-0076: Add overrides taking an UnsafePointer source to non-destructive copying methods on UnsafeMutablePointer" begins now and runs through May 9. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0076-copying-to-unsafe-mutable-pointer-with-unsafe-pointer-source.md

* What is your evaluation of the proposal?

The new methods are needed, but they don’t need to be overloads. I have no idea why the argument type was originally declared Mutable.

func assignBackwardFrom(source: UnsafePointer<Pointee>, count: Int
)

func assignFrom(source: UnsafePointer<Pointee>, count: Int
)

func initializeFrom(source: UnsafePointer<Pointee>, count: Int)

FWIW: I made precisely this change a while back on an experimental branch while experimenting with UnsafePointer conversion. I don’t see a problem with it.

Implicit argument conversion from UnsafeMutablePointer<Pointee> to UnsafePointer<Pointee> is normal and extremely obvious.

Yeah, Andy's approach seems cleaner than overloading.

:-( …but it’s an implicit conversion. Which we’re trying to expunge from the language. (Sort of.)

jrose · 2016-05-04T17:15:59.000Z

String/Array to UnsafePointer can't be safely expressed in the language right now without the lifetime-extending wrapper closure, so I feel less bad about those. Inout-to-pointer conversions don't feel implicit because of the '&'.

For other conversions, the remaining implicit value-to-reference bridging conversion is up for review, and we've talked about tightening up when implicit optional wrapping can occur. IUOs are out of the type system, which is probably as far as they'll go. I think subclass-to-superclass and covariant function pointer conversions are likely to stay as is, though.

I think converting initialization from UnsafeMutablePointer<T> to UnsafePointer<T> is always safe, but all the other initializers may need to grow a label (under our rules). That doesn't mean there should be an implicit conversion for it, as convenient as it may be.

(The counter-movement is people who want implicit conversions from smaller to larger integer and floating-point types, so the tide may shift here.)

Jordan

···

On May 4, 2016, at 10:07, Andrew Trick <atrick@apple.com> wrote:

On May 4, 2016, at 9:40 AM, Jordan Rose <jordan_rose@apple.com <mailto:jordan_rose@apple.com>> wrote:

On May 4, 2016, at 09:18, Joe Groff via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

On May 3, 2016, at 9:39 PM, Andrew Trick via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

On May 3, 2016, at 8:56 PM, Chris Lattner <clattner@apple.com <mailto:clattner@apple.com>> wrote:

Hello Swift community,

The review of "SE-0076: Add overrides taking an UnsafePointer source to non-destructive copying methods on UnsafeMutablePointer" begins now and runs through May 9. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0076-copying-to-unsafe-mutable-pointer-with-unsafe-pointer-source.md

* What is your evaluation of the proposal?

The new methods are needed, but they don’t need to be overloads. I have no idea why the argument type was originally declared Mutable.

func assignBackwardFrom(source: UnsafePointer<Pointee>, count: Int
)

func assignFrom(source: UnsafePointer<Pointee>, count: Int
)

func initializeFrom(source: UnsafePointer<Pointee>, count: Int)

FWIW: I made precisely this change a while back on an experimental branch while experimenting with UnsafePointer conversion. I don’t see a problem with it.

Implicit argument conversion from UnsafeMutablePointer<Pointee> to UnsafePointer<Pointee> is normal and extremely obvious.

Yeah, Andy's approach seems cleaner than overloading.

:-( …but it’s an implicit conversion. Which we’re trying to expunge from the language. (Sort of.)

I’ve heard exactly the opposite argument recently. Namely that explicit UnsafePointer construction indicates an “unsafe” cast (I personally don’t agree with that argument though).

Tangential: Which of our current implicit conversions are considered bad? I can’t think of a good alternative, particularly for String/Array to UnsafePointer.

Joe_Groff · 2016-05-04T17:32:25.000Z

Hello Swift community,

The review of "SE-0076: Add overrides taking an UnsafePointer source to non-destructive copying methods on UnsafeMutablePointer" begins now and runs through May 9. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0076-copying-to-unsafe-mutable-pointer-with-unsafe-pointer-source.md

* What is your evaluation of the proposal?

The new methods are needed, but they don’t need to be overloads. I have no idea why the argument type was originally declared Mutable.

func assignBackwardFrom(source: UnsafePointer<Pointee>, count: Int
)

func assignFrom(source: UnsafePointer<Pointee>, count: Int
)

func initializeFrom(source: UnsafePointer<Pointee>, count: Int)

FWIW: I made precisely this change a while back on an experimental branch while experimenting with UnsafePointer conversion. I don’t see a problem with it.

Implicit argument conversion from UnsafeMutablePointer<Pointee> to UnsafePointer<Pointee> is normal and extremely obvious.

Yeah, Andy's approach seems cleaner than overloading.

:-( …but it’s an implicit conversion. Which we’re trying to expunge from the language. (Sort of.)

I’ve heard exactly the opposite argument recently. Namely that explicit UnsafePointer construction indicates an “unsafe” cast (I personally don’t agree with that argument though).

Tangential: Which of our current implicit conversions are considered bad? I can’t think of a good alternative, particularly for String/Array to UnsafePointer.

String/Array to UnsafePointer can't be safely expressed in the language right now without the lifetime-extending wrapper closure, so I feel less bad about those. Inout-to-pointer conversions don't feel implicit because of the '&'.

These conversions are also geared toward interop with well-behaved C APIs, and have caused problems when they fire in unwanted contexts (for example, - ends up taking the pointer difference of the two arrays' buffers). We've discussed limiting those conversions in particular to imported APIs for these reasons.

For other conversions, the remaining implicit value-to-reference bridging conversion is up for review, and we've talked about tightening up when implicit optional wrapping can occur. IUOs are out of the type system, which is probably as far as they'll go. I think subclass-to-superclass and covariant function pointer conversions are likely to stay as is, though.

I think converting initialization from UnsafeMutablePointer<T> to UnsafePointer<T> is always safe, but all the other initializers may need to grow a label (under our rules). That doesn't mean there should be an implicit conversion for it, as convenient as it may be.

(The counter-movement is people who want implicit conversions from smaller to larger integer and floating-point types, so the tide may shift here.)

Yeah, UnsafeMutablePointer to UnsafePointer conversion feels to me like it fits in the same bucket of "safe" conversions as Int8-to-Int16 etc. There's a well-behaved unidirectional subtype-ish relationship between the types, so many of the problems with ad-hoc user-defined implicit conversions don't apply.

-Joe

···

On May 4, 2016, at 10:15 AM, Jordan Rose <jordan_rose@apple.com> wrote:

On May 4, 2016, at 10:07, Andrew Trick <atrick@apple.com> wrote:

On May 4, 2016, at 9:40 AM, Jordan Rose <jordan_rose@apple.com> wrote:

On May 4, 2016, at 09:18, Joe Groff via swift-evolution <swift-evolution@swift.org> wrote:

On May 3, 2016, at 9:39 PM, Andrew Trick via swift-evolution <swift-evolution@swift.org> wrote:

On May 3, 2016, at 8:56 PM, Chris Lattner <clattner@apple.com> wrote:

Andrew_Trick · 2016-05-04T17:34:10.000Z

Hello Swift community,

The review of "SE-0076: Add overrides taking an UnsafePointer source to non-destructive copying methods on UnsafeMutablePointer" begins now and runs through May 9. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0076-copying-to-unsafe-mutable-pointer-with-unsafe-pointer-source.md

* What is your evaluation of the proposal?

The new methods are needed, but they don’t need to be overloads. I have no idea why the argument type was originally declared Mutable.

func assignBackwardFrom(source: UnsafePointer<Pointee>, count: Int
)

func assignFrom(source: UnsafePointer<Pointee>, count: Int
)

func initializeFrom(source: UnsafePointer<Pointee>, count: Int)

FWIW: I made precisely this change a while back on an experimental branch while experimenting with UnsafePointer conversion. I don’t see a problem with it.

Implicit argument conversion from UnsafeMutablePointer<Pointee> to UnsafePointer<Pointee> is normal and extremely obvious.

Yeah, Andy's approach seems cleaner than overloading.

:-( …but it’s an implicit conversion. Which we’re trying to expunge from the language. (Sort of.)

I’ve heard exactly the opposite argument recently. Namely that explicit UnsafePointer construction indicates an “unsafe” cast (I personally don’t agree with that argument though).

Tangential: Which of our current implicit conversions are considered bad? I can’t think of a good alternative, particularly for String/Array to UnsafePointer.

String/Array to UnsafePointer can't be safely expressed in the language right now without the lifetime-extending wrapper closure, so I feel less bad about those. Inout-to-pointer conversions don't feel implicit because of the '&'.

For other conversions, the remaining implicit value-to-reference bridging conversion is up for review, and we've talked about tightening up when implicit optional wrapping can occur. IUOs are out of the type system, which is probably as far as they'll go. I think subclass-to-superclass and covariant function pointer conversions are likely to stay as is, though.

I think converting initialization from UnsafeMutablePointer<T> to UnsafePointer<T> is always safe, but all the other initializers may need to grow a label (under our rules). That doesn't mean there should be an implicit conversion for it, as convenient as it may be.

That’s good to hear. I completely agree that unsafe conversions need to grow a label.

Although I still somewhat prefer implicit Mutable to Immutable conversions not only because it is so convenient but because programmers will expect that behavior. It feels to me like we are mimicking a type qualifier. AFAIK it doesn’t introduce implementation complexity or strange corner cases.

-Andy

···

On May 4, 2016, at 10:15 AM, Jordan Rose <jordan_rose@apple.com> wrote:

On May 4, 2016, at 10:07, Andrew Trick <atrick@apple.com <mailto:atrick@apple.com>> wrote:

On May 4, 2016, at 9:40 AM, Jordan Rose <jordan_rose@apple.com <mailto:jordan_rose@apple.com>> wrote:

On May 4, 2016, at 09:18, Joe Groff via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

On May 3, 2016, at 9:39 PM, Andrew Trick via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

On May 3, 2016, at 8:56 PM, Chris Lattner <clattner@apple.com <mailto:clattner@apple.com>> wrote:

(The counter-movement is people who want implicit conversions from smaller to larger integer and floating-point types, so the tide may shift here.)

Jordan