[Review] SE-0073: Marking closures as executing exactly once


(Chris Lattner) #1

Hello Swift community,

The review of "SE-0073: Marking closures as executing exactly once" begins now and runs through May 9. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0073-noescape-once.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


(David Hart) #2

   * What is your evaluation of the proposal?

I like the proposal and I think it's a good idea, but I'm really not sure it is important enough to fix. Looking at the proposals already accepted, half of them are still waiting for an implementation. Several will probably never make it in time for Swift 3. And I don't feel like the current issue is less important, and will require more implementation effort, than many other issues still waiting for an implementation.

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

See above, but I think not.

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

I does fit well.

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

No.

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

A detailed read of the proposal.


(Pyry Jahkola) #3

Here's my review of "SE-0073 <https://github.com/apple/swift-evolution/blob/master/proposals/0073-noescape-once.md>: Marking closures as executing exactly once".

What is your evaluation of the proposal?

+1. I think this is a good idea and should be accepted (without extending the proposed scope).

However, I think the proposal should be more explicit about the case when (and whether) the block itself throws. Specifically, I think we should highlight that the criterion that

it must not be executed on any path that throws

implies that a @noescape(once) parameter itself cannot throw (until another language change allows otherwise). Consider this use case:

    final class Database {
      final class Transaction { ... }
      // Not allowed by SE-0073:
      func transact(_ apply: @noescape(once) (Transaction) throws -> ()) rethrows
    }

    func incrementScore(db: Database, game: String) throws -> Int {
      let oldScore: Int
      // This use of "@noescape(once) ... throws" *could* be ok if the error was
      // unconditionally propagated out of the scope of uninitialised variables:
      try db.transact { tx in
        oldScore = try tx.read(key: "\(game).score")
        try tx.update(key: "\(game).score", value: oldScore + 1)
        try tx.update(key: "\(game).updatedAt", value: NSDate())
      }
      return oldScore
    }

Being able to throw out of a @noescape(once) block would be useful in cases like this, as it would naturally allow rolling back a transaction. But it would complicate the language by requiring that no one catches the error in the scope where uninitialised variables are defined. I suggest adding this remark to the Future directions.

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

Yes. I'm looking forward to being able to initialise immutable variables in dispatch_sync blocks and such without needing to resort to defining them as `var`.

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

Yes it does, it aligns nicely with the trailing closure syntax that allows for the emulation of control flow constructs with library code. And as we know, the control flow constructs already allow the delayed initialisation of local variables.

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

With a similar feature, I have only used languages (in the sense of "used in anger") where the delayed initialisation is allowed by allowing an undefined state (e.g. C++). For a not-really-applicable-to-Swift alternative approach, Haskell's laziness allows the recursive definition of variables for a similar effect.

Finally, I think it's interesting that the requirement for @noescape(once) arguments to be unconditionally executed has a similarity to linear type systems (Wikipedia <https://en.wikipedia.org/wiki/Substructural_type_system>) and uniqueness or references. Except in this case, the reference is not to an object but a function. Such a function reference bears a certain similarity to the deinit of a uniquely held object. I think the proposed feature may later merge with a bigger language update that brings reference uniqueness to the type system.

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

Between quick reading and in-depth study.

— Pyry


(Nicola Salmoria) #4

* What is your evaluation of the proposal?

-1. The proposed change seems to require a somewhat complex implementation
in the compiler, and I fail to see clear benefits in the change.

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

Swift?

Frankly, I don't think the motivating example in the proposal is strong
enough. If the only purpose is to simplify certain imperative constructs, I
think there are better, functional-style, solutions.

The example related to autoreleasepool is particularly significant:

// Current Swift:
var x: Int = 0 // `var` declaration, with some irrelevant value
autoreleasepool {
    x = 1
}

// Should SE-0061 be accepted:
let x = autoreleasepool {
    return 1
}

// Should this proposal be accepted:
let x: Int
let y: String
autoreleasepool {
    x = 1
    y = "foo"
}

