Pitch: only allow capture of inout parameters in @noescape closures


(Joe Groff) #1

I think the time has come for us to limit the implicit capture of 'inout' parameters to @noescape closures. In the early days before @noescape, we designed a semantics for capturing "inout" parameters that attempted to balance the constraints on inout with the usability of higher-order functions. Inout parameters don't pass a first-class reference as in other languages, but instead provide a limited lease to mutate a property for the duration of a call; this means closures can't extend the lifetime of that lease, so closures instead capture the local mutable copy of the inout parameter. This gives the expected behavior as long as closures over an inout parameter never escape their original scope, but leads to surprises as soon as those closures escape. This unintuitive behavior has made several "Swift gotchas" lists. Now that we can explicitly mark closures as not escaping, I think we should prevent 'inout' parameters from being implicitly captured by escapable closures. There are several ways we can still support today's behavior:

- Encourage people to write the local copy themselves, if that's what they want:

func foo(inout x: Int) {
  var localX = x; defer { x = localX }

  // Asynchronously mutate localX, then wait for it to finish
  dispatch_async { mutate(&localX) }
  dispatch_sync {}
}

- You can explicitly capture an immutable copy:

func foo(inout x: Int) {
  // We don't need to mutate or observe mutations on x in the async job
  dispatch_async {[x] in doStuffWithoutMutating(x) }
}

and/or we could introduce a new explicit capture kind for the current behavior:

func foo(inout x: Int) {
  // Explicitly capture the 'inout' mutable shadow copy of x
  dispatch_async {[inout x] in mutate(&x) }
  dispatch_sync { }
}

Making it explicit should make the behavior less surprising when it occurs. We should able to provide fixits to migrate code that relies on the current behavior as well. What do you all think?

-Joe


(Lily Ballard) #2

I'm generally in favor of this. My biggest concern is when using
closures that are known not to escape the function but aren't marked as
@noescape. Sometimes this is just due to the frameworks not annotating
types properly, e.g. dispatch_sync()'s closure isn't @noescaping.
Sometimes this is due to a function taking a closure that may escape
sometimes, but not escape at other times (e.g. UIView animation methods
usually don't escape the animation block, unless a non-zero delay is
provided). And sometimes this is due to synchronization at a level the
API doesn't know about, such as using dispatch_async + dispatch_group to
wait until a block has executed.

That said, if dispatch_sync() is fixed, then I think it's reasonable to
require people to use manual workarounds for the remaining issues (e.g.
writing a local copy themselves). Especially because capturing an inout
parameter in a block is a relatively rare thing to do anyway (I don't
think I've ever actually done that myself, which is why I wasn't even
sure how Swift handled it).

The idea of using a capture list of [inout x] is interesting, but
ultimately confusing, because inout implies writeback semantics, when in
fact the closure wouldn't be doing writeback but would in fact be
closing over a shared mutable value (e.g. writeback implies there's a
specific point where the modified value becomes visible to the enclosing
scope, but that can't possibly work with non-@noescape closures because
there's no single point where the compiler knows it's appropriate to
insert the writeback). Based on that, I think the local copy version is
the most appropriate workaround.

So, given all that, +1 from me, but I really want to see dispatch_sync()
fixed as soon as possible (I want it fixed anyway, but I think
dispatch_sync() is the most likely candidate for unexpectedly hitting
this proposed limitation in Swift). I recognize that dispatch_sync()
isn't something the Swift team can fix (though Swift does have its own
copy of libdispatch, I assume that changes made to that don't actually
propagate upstream back to Apple's version), but since Swift core team
members are Apple employees, perhaps they can put some pressure on the
appropriate team to add the noescaping attribute to the appropriate
libdispatch APIs.

-Kevin Ballard

···

On Thu, Jan 28, 2016, at 11:58 AM, Joe Groff via swift-evolution wrote:

I think the time has come for us to limit the implicit capture of
'inout' parameters to @noescape closures. In the early days before
@noescape, we designed a semantics for capturing "inout" parameters
that attempted to balance the constraints on inout with the usability
of higher-order functions. Inout parameters don't pass a first-class
reference as in other languages, but instead provide a limited lease
to mutate a property for the duration of a call; this means closures
can't extend the lifetime of that lease, so closures instead capture
the local mutable copy of the inout parameter. This gives the expected
behavior as long as closures over an inout parameter never escape
their original scope, but leads to surprises as soon as those closures
escape. This unintuitive behavior has made several "Swift gotchas"
lists. Now that we can explicitly mark closures as not escaping, I
think we should prevent 'inout' parameters from being implicitly
captured by escapable closures. There are several ways we can still
support today's behavior:

