Skip to content

[Concurrency] explicit nonisolated specification for closures #72175

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions include/swift/AST/DiagnosticsParse.def
Original file line number Diff line number Diff line change
Expand Up @@ -1296,6 +1296,8 @@ ERROR(anon_closure_tuple_param_destructuring,none,
"closure tuple parameter does not support destructuring", ())
ERROR(expected_closure_parameter_name,none,
"expected the name of a closure parameter", ())
ERROR(nonisolated_closure_parameter_parentheses,none,
"the parameter list of a 'nonisolated' closure requires parentheses", ())
ERROR(expected_capture_specifier,none,
"expected 'weak', 'unowned', or no specifier in capture list", ())
ERROR(expected_capture_specifier_name,none,
Expand Down
3 changes: 3 additions & 0 deletions include/swift/Basic/Features.def
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,9 @@ EXPERIMENTAL_FEATURE(DynamicActorIsolation, false)
// Allow for `switch` of noncopyable values to be borrowing or consuming.
EXPERIMENTAL_FEATURE(BorrowingSwitch, true)

// Enable explicit isolation of closures.
EXPERIMENTAL_FEATURE(ClosureIsolation, true)

// Enable isolated(any) attribute on function types.
CONDITIONALLY_SUPPRESSIBLE_EXPERIMENTAL_FEATURE(IsolatedAny, true)

Expand Down
4 changes: 4 additions & 0 deletions include/swift/Parse/Parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ class Parser {
void operator=(const Parser&) = delete;

bool IsInputIncomplete = false;
bool EnableParameterizedNonisolated = true; // HACK: for closures
std::vector<Token> SplitTokens;

public:
Expand Down Expand Up @@ -1042,6 +1043,9 @@ class Parser {
bool IfConfigsAreDeclAttrs,
PatternBindingInitializer *initContext);

/// Parse the optional attributes before a closure declaration.
ParserStatus parseClosureDeclAttributeList(DeclAttributes &Attributes);

/// Parse the optional modifiers before a declaration.
ParserStatus parseDeclModifierList(DeclAttributes &Attributes,
SourceLoc &StaticLoc,
Expand Down
5 changes: 4 additions & 1 deletion lib/AST/ASTDumper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2792,10 +2792,13 @@ class PrintExpr : public ExprVisitor<PrintExpr, void, StringRef>,

switch (auto isolation = E->getActorIsolation()) {
case ActorIsolation::Unspecified:
case ActorIsolation::Nonisolated:
case ActorIsolation::NonisolatedUnsafe:
break;

case ActorIsolation::Nonisolated:
printFlag(true, "nonisolated", CapturesColor);
break;

case ActorIsolation::Erased:
printFlag(true, "dynamically_isolated", CapturesColor);
break;
Expand Down
2 changes: 2 additions & 0 deletions lib/AST/FeatureSet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,8 @@ static bool usesFeatureDynamicActorIsolation(Decl *decl) {

UNINTERESTING_FEATURE(BorrowingSwitch)

UNINTERESTING_FEATURE(ClosureIsolation)

static bool usesFeatureIsolatedAny(Decl *decl) {
return usesTypeMatching(decl, [](Type type) {
if (auto fnType = type->getAs<AnyFunctionType>()) {
Expand Down
45 changes: 40 additions & 5 deletions lib/Parse/ParseDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3877,11 +3877,14 @@ ParserStatus Parser::parseNewDeclAttribute(DeclAttributes &Attributes,
break;
}
case DeclAttrKind::Nonisolated: {
auto isUnsafe =
parseSingleAttrOption<bool>(*this, Loc, AttrRange, AttrName, DK,
{{Context.Id_unsafe, true}}, false);
if (!isUnsafe) {
return makeParserSuccess();
std::optional<bool> isUnsafe(false);
if (EnableParameterizedNonisolated) {
isUnsafe =
parseSingleAttrOption<bool>(*this, Loc, AttrRange, AttrName, DK,
{{Context.Id_unsafe, true}}, *isUnsafe);
if (!isUnsafe) {
return makeParserSuccess();
}
}

if (!DiscardAttribute) {
Expand Down Expand Up @@ -5180,6 +5183,38 @@ ParserStatus Parser::parseDeclAttributeList(
return parseDeclAttributeList(Attributes, IfConfigsAreDeclAttrs, initContext);
}

// effectively parseDeclAttributeList but with selective modifier handling
ParserStatus Parser::parseClosureDeclAttributeList(DeclAttributes &Attributes) {
auto parsingNonisolated = [this] {
return Context.LangOpts.hasFeature(Feature::ClosureIsolation) &&
Tok.isContextualKeyword("nonisolated");
};

if (Tok.isNot(tok::at_sign, tok::pound_if) && !parsingNonisolated())
return makeParserSuccess();

PatternBindingInitializer *initContext = nullptr;
constexpr bool ifConfigsAreDeclAttrs = false;
ParserStatus Status;
while (Tok.isAny(tok::at_sign, tok::pound_if) || parsingNonisolated()) {
if (Tok.is(tok::at_sign)) {
SourceLoc AtEndLoc = Tok.getRange().getEnd();
SourceLoc AtLoc = consumeToken();
Status |= parseDeclAttribute(Attributes, AtLoc, AtEndLoc, initContext);
} else if (parsingNonisolated()) {
Status |=
parseNewDeclAttribute(Attributes, {}, DeclAttrKind::Nonisolated);
} else {
if (!ifConfigsAreDeclAttrs && !ifConfigContainsOnlyAttributes()) {
break;
}
Status |= parseIfConfigDeclAttributes(Attributes, ifConfigsAreDeclAttrs,
initContext);
}
}
return Status;
}

/// \verbatim
/// modifier-list
/// /* empty */
Expand Down
42 changes: 33 additions & 9 deletions lib/Parse/ParseExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,22 @@
//
//===----------------------------------------------------------------------===//

#include "swift/Parse/Parser.h"
#include "swift/AST/ASTWalker.h"
#include "swift/AST/Attr.h"
#include "swift/AST/DiagnosticsParse.h"
#include "swift/AST/TypeRepr.h"
#include "swift/Basic/Defer.h"
#include "swift/Basic/EditorPlaceholder.h"
#include "swift/Basic/StringExtras.h"
#include "swift/Parse/IDEInspectionCallbacks.h"
#include "swift/Parse/Parser.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringSwitch.h"
#include "llvm/ADT/Twine.h"
#include "swift/Basic/Defer.h"
#include "swift/Basic/StringExtras.h"
#include "llvm/Support/Compiler.h"
#include "llvm/Support/SaveAndRestore.h"
#include "llvm/Support/raw_ostream.h"
#include <utility>

using namespace swift;

Expand Down Expand Up @@ -2579,8 +2580,16 @@ ParserStatus Parser::parseClosureSignatureIfPresent(
BacktrackingScope backtrack(*this);

// Consume attributes.
while (Tok.is(tok::at_sign)) {
skipAnyAttribute();
auto parsingNonisolated = [this] {
return Context.LangOpts.hasFeature(Feature::ClosureIsolation) &&
Tok.isContextualKeyword("nonisolated");
};
while (Tok.is(tok::at_sign) || parsingNonisolated()) {
if (parsingNonisolated()) {
consumeToken();
} else {
skipAnyAttribute();
}
}

// Skip by a closure capture list if present.
Expand Down Expand Up @@ -2644,7 +2653,12 @@ ParserStatus Parser::parseClosureSignatureIfPresent(
return makeParserSuccess();
}
ParserStatus status;
(void)parseDeclAttributeList(attributes);
// 'nonisolated' cannot be parameterized in a closure due to ambiguity with
// closure parameters.
const auto entryNonisolatedState =
std::exchange(EnableParameterizedNonisolated, false);
(void)parseClosureDeclAttributeList(attributes);
EnableParameterizedNonisolated = entryNonisolatedState;

if (Tok.is(tok::l_square) && peekToken().is(tok::r_square)) {
SourceLoc lBracketLoc = consumeToken(tok::l_square);
Expand Down Expand Up @@ -2814,6 +2828,7 @@ ParserStatus Parser::parseClosureSignatureIfPresent(
} else {
// Parse identifier (',' identifier)*
SmallVector<ParamDecl*, 4> elements;
const auto parameterListLoc = Tok.getLoc();
bool HasNext;
do {
if (Tok.isNot(tok::identifier, tok::kw__, tok::code_complete)) {
Expand Down Expand Up @@ -2843,6 +2858,15 @@ ParserStatus Parser::parseClosureSignatureIfPresent(
} while (HasNext);

params = ParameterList::create(Context, elements);

if (Context.LangOpts.hasFeature(Feature::ClosureIsolation) && params &&
(params->size() > 0) && attributes.hasAttribute<NonisolatedAttr>()) {
diagnose(parameterListLoc,
diag::nonisolated_closure_parameter_parentheses)
.fixItInsert(parameterListLoc, "(")
.fixItInsert(Tok.getLoc(), ")");
status.setIsParseError();
}
}

TypeRepr *thrownTypeRepr = nullptr;
Expand Down Expand Up @@ -2973,13 +2997,13 @@ ParserResult<Expr> Parser::parseExprClosure() {
DeclAttributes attributes;
SourceRange bracketRange;
SmallVector<CaptureListEntry, 2> captureList;
VarDecl *capturedSelfDecl;
VarDecl *capturedSelfDecl = nullptr;
ParameterList *params = nullptr;
SourceLoc asyncLoc;
SourceLoc throwsLoc;
TypeExpr *thrownType;
TypeExpr *thrownType = nullptr;
SourceLoc arrowLoc;
TypeExpr *explicitResultType;
TypeExpr *explicitResultType = nullptr;
SourceLoc inLoc;
Status |= parseClosureSignatureIfPresent(
attributes, bracketRange, captureList, capturedSelfDecl, params, asyncLoc,
Expand Down
7 changes: 7 additions & 0 deletions lib/Sema/TypeCheckAttr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7374,6 +7374,13 @@ class ClosureAttributeChecker
// Nothing else to check.
}

void visitNonisolatedAttr(NonisolatedAttr *attr) {
if (attr->isUnsafe() ||
!ctx.LangOpts.hasFeature(Feature::ClosureIsolation)) {
visitDeclAttribute(attr);
}
}

void visitCustomAttr(CustomAttr *attr) {
// Check whether this custom attribute is the global actor attribute.
auto globalActorAttr = evaluateOrDefault(
Expand Down
7 changes: 7 additions & 0 deletions lib/Sema/TypeCheckConcurrency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4050,6 +4050,13 @@ namespace {
if (Type globalActor = resolveGlobalActorType(explicitClosure))
return ActorIsolation::forGlobalActor(globalActor)
.withPreconcurrency(preconcurrency);

if (auto *attr =
explicitClosure->getAttrs().getAttribute<NonisolatedAttr>();
attr && ctx.LangOpts.hasFeature(Feature::ClosureIsolation)) {
return ActorIsolation::forNonisolated(attr->isUnsafe())
.withPreconcurrency(preconcurrency);
}
}

// If a closure has an isolated parameter, it is isolated to that
Expand Down
8 changes: 7 additions & 1 deletion test/Concurrency/closure_isolation.swift
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// RUN: %target-swift-frontend -dump-ast %s -disable-availability-checking | %FileCheck %s
// RUN: %target-swift-frontend -dump-ast %s -disable-availability-checking -enable-experimental-feature ClosureIsolation | %FileCheck %s

// REQUIRES: concurrency
// REQUIRES: asserts

func acceptClosure<T>(_: () -> T) { }
func acceptSendableClosure<T>(_: @Sendable () -> T) { }
Expand Down Expand Up @@ -45,6 +46,11 @@ extension MyActor {
// CHECK: closure_expr
// CHECK: actor_isolated
acceptEscapingAsyncClosure { () async in print(self) }

// CHECK: acceptClosure
// CHECK: closure_expr
// CHECK: nonisolated
acceptClosure { nonisolated in print() }
}
}

Expand Down
4 changes: 2 additions & 2 deletions test/Frontend/dump-parse.swift
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,14 @@ escaping({ $0 })
// CHECK-AST: (declref_expr type="(@escaping (Int) -> Int) -> ()"
// CHECK-AST-NEXT: (argument_list
// CHECK-AST-NEXT: (argument
// CHECK-AST-NEXT: (closure_expr type="(Int) -> Int" {{.*}} discriminator=0 escaping single_expression
// CHECK-AST-NEXT: (closure_expr type="(Int) -> Int" {{.*}} discriminator=0 nonisolated escaping single_expression

func nonescaping(_: (Int) -> Int) {}
nonescaping({ $0 })
// CHECK-AST: (declref_expr type="((Int) -> Int) -> ()"
// CHECK-AST-NEXT: (argument_list
// CHECK-AST-NEXT: (argument
// CHECK-AST-NEXT: (closure_expr type="(Int) -> Int" {{.*}} discriminator=1 single_expression
// CHECK-AST-NEXT: (closure_expr type="(Int) -> Int" {{.*}} discriminator=1 nonisolated single_expression

// CHECK-LABEL: (struct_decl range=[{{.+}}] "MyStruct")
struct MyStruct {}
Expand Down
14 changes: 13 additions & 1 deletion test/Parse/trailing_closures.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
// RUN: %target-typecheck-verify-swift
// RUN: %target-typecheck-verify-swift -enable-experimental-feature ClosureIsolation

// REQUIRES: asserts

func foo<T, U>(a: () -> T, b: () -> U) {}

Expand Down Expand Up @@ -120,3 +122,13 @@ struct TrickyTest {
3
}
}

struct IsolationTest {
func acceptClosureWithParameter(_: (Int) -> Int) {}

func test() {
acceptClosureWithParameter {
nonisolated parameter in return parameter // expected-error {{the parameter list of a 'nonisolated' closure requires parentheses}} {{19-19=(}} {{29-29=)}}
}
}
}
8 changes: 4 additions & 4 deletions test/Sema/property_wrappers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -70,17 +70,17 @@ public class W_58201<Value> {
struct S1_58201 {
// CHECK: argument_list implicit labels="wrappedValue:"
// CHECK-NEXT: argument label="wrappedValue"
// CHECK-NEXT: autoclosure_expr implicit type="() -> Bool?" discriminator=0 captures=(<opaque_value> ) escaping
// CHECK: autoclosure_expr implicit type="() -> Bool?" discriminator=1 escaping
// CHECK-NEXT: autoclosure_expr implicit type="() -> Bool?" discriminator=0 nonisolated captures=(<opaque_value> ) escaping
// CHECK: autoclosure_expr implicit type="() -> Bool?" discriminator=1 nonisolated escaping
@W_58201 var a: Bool?
}

// CHECK-LABEL: struct_decl{{.*}}S2_58201
struct S2_58201 {
// CHECK: argument_list implicit labels="wrappedValue:"
// CHECK-NEXT: argument label="wrappedValue"
// CHECK-NEXT: autoclosure_expr implicit type="() -> Bool" location={{.*}}.swift:[[@LINE+2]]:26 range=[{{.+}}] discriminator=0 captures=(<opaque_value> ) escaping
// CHECK: autoclosure_expr implicit type="() -> Bool" location={{.*}}.swift:[[@LINE+1]]:26 range=[{{.+}}] discriminator=1 escaping
// CHECK-NEXT: autoclosure_expr implicit type="() -> Bool" location={{.*}}.swift:[[@LINE+2]]:26 range=[{{.+}}] discriminator=0 nonisolated captures=(<opaque_value> ) escaping
// CHECK: autoclosure_expr implicit type="() -> Bool" location={{.*}}.swift:[[@LINE+1]]:26 range=[{{.+}}] discriminator=1 nonisolated escaping
@W_58201 var b: Bool = false
}

Expand Down
2 changes: 1 addition & 1 deletion test/expr/capture/dynamic_self.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

class Android {
func clone() -> Self {
// CHECK: closure_expr type="() -> Self" {{.*}} discriminator=0 captures=(<dynamic_self> self<direct>)
// CHECK: closure_expr type="() -> Self" {{.*}} discriminator=0 nonisolated captures=(<dynamic_self> self<direct>)
let fn = { return self }
return fn()
}
Expand Down
6 changes: 3 additions & 3 deletions test/expr/capture/generic_params.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,16 @@ func doSomething<T>(_ t: T) {}

func outerGeneric<T>(t: T, x: AnyObject) {
// Simple case -- closure captures outer generic parameter
// CHECK: closure_expr type="() -> ()" {{.*}} discriminator=0 captures=(<generic> t<direct>) escaping single_expression
// CHECK: closure_expr type="() -> ()" {{.*}} discriminator=0 nonisolated captures=(<generic> t<direct>) escaping single_expression
_ = { doSomething(t) }

// Special case -- closure does not capture outer generic parameters
// CHECK: closure_expr type="() -> ()" {{.*}} discriminator=1 captures=(x<direct>) escaping single_expression
// CHECK: closure_expr type="() -> ()" {{.*}} discriminator=1 nonisolated captures=(x<direct>) escaping single_expression
_ = { doSomething(x) }

// Special case -- closure captures outer generic parameter, but it does not
// appear as the type of any expression
// CHECK: closure_expr type="() -> ()" {{.*}} discriminator=2 captures=(<generic> x<direct>)
// CHECK: closure_expr type="() -> ()" {{.*}} discriminator=2 nonisolated captures=(<generic> x<direct>)
_ = { if x is T {} }

// Nested generic functions always capture outer generic parameters, even if
Expand Down
4 changes: 2 additions & 2 deletions test/expr/capture/nested.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

// CHECK: func_decl{{.*}}"foo2(_:)"
func foo2(_ x: Int) -> (Int) -> (Int) -> Int {
// CHECK: closure_expr type="(Int) -> (Int) -> Int" {{.*}} discriminator=0 captures=(x)
// CHECK: closure_expr type="(Int) -> (Int) -> Int" {{.*}} discriminator=0 nonisolated captures=(x)
return {(bar: Int) -> (Int) -> Int in
// CHECK: closure_expr type="(Int) -> Int" {{.*}} discriminator=0 captures=(x<direct>, bar<direct>)
// CHECK: closure_expr type="(Int) -> Int" {{.*}} discriminator=0 nonisolated captures=(x<direct>, bar<direct>)
return {(bas: Int) -> Int in
return x + bar + bas
}
Expand Down