[Concurrency] explicit nonisolated specification for closures

[sophiapoirier]

https://github.com/sophiapoirier/swift-evolution/blob/closure-isolation/proposals/nnnn-closure-isolation-control.md

[ktoso]

This reminded me about the let nonisolated = ""; Task { nonisolated in } case, did we figure out how to deal with those, like that I guess or we ignore that weird case?

Task {`nonisolated` in }

[sophiapoirier]

I believe that is not an issue. Your example already is valid code and, without this change, would declare an inferred-type parameter to the closure nonisolated, so that would be inner scope and not conflict with the outer declaration of a variable named nonisolated. The change does introduce a source break in the case that someone intended to have a closure parameter named nonisolated, which I do address in the proposal.

[ahoppen]

I don’t think this approach works.

For example the following is currently valid Swift code:

let closure = { open in
    print(open)
}
closure("abc")

But with this change I assume that open would get parsed as a decl modifier and it would stop working. It would definitely be good to add a test case for something like this.

---

Also, this needs implementation in the new parser as well

[sophiapoirier]

I don’t think this approach works.

For example the following is currently valid Swift code:

let closure = { open in
    print(open)
}
closure("abc")

But with this change I assume that open would get parsed as a decl modifier and it would stop working. It would definitely be good to add a test case for something like this.

Ah good point. My current implementation will at least fail later any modifier other than nonisolated, however that will increase the potential for source breaking to stop allowing other modifiers or contextual keywords to be used as closure parameter names. I will restore an earlier albeit more complicated implementation that I had been using...

Also, this needs implementation in the new parser as well

Yes sorry, I meant to comment that I am working on that, but due to the nature of the change, the lack of it does not block CI here so I wanted to open this for review earlier rather than wait.

[ahoppen]

FWIW I think that even the source break for nonisolated is a blocker for implementing decl modifiers in closures like this.

[sophiapoirier]

FWIW I think that even the source break for nonisolated is a blocker for implementing decl modifiers in closures like this.

No one on the pitch as of yet has expressed concern about this, so you may want to if it worries you: https://forums.swift.org/t/closure-isolation-control/70378

[xedin]

Does this flag need to be serialized into AbstractClosureExprLayout as well?

[xedin]

Since you added -enable-experimental-feature this test-case now needs // REQUIRES: asserts

[sophiapoirier]

Does this flag need to be serialized into AbstractClosureExprLayout as well?

I do not believe so. Technically speaking, no flag or new state has been added. Closures already have had formal isolation, just not the ability to explicitly specify the option of nonisolated. Looking at SerializedAbstractClosureExpr, its purpose is described to preserve AST structure. Discussing with @rjmccall , it seems that if there a need arose to know the isolation for a DeclContext, we should serialize the isolation property itself. We do not currently serialize anything from the context.

[xedin]

@sophiapoirier What I meant by "flag" is whether nonisolated is stated on the closure or not. If I understand it correctly, previously isolation was always inferred now it could be explicit, so if the closure is deserialized and something asks for its isolation without the nonisolated bit preserved is there a possibility such a request would produce an incorrect result?

[sophiapoirier]

@sophiapoirier What I meant by "flag" is whether nonisolated is stated on the closure or not. If I understand it correctly, previously isolation was always inferred now it could be explicit, so if the closure is deserialized and something asks for its isolation without the nonisolated bit preserved is there a possibility such a request would produce an incorrect result?

Ah I see. I would still go back though to what I said about currently SerializedAbstractClosureExpr and its counterpart AbstractClosureExprLayout serialize nothing about the context and no other attributes, no parameters, no effect specifiers, etc.

[sophiapoirier]

Due to bike shedding on syntax, I am holding off on swift-syntax implementation pending language evolution review decisions on a final syntax direction. Currently I have omitted one test that would fail due to different results from swift-syntax, but I will introduce it along with a final syntax implementation.

[sophiapoirier]

squashing resets CI results??? RUDE.

[sophiapoirier]

@swift-ci please test