- Encourage people to write the local copy themselves, if that's what
  they want:

func foo(inout x: Int) { var localX = x; defer { x = localX }

// Asynchronously mutate localX, then wait for it to finish
dispatch_async { mutate(&localX) } dispatch_sync {} }

- You can explicitly capture an immutable copy:

func foo(inout x: Int) { // We don't need to mutate or observe
mutations on x in the async job dispatch_async {[x] in
doStuffWithoutMutating(x) } }

and/or we could introduce a new explicit capture kind for the current
behavior:

func foo(inout x: Int) { // Explicitly capture the 'inout' mutable
shadow copy of x dispatch_async {[inout x] in mutate(&x) }
dispatch_sync { } }

Making it explicit should make the behavior less surprising when it
occurs. We should able to provide fixits to migrate code that relies
on the current behavior as well. What do you all think?

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


(Jordan Rose) #3

+1 from me, with no new capture kind. I think the workaround is easy enough that we don't need to worry about APIs that haven't been annotated yet, or APIs that are synchronous this time. Of course, it would be nice if the compiler could even insert the copy/writeback in a note fix-it.

Jordan

···

On Jan 28, 2016, at 11:58, Joe Groff via swift-evolution <swift-evolution@swift.org> wrote:

I think the time has come for us to limit the implicit capture of 'inout' parameters to @noescape closures. In the early days before @noescape, we designed a semantics for capturing "inout" parameters that attempted to balance the constraints on inout with the usability of higher-order functions. Inout parameters don't pass a first-class reference as in other languages, but instead provide a limited lease to mutate a property for the duration of a call; this means closures can't extend the lifetime of that lease, so closures instead capture the local mutable copy of the inout parameter. This gives the expected behavior as long as closures over an inout parameter never escape their original scope, but leads to surprises as soon as those closures escape. This unintuitive behavior has made several "Swift gotchas" lists. Now that we can explicitly mark closures as not escaping, I think we should prevent 'inout' parameters from being implicitly captured by escapable closures. There are several ways we can still support today's behavior:

- Encourage people to write the local copy themselves, if that's what they want:

func foo(inout x: Int) {
  var localX = x; defer { x = localX }

  // Asynchronously mutate localX, then wait for it to finish
  dispatch_async { mutate(&localX) }
  dispatch_sync {}
}

- You can explicitly capture an immutable copy:

func foo(inout x: Int) {
  // We don't need to mutate or observe mutations on x in the async job
  dispatch_async {[x] in doStuffWithoutMutating(x) }
}

and/or we could introduce a new explicit capture kind for the current behavior:

func foo(inout x: Int) {
  // Explicitly capture the 'inout' mutable shadow copy of x
  dispatch_async {[inout x] in mutate(&x) }
  dispatch_sync { }
}

Making it explicit should make the behavior less surprising when it occurs. We should able to provide fixits to migrate code that relies on the current behavior as well. What do you all think?

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


(Dave Abrahams) #4

I think the time has come for us to limit the implicit capture of
'inout' parameters to @noescape closures. In the early days before
@noescape, we designed a semantics for capturing "inout" parameters
that attempted to balance the constraints on inout with the usability
of higher-order functions. Inout parameters don't pass a first-class
reference as in other languages, but instead provide a limited lease
to mutate a property for the duration of a call; this means closures
can't extend the lifetime of that lease, so closures instead capture
the local mutable copy of the inout parameter. This gives the expected
behavior as long as closures over an inout parameter never escape
their original scope, but leads to surprises as soon as those closures
escape. This unintuitive behavior has made several "Swift gotchas"
lists. Now that we can explicitly mark closures as not escaping, I
think we should prevent 'inout' parameters from being implicitly
captured by escapable closures. There are several ways we can still
support today's behavior:

- Encourage people to write the local copy themselves, if that's what they want:

func foo(inout x: Int) {
  var localX = x; defer { x = localX }

  // Asynchronously mutate localX, then wait for it to finish
  dispatch_async { mutate(&localX) }
  dispatch_sync {}
}

+1

As the author of the current semantics, I can say with confidence that
if we'd had @noescape at our disposal originally we would never have
done this.

- You can explicitly capture an immutable copy:

func foo(inout x: Int) {
  // We don't need to mutate or observe mutations on x in the async job
  dispatch_async {[x] in doStuffWithoutMutating(x) }
}

