Skip to content

Commit 45497ac

Browse files
committed
[Concurrency] Make "self" parameter of actor-isolated functions 'isolated'.
The notion of "actor-isolated" currently exists at the declaration level. For functions, it is going to be captured in the function type itself, where 'self' is declared to be 'isolated'. Model isolation both ways: the 'self' of a method that is isolated to an actor instance will be 'isolated' as well. We are still using declaration-based checking of actor isolation. However, by mirroring this information we can move more incrementally over to doing checking based on 'isolated' parameters. (cherry picked from commit 5004a54)
1 parent db49cb1 commit 45497ac

23 files changed

+257
-117
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4528,9 +4528,6 @@ ERROR(nonisolated_local_var,none,
45284528
"'nonisolated' can not be applied to local variables",
45294529
())
45304530

4531-
ERROR(isolated_param_non_actor,none,
4532-
"'isolated' parameter cannot have not-actor type %0", (Type))
4533-
45344531
ERROR(actor_instance_property_wrapper,none,
45354532
"%0 property in property wrapper type %1 cannot be isolated to "
45364533
"the actor instance; consider 'nonisolated'",

include/swift/AST/TypeCheckRequests.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1023,6 +1023,20 @@ class ActorIsolationRequest :
10231023
bool isCached() const { return true; }
10241024
};
10251025

1026+
/// Determine whether the given function should have an isolated 'self'.
1027+
class HasIsolatedSelfRequest :
1028+
public SimpleRequest<HasIsolatedSelfRequest,
1029+
bool(ValueDecl *),
1030+
RequestFlags::Uncached> {
1031+
public:
1032+
using SimpleRequest::SimpleRequest;
1033+
1034+
private:
1035+
friend SimpleRequest;
1036+
1037+
bool evaluate(Evaluator &evaluator, ValueDecl *func) const;
1038+
};
1039+
10261040
/// Request whether the storage has a mutating getter.
10271041
class IsGetterMutatingRequest :
10281042
public SimpleRequest<IsGetterMutatingRequest,

