Skip to content

[concurrency] implement the 'unsafe' option for @actorIndependent #34260

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 3 commits into from
Oct 21, 2020
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
20 changes: 18 additions & 2 deletions include/swift/AST/ActorIsolation.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ class ActorIsolation {
/// meaning that it can be used from any actor but is also unable to
/// refer to the isolated state of any given actor.
Independent,
/// The declaration is explicitly specified to be independent of any actor,
/// but the programmer promises to protect the declaration from concurrent
/// accesses manually. Thus, it is okay if this declaration is a mutable
/// variable that creates storage.
IndependentUnsafe,
/// The declaration is isolated to a global actor. It can refer to other
/// entities with the same global actor.
GlobalActor,
Expand All @@ -70,8 +75,18 @@ class ActorIsolation {
return ActorIsolation(Unspecified, nullptr);
}

static ActorIsolation forIndependent() {
return ActorIsolation(Independent, nullptr);
static ActorIsolation forIndependent(ActorIndependentKind indepKind) {
ActorIsolation::Kind isoKind;
switch (indepKind) {
case ActorIndependentKind::Safe:
isoKind = Independent;
break;

case ActorIndependentKind::Unsafe:
isoKind = IndependentUnsafe;
break;
}
return ActorIsolation(isoKind, nullptr);
}

static ActorIsolation forActorInstance(ClassDecl *actor) {
Expand Down Expand Up @@ -112,6 +127,7 @@ class ActorIsolation {

switch (lhs.kind) {
case Independent:
case IndependentUnsafe:
case Unspecified:
return true;

Expand Down
2 changes: 1 addition & 1 deletion include/swift/AST/Attr.def
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ CONTEXTUAL_SIMPLE_DECL_ATTR(actor, Actor,
APIBreakingToAdd | APIBreakingToRemove,
102)

SIMPLE_DECL_ATTR(actorIndependent, ActorIndependent,
DECL_ATTR(actorIndependent, ActorIndependent,
OnFunc | OnVar | OnSubscript | ConcurrencyOnly |
ABIStableToAdd | ABIStableToRemove |
APIStableToAdd | APIBreakingToRemove,
Expand Down
23 changes: 23 additions & 0 deletions include/swift/AST/Attr.h
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,10 @@ class DeclAttribute : public AttributeBase {
kind : NumInlineKindBits
);

SWIFT_INLINE_BITFIELD(ActorIndependentAttr, DeclAttribute, NumActorIndependentKindBits,
kind : NumActorIndependentKindBits
);

SWIFT_INLINE_BITFIELD(OptimizeAttr, DeclAttribute, NumOptimizationModeBits,
mode : NumOptimizationModeBits
);
Expand Down Expand Up @@ -1329,6 +1333,25 @@ class ReferenceOwnershipAttr : public DeclAttribute {
}
};

/// Represents an actorIndependent/actorIndependent(unsafe) decl attribute.
class ActorIndependentAttr : public DeclAttribute {
public:
ActorIndependentAttr(SourceLoc atLoc, SourceRange range, ActorIndependentKind kind)
: DeclAttribute(DAK_ActorIndependent, atLoc, range, /*Implicit=*/false) {
Bits.ActorIndependentAttr.kind = unsigned(kind);
}

ActorIndependentAttr(ActorIndependentKind kind, bool IsImplicit=false)
: ActorIndependentAttr(SourceLoc(), SourceRange(), kind) {
setImplicit(IsImplicit);
}

ActorIndependentKind getKind() const { return ActorIndependentKind(Bits.ActorIndependentAttr.kind); }
static bool classof(const DeclAttribute *DA) {
return DA->getKind() == DAK_ActorIndependent;
}
};

/// Defines the attribute that we use to model documentation comments.
class RawDocCommentAttr : public DeclAttribute {
/// Source range of the attached comment. This comment is located before
Expand Down
11 changes: 11 additions & 0 deletions include/swift/AST/AttrKind.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,17 @@ enum class InlineKind : uint8_t {
enum : unsigned { NumInlineKindBits =
countBitsUsed(static_cast<unsigned>(InlineKind::Last_InlineKind)) };


/// Indicates whether an actorIndependent decl is unsafe or not
enum class ActorIndependentKind : uint8_t {
Safe = 0,
Unsafe = 1,
Last_InlineKind = Unsafe
};

enum : unsigned { NumActorIndependentKindBits =
countBitsUsed(static_cast<unsigned>(ActorIndependentKind::Last_InlineKind)) };

/// This enum represents the possible values of the @_effects attribute.
/// These values are ordered from the strongest guarantee to the weakest,
/// so please do not reorder existing values.
Expand Down
14 changes: 6 additions & 8 deletions include/swift/AST/DiagnosticsParse.def
Original file line number Diff line number Diff line change
Expand Up @@ -1358,6 +1358,12 @@ ERROR(attr_expected_comma,none,
ERROR(attr_expected_string_literal,none,
"expected string literal in '%0' attribute", (StringRef))

ERROR(attr_expected_option_such_as,none,
"expected '%0' option such as '%1'", (StringRef, StringRef))

ERROR(attr_unknown_option,none,
"unknown option '%0' for attribute '%1'", (StringRef, StringRef))

ERROR(attr_missing_label,PointsToFirstBadToken,
"missing label '%0:' in '@%1' attribute", (StringRef, StringRef))
ERROR(attr_expected_label,none,
Expand Down Expand Up @@ -1533,17 +1539,9 @@ ERROR(opened_attribute_id_value,none,
ERROR(opened_attribute_expected_rparen,none,
"expected ')' after id value for 'opened' attribute", ())

// inline, optimize
ERROR(optimization_attribute_expect_option,none,
"expected '%0' option such as '%1'", (StringRef, StringRef))
ERROR(optimization_attribute_unknown_option,none,
"unknown option '%0' for attribute '%1'", (StringRef, StringRef))

// effects
ERROR(effects_attribute_expect_option,none,
"expected '%0' option (readnone, readonly, readwrite)", (StringRef))
ERROR(effects_attribute_unknown_option,none,
"unknown option '%0' for attribute '%1'", (StringRef, StringRef))

// unowned
ERROR(attr_unowned_invalid_specifier,none,
Expand Down
20 changes: 10 additions & 10 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -4207,22 +4207,22 @@ ERROR(global_actor_isolated_requirement_witness_conflict,none,
"requirement from protocol %3 isolated to global actor %4",
(DescriptiveDeclKind, DeclName, Type, Identifier, Type))

ERROR(actorisolated_let,none,
"'@actorIsolated' is meaningless on 'let' declarations because "
ERROR(actorindependent_let,none,
"'@actorIndependent' is meaningless on 'let' declarations because "
"they are immutable",
())
ERROR(actorisolated_mutable_storage,none,
"'@actorIsolated' can not be applied to stored properties",
ERROR(actorindependent_mutable_storage,none,
"'@actorIndependent' can not be applied to stored properties",
())
ERROR(actorisolated_local_var,none,
"'@actorIsolated' can not be applied to local variables",
ERROR(actorindependent_local_var,none,
"'@actorIndependent' can not be applied to local variables",
())
ERROR(actorisolated_not_actor_member,none,
"'@actorIsolated' can only be applied to actor members and "
ERROR(actorindependent_not_actor_member,none,
"'@actorIndependent' can only be applied to actor members and "
"global/static variables",
())
ERROR(actorisolated_not_actor_instance_member,none,
"'@actorIsolated' can only be applied to instance members of actors",
ERROR(actorindependent_not_actor_instance_member,none,
"'@actorIndependent' can only be applied to instance members of actors",
())

ERROR(concurrency_lib_missing,none,
Expand Down
10 changes: 10 additions & 0 deletions lib/AST/Attr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -769,6 +769,7 @@ bool DeclAttribute::printImpl(ASTPrinter &Printer, const PrintOptions &Options,
case DAK_ReferenceOwnership:
case DAK_Effects:
case DAK_Optimize:
case DAK_ActorIndependent:
if (DeclAttribute::isDeclModifier(getKind())) {
Printer.printKeyword(getAttrName(), Options);
} else {
Expand Down Expand Up @@ -1136,6 +1137,15 @@ StringRef DeclAttribute::getAttrName() const {
}
llvm_unreachable("Invalid inline kind");
}
case DAK_ActorIndependent: {
switch (cast<ActorIndependentAttr>(this)->getKind()) {
case ActorIndependentKind::Safe:
return "actorIndependent";
case ActorIndependentKind::Unsafe:
return "actorIndependent(unsafe)";
}
llvm_unreachable("Invalid actorIndependent kind");
}
case DAK_Optimize: {
switch (cast<OptimizeAttr>(this)->getMode()) {
case OptimizationMode::NoOptimization:
Expand Down
4 changes: 4 additions & 0 deletions lib/AST/DiagnosticEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -676,6 +676,10 @@ static void formatDiagnosticArgument(StringRef Modifier,
Out << "actor-independent";
break;

case ActorIsolation::IndependentUnsafe:
Out << "actor-independent-unsafe";
break;

case ActorIsolation::Unspecified:
Out << "non-actor-isolated";
break;
Expand Down
6 changes: 6 additions & 0 deletions lib/AST/TypeCheckRequests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1495,6 +1495,7 @@ bool ActorIsolation::requiresSubstitution() const {
switch (kind) {
case ActorInstance:
case Independent:
case IndependentUnsafe:
case Unspecified:
return false;

Expand All @@ -1508,6 +1509,7 @@ ActorIsolation ActorIsolation::subst(SubstitutionMap subs) const {
switch (kind) {
case ActorInstance:
case Independent:
case IndependentUnsafe:
case Unspecified:
return *this;

Expand All @@ -1528,6 +1530,10 @@ void swift::simple_display(
out << "actor-independent";
break;

case ActorIsolation::IndependentUnsafe:
out << "actor-independent (unsafe)";
break;

case ActorIsolation::Unspecified:
out << "unspecified actor isolation";
break;
Expand Down
53 changes: 44 additions & 9 deletions lib/Parse/ParseDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1618,7 +1618,7 @@ bool Parser::parseNewDeclAttribute(DeclAttributes &Attributes, SourceLoc AtLoc,
else if (Tok.getText() == "releasenone")
kind = EffectsKind::ReleaseNone;
else {
diagnose(Loc, diag::effects_attribute_unknown_option,
diagnose(Loc, diag::attr_unknown_option,
Tok.getText(), AttrName);
return false;
}
Expand All @@ -1644,8 +1644,7 @@ bool Parser::parseNewDeclAttribute(DeclAttributes &Attributes, SourceLoc AtLoc,
}

if (Tok.isNot(tok::identifier)) {
diagnose(Loc, diag::optimization_attribute_expect_option, AttrName,
"none");
diagnose(Loc, diag::attr_expected_option_such_as, AttrName, "none");
return false;
}

Expand All @@ -1655,8 +1654,7 @@ bool Parser::parseNewDeclAttribute(DeclAttributes &Attributes, SourceLoc AtLoc,
else if (Tok.getText() == "__always")
kind = InlineKind::Always;
else {
diagnose(Loc, diag::optimization_attribute_unknown_option,
Tok.getText(), AttrName);
diagnose(Loc, diag::attr_unknown_option, Tok.getText(), AttrName);
return false;
}
consumeToken(tok::identifier);
Expand All @@ -1674,6 +1672,45 @@ bool Parser::parseNewDeclAttribute(DeclAttributes &Attributes, SourceLoc AtLoc,
break;
}

case DAK_ActorIndependent: {
// if no option is provided, then it's the 'safe' version.
if (!consumeIf(tok::l_paren)) {
if (!DiscardAttribute) {
AttrRange = SourceRange(Loc, Tok.getRange().getStart());
Attributes.add(new (Context) ActorIndependentAttr(AtLoc, AttrRange,
ActorIndependentKind::Safe));
}
break;
}

// otherwise, make sure it looks like an identifier.
if (Tok.isNot(tok::identifier)) {
diagnose(Loc, diag::attr_expected_option_such_as, AttrName, "unsafe");
return false;
}

// make sure the identifier is 'unsafe'
if (Tok.getText() != "unsafe") {
diagnose(Loc, diag::attr_unknown_option, Tok.getText(), AttrName);
return false;
}

consumeToken(tok::identifier);
AttrRange = SourceRange(Loc, Tok.getRange().getStart());

if (!consumeIf(tok::r_paren)) {
diagnose(Loc, diag::attr_expected_rparen, AttrName,
DeclAttribute::isDeclModifier(DK));
return false;
}

if (!DiscardAttribute)
Attributes.add(new (Context) ActorIndependentAttr(AtLoc, AttrRange,
ActorIndependentKind::Unsafe));

break;
}

case DAK_Optimize: {
if (!consumeIf(tok::l_paren)) {
diagnose(Loc, diag::attr_expected_lparen, AttrName,
Expand All @@ -1682,8 +1719,7 @@ bool Parser::parseNewDeclAttribute(DeclAttributes &Attributes, SourceLoc AtLoc,
}

if (Tok.isNot(tok::identifier)) {
diagnose(Loc, diag::optimization_attribute_expect_option, AttrName,
"speed");
diagnose(Loc, diag::attr_expected_option_such_as, AttrName, "speed");
return false;
}

Expand All @@ -1695,8 +1731,7 @@ bool Parser::parseNewDeclAttribute(DeclAttributes &Attributes, SourceLoc AtLoc,
else if (Tok.getText() == "size")
optMode = OptimizationMode::ForSize;
else {
diagnose(Loc, diag::optimization_attribute_unknown_option,
Tok.getText(), AttrName);
diagnose(Loc, diag::attr_unknown_option, Tok.getText(), AttrName);
return false;
}
consumeToken(tok::identifier);
Expand Down
7 changes: 3 additions & 4 deletions lib/Sema/DerivedConformanceActor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -236,10 +236,9 @@ static ValueDecl *deriveActor_enqueuePartialTask(DerivedConformance &derived) {
func->copyFormalAccessFrom(derived.Nominal);
func->setBodySynthesizer(deriveBodyActor_enqueuePartialTask);
func->setSynthesized();

// FIXME: This function should be "actor-unsafe", not "actor-independent", but
// the latter is all we have at the moment.
func->getAttrs().add(new (ctx) ActorIndependentAttr(/*IsImplicit=*/true));
// mark as @actorIndependent(unsafe)
func->getAttrs().add(new (ctx) ActorIndependentAttr(
ActorIndependentKind::Unsafe, /*IsImplicit=*/true));

// Actor storage property and its initialization.
auto actorStorage = new (ctx) VarDecl(
Expand Down
21 changes: 14 additions & 7 deletions lib/Sema/TypeCheckAttr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -286,19 +286,26 @@ class AttributeChecker : public AttributeVisitor<AttributeChecker> {
if (auto var = dyn_cast<VarDecl>(D)) {
// @actorIndependent is meaningless on a `let`.
if (var->isLet()) {
diagnoseAndRemoveAttr(attr, diag::actorisolated_let);
diagnoseAndRemoveAttr(attr, diag::actorindependent_let);
return;
}

// @actorIndependent can not be applied to stored properties.
// @actorIndependent can not be applied to stored properties, unless if
// the 'unsafe' option was specified
if (var->hasStorage()) {
diagnoseAndRemoveAttr(attr, diag::actorisolated_mutable_storage);
return;
switch (attr->getKind()) {
case ActorIndependentKind::Safe:
diagnoseAndRemoveAttr(attr, diag::actorindependent_mutable_storage);
return;

case ActorIndependentKind::Unsafe:
break;
}
}

// @actorIndependent can not be applied to local properties.
if (dc->isLocalContext()) {
diagnoseAndRemoveAttr(attr, diag::actorisolated_local_var);
diagnoseAndRemoveAttr(attr, diag::actorindependent_local_var);
return;
}

Expand All @@ -315,14 +322,14 @@ class AttributeChecker : public AttributeVisitor<AttributeChecker> {
// @actorIndependent only makes sense on an actor instance member.
if (!dc->getSelfClassDecl() ||
!dc->getSelfClassDecl()->isActor()) {
diagnoseAndRemoveAttr(attr, diag::actorisolated_not_actor_member);
diagnoseAndRemoveAttr(attr, diag::actorindependent_not_actor_member);
return;
}

auto VD = cast<ValueDecl>(D);
if (!VD->isInstanceMember()) {
diagnoseAndRemoveAttr(
attr, diag::actorisolated_not_actor_instance_member);
attr, diag::actorindependent_not_actor_instance_member);
return;
}

Expand Down
Loading