and/or we could introduce a new explicit capture kind for the current behavior:

func foo(inout x: Int) {
  // Explicitly capture the 'inout' mutable shadow copy of x
  dispatch_async {[inout x] in mutate(&x) }
  dispatch_sync { }
}

-1. Let's not complicate the language for this rare case.

···

on Thu Jan 28 2016, Joe Groff <swift-evolution@swift.org> wrote:

Making it explicit should make the behavior less surprising when it
occurs. We should able to provide fixits to migrate code that relies
on the current behavior as well. What do you all think?

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

--
-Dave


(Joe Groff) #5

Thanks for the feedback, everyone. I've submitted a formal proposal:

https://github.com/jckarter/swift-evolution/blob/e89479c1a3ae9f37c217c8c3130c24b71c8909b3/proposals/XXXX-limit-inout-capture.md

-Joe

···

On Jan 28, 2016, at 11:58 AM, Joe Groff via swift-evolution <swift-evolution@swift.org> wrote:

I think the time has come for us to limit the implicit capture of 'inout' parameters to @noescape closures. In the early days before @noescape, we designed a semantics for capturing "inout" parameters that attempted to balance the constraints on inout with the usability of higher-order functions. Inout parameters don't pass a first-class reference as in other languages, but instead provide a limited lease to mutate a property for the duration of a call; this means closures can't extend the lifetime of that lease, so closures instead capture the local mutable copy of the inout parameter. This gives the expected behavior as long as closures over an inout parameter never escape their original scope, but leads to surprises as soon as those closures escape. This unintuitive behavior has made several "Swift gotchas" lists. Now that we can explicitly mark closures as not escaping, I think we should prevent 'inout' parameters from being implicitly captured by escapable closures. There are several ways we can still support today's behavior:

- Encourage people to write the local copy themselves, if that's what they want:

func foo(inout x: Int) {
  var localX = x; defer { x = localX }

  // Asynchronously mutate localX, then wait for it to finish
  dispatch_async { mutate(&localX) }
  dispatch_sync {}
}

- You can explicitly capture an immutable copy:

func foo(inout x: Int) {
  // We don't need to mutate or observe mutations on x in the async job
  dispatch_async {[x] in doStuffWithoutMutating(x) }
}

and/or we could introduce a new explicit capture kind for the current behavior:

func foo(inout x: Int) {
  // Explicitly capture the 'inout' mutable shadow copy of x
  dispatch_async {[inout x] in mutate(&x) }
  dispatch_sync { }
}

Making it explicit should make the behavior less surprising when it occurs. We should able to provide fixits to migrate code that relies on the current behavior as well. What do you all think?

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


(Jacob Bandes-Storch) #6

[ shameless plug for
https://github.com/apple/swift-evolution/blob/master/proposals/0012-add-noescape-to-public-library-api.md
]

···

On Thu, Jan 28, 2016 at 1:46 PM, Kevin Ballard via swift-evolution < swift-evolution@swift.org> wrote:

I'm generally in favor of this. My biggest concern is when using closures
that are known not to escape the function but aren't marked as @noescape.
Sometimes this is just due to the frameworks not annotating types properly,
e.g. dispatch_sync()'s closure isn't @noescaping. Sometimes this is due to
a function taking a closure that may escape sometimes, but not escape at
other times (e.g. UIView animation methods usually don't escape the
animation block, unless a non-zero delay is provided). And sometimes this
is due to synchronization at a level the API doesn't know about, such as
using dispatch_async + dispatch_group to wait until a block has executed.

That said, if dispatch_sync() is fixed, then I think it's reasonable to
require people to use manual workarounds for the remaining issues (e.g.
writing a local copy themselves). Especially because capturing an inout
parameter in a block is a relatively rare thing to do anyway (I don't think
I've ever actually done that myself, which is why I wasn't even sure how
Swift handled it).

The idea of using a capture list of [inout x] is interesting, but
ultimately confusing, because inout implies writeback semantics, when in
fact the closure wouldn't be doing writeback but would in fact be closing
over a shared mutable value (e.g. writeback implies there's a specific
point where the modified value becomes visible to the enclosing scope, but
that can't possibly work with non-@noescape closures because there's no
single point where the compiler knows it's appropriate to insert the
writeback). Based on that, I think the local copy version is the most
appropriate workaround.

