[Review] SE-0127: Cleaning up stdlib Pointer and Buffer Routines


(Chris Lattner) #1

Hello Swift community,

The review of "SE-0127: Cleaning up stdlib Pointer and Buffer Routines" begins now and runs through July 24. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0127-cleaning-up-stdlib-ptr-buffer.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


(Bob Wilson) #2

I have been looking at the parts of this proposal related to withUnsafe[Mutable]Pointer:

- https://bugs.swift.org/browse/SR-1937: <https://bugs.swift.org/browse/SR-1937:> withUnsafePointer(&x) should have an argument label 'to:’
- https://bugs.swift.org/browse/SR-1956: <https://bugs.swift.org/browse/SR-1956:> `withUnsafePointer` shouldn't take its argument as `inout`

Adding the “to:” argument label seems like an obviously good thing to do for the one-argument version of these functions. Besides that, I'd like to suggest that we remove the 2- and 3-argument versions: withUnsafe[Mutable]Pointers. The naming is already different than the 1-argument functions (plural vs. singular) and the argument label doesn't work so well with multiple arguments. These extra functions are not a complete solution (if you need more than 3 arguments) and they're not used much -- there is only one real use in the stdlib, with one more use in a validation-test. We can replace them by composing nested uses of the 1-argument functions in the few places where they are needed. I have already prototyped that and it seems to work fine.