I would argue that of these three examples, the second is the clearest and
most readable; the last one, which should support the proposal, could simply
be rewritten as

let (x, y) = autoreleasepool {
    return (1, "foo")
}

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

While it does build on the delayed initialization of let variables, which is
a peculiar feature of Swift, I don't think it fits the overall trend of
favoring functional constructs and discouraging reliance on side effects.

* If you have used other languages or libraries with a similar feature,

how do you feel that this proposal

compares to those?

I've never seen this kind of feature.

* How much effort did you put into your review? A glance, a quick reading,

or an in-depth study?

A quick reading.

···

--
Nicola


(Matthew Johnson) #5

* What is your evaluation of the proposal?

+1. I like semantic guarantees provided by the compiler.

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

Yes.

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

Yes.

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

Not that I can think of.

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

I participated in the earlier threads, read the proposal thoroughly, and have followed the review closely.

···

More information about the Swift evolution process is available at

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

Thank you,

-Chris Lattner
Review Manager

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


(Dave Abrahams) #6

I think it's of questionable importance and doesn't generalize well.
For example, you can't use this to construct something like

  var x: Int
  functionThatActsLikeIf( someTest(), then: { x = 1 }, else: { x = 2} )

If you need to initialize something in an outer scope with something
computed by a closure, it's much better to arrange something like this:
    
  var x = functionThatActsLikeIf( someTest(), then: { 1 }, else: { 2 } )

···

on Tue May 03 2016, Chris Lattner <swift-evolution@swift.org> wrote:

Hello Swift community,

The review of "SE-0073: Marking closures as executing exactly once"
begins now and runs through May 9. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0073-noescape-once.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?

--
Dave


(Greg Parker) #7

I dislike the proposed syntax.

noescape-ness and once-ness are semantically orthogonal. The compiler's ability to verify once-ness depends on noescape and some other semantics all being present, as noted in the above quote from the proposal. The fact that the compiler is limited should not forgive the semantic transgression. Note that semantically the once-ness is the more important part for the designed usage; noescape-ness is only dragged in because of the desire to enforce once-ness at compile time. The @noescape(once) syntax is therefore backwards: the feature is not some small modification to or variation of plain @noescape.

The proposed behavior includes restrictions that are required for its designed usage but are unrelated to noescape-ness and once-ness. ("A @noescape(once) closure may only read from variables that were initialized before it was formed.") That suggests the proposed functionality should use a new higher-level name rather than being bolted on to @noescape.

···

On May 3, 2016, at 8:53 PM, Chris Lattner via swift-evolution <swift-evolution@swift.org> wrote:

Hello Swift community,

The review of "SE-0073: Marking closures as executing exactly once" begins now and runs through May 9. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0073-noescape-once.md

"A @once parameter

It was mentioned in the discussion that the "once" behavior and @noescape look orthogonal, and the "once" behavior could be useful on closures that escape. However, it is only possible to verify that a closure has been executed exactly once if it does not escape. Because of this, "once" and @noescape are better left together."

--
Greg Parker gparker@apple.com Runtime Wrangler


#8

Hello Pyry,

I quite expect being able to throw out of a @noescape(once) block. Maybe the sentence "it must not be executed on any path that throws" should be removed from the proposal, should it have the implications you describe.

Here is below what I expect this proposal to allow. So you see one problematic case?

  // Function which rethrows closure errors:
  func f1(closure: @noescape(once) () throws -> ()) rethrows {
    try closure()
  }

  // Function which may throw before, inside, or after the closure:
  func f2(closure: @noescape(once) () throws -> ()) throws {
    try mayFailBefore()
    try closure()
    try mayFailAfter()
  }
  
  // Support function
  func getX() throws -> Int { return 1 }
  
Case 1:

  let x: Int
  f1 {
    x = 1
    // use x
  }
  // use x
  
Case 2:

  let x: Int
  do {
    try f1 {
      x = try getX()
      // use x
    }
    // use x
  } catch {
    // can't use x
  }
  // can't use x
  