include/swift/AST/TypeCheckerTypeIDZone.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,9 @@ SWIFT_REQUEST(TypeChecker, GlobalActorAttributeRequest,
112112
SWIFT_REQUEST(TypeChecker, ActorIsolationRequest,
113113
ActorIsolationState(ValueDecl *),
114114
Cached, NoLocationInfo)
115+
SWIFT_REQUEST(TypeChecker, HasIsolatedSelfRequest,
116+
bool(ValueDecl *),
117+
Uncached, NoLocationInfo)
115118
SWIFT_REQUEST(TypeChecker, FunctionOperatorRequest, OperatorDecl *(FuncDecl *),
116119
Cached, NoLocationInfo)
117120
SWIFT_REQUEST(NameLookup, GenericSignatureRequest,

include/swift/AST/Types.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2870,6 +2870,9 @@ class AnyFunctionType : public TypeBase {
28702870
/// Whether the parameter is marked '@_nonEphemeral'
28712871
bool isNonEphemeral() const { return Flags.isNonEphemeral(); }
28722872

2873+
/// Whether the parameter is 'isolated'.
2874+
bool isIsolated() const { return Flags.isIsolated(); }
2875+
28732876
/// Whether the parameter is marked '@noDerivative'.
28742877
bool isNoDerivative() const { return Flags.isNoDerivative(); }
28752878

include/swift/Parse/Parser.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1362,6 +1362,9 @@ class Parser {
13621362
EnumElement,
13631363
};
13641364

1365+
/// Whether we are at the start of a parameter name when parsing a parameter.
1366+
bool startsParameterName(bool isClosure);
1367+
13651368
/// Parse a parameter-clause.
13661369
///
13671370
/// \verbatim

lib/AST/ASTContext.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2687,6 +2687,7 @@ AnyFunctionType::Param swift::computeSelfParam(AbstractFunctionDecl *AFD,
26872687
bool isStatic = false;
26882688
SelfAccessKind selfAccess = SelfAccessKind::NonMutating;
26892689
bool isDynamicSelf = false;
2690+
bool isIsolated = false;
26902691

26912692
if (auto *FD = dyn_cast<FuncDecl>(AFD)) {
26922693
isStatic = FD->isStatic();
@@ -2703,6 +2704,11 @@ AnyFunctionType::Param swift::computeSelfParam(AbstractFunctionDecl *AFD,
27032704
// FIXME: All methods of non-final classes should have this.
27042705
else if (wantDynamicSelf && FD->hasDynamicSelfResult())
27052706
isDynamicSelf = true;
2707+
2708+
// If this is a non-static method within an actor, the 'self' parameter
2709+
// is isolated if this declaration is isolated.
2710+
isIsolated = evaluateOrDefault(
2711+
Ctx.evaluator, HasIsolatedSelfRequest{AFD}, false);
27062712
} else if (auto *CD = dyn_cast<ConstructorDecl>(AFD)) {
27072713
if (isInitializingCtor) {
27082714
// initializing constructors of value types always have an implicitly
@@ -2736,7 +2742,7 @@ AnyFunctionType::Param swift::computeSelfParam(AbstractFunctionDecl *AFD,
27362742
if (isStatic)
27372743
return AnyFunctionType::Param(MetatypeType::get(selfTy, Ctx));
27382744

2739-
auto flags = ParameterTypeFlags();
2745+
auto flags = ParameterTypeFlags().withIsolated(isIsolated);
27402746
switch (selfAccess) {
27412747
case SelfAccessKind::Consuming:
27422748
flags = flags.withOwned(true);

lib/AST/ASTPrinter.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3240,6 +3240,9 @@ static void printParameterFlags(ASTPrinter &printer,
32403240
break;
32413241
}
32423242

3243+
if (flags.isIsolated())
3244+
printer.printKeyword("isolated", options, " ");
3245+
32433246
if (!options.excludeAttrKind(TAK_escaping) && escaping)
32443247
printer.printKeyword("@escaping", options, " ");
32453248
}

lib/AST/ASTVerifier.cpp

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1762,6 +1762,26 @@ class Verifier : public ASTWalker {
17621762
}
17631763
}
17641764

1765+
/// A version of AnyFunctionType::equalParams() that ignores "isolated"
1766+
/// parameters, which aren't represented in the type system.
1767+
static bool equalParamsIgnoringIsolation(
1768+
ArrayRef<AnyFunctionType::Param> a,
1769+
ArrayRef<AnyFunctionType::Param> b) {
1770+
auto withoutIsolation = [](AnyFunctionType::Param param) {
1771+
return param.withFlags(param.getParameterFlags().withIsolated(false));
1772+
};
1773+
1774+
if (a.size() != b.size())
1775+
return false;
1776+
1777+
for (unsigned i = 0, n = a.size(); i != n; ++i) {
1778+
if (withoutIsolation(a[i]) != withoutIsolation(b[i]))
1779+
return false;
1780+
}
1781+
1782+
return true;
1783+
}
1784+
17651785
void verifyChecked(ApplyExpr *E) {
17661786
PrettyStackTraceExpr debugStack(Ctx, "verifying ApplyExpr", E);
17671787

@@ -1786,7 +1806,7 @@ class Verifier : public ASTWalker {
17861806
Type InputExprTy = E->getArg()->getType();
17871807
AnyFunctionType::decomposeInput(InputExprTy, Args);
17881808
auto Params = FT->getParams();
1789-
if (!AnyFunctionType::equalParams(Args, Params)) {
1809+
if (!equalParamsIgnoringIsolation(Args, Params)) {
17901810
Out << "Argument type does not match parameter type in ApplyExpr:"
17911811
"\nArgument type: ";
17921812
InputExprTy.print(Out);

lib/Parse/ParsePattern.cpp

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -150,22 +150,37 @@ static ParserStatus parseDefaultArgument(
150150

151151
/// Determine whether we are at the start of a parameter name when
152152
/// parsing a parameter.
153-
static bool startsParameterName(Parser &parser, bool isClosure) {
153+
bool Parser::startsParameterName(bool isClosure) {
154154
// '_' cannot be a type, so it must be a parameter name.
155-
if (parser.Tok.is(tok::kw__))
155+
if (Tok.is(tok::kw__))
156156
return true;
157157

158158
// To have a parameter name here, we need a name.
159-
if (!parser.Tok.canBeArgumentLabel())
159+
if (!Tok.canBeArgumentLabel())
160160
return false;
161161

162-
// If the next token can be an argument label or is ':', this is a name.
163-
const auto &nextTok = parser.peekToken();
164-
if (nextTok.is(tok::colon) || nextTok.canBeArgumentLabel())
162+
// If the next token is ':', this is a name.
163+
const auto &nextTok = peekToken();
164+
if (nextTok.is(tok::colon))
165165
return true;
166166

167-
if (parser.isOptionalToken(nextTok)
168-
|| parser.isImplicitlyUnwrappedOptionalToken(nextTok))
167+
// If the next token can be an argument label, we might have a name.
168+
if (nextTok.canBeArgumentLabel()) {
169+
// If the first name wasn't "isolated", we're done.
170+
if (!Tok.isContextualKeyword("isolated"))
171+
return true;
172+
173+
// "isolated" can be an argument label, but it's also a contextual keyword,
174+
// so look ahead one more token see if we have a ':' that would indicate
175+
// that this is an argument label.
176+
BacktrackingScope backtrackScope(*this);
177+
consumeToken();
178+
consumeToken();
179+
return Tok.is(tok::colon);
180+
}
181+
182+
if (isOptionalToken(nextTok)
183+
|| isImplicitlyUnwrappedOptionalToken(nextTok))
169184
return false;
170185

171186
// The identifier could be a name or it could be a type. In a closure, we
@@ -289,7 +304,7 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
289304
.fixItReplace(Tok.getLoc(), "`" + Tok.getText().str() + "`");
290305
}
291306

292-
if (startsParameterName(*this, isClosure)) {
307+
if (startsParameterName(isClosure)) {
293308
// identifier-or-none for the first name
294309
param.FirstNameLoc = consumeArgumentLabel(param.FirstName,
295310
/*diagnoseDollarPrefix=*/!isClosure);

lib/Parse/ParseType.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1027,9 +1027,7 @@ ParserResult<TypeRepr> Parser::parseTypeTupleBody() {
10271027
// If the tuple element starts with a potential argument label followed by a
10281028
// ':' or another potential argument label, then the identifier is an
10291029
// element tag, and it is followed by a type annotation.
1030-
if (Tok.canBeArgumentLabel()
1031-
&& (peekToken().is(tok::colon)
1032-
|| peekToken().canBeArgumentLabel())) {
1030+
if (startsParameterName(false)) {
10331031
// Consume a name.
10341032
element.NameLoc = consumeArgumentLabel(element.Name,
10351033
/*diagnoseDollarPrefix=*/true);

0 commit comments

Comments
 (0)