Proposal: Allow `[strong self]` capture in closures and remove the `self` requirement therein

gparker42 · December 15, 2015, 11:02pm

Swift currently requires that `self` be used explicitly inside closures, to help avoid bugs from unintentional capture. This is annoying when a closure uses `self` a lot. Closures should be allowed to name `[strong self]` in their capture list and thereafter not be required to write `self` everywhere.

I wrote code this weekend that looked something like this:

    data = ...
    running = true
    delegate.notifyBegin(data)

    dispatch_async(queue) {
        self.processData(self.data)
        self.running = false
        self.delegate.notifyEnd(self.data)
    }

Note the asymmetry: the dispatched code needs to use `self` and the non-dispatched code does not. It is clear that the dispatched closure captures `self`, but it's annoying that it needed to be mentioned five different times. The noise gets worse with longer closures. The annoyance gets worse when moving code in and out of dispatches or other closures, with lots of editing required each time.

The proposal would allow the same code to be written like this:

    data = ...
    running = true
    delegate.notifyBegin(data)

    dispatch_async(queue) {
        [strong self] in
        processData(data)
        running = false
        delegate.notifyEnd(data)
    }

Advantages:
* The dispatch'ed code looks like the non-dispatched code.
* The capture of `self` is still obvious.
* The code's action is clearer without the `self` noise.

Disadvantages:
* The capture behavior of self's properties is less obvious. For example, neither closure above captured its own copy of `self.data`, but that behavior is not immediately visible in the second closure.

What about [weak self] and [unowned self] ? I do not propose to change the `self` requirement for those closures. In the weak case it is critically important to know where `self` is accessed, because it could potentially become nil between any two accesses. Unowned self might be reasonable to change, but for simplicity I won't do so here.

···

--
Greg Parker gparker@apple.com Runtime Wrangler

Brent_Royal-Gordon · December 15, 2015, 11:14pm

Swift currently requires that `self` be used explicitly inside closures, to help avoid bugs from unintentional capture. This is annoying when a closure uses `self` a lot. Closures should be allowed to name `[strong self]` in their capture list and thereafter not be required to write `self` everywhere.

Big +1 from me. I’ve been meaning to suggest this.

What about [weak self] and [unowned self] ? I do not propose to change the `self` requirement for those closures. In the weak case it is critically important to know where `self` is accessed, because it could potentially become nil between any two accesses. Unowned self might be reasonable to change, but for simplicity I won't do so here.

`weak` is unfortunate, but your logic here makes sense; otherwise you’d have to write `map { $0.whatever() }`, which is even worse than `self?.whatever()`. I do think `unowned` makes sense to include, though.

···

--
Brent Royal-Gordon
Architechies

Drew_Crawford1 · December 15, 2015, 11:18pm

Big -1

Consider the following

//somewherelse.swift

var strongReference: Foo! = nil
func evil(foo: Foo) {
strongReference = foo
}

and

//Foo.swift
class Bar {
    var f: Foo
    dispatch_async(queue) { [strong self] in
         evil(f)
    }
}

IMHO, `evil(self.f)` is *far* better and *far* more likely to look like a bug. I feel so strongly about this that I would take the extraordinary step of banning `strong self` from my codebase. I have spent waaaaaaay too much time playing "spot func evil".

Obviously reasonable people can disagree about "more likely" but "this sort of retain cycle creep" is how we ended up with the problems in this post <http://sealedabstract.com/code/nsnotificationcenter-with-blocks-considered-harmful/>, and one of the secret advantages of Swift is those bugs are SO MUCH HARDER.

Additional compiler diagnostics may help here, (which I want to see regardless) but unless those come in as part of the same proposal I'm -1.

···

On Dec 15, 2015, at 5:02 PM, Greg Parker via swift-evolution <swift-evolution@swift.org> wrote:

Swift currently requires that `self` be used explicitly inside closures, to help avoid bugs from unintentional capture. This is annoying when a closure uses `self` a lot. Closures should be allowed to name `[strong self]` in their capture list and thereafter not be required to write `self` everywhere.

I wrote code this weekend that looked something like this:

   data = ...
   running = true
   delegate.notifyBegin(data)

   dispatch_async(queue) {
       self.processData(self.data)
       self.running = false
       self.delegate.notifyEnd(self.data)
   }

Note the asymmetry: the dispatched code needs to use `self` and the non-dispatched code does not. It is clear that the dispatched closure captures `self`, but it's annoying that it needed to be mentioned five different times. The noise gets worse with longer closures. The annoyance gets worse when moving code in and out of dispatches or other closures, with lots of editing required each time.

The proposal would allow the same code to be written like this:

   data = ...
   running = true
   delegate.notifyBegin(data)

   dispatch_async(queue) {
       [strong self] in
       processData(data)
       running = false
       delegate.notifyEnd(data)
   }

Advantages:
* The dispatch'ed code looks like the non-dispatched code.
* The capture of `self` is still obvious.
* The code's action is clearer without the `self` noise.

Disadvantages:
* The capture behavior of self's properties is less obvious. For example, neither closure above captured its own copy of `self.data`, but that behavior is not immediately visible in the second closure.

What about [weak self] and [unowned self] ? I do not propose to change the `self` requirement for those closures. In the weak case it is critically important to know where `self` is accessed, because it could potentially become nil between any two accesses. Unowned self might be reasonable to change, but for simplicity I won't do so here.

--
Greg Parker gparker@apple.com Runtime Wrangler

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

fluidsonic · December 16, 2015, 1:42am

+1 from me. Developers will likely only ever write the rather uncommon [strong
self] capture when they know what they are doing.
Misuse can happen with any feature, but it is unlikely in this case.

···