It is not so clear what to do about SR-1956. (Charlie and I had some comments on this in https://github.com/apple/swift-evolution/pull/437.) Jordan raised the objection that when using withUnsafePointer with a global, there is an expectation that you’ll get the same address every time. Removing inout would cause the argument to be passed by value and the address would refer to a copy. Dmitri agreed that this could be a problem. On the other hand, if you don’t care about the address, or if you’re not using a value type, it would indeed be convenient to have a version of withUnsafePointer that does not require an inout argument.

Option 1: Keep inout (not addressing SR-1956). In this case, there’s no reason to have both withUnsafePointer and withUnsafeMutablePointer. If you want to call a function that expects an UnsafePointer, you can give it an UnsafeMutablePointer and there will be an implicit conversion to make it work. I discussed this with Apple’s stdlib team and they recommended that if we have only one function we use the shorter name “withUnsafePointer” and have it use an UnsafeMutablePointer.

Option 2: Fix SR-1956 and have two functions, one with inout and the other not. This would address the inconvenience of not being able to use withUnsafePointer with immutable values, while still supporting the existing behavior. The question then would be what to call these two functions.

- Option 2a. Combine the two existing functions as in Option 1 and use a new name for the non-inout version, e.g., withUnsafePointer(toCopyOf:), so that it won’t be confused with the old function. (That particular name doesn’t work very well when dealing with references to objects, since the object itself would not be copied. I haven’t yet come up with a better name, though.) One advantage of this approach is that we would not need to rush the new function into Swift 3 since it would be an additive change.

- Option 2b. Switch to use withUnsafeMutablePointer for all the cases where you care about the getting the same address. Change withUnsafePointer to be the non-inout version. Charlie suggested that we could have the migrator convert all existing uses on withUnsafePointer in Swift 2 code to use withUnsafeMutablePointer in Swift 3, but I’m not sure how well that would work.


(Dmitri Gribenko) #3

I have been looking at the parts of this proposal related to
withUnsafe[Mutable]Pointer:

[...]

I agree with everything that Bob said, and I would like to comment on this part:

unsafeAddressOf is removed, in favor of adding a unsafeAddress field on ObjectIdentifier. ObjectIdentifier already contains a raw pointer in the internal _value field and can be initialized with AnyObject just like the argument of unsafeAddressOf.

I think we should not add the ObjectIdentifier.unsafeAddress API. I
don't agree with the motivation from the proposal:

Remove unsafeAddressOf and use Unmanaged.takeUnretainedValue(_:slight_smile: instead. This, however, requires the caller to deal with retain logic for something as simple as getting an object address.

We want users to be explicit about their reference counting semantics
when working unsafely with object addresses. Otherwise it is not
clear for how long the resulting pointer is valid. Getting an unsafe
object address is not "simple", it is not commonly when working with
Swift or Objective-C APIs, and there should be no need to have
shorthand convenience syntax for it. The current way to perform
manual reference counting and bridging of objects to the unsafe world
is through Unmanaged, so the conversion from object to a pointer
should be on Unmanaged (where it is now).

Dmitri

···

On Fri, Jul 22, 2016 at 11:16 AM, Bob Wilson via swift-evolution <swift-evolution@swift.org> wrote:

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/


(Dave Abrahams) #4

It is not so clear what to do about SR-1956. (Charlie and I had some
comments on this in https://github.com/apple/swift-evolution/pull/437
<https://github.com/apple/swift-evolution/pull/437>.) Jordan raised
the objection that when using withUnsafePointer with a global, there
is an expectation that you’ll get the same address every
time. Removing inout would cause the argument to be passed by value
and the address would refer to a copy. Dmitri agreed that this could
be a problem. On the other hand, if you don’t care about the address,
or if you’re not using a value type, it would indeed be convenient to
have a version of withUnsafePointer that does not require an inout
argument.

Option 1: Keep inout (not addressing SR-1956). In this case, there’s
no reason to have both withUnsafePointer and
withUnsafeMutablePointer. If you want to call a function that expects
an UnsafePointer, you can give it an UnsafeMutablePointer and there
will be an implicit conversion to make it work. I discussed this with
Apple’s stdlib team and they recommended that if we have only one
function we use the shorter name “withUnsafePointer” and have it use
an UnsafeMutablePointer.

Very much in favor of Option 1.

Option 2: Fix SR-1956 and have two functions, one with inout and the
other not. This would address the inconvenience of not being able to
use withUnsafePointer with immutable values, while still supporting
the existing behavior. The question then would be what to call these
two functions.

We do not need to support new use-cases in this release, and this would
be unsatisfying because the “address of a global” property that Jordan
argued for would not hold for the immutable version.

- Option 2a. Combine the two existing functions as in Option 1 and use
a new name for the non-inout version, e.g.,
withUnsafePointer(toCopyOf:), so that it won’t be confused with the
old function. (That particular name doesn’t work very well when
dealing with references to objects, since the object itself would not
be copied. I haven’t yet come up with a better name, though.) One
advantage of this approach is that we would not need to rush the new
function into Swift 3 since it would be an additive change.

Not rushing that into Swift 3 is the same as Option 1.

- Option 2b. Switch to use withUnsafeMutablePointer for all the cases
where you care about the getting the same address. Change
withUnsafePointer to be the non-inout version. Charlie suggested that
we could have the migrator convert all existing uses on
withUnsafePointer in Swift 2 code to use withUnsafeMutablePointer in
Swift 3, but I’m not sure how well that would work.

That's exactly the same outcome, with respect to the language/library
surface, as Option 2 AFAICT. Can we simplify this list of options?

···

on Fri Jul 22 2016, Bob Wilson <swift-evolution@swift.org> wrote:

--
Dave


(Xiaodi Wu) #5

> It is not so clear what to do about SR-1956. (Charlie and I had some
> comments on this in https://github.com/apple/swift-evolution/pull/437
> <https://github.com/apple/swift-evolution/pull/437>.) Jordan raised
> the objection that when using withUnsafePointer with a global, there
> is an expectation that you’ll get the same address every
> time. Removing inout would cause the argument to be passed by value
> and the address would refer to a copy. Dmitri agreed that this could
> be a problem. On the other hand, if you don’t care about the address,
> or if you’re not using a value type, it would indeed be convenient to
> have a version of withUnsafePointer that does not require an inout
> argument.
>
> Option 1: Keep inout (not addressing SR-1956). In this case, there’s
> no reason to have both withUnsafePointer and
> withUnsafeMutablePointer. If you want to call a function that expects
> an UnsafePointer, you can give it an UnsafeMutablePointer and there
> will be an implicit conversion to make it work. I discussed this with
> Apple’s stdlib team and they recommended that if we have only one
> function we use the shorter name “withUnsafePointer” and have it use
> an UnsafeMutablePointer.

Very much in favor of Option 1.

Ditto, except that I think there is some value in keeping both (i.e. doing
nothing): allowing the user to document intent. It would be inconsistent
and potentially confusing to call the function that returns an
`UnsafeMutablePointer` `withUnsafePointer`. It's rarely used enough, and
the shorter name needlessly raises the question of where I'm really
"supposed to be" mutating the pointee. I've not had to use these functions
much, but the distinction between `Array.withUnsafeBufferPointer(_:)` and
`Array.withUnsafeMutableBufferPointer(_:)` has conditioned me to mutate the
pointee using only "mutable" functions.

···

On Fri, Jul 22, 2016 at 5:06 PM, Dave Abrahams via swift-evolution < swift-evolution@swift.org> wrote:

on Fri Jul 22 2016, Bob Wilson <swift-evolution@swift.org> wrote:

>
> Option 2: Fix SR-1956 and have two functions, one with inout and the
> other not. This would address the inconvenience of not being able to
> use withUnsafePointer with immutable values, while still supporting
> the existing behavior. The question then would be what to call these
> two functions.

We do not need to support new use-cases in this release, and this would
be unsatisfying because the “address of a global” property that Jordan
argued for would not hold for the immutable version.

> - Option 2a. Combine the two existing functions as in Option 1 and use
> a new name for the non-inout version, e.g.,
> withUnsafePointer(toCopyOf:), so that it won’t be confused with the
> old function. (That particular name doesn’t work very well when
> dealing with references to objects, since the object itself would not
> be copied. I haven’t yet come up with a better name, though.) One
> advantage of this approach is that we would not need to rush the new
> function into Swift 3 since it would be an additive change.

Not rushing that into Swift 3 is the same as Option 1.

> - Option 2b. Switch to use withUnsafeMutablePointer for all the cases
> where you care about the getting the same address. Change
> withUnsafePointer to be the non-inout version. Charlie suggested that
> we could have the migrator convert all existing uses on
> withUnsafePointer in Swift 2 code to use withUnsafeMutablePointer in
> Swift 3, but I’m not sure how well that would work.

That's exactly the same outcome, with respect to the language/library
surface, as Option 2 AFAICT. Can we simplify this list of options?

--
Dave

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


(Charlie Monroe) #6

I have been looking at the parts of this proposal related to
withUnsafe[Mutable]Pointer:

[...]

I agree with everything that Bob said, and I would like to comment on this part:

unsafeAddressOf is removed, in favor of adding a unsafeAddress field on ObjectIdentifier. ObjectIdentifier already contains a raw pointer in the internal _value field and can be initialized with AnyObject just like the argument of unsafeAddressOf.

I think we should not add the ObjectIdentifier.unsafeAddress API. I
don't agree with the motivation from the proposal:

Remove unsafeAddressOf and use Unmanaged.takeUnretainedValue(_:slight_smile: instead. This, however, requires the caller to deal with retain logic for something as simple as getting an object address.

We want users to be explicit about their reference counting semantics
when working unsafely with object addresses. Otherwise it is not
clear for how long the resulting pointer is valid. Getting an unsafe
object address is not "simple", it is not commonly when working with
Swift or Objective-C APIs, and there should be no need to have
shorthand convenience syntax for it. The current way to perform
manual reference counting and bridging of objects to the unsafe world
is through Unmanaged, so the conversion from object to a pointer
should be on Unmanaged (where it is now).

The general consensus on where the unsafeAddressOf is used the mosed has been settled (here in the original discussion and what I've googled on Stackoverflow) on that it's mostly used for logging an object address as part of description or debugDescription (or various other debugging purposes).

In the original discussion about the pointer and buffer routines cleanup here Jordan suggested using ObjectIdentifier instead (http://article.gmane.org/gmane.comp.lang.swift.evolution/23168) - and I have to agree with him. Not that the entire ObjectIdentifier should be interpoled into the description string, but that ObjectIdentifier expresses what you want.

The ObjectIdentifier IMHO has potential for more - a lot of various debugging purposes come to mind since it can point to an object that is no longer allocated. In this sense, it could also hold dynamicType of the object it was created with, but that's purely additive, so I left it out of this proposal.

I still believe that the ObjectIdentifier is missing the "unsafeAddress" property that would expose the already-contained internal raw pointer. And for most uses of the current unsafeAddressOf, this is the equivalent behavior.

As Xiaodi has mentioned as an argument for keeping both withUnsafePointer and withUnsafeMutablePointer for the sake of readibility and expressing your intentions, this is a similar case:

print(Unmanaged.takeUnretained(obj))

vs.

print(ObjectIdentifer(obj).unsafeAddress)

The first doesn't express the intentions at all, while the latter does. Using the first one seems like an abuse of the fact that the "Unretained" returns the same address - AFAIK, if you have an ObjC class that implements its own retain selector, it can theoretically return another value via takeRetained.

Or, another alternative is to use

unsafeBitCast(obj, to: UnsafePointer<Void>.self)

···

On Jul 22, 2016, at 10:37 PM, Dmitri Gribenko via swift-evolution <swift-evolution@swift.org> wrote:
On Fri, Jul 22, 2016 at 11:16 AM, Bob Wilson via swift-evolution > <swift-evolution@swift.org> wrote:

Dmitri

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Dave Abrahams) #7

> It is not so clear what to do about SR-1956. (Charlie and I had some
> comments on this in https://github.com/apple/swift-evolution/pull/437
> <https://github.com/apple/swift-evolution/pull/437>.) Jordan raised
> the objection that when using withUnsafePointer with a global, there
> is an expectation that you’ll get the same address every
> time. Removing inout would cause the argument to be passed by value
> and the address would refer to a copy. Dmitri agreed that this could
> be a problem. On the other hand, if you don’t care about the address,
> or if you’re not using a value type, it would indeed be convenient to
> have a version of withUnsafePointer that does not require an inout
> argument.
>
> Option 1: Keep inout (not addressing SR-1956). In this case, there’s
> no reason to have both withUnsafePointer and
> withUnsafeMutablePointer. If you want to call a function that expects
> an UnsafePointer, you can give it an UnsafeMutablePointer and there
> will be an implicit conversion to make it work. I discussed this with
> Apple’s stdlib team and they recommended that if we have only one
> function we use the shorter name “withUnsafePointer” and have it use
> an UnsafeMutablePointer.

Very much in favor of Option 1.

Ditto, except that I think there is some value in keeping both (i.e. doing
nothing): allowing the user to document intent. It would be inconsistent
and potentially confusing to call the function that returns an
`UnsafeMutablePointer` `withUnsafePointer`.

It doesn't return an `UnsafeMutablePointer`, it passes an
`UnsafeMutablePointer` to the body of the closure.

It's rarely used enough, and the shorter name needlessly raises the
question of where I'm really "supposed to be" mutating the
pointee.

I don't understand; you only have the pointee inside the closure.
That's where you mutate it (obviously?)

I've not had to use these functions much, but the distinction between
`Array.withUnsafeBufferPointer(_:)` and
`Array.withUnsafeMutableBufferPointer(_:)` has conditioned me to
mutate the pointee using only "mutable" functions.

Not sure if you're just drawing an analogy, but if not, those two
methods are not under discussion here. They are meaningfully different,
whereas the existing functions are not, and the one currently called
withUnsafePointer is always going to cause people to complain about
having to pass a mutable variable.

As a fallback position, I would suggest we only provide the mutating
one, but with its existing name. But I still prefer the shorter name.

···

on Fri Jul 22 2016, Xiaodi Wu <swift-evolution@swift.org> wrote:

On Fri, Jul 22, 2016 at 5:06 PM, Dave Abrahams via swift-evolution < > swift-evolution@swift.org> wrote:

on Fri Jul 22 2016, Bob Wilson <swift-evolution@swift.org> wrote:

>
> Option 2: Fix SR-1956 and have two functions, one with inout and the
> other not. This would address the inconvenience of not being able to
> use withUnsafePointer with immutable values, while still supporting
> the existing behavior. The question then would be what to call these
> two functions.

We do not need to support new use-cases in this release, and this would
be unsatisfying because the “address of a global” property that Jordan
argued for would not hold for the immutable version.

> - Option 2a. Combine the two existing functions as in Option 1 and use
> a new name for the non-inout version, e.g.,
> withUnsafePointer(toCopyOf:), so that it won’t be confused with the
> old function. (That particular name doesn’t work very well when
> dealing with references to objects, since the object itself would not
> be copied. I haven’t yet come up with a better name, though.) One
> advantage of this approach is that we would not need to rush the new
> function into Swift 3 since it would be an additive change.

Not rushing that into Swift 3 is the same as Option 1.

> - Option 2b. Switch to use withUnsafeMutablePointer for all the cases
> where you care about the getting the same address. Change
> withUnsafePointer to be the non-inout version. Charlie suggested that
> we could have the migrator convert all existing uses on
> withUnsafePointer in Swift 2 code to use withUnsafeMutablePointer in
> Swift 3, but I’m not sure how well that would work.

That's exactly the same outcome, with respect to the language/library
surface, as Option 2 AFAICT. Can we simplify this list of options?

--
Dave

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

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

--
Dave


(Xiaodi Wu) #8

>
>>
>>
>> > It is not so clear what to do about SR-1956. (Charlie and I had some
>> > comments on this in https://github.com/apple/swift-evolution/pull/437
>> > <https://github.com/apple/swift-evolution/pull/437>.) Jordan raised
>> > the objection that when using withUnsafePointer with a global, there
>> > is an expectation that you’ll get the same address every
>> > time. Removing inout would cause the argument to be passed by value
>> > and the address would refer to a copy. Dmitri agreed that this could
>> > be a problem. On the other hand, if you don’t care about the address,
>> > or if you’re not using a value type, it would indeed be convenient to
>> > have a version of withUnsafePointer that does not require an inout
>> > argument.
>> >
>> > Option 1: Keep inout (not addressing SR-1956). In this case, there’s
>> > no reason to have both withUnsafePointer and
>> > withUnsafeMutablePointer. If you want to call a function that expects
>> > an UnsafePointer, you can give it an UnsafeMutablePointer and there
>> > will be an implicit conversion to make it work. I discussed this with
>> > Apple’s stdlib team and they recommended that if we have only one
>> > function we use the shorter name “withUnsafePointer” and have it use
>> > an UnsafeMutablePointer.
>>
>> Very much in favor of Option 1.
>>
>
> Ditto, except that I think there is some value in keeping both (i.e.
doing
> nothing): allowing the user to document intent. It would be inconsistent
> and potentially confusing to call the function that returns an
> `UnsafeMutablePointer` `withUnsafePointer`.

It doesn't return an `UnsafeMutablePointer`, it passes an
`UnsafeMutablePointer` to the body of the closure.

Brainfart. Yes, that's what I meant to write. Sorry.

> It's rarely used enough, and the shorter name needlessly raises the
> question of where I'm really "supposed to be" mutating the
> pointee.

I don't understand; you only have the pointee inside the closure.
That's where you mutate it (obviously?)

If my closure does not mutate the pointee, `withUnsafePointer(_:)` allows
me to document that. Everything *works* with
`withUnsafeMutablePointer(_:)`, but I cannot read the code and understand
that no mutation has happened within the body of the closure. [Am I wrong
on this?]

For instance, I've been working with some of the Accelerate.framework
functions and the arguments are often cryptic. Take this call:

cblas_sgemm(CblasColMajor, CblasNoTrans, CblasNoTrans, m, n, k, 1, matrix,
m, b, k, 1, &c, m)

There are times when I'd want to
call `cblas_sgemm(_:_:_:_:_:_:_:_:_:_:_:_:_:_:)` inside an
`withUnsafe[Mutable]Pointer(_:)` closure. Distinguishing
`withUnsafePointer(_:)` and `withUnsafeMutablePointer(_:)` would allow a
reader to know from the outset if `$0.pointee` is mutated without having to
know that the second-from-last argument is the one that stores the result
(it is not consistently second-from-last; for vDSP_* functions, it's often
the third-from-last argument, and for others it can be the first argument).
Removing the current `withUnsafePointer(_:)` would decrease clarity for the
reader here.

I've not had to use these functions much, but the distinction between
> `Array.withUnsafeBufferPointer(_:)` and
> `Array.withUnsafeMutableBufferPointer(_:)` has conditioned me to
> mutate the pointee using only "mutable" functions.

Not sure if you're just drawing an analogy,

I was trying to. I guess ineffectively.

···

On Fri, Jul 22, 2016 at 6:49 PM, Dave Abrahams via swift-evolution < swift-evolution@swift.org> wrote:

on Fri Jul 22 2016, Xiaodi Wu <swift-evolution@swift.org> wrote:
> On Fri, Jul 22, 2016 at 5:06 PM, Dave Abrahams via swift-evolution < > > swift-evolution@swift.org> wrote:
>> on Fri Jul 22 2016, Bob Wilson <swift-evolution@swift.org> wrote:

but if not, those two
methods are not under discussion here. They are meaningfully different,
whereas the existing functions are not, and the one currently called
withUnsafePointer is always going to cause people to complain about
having to pass a mutable variable.

As a fallback position, I would suggest we only provide the mutating
one, but with its existing name. But I still prefer the shorter name.

>
>> >
>> > Option 2: Fix SR-1956 and have two functions, one with inout and the
>> > other not. This would address the inconvenience of not being able to
>> > use withUnsafePointer with immutable values, while still supporting
>> > the existing behavior. The question then would be what to call these
>> > two functions.
>>
>> We do not need to support new use-cases in this release, and this would
>> be unsatisfying because the “address of a global” property that Jordan
>> argued for would not hold for the immutable version.
>>
>> > - Option 2a. Combine the two existing functions as in Option 1 and use
>> > a new name for the non-inout version, e.g.,
>> > withUnsafePointer(toCopyOf:), so that it won’t be confused with the
>> > old function. (That particular name doesn’t work very well when
>> > dealing with references to objects, since the object itself would not
>> > be copied. I haven’t yet come up with a better name, though.) One
>> > advantage of this approach is that we would not need to rush the new
>> > function into Swift 3 since it would be an additive change.
>>
>> Not rushing that into Swift 3 is the same as Option 1.
>>
>> > - Option 2b. Switch to use withUnsafeMutablePointer for all the cases
>> > where you care about the getting the same address. Change
>> > withUnsafePointer to be the non-inout version. Charlie suggested that
>> > we could have the migrator convert all existing uses on
>> > withUnsafePointer in Swift 2 code to use withUnsafeMutablePointer in
>> > Swift 3, but I’m not sure how well that would work.
>>
>> That's exactly the same outcome, with respect to the language/library
>> surface, as Option 2 AFAICT. Can we simplify this list of options?
>>
>> --
>> Dave
>>
>> _______________________________________________
>> swift-evolution mailing list
>> swift-evolution@swift.org
>> https://lists.swift.org/mailman/listinfo/swift-evolution
>>
> _______________________________________________
> swift-evolution mailing list
> swift-evolution@swift.org
> https://lists.swift.org/mailman/listinfo/swift-evolution
>

--
Dave

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


(Xiaodi Wu) #9

>
>> I have been looking at the parts of this proposal related to
>> withUnsafe[Mutable]Pointer:
>>
> [...]
>
> I agree with everything that Bob said, and I would like to comment on
this part:
>
>> unsafeAddressOf is removed, in favor of adding a unsafeAddress field on
ObjectIdentifier. ObjectIdentifier already contains a raw pointer in the
internal _value field and can be initialized with AnyObject just like the
argument of unsafeAddressOf.
>
> I think we should not add the ObjectIdentifier.unsafeAddress API. I
> don't agree with the motivation from the proposal:
>
>> Remove unsafeAddressOf and use Unmanaged.takeUnretainedValue(_:slight_smile:
instead. This, however, requires the caller to deal with retain logic for
something as simple as getting an object address.
>
> We want users to be explicit about their reference counting semantics
> when working unsafely with object addresses. Otherwise it is not
> clear for how long the resulting pointer is valid. Getting an unsafe
> object address is not "simple", it is not commonly when working with
> Swift or Objective-C APIs, and there should be no need to have
> shorthand convenience syntax for it. The current way to perform
> manual reference counting and bridging of objects to the unsafe world
> is through Unmanaged, so the conversion from object to a pointer
> should be on Unmanaged (where it is now).

The general consensus on where the unsafeAddressOf is used the mosed has
been settled (here in the original discussion and what I've googled on
Stackoverflow) on that it's mostly used for logging an object address as
part of description or debugDescription (or various other debugging
purposes).

In the original discussion about the pointer and buffer routines cleanup
here Jordan suggested using ObjectIdentifier instead (
http://article.gmane.org/gmane.comp.lang.swift.evolution/23168) - and I
have to agree with him. Not that the entire ObjectIdentifier should be
interpoled into the description string, but that ObjectIdentifier expresses
what you want.

ObjectIdentifier itself conforms to Hashable, and I recently patched it to
be CustomDebugStringConvertible, so that you can not only compare
ObjectIdentifier instances to know if objects live at the same address, you
can now log a unique debug description for each instance. That should be
sufficient for most use cases you describe above. If you actually need the
*address* and not just some identifier for the object (presumably because
you'll use that information to do something at that address), then surely
you should explicitly indicate what you're doing about reference counting.

···

On Sat, Jul 23, 2016 at 1:52 AM, Charlie Monroe via swift-evolution < swift-evolution@swift.org> wrote:

> On Jul 22, 2016, at 10:37 PM, Dmitri Gribenko via swift-evolution < > swift-evolution@swift.org> wrote:
> On Fri, Jul 22, 2016 at 11:16 AM, Bob Wilson via swift-evolution > > <swift-evolution@swift.org> wrote:

The ObjectIdentifier IMHO has potential for more - a lot of various
debugging purposes come to mind since it can point to an object that is no
longer allocated. In this sense, it could also hold dynamicType of the
object it was created with, but that's purely additive, so I left it out of
this proposal.

I still believe that the ObjectIdentifier is missing the "unsafeAddress"
property that would expose the already-contained internal raw pointer. And
for most uses of the current unsafeAddressOf, this is the equivalent
behavior.

As Xiaodi has mentioned as an argument for keeping both withUnsafePointer
and withUnsafeMutablePointer for the sake of readibility and expressing
your intentions, this is a similar case:

print(Unmanaged.takeUnretained(obj))

vs.

print(ObjectIdentifer(obj).unsafeAddress)

The first doesn't express the intentions at all, while the latter does.
Using the first one seems like an abuse of the fact that the "Unretained"
returns the same address - AFAIK, if you have an ObjC class that implements
its own retain selector, it can theoretically return another value via
takeRetained.

Or, another alternative is to use

unsafeBitCast(obj, to: UnsafePointer<Void>.self)

>
> Dmitri
>
> --
> main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
> (j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/
> _______________________________________________
> swift-evolution mailing list
> swift-evolution@swift.org
> https://lists.swift.org/mailman/listinfo/swift-evolution

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


(Charlie Monroe) #10

> We want users to be explicit about their reference counting semantics
> when working unsafely with object addresses. Otherwise it is not
> clear for how long the resulting pointer is valid. Getting an unsafe
> object address is not "simple", it is not commonly when working with
> Swift or Objective-C APIs, and there should be no need to have
> shorthand convenience syntax for it. The current way to perform
> manual reference counting and bridging of objects to the unsafe world
> is through Unmanaged, so the conversion from object to a pointer
> should be on Unmanaged (where it is now).

Thanks for the patch. Nevertheless, see how ObjC prints an object:

<NSView: 0x7fc9d11b2640>

Using the ObjectIdentifier to mimic this behavior (using current implementation), you get

<NSView: ObjectIdentifier(0x7fc9d11b2640)>

Not everyone needs to be happy with this. Yes, you could really do this now:

let identifier = ObjectIdentifier(obj)
let ptr = (nil as UnsafePointer<Void>).advancedBy(Int(identifier.uintValue))

Since you can get the uintValue, which is in fact the numeric value of the pointer, I don't see the harm in exposing the pointer value as well, explicitly naming it "unsafeAddress".

···

The general consensus on where the unsafeAddressOf is used the mosed has been settled (here in the original discussion and what I've googled on Stackoverflow) on that it's mostly used for logging an object address as part of description or debugDescription (or various other debugging purposes).

In the original discussion about the pointer and buffer routines cleanup here Jordan suggested using ObjectIdentifier instead (http://article.gmane.org/gmane.comp.lang.swift.evolution/23168) - and I have to agree with him. Not that the entire ObjectIdentifier should be interpoled into the description string, but that ObjectIdentifier expresses what you want.

ObjectIdentifier itself conforms to Hashable, and I recently patched it to be CustomDebugStringConvertible, so that you can not only compare ObjectIdentifier instances to know if objects live at the same address, you can now log a unique debug description for each instance. That should be sufficient for most use cases you describe above. If you actually need the *address* and not just some identifier for the object (presumably because you'll use that information to do something at that address), then surely you should explicitly indicate what you're doing about reference counting.

The ObjectIdentifier IMHO has potential for more - a lot of various debugging purposes come to mind since it can point to an object that is no longer allocated. In this sense, it could also hold dynamicType of the object it was created with, but that's purely additive, so I left it out of this proposal.

I still believe that the ObjectIdentifier is missing the "unsafeAddress" property that would expose the already-contained internal raw pointer. And for most uses of the current unsafeAddressOf, this is the equivalent behavior.

As Xiaodi has mentioned as an argument for keeping both withUnsafePointer and withUnsafeMutablePointer for the sake of readibility and expressing your intentions, this is a similar case:

print(Unmanaged.takeUnretained(obj))

vs.

print(ObjectIdentifer(obj).unsafeAddress)

The first doesn't express the intentions at all, while the latter does. Using the first one seems like an abuse of the fact that the "Unretained" returns the same address - AFAIK, if you have an ObjC class that implements its own retain selector, it can theoretically return another value via takeRetained.

Or, another alternative is to use

unsafeBitCast(obj, to: UnsafePointer<Void>.self)

>
> Dmitri
>
> --
> main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
> (j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com <mailto:gribozavr@gmail.com>>*/
> _______________________________________________
> swift-evolution mailing list
> swift-evolution@swift.org <mailto:swift-evolution@swift.org>
> https://lists.swift.org/mailman/listinfo/swift-evolution

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


(Dave Abrahams) #11

>
>>
>>
>> > It is not so clear what to do about SR-1956. (Charlie and I had some
>> > comments on this in https://github.com/apple/swift-evolution/pull/437
>> > <https://github.com/apple/swift-evolution/pull/437>.) Jordan raised
>> > the objection that when using withUnsafePointer with a global, there
>> > is an expectation that you’ll get the same address every
>> > time. Removing inout would cause the argument to be passed by value
>> > and the address would refer to a copy. Dmitri agreed that this could
>> > be a problem. On the other hand, if you don’t care about the address,
>> > or if you’re not using a value type, it would indeed be convenient to
>> > have a version of withUnsafePointer that does not require an inout
>> > argument.
>> >
>> > Option 1: Keep inout (not addressing SR-1956). In this case, there’s
>> > no reason to have both withUnsafePointer and
>> > withUnsafeMutablePointer. If you want to call a function that expects
>> > an UnsafePointer, you can give it an UnsafeMutablePointer and there
>> > will be an implicit conversion to make it work. I discussed this with
>> > Apple’s stdlib team and they recommended that if we have only one
>> > function we use the shorter name “withUnsafePointer” and have it use
>> > an UnsafeMutablePointer.
>>
>> Very much in favor of Option 1.
>>
>
> Ditto, except that I think there is some value in keeping both (i.e.
doing
> nothing): allowing the user to document intent. It would be inconsistent
> and potentially confusing to call the function that returns an
> `UnsafeMutablePointer` `withUnsafePointer`.

It doesn't return an `UnsafeMutablePointer`, it passes an
`UnsafeMutablePointer` to the body of the closure.

Brainfart. Yes, that's what I meant to write. Sorry.

> It's rarely used enough, and the shorter name needlessly raises the
> question of where I'm really "supposed to be" mutating the
> pointee.

I don't understand; you only have the pointee inside the closure.
That's where you mutate it (obviously?)

If my closure does not mutate the pointee, `withUnsafePointer(_:)` allows
me to document that. Everything *works* with
`withUnsafeMutablePointer(_:)`, but I cannot read the code and understand
that no mutation has happened within the body of the closure. [Am I wrong
on this?]

No, you're right.

For instance, I've been working with some of the Accelerate.framework
functions and the arguments are often cryptic. Take this call:

cblas_sgemm(CblasColMajor, CblasNoTrans, CblasNoTrans, m, n, k, 1, matrix,
m, b, k, 1, &c, m)

There are times when I'd want to
call `cblas_sgemm(_:_:_:_:_:_:_:_:_:_:_:_:_:_:)` inside an
`withUnsafe[Mutable]Pointer(_:)` closure. Distinguishing
`withUnsafePointer(_:)` and `withUnsafeMutablePointer(_:)` would allow a
reader to know from the outset if `$0.pointee is mutated without having to
know that the second-from-last argument is the one that stores the result
(it is not consistently second-from-last; for vDSP_* functions, it's often
the third-from-last argument, and for others it can be the first argument).
Removing the current `withUnsafePointer(_:)` would decrease clarity for the
reader here.

Okay, fair enough.

···

on Fri Jul 22 2016, Xiaodi Wu <xiaodi.wu-AT-gmail.com> wrote:

On Fri, Jul 22, 2016 at 6:49 PM, Dave Abrahams via swift-evolution < > swift-evolution@swift.org> wrote:

on Fri Jul 22 2016, Xiaodi Wu <swift-evolution@swift.org> wrote:
> On Fri, Jul 22, 2016 at 5:06 PM, Dave Abrahams via swift-evolution < >> > swift-evolution@swift.org> wrote:
>> on Fri Jul 22 2016, Bob Wilson <swift-evolution@swift.org> wrote:

I've not had to use these functions much, but the distinction between
> `Array.withUnsafeBufferPointer(_:)` and
> `Array.withUnsafeMutableBufferPointer(_:)` has conditioned me to
> mutate the pointee using only "mutable" functions.

Not sure if you're just drawing an analogy,

I was trying to. I guess ineffectively.

but if not, those two
methods are not under discussion here. They are meaningfully different,
whereas the existing functions are not, and the one currently called
withUnsafePointer is always going to cause people to complain about
having to pass a mutable variable.

As a fallback position, I would suggest we only provide the mutating
one, but with its existing name. But I still prefer the shorter name.

>
>> >
>> > Option 2: Fix SR-1956 and have two functions, one with inout and the
>> > other not. This would address the inconvenience of not being able to
>> > use withUnsafePointer with immutable values, while still supporting
>> > the existing behavior. The question then would be what to call these
>> > two functions.
>>
>> We do not need to support new use-cases in this release, and this would
>> be unsatisfying because the “address of a global” property that Jordan
>> argued for would not hold for the immutable version.
>>
>> > - Option 2a. Combine the two existing functions as in Option 1 and use
>> > a new name for the non-inout version, e.g.,
>> > withUnsafePointer(toCopyOf:), so that it won’t be confused with the
>> > old function. (That particular name doesn’t work very well when
>> > dealing with references to objects, since the object itself would not
>> > be copied. I haven’t yet come up with a better name, though.) One
>> > advantage of this approach is that we would not need to rush the new
>> > function into Swift 3 since it would be an additive change.
>>
>> Not rushing that into Swift 3 is the same as Option 1.
>>
>> > - Option 2b. Switch to use withUnsafeMutablePointer for all the cases
>> > where you care about the getting the same address. Change
>> > withUnsafePointer to be the non-inout version. Charlie suggested that
>> > we could have the migrator convert all existing uses on
>> > withUnsafePointer in Swift 2 code to use withUnsafeMutablePointer in
>> > Swift 3, but I’m not sure how well that would work.
>>
>> That's exactly the same outcome, with respect to the language/library
>> surface, as Option 2 AFAICT. Can we simplify this list of options?
>>
>> --
>> Dave
>>
>> _______________________________________________
>> swift-evolution mailing list
>> swift-evolution@swift.org
>> https://lists.swift.org/mailman/listinfo/swift-evolution
>>
> _______________________________________________
> swift-evolution mailing list
> swift-evolution@swift.org
> https://lists.swift.org/mailman/listinfo/swift-evolution
>

--
Dave

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

--
Dave


(Charlie Monroe) #12

Ok, I'll update the proposal to withdraw the change of inout for withUnsafePointer.

Under the discussion for the pull request, Bob mentioned possible removal of the multi-pointer variants (i.e. withUnsafe[Mutable]Pointers) - is this something that you'd agree on?

If so, I'd add this to the proposal since I agree that this is an API that is rarely used, can be used by nesting two withUnsafe[Mutable]Pointer calls and is limited to three pointers at max anyway. It almost feels like NSAssert1, NSAssert2, etc.

···

On Jul 23, 2016, at 3:35 AM, Dave Abrahams via swift-evolution <swift-evolution@swift.org> wrote:

on Fri Jul 22 2016, Xiaodi Wu <xiaodi.wu-AT-gmail.com> wrote:

On Fri, Jul 22, 2016 at 6:49 PM, Dave Abrahams via swift-evolution < >> swift-evolution@swift.org> wrote:

on Fri Jul 22 2016, Xiaodi Wu <swift-evolution@swift.org> wrote:

On Fri, Jul 22, 2016 at 5:06 PM, Dave Abrahams via swift-evolution < >>>> swift-evolution@swift.org> wrote:

on Fri Jul 22 2016, Bob Wilson <swift-evolution@swift.org> wrote:

It is not so clear what to do about SR-1956. (Charlie and I had some
comments on this in https://github.com/apple/swift-evolution/pull/437
<https://github.com/apple/swift-evolution/pull/437>.) Jordan raised
the objection that when using withUnsafePointer with a global, there
is an expectation that you’ll get the same address every
time. Removing inout would cause the argument to be passed by value
and the address would refer to a copy. Dmitri agreed that this could
be a problem. On the other hand, if you don’t care about the address,
or if you’re not using a value type, it would indeed be convenient to
have a version of withUnsafePointer that does not require an inout
argument.

Option 1: Keep inout (not addressing SR-1956). In this case, there’s
no reason to have both withUnsafePointer and
withUnsafeMutablePointer. If you want to call a function that expects
an UnsafePointer, you can give it an UnsafeMutablePointer and there
will be an implicit conversion to make it work. I discussed this with
Apple’s stdlib team and they recommended that if we have only one
function we use the shorter name “withUnsafePointer” and have it use
an UnsafeMutablePointer.

Very much in favor of Option 1.

Ditto, except that I think there is some value in keeping both (i.e.

doing

nothing): allowing the user to document intent. It would be inconsistent
and potentially confusing to call the function that returns an
`UnsafeMutablePointer` `withUnsafePointer`.

It doesn't return an `UnsafeMutablePointer`, it passes an
`UnsafeMutablePointer` to the body of the closure.

Brainfart. Yes, that's what I meant to write. Sorry.

It's rarely used enough, and the shorter name needlessly raises the
question of where I'm really "supposed to be" mutating the
pointee.

I don't understand; you only have the pointee inside the closure.
That's where you mutate it (obviously?)

If my closure does not mutate the pointee, `withUnsafePointer(_:)` allows
me to document that. Everything *works* with
`withUnsafeMutablePointer(_:)`, but I cannot read the code and understand
that no mutation has happened within the body of the closure. [Am I wrong
on this?]

No, you're right.

For instance, I've been working with some of the Accelerate.framework
functions and the arguments are often cryptic. Take this call:

cblas_sgemm(CblasColMajor, CblasNoTrans, CblasNoTrans, m, n, k, 1, matrix,
m, b, k, 1, &c, m)

There are times when I'd want to
call `cblas_sgemm(_:_:_:_:_:_:_:_:_:_:_:_:_:_:)` inside an
`withUnsafe[Mutable]Pointer(_:)` closure. Distinguishing
`withUnsafePointer(_:)` and `withUnsafeMutablePointer(_:)` would allow a
reader to know from the outset if `$0.pointee is mutated without having to
know that the second-from-last argument is the one that stores the result
(it is not consistently second-from-last; for vDSP_* functions, it's often
the third-from-last argument, and for others it can be the first argument).
Removing the current `withUnsafePointer(_:)` would decrease clarity for the
reader here.

Okay, fair enough.

I've not had to use these functions much, but the distinction between

`Array.withUnsafeBufferPointer(_:)` and
`Array.withUnsafeMutableBufferPointer(_:)` has conditioned me to
mutate the pointee using only "mutable" functions.

Not sure if you're just drawing an analogy,

I was trying to. I guess ineffectively.

but if not, those two
methods are not under discussion here. They are meaningfully different,
whereas the existing functions are not, and the one currently called
withUnsafePointer is always going to cause people to complain about
having to pass a mutable variable.

As a fallback position, I would suggest we only provide the mutating
one, but with its existing name. But I still prefer the shorter name.

Option 2: Fix SR-1956 and have two functions, one with inout and the
other not. This would address the inconvenience of not being able to
use withUnsafePointer with immutable values, while still supporting
the existing behavior. The question then would be what to call these
two functions.

We do not need to support new use-cases in this release, and this would
be unsatisfying because the “address of a global” property that Jordan
argued for would not hold for the immutable version.

- Option 2a. Combine the two existing functions as in Option 1 and use
a new name for the non-inout version, e.g.,
withUnsafePointer(toCopyOf:), so that it won’t be confused with the
old function. (That particular name doesn’t work very well when
dealing with references to objects, since the object itself would not
be copied. I haven’t yet come up with a better name, though.) One
advantage of this approach is that we would not need to rush the new
function into Swift 3 since it would be an additive change.

Not rushing that into Swift 3 is the same as Option 1.

- Option 2b. Switch to use withUnsafeMutablePointer for all the cases
where you care about the getting the same address. Change
withUnsafePointer to be the non-inout version. Charlie suggested that
we could have the migrator convert all existing uses on
withUnsafePointer in Swift 2 code to use withUnsafeMutablePointer in
Swift 3, but I’m not sure how well that would work.

That's exactly the same outcome, with respect to the language/library
surface, as Option 2 AFAICT. Can we simplify this list of options?

--
Dave

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

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

--
Dave

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

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


(Dave Abrahams) #13

Ok, I'll update the proposal to withdraw the change of inout for
withUnsafePointer.

We could also add

    withUnsafePointer(toCopyOf: x)

but that can also come later if we decide we need it.

Under the discussion for the pull request, Bob mentioned possible
removal of the multi-pointer variants
(i.e. withUnsafe[Mutable]Pointers) - is this something that you'd
agree on?

Absolutely.

···

on Fri Jul 22 2016, Charlie Monroe <charlie-AT-charliemonroe.net> wrote:

If so, I'd add this to the proposal since I agree that this is an API
that is rarely used, can be used by nesting two
withUnsafe[Mutable]Pointer calls and is limited to three pointers at
max anyway. It almost feels like NSAssert1, NSAssert2, etc.

On Jul 23, 2016, at 3:35 AM, Dave Abrahams via swift-evolution <swift-evolution@swift.org> wrote:

on Fri Jul 22 2016, Xiaodi Wu <xiaodi.wu-AT-gmail.com> wrote:

On Fri, Jul 22, 2016 at 6:49 PM, Dave Abrahams via swift-evolution < >>> swift-evolution@swift.org> wrote:

on Fri Jul 22 2016, Xiaodi Wu <swift-evolution@swift.org> wrote:

On Fri, Jul 22, 2016 at 5:06 PM, Dave Abrahams via swift-evolution < >>>>> swift-evolution@swift.org> wrote:

on Fri Jul 22 2016, Bob Wilson <swift-evolution@swift.org> wrote:

It is not so clear what to do about SR-1956. (Charlie and I had some
comments on this in https://github.com/apple/swift-evolution/pull/437
<https://github.com/apple/swift-evolution/pull/437>.) Jordan raised
the objection that when using withUnsafePointer with a global, there
is an expectation that you’ll get the same address every
time. Removing inout would cause the argument to be passed by value
and the address would refer to a copy. Dmitri agreed that this could
be a problem. On the other hand, if you don’t care about the address,
or if you’re not using a value type, it would indeed be convenient to
have a version of withUnsafePointer that does not require an inout
argument.

Option 1: Keep inout (not addressing SR-1956). In this case, there’s
no reason to have both withUnsafePointer and
withUnsafeMutablePointer. If you want to call a function that expects
an UnsafePointer, you can give it an UnsafeMutablePointer and there
will be an implicit conversion to make it work. I discussed this with
Apple’s stdlib team and they recommended that if we have only one
function we use the shorter name “withUnsafePointer” and have it use
an UnsafeMutablePointer.

Very much in favor of Option 1.

Ditto, except that I think there is some value in keeping both (i.e.

doing

nothing): allowing the user to document intent. It would be inconsistent
and potentially confusing to call the function that returns an
`UnsafeMutablePointer` `withUnsafePointer`.

It doesn't return an `UnsafeMutablePointer`, it passes an
`UnsafeMutablePointer` to the body of the closure.

Brainfart. Yes, that's what I meant to write. Sorry.

It's rarely used enough, and the shorter name needlessly raises the
question of where I'm really "supposed to be" mutating the
pointee.

I don't understand; you only have the pointee inside the closure.
That's where you mutate it (obviously?)

If my closure does not mutate the pointee, `withUnsafePointer(_:)` allows
me to document that. Everything *works* with
`withUnsafeMutablePointer(_:)`, but I cannot read the code and understand
that no mutation has happened within the body of the closure. [Am I wrong
on this?]

No, you're right.

For instance, I've been working with some of the Accelerate.framework
functions and the arguments are often cryptic. Take this call:

cblas_sgemm(CblasColMajor, CblasNoTrans, CblasNoTrans, m, n, k, 1, matrix,
m, b, k, 1, &c, m)

There are times when I'd want to
call `cblas_sgemm(_:_:_:_:_:_:_:_:_:_:_:_:_:_:)` inside an
`withUnsafe[Mutable]Pointer(_:)` closure. Distinguishing
`withUnsafePointer(_:)` and `withUnsafeMutablePointer(_:)` would allow a
reader to know from the outset if `$0.pointee is mutated without having to
know that the second-from-last argument is the one that stores the result
(it is not consistently second-from-last; for vDSP_* functions, it's often
the third-from-last argument, and for others it can be the first argument).
Removing the current `withUnsafePointer(_:)` would decrease clarity for the
reader here.

Okay, fair enough.

I've not had to use these functions much, but the distinction between

`Array.withUnsafeBufferPointer(_:)` and
`Array.withUnsafeMutableBufferPointer(_:)` has conditioned me to
mutate the pointee using only "mutable" functions.

Not sure if you're just drawing an analogy,

I was trying to. I guess ineffectively.

but if not, those two
methods are not under discussion here. They are meaningfully different,
whereas the existing functions are not, and the one currently called
withUnsafePointer is always going to cause people to complain about
having to pass a mutable variable.

As a fallback position, I would suggest we only provide the mutating
one, but with its existing name. But I still prefer the shorter name.

Option 2: Fix SR-1956 and have two functions, one with inout and the
other not. This would address the inconvenience of not being able to
use withUnsafePointer with immutable values, while still supporting
the existing behavior. The question then would be what to call these
two functions.

We do not need to support new use-cases in this release, and this would
be unsatisfying because the “address of a global” property that Jordan
argued for would not hold for the immutable version.

- Option 2a. Combine the two existing functions as in Option 1 and use
a new name for the non-inout version, e.g.,
withUnsafePointer(toCopyOf:), so that it won’t be confused with the
old function. (That particular name doesn’t work very well when
dealing with references to objects, since the object itself would not
be copied. I haven’t yet come up with a better name, though.) One
advantage of this approach is that we would not need to rush the new
function into Swift 3 since it would be an additive change.

Not rushing that into Swift 3 is the same as Option 1.

- Option 2b. Switch to use withUnsafeMutablePointer for all the cases
where you care about the getting the same address. Change
withUnsafePointer to be the non-inout version. Charlie suggested that
we could have the migrator convert all existing uses on
withUnsafePointer in Swift 2 code to use withUnsafeMutablePointer in
Swift 3, but I’m not sure how well that would work.

That's exactly the same outcome, with respect to the language/library
surface, as Option 2 AFAICT. Can we simplify this list of options?

--
Dave

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

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

--
Dave

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

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


(Charlie Monroe) #14

Ok, I'll update the proposal to withdraw the change of inout for
withUnsafePointer.

We could also add

   withUnsafePointer(toCopyOf: x)

but that can also come later if we decide we need it.

I don't think this is correct naming. It might be in case of structs, but in case of a pointer on the heap, I think this is misleading. It would be absolutely valid to write:

let obj = NSObject()
withUnsafePointer(toCopyOf: obj) { ptr in
  print(ptr)
}

and it doesn't create any copy of anything. I'd personally prefer renaming the current behavior to withUnsafePointer(byReferenceTo:) which is more descriptive.

Under the discussion for the pull request, Bob mentioned possible

removal of the multi-pointer variants
(i.e. withUnsafe[Mutable]Pointers) - is this something that you'd
agree on?

Absolutely.

I've updated the proposal and created a pull request.

···

If so, I'd add this to the proposal since I agree that this is an API
that is rarely used, can be used by nesting two
withUnsafe[Mutable]Pointer calls and is limited to three pointers at
max anyway. It almost feels like NSAssert1, NSAssert2, etc.

On Jul 23, 2016, at 3:35 AM, Dave Abrahams via swift-evolution <swift-evolution@swift.org> wrote:

on Fri Jul 22 2016, Xiaodi Wu <xiaodi.wu-AT-gmail.com> wrote:

On Fri, Jul 22, 2016 at 6:49 PM, Dave Abrahams via swift-evolution < >>>> swift-evolution@swift.org> wrote:

on Fri Jul 22 2016, Xiaodi Wu <swift-evolution@swift.org> wrote:

On Fri, Jul 22, 2016 at 5:06 PM, Dave Abrahams via swift-evolution < >>>>>> swift-evolution@swift.org> wrote:

on Fri Jul 22 2016, Bob Wilson <swift-evolution@swift.org> wrote:

It is not so clear what to do about SR-1956. (Charlie and I had some
comments on this in https://github.com/apple/swift-evolution/pull/437
<https://github.com/apple/swift-evolution/pull/437>.) Jordan raised
the objection that when using withUnsafePointer with a global, there
is an expectation that you’ll get the same address every
time. Removing inout would cause the argument to be passed by value
and the address would refer to a copy. Dmitri agreed that this could
be a problem. On the other hand, if you don’t care about the address,
or if you’re not using a value type, it would indeed be convenient to
have a version of withUnsafePointer that does not require an inout
argument.

Option 1: Keep inout (not addressing SR-1956). In this case, there’s
no reason to have both withUnsafePointer and
withUnsafeMutablePointer. If you want to call a function that expects
an UnsafePointer, you can give it an UnsafeMutablePointer and there
will be an implicit conversion to make it work. I discussed this with
Apple’s stdlib team and they recommended that if we have only one
function we use the shorter name “withUnsafePointer” and have it use
an UnsafeMutablePointer.

Very much in favor of Option 1.

Ditto, except that I think there is some value in keeping both (i.e.

doing

nothing): allowing the user to document intent. It would be inconsistent
and potentially confusing to call the function that returns an
`UnsafeMutablePointer` `withUnsafePointer`.

It doesn't return an `UnsafeMutablePointer`, it passes an
`UnsafeMutablePointer` to the body of the closure.

Brainfart. Yes, that's what I meant to write. Sorry.

It's rarely used enough, and the shorter name needlessly raises the
question of where I'm really "supposed to be" mutating the
pointee.

I don't understand; you only have the pointee inside the closure.
That's where you mutate it (obviously?)

If my closure does not mutate the pointee, `withUnsafePointer(_:)` allows
me to document that. Everything *works* with
`withUnsafeMutablePointer(_:)`, but I cannot read the code and understand
that no mutation has happened within the body of the closure. [Am I wrong
on this?]

No, you're right.

For instance, I've been working with some of the Accelerate.framework
functions and the arguments are often cryptic. Take this call:

cblas_sgemm(CblasColMajor, CblasNoTrans, CblasNoTrans, m, n, k, 1, matrix,
m, b, k, 1, &c, m)

There are times when I'd want to
call `cblas_sgemm(_:_:_:_:_:_:_:_:_:_:_:_:_:_:)` inside an
`withUnsafe[Mutable]Pointer(_:)` closure. Distinguishing
`withUnsafePointer(_:)` and `withUnsafeMutablePointer(_:)` would allow a
reader to know from the outset if `$0.pointee is mutated without having to
know that the second-from-last argument is the one that stores the result
(it is not consistently second-from-last; for vDSP_* functions, it's often
the third-from-last argument, and for others it can be the first argument).
Removing the current `withUnsafePointer(_:)` would decrease clarity for the
reader here.

Okay, fair enough.

I've not had to use these functions much, but the distinction between

`Array.withUnsafeBufferPointer(_:)` and
`Array.withUnsafeMutableBufferPointer(_:)` has conditioned me to
mutate the pointee using only "mutable" functions.

Not sure if you're just drawing an analogy,

I was trying to. I guess ineffectively.

but if not, those two
methods are not under discussion here. They are meaningfully different,
whereas the existing functions are not, and the one currently called
withUnsafePointer is always going to cause people to complain about
having to pass a mutable variable.

As a fallback position, I would suggest we only provide the mutating
one, but with its existing name. But I still prefer the shorter name.

Option 2: Fix SR-1956 and have two functions, one with inout and the
other not. This would address the inconvenience of not being able to
use withUnsafePointer with immutable values, while still supporting
the existing behavior. The question then would be what to call these
two functions.

We do not need to support new use-cases in this release, and this would
be unsatisfying because the “address of a global” property that Jordan
argued for would not hold for the immutable version.

- Option 2a. Combine the two existing functions as in Option 1 and use
a new name for the non-inout version, e.g.,
withUnsafePointer(toCopyOf:), so that it won’t be confused with the
old function. (That particular name doesn’t work very well when
dealing with references to objects, since the object itself would not
be copied. I haven’t yet come up with a better name, though.) One
advantage of this approach is that we would not need to rush the new
function into Swift 3 since it would be an additive change.

Not rushing that into Swift 3 is the same as Option 1.

- Option 2b. Switch to use withUnsafeMutablePointer for all the cases
where you care about the getting the same address. Change
withUnsafePointer to be the non-inout version. Charlie suggested that
we could have the migrator convert all existing uses on
withUnsafePointer in Swift 2 code to use withUnsafeMutablePointer in
Swift 3, but I’m not sure how well that would work.

That's exactly the same outcome, with respect to the language/library
surface, as Option 2 AFAICT. Can we simplify this list of options?

--
Dave

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

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

--
Dave

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

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


(Dave Abrahams) #15

Ok, I'll update the proposal to withdraw the change of inout for
withUnsafePointer.

We could also add

   withUnsafePointer(toCopyOf: x)

but that can also come later if we decide we need it.

I don't think this is correct naming. It might be in case of structs,
but in case of a pointer on the heap, I think this is misleading. It
would be absolutely valid to write:

let obj = NSObject()
withUnsafePointer(toCopyOf: obj) { ptr in
  print(ptr)
}

and it doesn't create any copy of anything.

No, it actually does create a copy of the argument. When you copy a
reference, that's a real thing, even though it doesn't copy the instance
the reference refers to.

I'd personally prefer renaming the current behavior to
withUnsafePointer(byReferenceTo:) which is more descriptive.

I don't see how that helps anything.

···

on Sun Jul 24 2016, Charlie Monroe <charlie-AT-charliemonroe.net> wrote:

Under the discussion for the pull request, Bob mentioned possible

removal of the multi-pointer variants
(i.e. withUnsafe[Mutable]Pointers) - is this something that you'd
agree on?

Absolutely.

I've updated the proposal and created a pull request.

If so, I'd add this to the proposal since I agree that this is an API
that is rarely used, can be used by nesting two
withUnsafe[Mutable]Pointer calls and is limited to three pointers at
max anyway. It almost feels like NSAssert1, NSAssert2, etc.

On Jul 23, 2016, at 3:35 AM, Dave Abrahams via swift-evolution <swift-evolution@swift.org> wrote:

on Fri Jul 22 2016, Xiaodi Wu <xiaodi.wu-AT-gmail.com> wrote:

On Fri, Jul 22, 2016 at 6:49 PM, Dave Abrahams via swift-evolution < >>>>> swift-evolution@swift.org> wrote:

on Fri Jul 22 2016, Xiaodi Wu <swift-evolution@swift.org> wrote:

On Fri, Jul 22, 2016 at 5:06 PM, Dave Abrahams via swift-evolution < >>>>>>> swift-evolution@swift.org> wrote:

on Fri Jul 22 2016, Bob Wilson <swift-evolution@swift.org> wrote:

It is not so clear what to do about SR-1956. (Charlie and I had some
comments on this in https://github.com/apple/swift-evolution/pull/437
<https://github.com/apple/swift-evolution/pull/437>.) Jordan raised
the objection that when using withUnsafePointer with a global, there
is an expectation that you’ll get the same address every
time. Removing inout would cause the argument to be passed by value
and the address would refer to a copy. Dmitri agreed that this could
be a problem. On the other hand, if you don’t care about the address,
or if you’re not using a value type, it would indeed be convenient to
have a version of withUnsafePointer that does not require an inout
argument.

Option 1: Keep inout (not addressing SR-1956). In this case, there’s
no reason to have both withUnsafePointer and
withUnsafeMutablePointer. If you want to call a function that expects
an UnsafePointer, you can give it an UnsafeMutablePointer and there
will be an implicit conversion to make it work. I discussed this with
Apple’s stdlib team and they recommended that if we have only one
function we use the shorter name “withUnsafePointer” and have it use
an UnsafeMutablePointer.

Very much in favor of Option 1.

Ditto, except that I think there is some value in keeping both (i.e.

doing

nothing): allowing the user to document intent. It would be inconsistent
and potentially confusing to call the function that returns an
`UnsafeMutablePointer` `withUnsafePointer`.

It doesn't return an `UnsafeMutablePointer`, it passes an
`UnsafeMutablePointer` to the body of the closure.

Brainfart. Yes, that's what I meant to write. Sorry.

It's rarely used enough, and the shorter name needlessly raises the
question of where I'm really "supposed to be" mutating the
pointee.

I don't understand; you only have the pointee inside the closure.
That's where you mutate it (obviously?)

If my closure does not mutate the pointee, `withUnsafePointer(_:)` allows
me to document that. Everything *works* with
`withUnsafeMutablePointer(_:)`, but I cannot read the code and understand
that no mutation has happened within the body of the closure. [Am I wrong
on this?]

No, you're right.

For instance, I've been working with some of the Accelerate.framework
functions and the arguments are often cryptic. Take this call:

cblas_sgemm(CblasColMajor, CblasNoTrans, CblasNoTrans, m, n, k, 1, matrix,
m, b, k, 1, &c, m)

There are times when I'd want to
call `cblas_sgemm(_:_:_:_:_:_:_:_:_:_:_:_:_:_:)` inside an
`withUnsafe[Mutable]Pointer(_:)` closure. Distinguishing
`withUnsafePointer(_:)` and `withUnsafeMutablePointer(_:)` would allow a
reader to know from the outset if `$0.pointee is mutated without having to
know that the second-from-last argument is the one that stores the result
(it is not consistently second-from-last; for vDSP_* functions, it's often
the third-from-last argument, and for others it can be the first argument).
Removing the current `withUnsafePointer(_:)` would decrease clarity for the
reader here.

Okay, fair enough.

I've not had to use these functions much, but the distinction between

`Array.withUnsafeBufferPointer(_:)` and
`Array.withUnsafeMutableBufferPointer(_:)` has conditioned me to
mutate the pointee using only "mutable" functions.

Not sure if you're just drawing an analogy,

I was trying to. I guess ineffectively.

but if not, those two
methods are not under discussion here. They are meaningfully different,
whereas the existing functions are not, and the one currently called
withUnsafePointer is always going to cause people to complain about
having to pass a mutable variable.

As a fallback position, I would suggest we only provide the mutating
one, but with its existing name. But I still prefer the shorter name.

Option 2: Fix SR-1956 and have two functions, one with inout and the
other not. This would address the inconvenience of not being able to
use withUnsafePointer with immutable values, while still supporting
the existing behavior. The question then would be what to call these
two functions.

We do not need to support new use-cases in this release, and this would
be unsatisfying because the “address of a global” property that Jordan
argued for would not hold for the immutable version.

- Option 2a. Combine the two existing functions as in Option 1 and use
a new name for the non-inout version, e.g.,
withUnsafePointer(toCopyOf:), so that it won’t be confused with the
old function. (That particular name doesn’t work very well when
dealing with references to objects, since the object itself would not
be copied. I haven’t yet come up with a better name, though.) One
advantage of this approach is that we would not need to rush the new
function into Swift 3 since it would be an additive change.

Not rushing that into Swift 3 is the same as Option 1.

- Option 2b. Switch to use withUnsafeMutablePointer for all the cases
where you care about the getting the same address. Change
withUnsafePointer to be the non-inout version. Charlie suggested that
we could have the migrator convert all existing uses on
withUnsafePointer in Swift 2 code to use withUnsafeMutablePointer in
Swift 3, but I’m not sure how well that would work.

That's exactly the same outcome, with respect to the language/library
surface, as Option 2 AFAICT. Can we simplify this list of options?

--
Dave

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

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

--
Dave

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

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