Case 3:

  let x: Int
  do {
    try f1 {
      x = try getX()
      // use x
    }
    // use x
  } catch {
    x = 1
  }
  // use x
  
Case 4:

  let x: Int
  do {
    try f2 {
      x = try getX()
      // use x
    }
    // use x
  } catch {
    // can't use x
  }
  // can't use x

Case 5:

  let x: Int
  do {
    try f2 {
      x = try getX()
      // use x
    }
    // use x
  } catch {
    x = 1
  }
  // use x

Gwendal Roué

···

Le 4 mai 2016 à 08:28, Pyry Jahkola via swift-evolution <swift-evolution@swift.org> a écrit :

Here's my review of "SE-0073: Marking closures as executing exactly once".

What is your evaluation of the proposal?

+1. I think this is a good idea and should be accepted (without extending the proposed scope).

However, I think the proposal should be more explicit about the case when (and whether) the block itself throws. Specifically, I think we should highlight that the criterion that

it must not be executed on any path that throws

implies that a @noescape(once) parameter itself cannot throw (until another language change allows otherwise).

[…]

Being able to throw out of a @noescape(once) block […] would complicate the language by requiring that no one catches the error in the scope where uninitialised variables are defined. I suggest adding this remark to the Future directions.


#9

Hello Nicola,

I don't have the slightest idea about the difficulty of implementation, I admit.

Gwendal

···

Le 4 mai 2016 à 14:49, Nicola Salmoria via swift-evolution <swift-evolution@swift.org> a écrit :

* What is your evaluation of the proposal?

-1. The proposed change seems to require a somewhat complex implementation
in the compiler, and I fail to see clear benefits in the change.


(TJ Usiyan) #10

* What is your evaluation of the proposal?
+1
        * Is the problem being addressed significant enough to warrant a
change to Swift?
I believe so.
        * Does this proposal fit well with the feel and direction of Swift?
Yes
        * If you have used other languages or libraries with a similar
feature, how do you feel that this proposal compares to those?
no
        * How much effort did you put into your review? A glance, a quick
reading, or an in-depth study?
I have followed the thread and read the proposal

···

On Fri, May 6, 2016 at 1:24 PM, Dave Abrahams via swift-evolution < swift-evolution@swift.org> wrote:

on Tue May 03 2016, Chris Lattner <swift-evolution@swift.org> wrote:

> Hello Swift community,
>
> The review of "SE-0073: Marking closures as executing exactly once"
> begins now and runs through May 9. The proposal is available here:
>
>
https://github.com/apple/swift-evolution/blob/master/proposals/0073-noescape-once.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?

I think it's of questionable importance and doesn't generalize well.
For example, you can't use this to construct something like

  var x: Int
  functionThatActsLikeIf( someTest(), then: { x = 1 }, else: { x = 2} )

If you need to initialize something in an outer scope with something
computed by a closure, it's much better to arrange something like this:

  var x = functionThatActsLikeIf( someTest(), then: { 1 }, else: { 2 } )

--
Dave

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


(Rod Brown) #11

Agreed.

I think you bring up points that articulate well my issues with this proposal.

···

On 7 May 2016, at 10:31 AM, Greg Parker via swift-evolution <swift-evolution@swift.org> wrote:

On May 3, 2016, at 8:53 PM, Chris Lattner via swift-evolution <swift-evolution@swift.org> wrote:

Hello Swift community,

The review of "SE-0073: Marking closures as executing exactly once" begins now and runs through May 9. The proposal is available here:

   https://github.com/apple/swift-evolution/blob/master/proposals/0073-noescape-once.md

"A @once parameter

It was mentioned in the discussion that the "once" behavior and @noescape look orthogonal, and the "once" behavior could be useful on closures that escape. However, it is only possible to verify that a closure has been executed exactly once if it does not escape. Because of this, "once" and @noescape are better left together."

I dislike the proposed syntax.