On Wed, Dec 16, 2015 at 12:02 AM, Greg Parker via swift-evolution < swift-evolution@swift.org> wrote:

Swift currently requires that `self` be used explicitly inside closures,
to help avoid bugs from unintentional capture. This is annoying when a
closure uses `self` a lot. Closures should be allowed to name `[strong
self]` in their capture list and thereafter not be required to write `self`
everywhere.

I wrote code this weekend that looked something like this:

    data = ...
    running = true
    delegate.notifyBegin(data)

    dispatch_async(queue) {
        self.processData(self.data)
        self.running = false
        self.delegate.notifyEnd(self.data)
    }

Note the asymmetry: the dispatched code needs to use `self` and the
non-dispatched code does not. It is clear that the dispatched closure
captures `self`, but it's annoying that it needed to be mentioned five
different times. The noise gets worse with longer closures. The annoyance
gets worse when moving code in and out of dispatches or other closures,
with lots of editing required each time.

The proposal would allow the same code to be written like this:

    data = ...
    running = true
    delegate.notifyBegin(data)

    dispatch_async(queue) {
        [strong self] in
        processData(data)
        running = false
        delegate.notifyEnd(data)
    }

Advantages:
* The dispatch'ed code looks like the non-dispatched code.
* The capture of `self` is still obvious.
* The code's action is clearer without the `self` noise.

Disadvantages:
* The capture behavior of self's properties is less obvious. For example,
neither closure above captured its own copy of `self.data`, but that
behavior is not immediately visible in the second closure.

What about [weak self] and [unowned self] ? I do not propose to change the
`self` requirement for those closures. In the weak case it is critically
important to know where `self` is accessed, because it could potentially
become nil between any two accesses. Unowned self might be reasonable to
change, but for simplicity I won't do so here.

--
Greg Parker gparker@apple.com Runtime Wrangler

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

salutis · December 17, 2015, 10:20pm

+1

I've been writing a lot of experimental UIKit animation code past couple of days and spent most of my time adding and removing "self".

R+

···

Sent from my iPhone

On 16 Dec 2015, at 00:02, Greg Parker via swift-evolution <swift-evolution@swift.org> wrote:

Swift currently requires that `self` be used explicitly inside closures, to help avoid bugs from unintentional capture. This is annoying when a closure uses `self` a lot. Closures should be allowed to name `[strong self]` in their capture list and thereafter not be required to write `self` everywhere.

I wrote code this weekend that looked something like this:

   data = ...
   running = true
   delegate.notifyBegin(data)

   dispatch_async(queue) {
       self.processData(self.data)
       self.running = false
       self.delegate.notifyEnd(self.data)
   }

Note the asymmetry: the dispatched code needs to use `self` and the non-dispatched code does not. It is clear that the dispatched closure captures `self`, but it's annoying that it needed to be mentioned five different times. The noise gets worse with longer closures. The annoyance gets worse when moving code in and out of dispatches or other closures, with lots of editing required each time.

The proposal would allow the same code to be written like this:

   data = ...
   running = true
   delegate.notifyBegin(data)

   dispatch_async(queue) {
       [strong self] in
       processData(data)
       running = false
       delegate.notifyEnd(data)
   }

Advantages:
* The dispatch'ed code looks like the non-dispatched code.
* The capture of `self` is still obvious.
* The code's action is clearer without the `self` noise.

Disadvantages:
* The capture behavior of self's properties is less obvious. For example, neither closure above captured its own copy of `self.data`, but that behavior is not immediately visible in the second closure.

What about [weak self] and [unowned self] ? I do not propose to change the `self` requirement for those closures. In the weak case it is critically important to know where `self` is accessed, because it could potentially become nil between any two accesses. Unowned self might be reasonable to change, but for simplicity I won't do so here.

--
Greg Parker gparker@apple.com Runtime Wrangler

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

gparker42 · December 15, 2015, 11:49pm

Can you explain what is so evil about func evil() when it is called from an asynchronously-executed closure? I don't see an obvious bug here.

···

On Dec 15, 2015, at 3:18 PM, Drew Crawford <drew@sealedabstract.com> wrote:

Big -1

Consider the following

//somewherelse.swift

var strongReference: Foo! = nil
func evil(foo: Foo) {
    strongReference = foo
}

and

//Foo.swift
class Bar {
    var f: Foo
    dispatch_async(queue) { [strong self] in
         evil(f)
    }
}

IMHO, `evil(self.f)` is *far* better and *far* more likely to look like a bug. I feel so strongly about this that I would take the extraordinary step of banning `strong self` from my codebase. I have spent waaaaaaay too much time playing "spot func evil".

--
Greg Parker gparker@apple.com Runtime Wrangler

gparker42 · December 16, 2015, 1:15am

I think I see. Your contention is that `self.metadata` is only just barely a sufficient clue to the capture behavior, and that `[strong self]` at the top of the closure would no longer be good enough.

We could make it more difficult for unaware programmers to *write* the `[strong self]` version, at least. When a programmer writes just `metadata` in a closure, the compiler could continue to suggest using `self.metadata` and not suggest using `[strong self]`.

···

On Dec 15, 2015, at 5:02 PM, Drew Crawford <drew@sealedabstract.com> wrote:

Can you explain what is so evil about func evil() when it is called from an asynchronously-executed closure? I don't see an obvious bug here.

Apologies for earlier brevity, perhaps it would be helpful for me to present a more complete, realistic example. Paste the following into main.swift:

import Foundation

print("Hello, World!")

typealias evilArg = [String:String]
var strongReference: evilArg! = nil
func evil(foo:evilArg ) {
    strongReference = foo
}

