Skip to content

Commit

Permalink
Merge pull request #34260 from kavon/actorUnsafe
Browse files Browse the repository at this point in the history
[concurrency] implement the 'unsafe' option for @actorIndependent
  • Loading branch information
DougGregor committed Oct 21, 2020
2 parents 9671299 + 0752121 commit ce90767
Show file tree
Hide file tree
Showing 20 changed files with 464 additions and 51 deletions.
20 changes: 18 additions & 2 deletions include/swift/AST/ActorIsolation.h
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
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
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
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
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
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
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
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
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
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
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
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

0 comments on commit ce90767

Please sign in to comment.