noescape-ness and once-ness are semantically orthogonal. The compiler's ability to verify once-ness depends on noescape and some other semantics all being present, as noted in the above quote from the proposal. The fact that the compiler is limited should not forgive the semantic transgression. Note that semantically the once-ness is the more important part for the designed usage; noescape-ness is only dragged in because of the desire to enforce once-ness at compile time. The @noescape(once) syntax is therefore backwards: the feature is not some small modification to or variation of plain @noescape.

The proposed behavior includes restrictions that are required for its designed usage but are unrelated to noescape-ness and once-ness. ("A @noescape(once) closure may only read from variables that were initialized before it was formed.") That suggests the proposed functionality should use a new higher-level name rather than being bolted on to @noescape.

--
Greg Parker gparker@apple.com Runtime Wrangler

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


(Andrew Bennett) #12

Hi Dave,

Sorry, Dave, sending a second time as I forgot to Reply-All.

I agree, this proposal doesn't allow multiple closures where only one of
them should be run, and it should only be run once. I personally don't
think lacking that functionality is worth blocking this proposal for,
another proposal can be built on top of this if it is desired.

These cases can also be handled by a more meaningful if/switch statement,
using @noescape(once), for example:
  let x: Int
  functionThatCallsAClosure(someTest()) { x = $0 ? 1 : 2 }

···

On Sat, May 7, 2016 at 6:24 AM, Dave Abrahams via swift-evolution < swift-evolution@swift.org> wrote:

on Tue May 03 2016, Chris Lattner <swift-evolution@swift.org> wrote:

> Hello Swift community,
>
> The review of "SE-0073: Marking closures as executing exactly once"
> begins now and runs through May 9. The proposal is available here:
>
>
https://github.com/apple/swift-evolution/blob/master/proposals/0073-noescape-once.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?

I think it's of questionable importance and doesn't generalize well.
For example, you can't use this to construct something like

  var x: Int
  functionThatActsLikeIf( someTest(), then: { x = 1 }, else: { x = 2} )

If you need to initialize something in an outer scope with something
computed by a closure, it's much better to arrange something like this:

  var x = functionThatActsLikeIf( someTest(), then: { 1 }, else: { 2 } )

--
Dave

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


(Dmitri Gribenko) #13

Hi Gwendal,

What about the following case?

// Function which rethrows closure errors:
func f1(closure: @noescape(once) () throws -> ()) rethrows {
  try closure()
}

let x: AnyObject
f1 {
  if someCondition() { x = MyClass() }
  if someOtherCondition() { throw MyError.Error() }
  x = MyOtherClass()
}

How do you handle memory management for 'x' on the path that throws?
If the rule is that upon returning from f1 via a throw the variable
'x' should not be initialized, then the closure passed to f1 has to
guarantee the deinitialization. But f1 accepts an arbitrary closure.

Dmitri

···

On Wed, May 4, 2016 at 2:24 AM, Gwendal Roué <swift-evolution@swift.org> wrote:

Le 4 mai 2016 à 08:28, Pyry Jahkola via swift-evolution <swift-evolution@swift.org> a écrit :

Here's my review of "SE-0073: Marking closures as executing exactly once".

What is your evaluation of the proposal?

+1. I think this is a good idea and should be accepted (without extending the proposed scope).

However, I think the proposal should be more explicit about the case when (and whether) the block itself throws. Specifically, I think we should highlight that the criterion that

it must not be executed on any path that throws

implies that a @noescape(once) parameter itself cannot throw (until another language change allows otherwise).

[…]

Being able to throw out of a @noescape(once) block […] would complicate the language by requiring that no one catches the error in the scope where uninitialised variables are defined. I suggest adding this remark to the Future directions.

Hello Pyry,

I quite expect being able to throw out of a @noescape(once) block. Maybe the sentence "it must not be executed on any path that throws" should be removed from the proposal, should it have the implications you describe.

Here is below what I expect this proposal to allow. So you see one problematic case?

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


(Pyry Jahkola) #14

Hi Gwendal,

Nice writeup. So I see that you recognise how this extra specification will complicate (while also facilitate) things. And I also see that you're a co-author of the proposal. So I'm more than happy if you can squeeze this extra into it.