final class Photo {
    var data = [UInt8](count: 100000000, repeatedValue: 0) //a large amount of data
    let metadata: [String: String] = [:] //a small amount of data
    func save() {
        dispatch_async(dispatch_get_global_queue(QOS_CLASS_DEFAULT, 0)) {
            evil(self.metadata)
        }
    }
}
let p = Photo()
p.save()
//leaks Photo, data, and metadata

let sema = dispatch_semaphore_create(0)
dispatch_semaphore_wait(sema, DISPATCH_TIME_FOREVER)

In this example, the memory usage at the end of the program is 100MB:

<Screen Shot 2015-12-15 at 6.26.52 PM.png>

This is because Photo, data, and metadata are all leaked by the evil function.

This is the (surprising?) behavior specified in the Swift Book:

[A capture occurs when] the closure’s body accesses a property of the instance, such as self.someProperty, or because the closure calls a method on the instance, such as self.someMethod(). In either case, these accesses cause the closure to “capture” self, creating a strong reference cycle.

Even though evil may seem (to the casual programmer who does not read language specifications for funsies) like the closure captures only the evil argument `metadata`, it *actually* captures Photo (and therefore data)

The capture of Photo (data) is somewhat clear when we write

evil(self.metadata)

But it is hidden when we write

evil(metadata)

As you propose.

I think that if we are going to have semantics that capture Photo (data), it had better look like it in a cursory inspection. The existing syntax is not as great as it could be, but it provides a clue.

--
Greg Parker gparker@apple.com Runtime Wrangler

Drew_Crawford1 · December 16, 2015, 1:12am

Can you explain what is so evil about func evil() when it is called from an asynchronously-executed closure?

Apologies for earlier brevity, perhaps it would be helpful for me to present a more complete, realistic example. Paste the following into main.swift:

import Foundation

print("Hello, World!")

typealias evilArg = [String:String]
var strongReference: evilArg! = nil
func evil(foo:evilArg ) {
strongReference = foo
}

final class Photo {
    var data = [UInt8](count: 100000000, repeatedValue: 0) //a large amount of data
    let metadata: [String: String] = [:] //a small amount of data
    func save() {
        dispatch_async(dispatch_get_global_queue(QOS_CLASS_DEFAULT, 0)) {
            evil(self.metadata)
        }
    }
}
let p = Photo()
p.save()
//leaks Photo, data, and metadata

let sema = dispatch_semaphore_create(0)
dispatch_semaphore_wait(sema, DISPATCH_TIME_FOREVER)

In this example, the memory usage at the end of the program is 100MB:

This is because Photo, data, and metadata are all leaked by the evil function.

This is the (surprising?) behavior specified in the Swift Book:

[A capture occurs when] the closure’s body accesses a property of the instance, such as self.someProperty, or because the closure calls a method on the instance, such as self.someMethod(). In either case, these accesses cause the closure to “capture” self, creating a strong reference cycle.

Even though evil may seem (to the casual programmer who does not read language specifications for funsies) like the closure captures only the evil argument `metadata`, it *actually* captures Photo (and therefore data)

The capture of Photo (data) is somewhat clear when we write

evil(self.metadata)

But it is hidden when we write

evil(metadata)

As you propose.

I think that if we are going to have semantics that capture Photo (data), it had better look like it in a cursory inspection. The existing syntax is not as great as it could be, but it provides a clue. I think even that the existing syntax isn't good enough, because I expect that many people are unaware of this particular dark corner. As you say:

Can you explain what is so evil about func evil() when it is called from an asynchronously-executed closure? I don't see an obvious bug here.

It may very well be that there is no *obvious* bug, and so what we may actually need is a proposal to make the programmer even more explicit when referencing self, not less.

But in any case. For `strong self` to make sense, we would need to do one of two things:

1. We could change Swift semantics to only capture metadata in this example, however this is a breaking change, and a very very nontrivial one. I do not know why it is specified this way in the first place, perhaps a designer can weigh in on that. I do know that ObjC is very similar, so there may be compatibility implications.

2. We could introduce additional syntax to provide compiler diagnostics to guard in this case, e.g.

func lessEvil(@noescape foo:evilArg ) { //@noescape currently not supported for non-closure parameters
self.strongReference = foo //should generate diagnostic about escaping a @noescape parameter
}

and then

dispatch_async(dispatch_get_global_queue(QOS_CLASS_DEFAULT, 0)) { [strong self] in
evil(metadata) //should generate diagnostic that self is required for functions without @noescape
lessEvil(metadata) //no diagnostic since parameter is @noescape
}

I think both of these fixes to the proposal are fairly impractical, but they would weaken my vote to -0.5.

Drew_Crawford1 · December 16, 2015, 1:23am

That's not (especially) the danger I'm worried about.

I'm worried about the case where I write `[strong self] in` and then years later I add the `evil(f)` to the bottom of the closure, because it turns out we need more code in there.

I'm also worried about the case where I write nonEvil(f) but at some later time the implementation of nonEvil(f) becomes evil.

I'm also worried about the case where nonEvil(f) is a call into closed-source UIKit and without any visibility to me, UIKit's implementation of nonEvil becomes evil between iOS10b3 and iOS10b4.

These things are "category 5" bugs waiting to happen, and they can't be competently addressed with a UI-level fix in Xcode.

···

On Dec 15, 2015, at 7:15 PM, Greg Parker <gparker@apple.com> wrote:

We could make it more difficult for unaware programmers to *write* the `[strong self]` version, at least. When a programmer writes just `metadata` in a closure, the compiler could continue to suggest using `self.metadata` and not suggest using `[strong self]`.

David_Rodrigues · December 16, 2015, 2:02am

+1 from me. I think this complements the options that we already have,
[weak self] and [unowned self], but we're dependent on the result of SE-0009
<https://github.com/apple/swift-evolution/blob/master/proposals/0009-require-self-for-accessing-instance-members.md>
which affects the viability of this change.