So, given all that, +1 from me, but I really want to see dispatch_sync()
fixed as soon as possible (I want it fixed anyway, but I think
dispatch_sync() is the most likely candidate for unexpectedly hitting this
proposed limitation in Swift). I recognize that dispatch_sync() isn't
something the Swift team can fix (though Swift does have its own copy of
libdispatch, I assume that changes made to that don't actually propagate
upstream back to Apple's version), but since Swift core team members are
Apple employees, perhaps they can put some pressure on the appropriate team
to add the noescaping attribute to the appropriate libdispatch APIs.

-Kevin Ballard

On Thu, Jan 28, 2016, at 11:58 AM, Joe Groff via swift-evolution wrote:

I think the time has come for us to limit the implicit capture of 'inout'
parameters to @noescape closures. In the early days before @noescape, we
designed a semantics for capturing "inout" parameters that attempted to
balance the constraints on inout with the usability of higher-order
functions. Inout parameters don't pass a first-class reference as in other
languages, but instead provide a limited lease to mutate a property for the
duration of a call; this means closures can't extend the lifetime of that
lease, so closures instead capture the local mutable copy of the inout
parameter. This gives the expected behavior as long as closures over an
inout parameter never escape their original scope, but leads to surprises
as soon as those closures escape. This unintuitive behavior has made
several "Swift gotchas" lists. Now that we can explicitly mark closures as
not escaping, I think we should prevent 'inout' parameters from being
implicitly captured by escapable closures. There are several ways we can
still support today's behavior:

- Encourage people to write the local copy themselves, if that's what they
want:

func foo(inout x: Int) {
  var localX = x; defer { x = localX }

  // Asynchronously mutate localX, then wait for it to finish
  dispatch_async { mutate(&localX) }
  dispatch_sync {}
}

- You can explicitly capture an immutable copy:

func foo(inout x: Int) {
  // We don't need to mutate or observe mutations on x in the async job
  dispatch_async {[x] in doStuffWithoutMutating(x) }
}

and/or we could introduce a new explicit capture kind for the current
behavior:

func foo(inout x: Int) {
  // Explicitly capture the 'inout' mutable shadow copy of x
  dispatch_async {[inout x] in mutate(&x) }
  dispatch_sync { }
}

Making it explicit should make the behavior less surprising when it
occurs. We should able to provide fixits to migrate code that relies on the
current behavior as well. What do you all think?

-Joe
*_______________________________________________*
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) #7

+1 from me, with no new capture kind.

Me too: +1 from me, with no new capture kind.

-Chris

···

On Jan 28, 2016, at 1:50 PM, Jordan Rose via swift-evolution <swift-evolution@swift.org> wrote:

I think the workaround is easy enough that we don't need to worry about APIs that haven't been annotated yet, or APIs that are synchronous this time. Of course, it would be nice if the compiler could even insert the copy/writeback in a note fix-it.

Jordan

On Jan 28, 2016, at 11:58, Joe Groff via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

I think the time has come for us to limit the implicit capture of 'inout' parameters to @noescape closures. In the early days before @noescape, we designed a semantics for capturing "inout" parameters that attempted to balance the constraints on inout with the usability of higher-order functions. Inout parameters don't pass a first-class reference as in other languages, but instead provide a limited lease to mutate a property for the duration of a call; this means closures can't extend the lifetime of that lease, so closures instead capture the local mutable copy of the inout parameter. This gives the expected behavior as long as closures over an inout parameter never escape their original scope, but leads to surprises as soon as those closures escape. This unintuitive behavior has made several "Swift gotchas" lists. Now that we can explicitly mark closures as not escaping, I think we should prevent 'inout' parameters from being implicitly captured by escapable closures. There are several ways we can still support today's behavior:

- Encourage people to write the local copy themselves, if that's what they want:

func foo(inout x: Int) {
  var localX = x; defer { x = localX }

  // Asynchronously mutate localX, then wait for it to finish
  dispatch_async { mutate(&localX) }
  dispatch_sync {}
}

- You can explicitly capture an immutable copy:

func foo(inout x: Int) {
  // We don't need to mutate or observe mutations on x in the async job
  dispatch_async {[x] in doStuffWithoutMutating(x) }
}

and/or we could introduce a new explicit capture kind for the current behavior:

func foo(inout x: Int) {
  // Explicitly capture the 'inout' mutable shadow copy of x
  dispatch_async {[inout x] in mutate(&x) }
  dispatch_sync { }
}

Making it explicit should make the behavior less surprising when it occurs. We should able to provide fixits to migrate code that relies on the current behavior as well. What do you all think?

-Joe
_______________________________________________
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