However reading your example code, I had to squint to see—without the compiler's squiggly red aid—what all the problematic cases are.

1) Is it obvious to everybody why you can't always use `x` in the end of Cases 2 and 4? For example, with `if—else` you MUST initialise the variable or escape its scope in all branches; you can't just write the following to complete the initialisation later:

    let x: Int
    if cond { x = 1 }
    // ...
    if !cond { x = 2 } // too late!

2) Should Cases 2 and 4 be made illegal? The requirement could then be that all `catch` blocks either:
2.a) initialise another value for `x`, or
2.b) escape the scope using `throw`, `return`, `fatalError`, whatnot…

3) If you consider including this addition to the proposal, it might also help other reviewers if you explained how the compiler will be able to help the programmer write a valid program. E.g. what would the error messages about partially initialised variables look like? And where and when would they appear? Could the compiler suggest certain fixits? Etc.

So, I'm all +1 and very glad if you can make it! (But also somewhat sceptical whether it could get accepted.)

— Pyry

···

On 04 May 2016, at 12:24, Gwendal Roué <gwendal.roue@gmail.com> wrote:

I quite expect being able to throw out of a @noescape(once) block. Maybe the sentence "it must not be executed on any path that throws" should be removed from the proposal, should it have the implications you describe.

Here is below what I expect this proposal to allow. So you see one problematic case?

  // Function which rethrows closure errors:
  func f1(closure: @noescape(once) () throws -> ()) rethrows {
    try closure()
  }

  // Function which may throw before, inside, or after the closure:
  func f2(closure: @noescape(once) () throws -> ()) throws {
    try mayFailBefore()
    try closure()
    try mayFailAfter()
  }
  
  // Support function
  func getX() throws -> Int { return 1 }
  
Case 1:

  let x: Int
  f1 {
    x = 1
    // use x
  }
  // use x
  
Case 2:

  let x: Int
  do {
    try f1 {
      x = try getX()
      // use x
    }
    // use x
  } catch {
    // can't use x
  }
  // can't use x
  
Case 3:

  let x: Int
  do {
    try f1 {
      x = try getX()
      // use x
    }
    // use x
  } catch {
    x = 1
  }
  // use x
  
Case 4:

  let x: Int
  do {
    try f2 {
      x = try getX()
      // use x
    }
    // use x
  } catch {
    // can't use x
  }
  // can't use x

Case 5:

  let x: Int
  do {
    try f2 {
      x = try getX()
      // use x
    }
    // use x
  } catch {
    x = 1
  }
  // use x

Gwendal Roué


#15

I quite expect being able to throw out of a @noescape(once) block. Maybe the sentence "it must not be executed on any path that throws" should be removed from the proposal, should it have the implications you describe.

Here is below what I expect this proposal to allow. So you see one problematic case?

Hi Gwendal,

What about the following case?

// Function which rethrows closure errors:
func f1(closure: @noescape(once) () throws -> ()) rethrows {
try closure()
}

let x: AnyObject
f1 {
if someCondition() { x = MyClass() }
if someOtherCondition() { throw MyError.Error() }
x = MyOtherClass()
}

How do you handle memory management for 'x' on the path that throws?
If the rule is that upon returning from f1 via a throw the variable
'x' should not be initialized, then the closure passed to f1 has to
guarantee the deinitialization. But f1 accepts an arbitrary closure.

Hello Dmitri,

To reason about @noescape(once) functions, the easiest way is to replace them with `do`:

    let x: AnyObject
    do {
        if someCondition() { x = MyClass() }
        if someOtherCondition() { throw MyError.error }
        x = MyOtherClass()
    }

This code does not compile because x can't be initialized to MyOtherClass().

But I don't think this is your point. Your point was "how can the compiler handle memory management ?".

I can't answer this question, because I'm not competent enough. But if it can handle the do { … } case, can't it also handle the f { … } case?

Gwendal Roué


(Dave Abrahams) #16

Hi Dave,

Sorry, Dave, sending a second time as I forgot to Reply-All.