···

2015-12-16 1:42 GMT+00:00 Marc Knaup via swift-evolution < swift-evolution@swift.org>:

+1 from me. Developers will likely only ever write the rather uncommon [strong
self] capture when they know what they are doing.
Misuse can happen with any feature, but it is unlikely in this case.

On Wed, Dec 16, 2015 at 12:02 AM, Greg Parker via swift-evolution < > swift-evolution@swift.org> wrote:

Swift currently requires that `self` be used explicitly inside closures,
to help avoid bugs from unintentional capture. This is annoying when a
closure uses `self` a lot. Closures should be allowed to name `[strong
self]` in their capture list and thereafter not be required to write `self`
everywhere.

I wrote code this weekend that looked something like this:

    data = ...
    running = true
    delegate.notifyBegin(data)

    dispatch_async(queue) {
        self.processData(self.data)
        self.running = false
        self.delegate.notifyEnd(self.data)
    }

Note the asymmetry: the dispatched code needs to use `self` and the
non-dispatched code does not. It is clear that the dispatched closure
captures `self`, but it's annoying that it needed to be mentioned five
different times. The noise gets worse with longer closures. The annoyance
gets worse when moving code in and out of dispatches or other closures,
with lots of editing required each time.

The proposal would allow the same code to be written like this:

    data = ...
    running = true
    delegate.notifyBegin(data)

    dispatch_async(queue) {
        [strong self] in
        processData(data)
        running = false
        delegate.notifyEnd(data)
    }

Advantages:
* The dispatch'ed code looks like the non-dispatched code.
* The capture of `self` is still obvious.
* The code's action is clearer without the `self` noise.

Disadvantages:
* The capture behavior of self's properties is less obvious. For example,
neither closure above captured its own copy of `self.data`, but that
behavior is not immediately visible in the second closure.

What about [weak self] and [unowned self] ? I do not propose to change
the `self` requirement for those closures. In the weak case it is
critically important to know where `self` is accessed, because it could
potentially become nil between any two accesses. Unowned self might be
reasonable to change, but for simplicity I won't do so here.

--
Greg Parker gparker@apple.com Runtime Wrangler

_______________________________________________
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

Andrey_Tarantsov · December 17, 2015, 9:46am

+1, this has been a noticeable source of annoyance. Marc Knaup has said it best:

Developers will likely only ever write the rather uncommon [strong self] capture when they know what they are doing.
Misuse can happen with any feature, but it is unlikely in this case.

I *am* concerned about an inconsistency here, though; is [strong self] just a magical workaround for one specific case, or could this be a part of a larger rule?

A.

Chris_Lattner · December 18, 2015, 12:21am

FWIW, I’m also +1 on this proposal, but it should just be a capture list of [self], since all captures are strong by default. This is also part of the intended design of capture lists. The only reason we don’t do this already is that there are some implementation limitations (aka, hacks) around name lookup of “self" that need to be unraveled.

-Chris

···

On Dec 17, 2015, at 2:20 PM, Rudolf Adamkovic via swift-evolution <swift-evolution@swift.org> wrote:

+1

I've been writing a lot of experimental UIKit animation code past couple of days and spent most of my time adding and removing "self".

R+

Sent from my iPhone

On 16 Dec 2015, at 00:02, Greg Parker via swift-evolution <swift-evolution@swift.org> wrote:

Swift currently requires that `self` be used explicitly inside closures, to help avoid bugs from unintentional capture. This is annoying when a closure uses `self` a lot. Closures should be allowed to name `[strong self]` in their capture list and thereafter not be required to write `self` everywhere.

I wrote code this weekend that looked something like this:

  data = ...
  running = true
  delegate.notifyBegin(data)

  dispatch_async(queue) {
      self.processData(self.data)
      self.running = false
      self.delegate.notifyEnd(self.data)
  }

Note the asymmetry: the dispatched code needs to use `self` and the non-dispatched code does not. It is clear that the dispatched closure captures `self`, but it's annoying that it needed to be mentioned five different times. The noise gets worse with longer closures. The annoyance gets worse when moving code in and out of dispatches or other closures, with lots of editing required each time.

The proposal would allow the same code to be written like this:

  data = ...
  running = true
  delegate.notifyBegin(data)

  dispatch_async(queue) {
      [strong self] in
      processData(data)
      running = false
      delegate.notifyEnd(data)
  }

Advantages:
* The dispatch'ed code looks like the non-dispatched code.
* The capture of `self` is still obvious.
* The code's action is clearer without the `self` noise.

Disadvantages:
* The capture behavior of self's properties is less obvious. For example, neither closure above captured its own copy of `self.data`, but that behavior is not immediately visible in the second closure.

What about [weak self] and [unowned self] ? I do not propose to change the `self` requirement for those closures. In the weak case it is critically important to know where `self` is accessed, because it could potentially become nil between any two accesses. Unowned self might be reasonable to change, but for simplicity I won't do so here.

--
Greg Parker gparker@apple.com Runtime Wrangler

_______________________________________________
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

Dennis_Lysenko · December 18, 2015, 2:01am

+1, this also solves one of the problems I brought forth with implicit
self, and it would be a sizeable step toward making that proposal fully
unnecessary.

···

On Thu, Dec 17, 2015, 7:21 PM Chris Lattner via swift-evolution < swift-evolution@swift.org> wrote:

FWIW, I’m also +1 on this proposal, but it should just be a capture list
of [self], since all captures are strong by default. This is also part of
the intended design of capture lists. The only reason we don’t do this
already is that there are some implementation limitations (aka, hacks)
around name lookup of “self" that need to be unraveled.

-Chris

> On Dec 17, 2015, at 2:20 PM, Rudolf Adamkovic via swift-evolution < > swift-evolution@swift.org> wrote:
>
> +1
>
> I've been writing a lot of experimental UIKit animation code past couple
of days and spent most of my time adding and removing "self".
>
> R+
>
> Sent from my iPhone
>
>> On 16 Dec 2015, at 00:02, Greg Parker via swift-evolution < > swift-evolution@swift.org> wrote:
>>
>> Swift currently requires that `self` be used explicitly inside
closures, to help avoid bugs from unintentional capture. This is annoying
when a closure uses `self` a lot. Closures should be allowed to name
`[strong self]` in their capture list and thereafter not be required to
write `self` everywhere.
>>
>> I wrote code this weekend that looked something like this:
>>
>> data = ...
>> running = true
>> delegate.notifyBegin(data)
>>
>> dispatch_async(queue) {
>> self.processData(self.data)
>> self.running = false
>> self.delegate.notifyEnd(self.data)
>> }
>>
>> Note the asymmetry: the dispatched code needs to use `self` and the
non-dispatched code does not. It is clear that the dispatched closure
captures `self`, but it's annoying that it needed to be mentioned five
different times. The noise gets worse with longer closures. The annoyance
gets worse when moving code in and out of dispatches or other closures,
with lots of editing required each time.
>>
>> The proposal would allow the same code to be written like this:
>>
>> data = ...
>> running = true
>> delegate.notifyBegin(data)
>>
>> dispatch_async(queue) {
>> [strong self] in
>> processData(data)
>> running = false
>> delegate.notifyEnd(data)
>> }
>>
>> Advantages:
>> * The dispatch'ed code looks like the non-dispatched code.
>> * The capture of `self` is still obvious.
>> * The code's action is clearer without the `self` noise.
>>
>> Disadvantages:
>> * The capture behavior of self's properties is less obvious. For
example, neither closure above captured its own copy of `self.data`, but
that behavior is not immediately visible in the second closure.
>>
>>
>> What about [weak self] and [unowned self] ? I do not propose to change
the `self` requirement for those closures. In the weak case it is
critically important to know where `self` is accessed, because it could
potentially become nil between any two accesses. Unowned self might be
reasonable to change, but for simplicity I won't do so here.
>>
>>
>> --
>> Greg Parker gparker@apple.com Runtime Wrangler
>>
>>
>> _______________________________________________
>> 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

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

fluidsonic · January 23, 2016, 9:26am

Is the self requirement when using [self] capture still necessary once the
limitations outlined by Chris Lattner are solved?
Or does this change still have to go through Swift Evolution?

···

On Fri, Dec 18, 2015 at 1:21 AM, Chris Lattner via swift-evolution < swift-evolution@swift.org> wrote:

FWIW, I’m also +1 on this proposal, but it should just be a capture list
of [self], since all captures are strong by default. This is also part of
the intended design of capture lists. The only reason we don’t do this
already is that there are some implementation limitations (aka, hacks)
around name lookup of “self" that need to be unraveled.

-Chris

> On Dec 17, 2015, at 2:20 PM, Rudolf Adamkovic via swift-evolution < > swift-evolution@swift.org> wrote:
>
> +1
>
> I've been writing a lot of experimental UIKit animation code past couple
of days and spent most of my time adding and removing "self".
>
> R+
>
> Sent from my iPhone
>
>> On 16 Dec 2015, at 00:02, Greg Parker via swift-evolution < > swift-evolution@swift.org> wrote:
>>
>> Swift currently requires that `self` be used explicitly inside
closures, to help avoid bugs from unintentional capture. This is annoying
when a closure uses `self` a lot. Closures should be allowed to name
`[strong self]` in their capture list and thereafter not be required to
write `self` everywhere.
>>
>> I wrote code this weekend that looked something like this:
>>
>> data = ...
>> running = true
>> delegate.notifyBegin(data)
>>
>> dispatch_async(queue) {
>> self.processData(self.data)
>> self.running = false
>> self.delegate.notifyEnd(self.data)
>> }
>>
>> Note the asymmetry: the dispatched code needs to use `self` and the
non-dispatched code does not. It is clear that the dispatched closure
captures `self`, but it's annoying that it needed to be mentioned five
different times. The noise gets worse with longer closures. The annoyance
gets worse when moving code in and out of dispatches or other closures,
with lots of editing required each time.
>>
>> The proposal would allow the same code to be written like this:
>>
>> data = ...
>> running = true
>> delegate.notifyBegin(data)
>>
>> dispatch_async(queue) {
>> [strong self] in
>> processData(data)
>> running = false
>> delegate.notifyEnd(data)
>> }
>>
>> Advantages:
>> * The dispatch'ed code looks like the non-dispatched code.
>> * The capture of `self` is still obvious.
>> * The code's action is clearer without the `self` noise.
>>
>> Disadvantages:
>> * The capture behavior of self's properties is less obvious. For
example, neither closure above captured its own copy of `self.data`, but
that behavior is not immediately visible in the second closure.
>>
>>
>> What about [weak self] and [unowned self] ? I do not propose to change
the `self` requirement for those closures. In the weak case it is
critically important to know where `self` is accessed, because it could
potentially become nil between any two accesses. Unowned self might be
reasonable to change, but for simplicity I won't do so here.
>>
>>
>> --
>> Greg Parker gparker@apple.com Runtime Wrangler
>>
>>
>> _______________________________________________
>> 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

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

anandabits · December 16, 2015, 2:06am

I believe it has been made clear upthread now, but only the closure is capturing self. Your evil function only stores a copy of the dictionary.

This behavior is identical whether you use [strong self] and metadata or you use self.metadata so I don’t understand why it would be an argument against the proposal. Either way the capture of self by the closure is explicitly represented in the code where it happens. [strong self] actually seems a bit more clear to me.