I agree, this proposal doesn't allow multiple closures where only one of them
should be run, and it should only be run once. I personally don't think lacking
that functionality is worth blocking this proposal for, another proposal can be
built on top of this if it is desired.

These cases can also be handled by a more meaningful if/switch statement, using
@noescape(once), for example:
let x: Int
functionThatCallsAClosure(someTest()) { x = $0 ? 1 : 2 }

Why is this better than

    let x = functionThatCallsAClosure(someTest()) { $0 ? 1 : 2 }

?

IMO separating initialization from declaration is *very* rarely needed
and very much better avoided altogether, because it leads to code that's
less clear. Just because we *can* do this doesn't mean we should.

···

on Fri May 06 2016, Andrew Bennett <swift-evolution@swift.org> wrote:

On Sat, May 7, 2016 at 6:24 AM, Dave Abrahams via swift-evolution > <swift-evolution@swift.org> wrote:

    on Tue May 03 2016, Chris Lattner > <swift-evolution@swift.org> wrote:

    > Hello Swift community,
    >
    > The review of "SE-0073: Marking closures as executing exactly once"
    > begins now and runs through May 9. The proposal is available here:
    >
    >
    https://github.com/apple/swift-evolution/blob/master/proposals/0073-noescape-once.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?

    I think it's of questionable importance and doesn't generalize well.
    For example, you can't use this to construct something like

    var x: Int
    functionThatActsLikeIf( someTest(), then: { x = 1 }, else: { x = 2} )

    If you need to initialize something in an outer scope with something
    computed by a closure, it's much better to arrange something like this:

    var x = functionThatActsLikeIf( someTest(), then: { 1 }, else: { 2 } )

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


(Andrew Bennett) #17

Replies inline:

> Hi Dave,
>
> Sorry, Dave, sending a second time as I forgot to Reply-All.
>
> I agree, this proposal doesn't allow multiple closures where only one of
them
> should be run, and it should only be run once. I personally don't think
lacking
> that functionality is worth blocking this proposal for, another proposal
can be
> built on top of this if it is desired.
>
> These cases can also be handled by a more meaningful if/switch
statement, using
> @noescape(once), for example:
> let x: Int
> functionThatCallsAClosure(someTest()) { x = $0 ? 1 : 2 }

Why is this better than

    let x = functionThatCallsAClosure(someTest()) { $0 ? 1 : 2 }

?

I'm not saying it's better, neither is the proposal. I do think both are
better than this though:

    functionThatActsLikeIf( someTest(), then: { x = 1 }, else: { x = 2} )

My opinion is that cases where *the proposal* are limited by multiple
closures seem to be cases where you would be better off with a single
closure and more explicit control-flow. I'd be interested if there are
other cases, but it currently seems like a straw-man argument to me.

···

On Sat, May 7, 2016 at 12:37 PM, Dave Abrahams via swift-evolution < swift-evolution@swift.org> wrote:

on Fri May 06 2016, Andrew Bennett <swift-evolution@swift.org> wrote:

--

It may be useful if Swift allowed things like this:

let x = switch { ... }
let x = if { ... } else { ... }
etc.

I think that's a much larger change/discussion, with no clear victor.

However until Swift has that support it's necessary to consider
separated initialization
and declaration. Likewise until all Swift is pure functional.

Even if this proposal didn't let you assign to let statements outside the
closure it still has value:

   - It lets the type system reduce programmer error
   - It allows protocol declarations to have a more explicit requirement
   - The user can guarantee that their code, and its side-effects, will be
   executed

IMO separating initialization from declaration is *very* rarely needed

and very much better avoided altogether, because it leads to code that's
less clear. Just because we *can* do this doesn't mean we should.