An example that behaves like you stated could be concocted if we modify your example to pass a bound method rather than a property value, however that seems pretty unlikely to happen unintentionally even if / when we can obtain references to bound property getters / setters.

typealias evilArg = [String:String]
var strongReference: () -> evilArg
func evil(foo: () -> evilArg ) {
strongReference = foo
}

final class Photo {
    var data = [UInt8](count: 100000000, repeatedValue: 0) //a large amount of data
    func metadata() -> [String: String] { return [:] }
    func save() {
        dispatch_async(dispatch_get_global_queue(QOS_CLASS_DEFAULT, 0)) {
            evil(self.metadata)
        }
    }
}

I understand why you’re afraid of anything that might lead to unintentionally storing a strong reference or creating a reference cycle. I don’t understand why this proposal is likely to make that happen. I would even argue it might make it less likely to happen. If developers get in the habit of using the capture list rather than explicitly using “self.” in the body they will be more likely to capture only what they need. In your example the developer may have specified [metadata = self.metadata] rather than using [strong self] if the metadata value at the time of closure capture was sufficient.

Matthew

···

On Dec 15, 2015, at 7:23 PM, Drew Crawford via swift-evolution <swift-evolution@swift.org> wrote:

On Dec 15, 2015, at 7:15 PM, Greg Parker <gparker@apple.com <mailto:gparker@apple.com>> wrote:

We could make it more difficult for unaware programmers to *write* the `[strong self]` version, at least. When a programmer writes just `metadata` in a closure, the compiler could continue to suggest using `self.metadata` and not suggest using `[strong self]`.

That's not (especially) the danger I'm worried about.

I'm worried about the case where I write `[strong self] in` and then years later I add the `evil(f)` to the bottom of the closure, because it turns out we need more code in there.

I'm also worried about the case where I write nonEvil(f) but at some later time the implementation of nonEvil(f) becomes evil.

I'm also worried about the case where nonEvil(f) is a call into closed-source UIKit and without any visibility to me, UIKit's implementation of nonEvil becomes evil between iOS10b3 and iOS10b4.

These things are "category 5" bugs waiting to happen, and they can't be competently addressed with a UI-level fix in Xcode.

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

fluidsonic · December 16, 2015, 1:24am

Waiiiit. That looks more like a bug in Swift than expected behavior which
would us prevent [strong self] from being used.

   - metadata is a value type and thus should be copied. There's should be
   no implicit reference back to the Photo instance.
   - Capturing "self" in the closure because a property is accessed is fine
   and intentional. The reference to self would normally be destroyed as soon
   as the closure is gone.

So something must erroneously keep either the closure alive or hold a
reference to self which it shouldn't.

···

On Wed, Dec 16, 2015 at 2:12 AM, Drew Crawford via swift-evolution < swift-evolution@swift.org> wrote:

Can you explain what is so evil about func evil() when it is called from
an asynchronously-executed closure?

Apologies for earlier brevity, perhaps it would be helpful for me to
present a more complete, realistic example. Paste the following into
main.swift:

import Foundation

print("Hello, World!")

typealias evilArg = [String:String]
var strongReference: evilArg! = nil
func evil(foo:evilArg ) {
    strongReference = foo
}

final class Photo {
    var data = [UInt8](count: 100000000, repeatedValue: 0) //a large
amount of data
    let metadata: [String: String] = [:] //a small amount of data
    func save() {
        dispatch_async(dispatch_get_global_queue(QOS_CLASS_DEFAULT, 0)) {
            evil(self.metadata)
        }
    }
}
let p = Photo()
p.save()
//leaks Photo, data, and metadata

let sema = dispatch_semaphore_create(0)
dispatch_semaphore_wait(sema, DISPATCH_TIME_FOREVER)

In this example, the memory usage at the end of the program is 100MB:

This is because Photo, data, and metadata are all leaked by the evil
function.

This is the (surprising?) behavior specified in the Swift Book:

[A capture occurs when] the closure’s body accesses a property of the
instance, such as self.someProperty, or because the closure calls a method
on the instance, such as self.someMethod(). In either case, these accesses
cause the closure to “capture” self, creating a strong reference cycle.

Even though evil may seem (to the casual programmer who does not read
language specifications for funsies) like the closure captures only the
evil argument `metadata`, it **actually** captures *Photo (and therefore
data)*

The capture of Photo (data) is somewhat clear when we write

evil(self.metadata)

But it is hidden when we write

evil(metadata)

As you propose.

I think that if we are going to have semantics that capture Photo (data),
it had better look like it in a cursory inspection. The existing syntax is
not as great as it could be, but it provides a clue. I think even that the
existing syntax isn't good enough, because I expect that many people are
unaware of this particular dark corner. As you say:

Can you explain what is so evil about func evil() when it is called from
an asynchronously-executed closure? I don't see an obvious bug here.

It may very well be that there is no *obvious* bug, and so what we may
actually need is a proposal to make the programmer even more explicit when
referencing self, not less.

But in any case. For `strong self` to make sense, we would need to do one
of two things:

1. We could change Swift semantics to only capture metadata in this
example, however this is a breaking change, and a very very nontrivial
one. I do not know why it is specified this way in the first place,
perhaps a designer can weigh in on that. I do know that ObjC is very
similar, so there may be compatibility implications.

2. We could introduce additional syntax to provide compiler diagnostics
to guard in this case, e.g.

func lessEvil(@noescape foo:evilArg ) { //@noescape currently not
supported for non-closure parameters
     self.strongReference = foo //should generate diagnostic about
escaping a @noescape parameter
}

and then

dispatch_async(dispatch_get_global_queue(QOS_CLASS_DEFAULT, 0)) { [strong
self] in
            evil(metadata) //should generate diagnostic that self is
required for functions without @noescape
            lessEvil(metadata) //no diagnostic since parameter is @noescape
}

I think both of these fixes to the proposal are fairly impractical, but
they would weaken my vote to -0.5.

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

gparker42 · December 17, 2015, 11:43pm

`self` is already special in closures. This is merely a change to its special behavior.

I can think of two possible amendments that would make self capture more consistent with other variables.
1. Allow or require plain `[self]` instead of `[strong self]`.
2. Allow or require other captures to use `strong` as the capture specifier: `[strong y = x]`

···

On Dec 17, 2015, at 1:46 AM, Andrey Tarantsov via swift-evolution <swift-evolution@swift.org> wrote:

+1, this has been a noticeable source of annoyance. Marc Knaup has said it best:

Developers will likely only ever write the rather uncommon [strong self] capture when they know what they are doing.
Misuse can happen with any feature, but it is unlikely in this case.

I *am* concerned about an inconsistency here, though; is [strong self] just a magical workaround for one specific case, or could this be a part of a larger rule?

--
Greg Parker gparker@apple.com Runtime Wrangler

stephencelis · December 18, 2015, 2:03am

I like the simple capture list of [self].

Stephen

···

On Thu, Dec 17, 2015 at 9:01 PM, Dennis Lysenko via swift-evolution < swift-evolution@swift.org> wrote:

+1, this also solves one of the problems I brought forth with implicit
self, and it would be a sizeable step toward making that proposal fully
unnecessary.

On Thu, Dec 17, 2015, 7:21 PM Chris Lattner via swift-evolution < > swift-evolution@swift.org> wrote:

FWIW, I’m also +1 on this proposal, but it should just be a capture list
of [self], since all captures are strong by default. This is also part of
the intended design of capture lists. The only reason we don’t do this
already is that there are some implementation limitations (aka, hacks)
around name lookup of “self" that need to be unraveled.

-Chris

> On Dec 17, 2015, at 2:20 PM, Rudolf Adamkovic via swift-evolution < >> swift-evolution@swift.org> wrote:
>
> +1
>
> I've been writing a lot of experimental UIKit animation code past
couple of days and spent most of my time adding and removing "self".
>
> R+
>
> Sent from my iPhone
>
>> On 16 Dec 2015, at 00:02, Greg Parker via swift-evolution < >> swift-evolution@swift.org> wrote:
>>
>> Swift currently requires that `self` be used explicitly inside
closures, to help avoid bugs from unintentional capture. This is annoying
when a closure uses `self` a lot. Closures should be allowed to name
`[strong self]` in their capture list and thereafter not be required to
write `self` everywhere.
>>
>> I wrote code this weekend that looked something like this:
>>
>> data = ...
>> running = true
>> delegate.notifyBegin(data)
>>
>> dispatch_async(queue) {
>> self.processData(self.data)
>> self.running = false
>> self.delegate.notifyEnd(self.data)
>> }
>>
>> Note the asymmetry: the dispatched code needs to use `self` and the
non-dispatched code does not. It is clear that the dispatched closure
captures `self`, but it's annoying that it needed to be mentioned five
different times. The noise gets worse with longer closures. The annoyance
gets worse when moving code in and out of dispatches or other closures,
with lots of editing required each time.
>>
>> The proposal would allow the same code to be written like this:
>>
>> data = ...
>> running = true
>> delegate.notifyBegin(data)
>>
>> dispatch_async(queue) {
>> [strong self] in
>> processData(data)
>> running = false
>> delegate.notifyEnd(data)
>> }
>>
>> Advantages:
>> * The dispatch'ed code looks like the non-dispatched code.
>> * The capture of `self` is still obvious.
>> * The code's action is clearer without the `self` noise.
>>
>> Disadvantages:
>> * The capture behavior of self's properties is less obvious. For
example, neither closure above captured its own copy of `self.data`, but
that behavior is not immediately visible in the second closure.
>>
>>
>> What about [weak self] and [unowned self] ? I do not propose to change
the `self` requirement for those closures. In the weak case it is
critically important to know where `self` is accessed, because it could
potentially become nil between any two accesses. Unowned self might be
reasonable to change, but for simplicity I won't do so here.
>>
>>
>> --
>> Greg Parker gparker@apple.com Runtime Wrangler
>>
>>
>> _______________________________________________
>> 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

_______________________________________________
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

Chris_Lattner · January 23, 2016, 5:38pm

Is the self requirement when using [self] capture still necessary once the limitations outlined by Chris Lattner are solved?
Or does this change still have to go through Swift Evolution?

Once we have [self] in closures, I think it is obvious that the "self.” requirement would be disabled for any closure that uses it.

-Chris

···

On Jan 23, 2016, at 1:26 AM, Marc Knaup <marc@knaup.koeln> wrote:

On Fri, Dec 18, 2015 at 1:21 AM, Chris Lattner via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
FWIW, I’m also +1 on this proposal, but it should just be a capture list of [self], since all captures are strong by default. This is also part of the intended design of capture lists. The only reason we don’t do this already is that there are some implementation limitations (aka, hacks) around name lookup of “self" that need to be unraveled.

-Chris