> On Sat, May 7, 2016 at 6:24 AM, Dave Abrahams via swift-evolution > > <swift-evolution@swift.org> wrote:
>
> on Tue May 03 2016, Chris Lattner > > <swift-evolution@swift.org> wrote:
>
> > Hello Swift community,
> >
> > The review of "SE-0073: Marking closures as executing exactly once"
> > begins now and runs through May 9. The proposal is available here:
> >
> >
>
https://github.com/apple/swift-evolution/blob/master/proposals/0073-noescape-once.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?
>
> I think it's of questionable importance and doesn't generalize well.
> For example, you can't use this to construct something like
>
> var x: Int
> functionThatActsLikeIf( someTest(), then: { x = 1 }, else: { x = 2} )
>
> If you need to initialize something in an outer scope with something
> computed by a closure, it's much better to arrange something like
this:
>
> var x = functionThatActsLikeIf( someTest(), then: { 1 }, else: { 2 }
)
>
> --
> 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


(Andrew Bennett) #18

* What is your evaluation of the proposal?
+1

I agree with others that there are opportunities to generalise this
proposal.

It would be pretty magical if it could be applied to escaping closures,
less magical if that's just adding a runtime assertion. It would also be
much more flexible if it could be used for multiple exclusive `if-else`
closures. However, I don't think this magic or flexibility is necessary to
make this proposal useful.

This proposal does add things you wouldn't be able to do otherwise, and
it's made clear to the user by the explicit pairing of @noescape and once.

···

--

It would be okay if (once) became @once, however I think this is only
useful if it was allowed on escaping closures. Adding compile-time
assurances to this is possible, but much more complicated (requiring strict
checks on storage, calling, passing).

I'm not sure if the added complexity of an escaping once is worth
considering until Swift concurrency (Swift 4?). I don't think the
guarantees would have any advantage for the compiler, and I don't think the
*currently* advantage to the user would be worth much more than a comment.

        * Is the problem being addressed significant enough to warrant a
change to Swift?
Yes
        * Does this proposal fit well with the feel and direction of Swift?
Yes
        * If you have used other languages or libraries with a similar
feature, how do you feel that this proposal compares to those?
no
        * How much effort did you put into your review? A glance, a quick
reading, or an in-depth study?
I have followed the thread, read the proposal, participated in the the
discussion.

On Sat, May 7, 2016 at 12:17 PM, Andrew Bennett <cacoyi@gmail.com> wrote:

Hi Dave,

Sorry, Dave, sending a second time as I forgot to Reply-All.

I agree, this proposal doesn't allow multiple closures where only one of
them should be run, and it should only be run once. I personally don't
think lacking that functionality is worth blocking this proposal for,
another proposal can be built on top of this if it is desired.

These cases can also be handled by a more meaningful if/switch statement,
using @noescape(once), for example:
  let x: Int
  functionThatCallsAClosure(someTest()) { x = $0 ? 1 : 2 }

On Sat, May 7, 2016 at 6:24 AM, Dave Abrahams via swift-evolution < > swift-evolution@swift.org> wrote:

on Tue May 03 2016, Chris Lattner <swift-evolution@swift.org> wrote:

> Hello Swift community,
>
> The review of "SE-0073: Marking closures as executing exactly once"
> begins now and runs through May 9. The proposal is available here:
>
>
https://github.com/apple/swift-evolution/blob/master/proposals/0073-noescape-once.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?

I think it's of questionable importance and doesn't generalize well.
For example, you can't use this to construct something like

  var x: Int
  functionThatActsLikeIf( someTest(), then: { x = 1 }, else: { x = 2} )

If you need to initialize something in an outer scope with something
computed by a closure, it's much better to arrange something like this:

  var x = functionThatActsLikeIf( someTest(), then: { 1 }, else: { 2 } )

--
Dave

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


(Dmitri Gribenko) #19

The difference is that the do{} case is type checked and codegen'ed
together with the rest of the function. The f{} case has two
functions that are type checked and codegen'ed more or less
separately (the function that contains the call to f, and the closure
itself). Moreover, with do{} the placement of the code block is
fixed. With f{}, you can save a closure in a variable and then call
f(myClosure). How would that affect the rules? How would you
implement the desired analysis?

Dmitri

···

On Thu, May 5, 2016 at 3:27 AM, Gwendal Roué <gwendal.roue@gmail.com> wrote:

I quite expect being able to throw out of a @noescape(once) block. Maybe the sentence "it must not be executed on any path that throws" should be removed from the proposal, should it have the implications you describe.

Here is below what I expect this proposal to allow. So you see one problematic case?

Hi Gwendal,

What about the following case?

// Function which rethrows closure errors:
func f1(closure: @noescape(once) () throws -> ()) rethrows {
try closure()
}

let x: AnyObject
f1 {
if someCondition() { x = MyClass() }
if someOtherCondition() { throw MyError.Error() }
x = MyOtherClass()
}

How do you handle memory management for 'x' on the path that throws?
If the rule is that upon returning from f1 via a throw the variable
'x' should not be initialized, then the closure passed to f1 has to
guarantee the deinitialization. But f1 accepts an arbitrary closure.

Hello Dmitri,

To reason about @noescape(once) functions, the easiest way is to replace them with `do`:

    let x: AnyObject
    do {
        if someCondition() { x = MyClass() }
        if someOtherCondition() { throw MyError.error }
        x = MyOtherClass()
    }

This code does not compile because x can't be initialized to MyOtherClass().

But I don't think this is your point. Your point was "how can the compiler handle memory management ?".

I can't answer this question, because I'm not competent enough. But if it can handle the do { … } case, can't it also handle the f { … } case?

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


#20

Hi Gwendal,

Nice writeup. So I see that you recognise how this extra specification will complicate (while also facilitate) things. And I also see that you're a co-author of the proposal. So I'm more than happy if you can squeeze this extra into it.

However reading your example code, I had to squint to see—without the compiler's squiggly red aid—what all the problematic cases are.

1) Is it obvious to everybody why you can't always use `x` in the end of Cases 2 and 4? For example, with `if—else` you MUST initialise the variable or escape its scope in all branches; you can't just write the following to complete the initialisation later:

    let x: Int
    if cond { x = 1 }
    // ...
    if !cond { x = 2 } // too late!
2) Should Cases 2 and 4 be made illegal? The requirement could then be that all `catch` blocks either:

Hello Pyry,

In case 2 and 4, you can't always use `x` at the end because there are code paths that do not initialize x.

Let's repeat the case 2 and 4, as a reminder:

Case 2:

  // Function which rethrows closure errors:
  func f1(closure: @noescape(once) () throws -> ()) rethrows {
    try closure()
  }
  let x: Int
  do {
    try f1 {
      x = try getX()
      // use x
    }
    // use x
  } catch {
    // can't use x
  }
  // can't use x

Case 4:

  // Function which may throw before, inside, or after the closure:
  func f2(closure: @noescape(once) () throws -> ()) throws {
    try mayFailBefore()
    try closure()
    try mayFailAfter()
  }
  let x: Int
  do {
    try f2 {
      x = try getX()
      // use x
    }
    // use x
  } catch {
    // can't use x
  }
  // can't use x

To better explain them, let's replace `f1` with `do`, and `f2` with a throwing function followed with do:

Rewritten case 2:

  let x: Int
  do {
    do {
      x = try getX()
      // use x
    }
    // use x
  } catch {
    // can't use x
  }
  // can't use x

Rewritten case 4:

  let x: Int
  do {
    try mayFail()
    do {
      x = try getX()
      // use x
    }
    // use x
  } catch {
    // can't use x
  }
  // can't use x

The examples above are already the behavior of the Swift compiler. I expect @noescape(once) closures to behave the same (the cases 2 and 4 above)

3) If you consider including this addition to the proposal, it might also help other reviewers if you explained how the compiler will be able to help the programmer write a valid program. E.g. what would the error messages about partially initialised variables look like? And where and when would they appear? Could the compiler suggest certain fixits? Etc.

For error messages about partially initialized variables, we just use the regular messages that we already have: `Constant 'x' used before being initialized` error.

Gwendal Roué

···

Le 4 mai 2016 à 11:55, Pyry Jahkola <pyry.jahkola@iki.fi> a écrit :