> On Dec 17, 2015, at 2:20 PM, Rudolf Adamkovic via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
>
> +1
>
> I've been writing a lot of experimental UIKit animation code past couple of days and spent most of my time adding and removing "self".
>
> R+
>
> Sent from my iPhone
>
>> On 16 Dec 2015, at 00:02, Greg Parker via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
>>
>> Swift currently requires that `self` be used explicitly inside closures, to help avoid bugs from unintentional capture. This is annoying when a closure uses `self` a lot. Closures should be allowed to name `[strong self]` in their capture list and thereafter not be required to write `self` everywhere.
>>
>> I wrote code this weekend that looked something like this:
>>
>> data = ...
>> running = true
>> delegate.notifyBegin(data)
>>
>> dispatch_async(queue) {
>> self.processData(self.data)
>> self.running = false
>> self.delegate.notifyEnd(self.data)
>> }
>>
>> Note the asymmetry: the dispatched code needs to use `self` and the non-dispatched code does not. It is clear that the dispatched closure captures `self`, but it's annoying that it needed to be mentioned five different times. The noise gets worse with longer closures. The annoyance gets worse when moving code in and out of dispatches or other closures, with lots of editing required each time.
>>
>> The proposal would allow the same code to be written like this:
>>
>> data = ...
>> running = true
>> delegate.notifyBegin(data)
>>
>> dispatch_async(queue) {
>> [strong self] in
>> processData(data)
>> running = false
>> delegate.notifyEnd(data)
>> }
>>
>> Advantages:
>> * The dispatch'ed code looks like the non-dispatched code.
>> * The capture of `self` is still obvious.
>> * The code's action is clearer without the `self` noise.
>>
>> Disadvantages:
>> * The capture behavior of self's properties is less obvious. For example, neither closure above captured its own copy of `self.data`, but that behavior is not immediately visible in the second closure.
>>
>>
>> What about [weak self] and [unowned self] ? I do not propose to change the `self` requirement for those closures. In the weak case it is critically important to know where `self` is accessed, because it could potentially become nil between any two accesses. Unowned self might be reasonable to change, but for simplicity I won't do so here.
>>
>>
>> --
>> Greg Parker gparker@apple.com <mailto:gparker@apple.com> Runtime Wrangler
>>
>>
>> _______________________________________________
>> 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

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

possen · January 23, 2016, 8:22pm

I am very sure this is covered elsewhere, could someone point me to a discussion of why we can’t automate the removal of self cycles for closures. Is it because it essentially means that we need a garbage collector? This seems like one of the biggest stumbling blocks for new developers and even experienced developers of iOS. ARC was brilliant in that it eliminated 95% writing code to deal with memory issues, but for this one. What about a solution that just solved this one problem with closures and not a general solution that solved all memory cycles? Is that feasible? The strong and weak is pretty easy to understand everywhere else except the closure case, I know lots of people who don’t fully understand when you need to use weakSelf and strongSelf etc. Anyway, i am sure this has been discussed in depth just want to understand all the issues. Also, this may not be strictly a swift topic.

···

On Jan 23, 2016, at 9:38 AM, Chris Lattner via swift-evolution <swift-evolution@swift.org> wrote:

On Jan 23, 2016, at 1:26 AM, Marc Knaup <marc@knaup.koeln <mailto:marc@knaup.koeln>> wrote:

Is the self requirement when using [self] capture still necessary once the limitations outlined by Chris Lattner are solved?
Or does this change still have to go through Swift Evolution?

Once we have [self] in closures, I think it is obvious that the "self.” requirement would be disabled for any closure that uses it.

-Chris

On Fri, Dec 18, 2015 at 1:21 AM, Chris Lattner via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
FWIW, I’m also +1 on this proposal, but it should just be a capture list of [self], since all captures are strong by default. This is also part of the intended design of capture lists. The only reason we don’t do this already is that there are some implementation limitations (aka, hacks) around name lookup of “self" that need to be unraveled.

-Chris

> On Dec 17, 2015, at 2:20 PM, Rudolf Adamkovic via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
>
> +1
>
> I've been writing a lot of experimental UIKit animation code past couple of days and spent most of my time adding and removing "self".
>
> R+
>
> Sent from my iPhone
>
>> On 16 Dec 2015, at 00:02, Greg Parker via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
>>
>> Swift currently requires that `self` be used explicitly inside closures, to help avoid bugs from unintentional capture. This is annoying when a closure uses `self` a lot. Closures should be allowed to name `[strong self]` in their capture list and thereafter not be required to write `self` everywhere.
>>
>> I wrote code this weekend that looked something like this:
>>
>> data = ...
>> running = true
>> delegate.notifyBegin(data)
>>
>> dispatch_async(queue) {
>> self.processData(self.data)
>> self.running = false
>> self.delegate.notifyEnd(self.data)
>> }
>>
>> Note the asymmetry: the dispatched code needs to use `self` and the non-dispatched code does not. It is clear that the dispatched closure captures `self`, but it's annoying that it needed to be mentioned five different times. The noise gets worse with longer closures. The annoyance gets worse when moving code in and out of dispatches or other closures, with lots of editing required each time.
>>
>> The proposal would allow the same code to be written like this:
>>
>> data = ...
>> running = true
>> delegate.notifyBegin(data)
>>
>> dispatch_async(queue) {
>> [strong self] in
>> processData(data)
>> running = false
>> delegate.notifyEnd(data)
>> }
>>
>> Advantages:
>> * The dispatch'ed code looks like the non-dispatched code.
>> * The capture of `self` is still obvious.
>> * The code's action is clearer without the `self` noise.
>>
>> Disadvantages:
>> * The capture behavior of self's properties is less obvious. For example, neither closure above captured its own copy of `self.data`, but that behavior is not immediately visible in the second closure.
>>
>>
>> What about [weak self] and [unowned self] ? I do not propose to change the `self` requirement for those closures. In the weak case it is critically important to know where `self` is accessed, because it could potentially become nil between any two accesses. Unowned self might be reasonable to change, but for simplicity I won't do so here.
>>
>>
>> --
>> Greg Parker gparker@apple.com <mailto:gparker@apple.com> Runtime Wrangler
>>
>>
>> _______________________________________________
>> 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

_______________________________________________
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
https://lists.swift.org/mailman/listinfo/swift-evolution