From 849e9d660f502fd67352305220ef83170e47a21d Mon Sep 17 00:00:00 2001 From: Kavon Farvardin Date: Mon, 12 Oct 2020 16:23:38 -0700 Subject: [PATCH 1/3] fix diagnostic messages that said '@actorIsolated' for @actorIndependent --- include/swift/AST/DiagnosticsSema.def | 20 ++++++++++---------- lib/Sema/TypeCheckAttr.cpp | 10 +++++----- test/attr/actorindependent.swift | 12 ++++++------ 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 6feed045acaf1..328a7fc06ba72 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -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, diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp index 6bc7a1e2babb5..111a28cf17f3c 100644 --- a/lib/Sema/TypeCheckAttr.cpp +++ b/lib/Sema/TypeCheckAttr.cpp @@ -286,19 +286,19 @@ class AttributeChecker : public AttributeVisitor { if (auto var = dyn_cast(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. if (var->hasStorage()) { - diagnoseAndRemoveAttr(attr, diag::actorisolated_mutable_storage); + diagnoseAndRemoveAttr(attr, diag::actorindependent_mutable_storage); return; } // @actorIndependent can not be applied to local properties. if (dc->isLocalContext()) { - diagnoseAndRemoveAttr(attr, diag::actorisolated_local_var); + diagnoseAndRemoveAttr(attr, diag::actorindependent_local_var); return; } @@ -315,14 +315,14 @@ class AttributeChecker : public AttributeVisitor { // @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(D); if (!VD->isInstanceMember()) { diagnoseAndRemoveAttr( - attr, diag::actorisolated_not_actor_instance_member); + attr, diag::actorindependent_not_actor_instance_member); return; } diff --git a/test/attr/actorindependent.swift b/test/attr/actorindependent.swift index 4cc8d3ef45acb..84abf990d6c33 100644 --- a/test/attr/actorindependent.swift +++ b/test/attr/actorindependent.swift @@ -1,6 +1,6 @@ // RUN: %target-swift-frontend -typecheck -verify %s -enable-experimental-concurrency -// expected-error@+1{{'@actorIsolated' can only be applied to actor members and global/static variables}} +// expected-error@+1{{'@actorIndependent' can only be applied to actor members and global/static variables}} @actorIndependent func globalFunction() { } @actorIndependent var globalComputedProperty1: Int { 17 } @@ -10,7 +10,7 @@ set { } } -// expected-error@+1{{'@actorIsolated' can not be applied to stored properties}} +// expected-error@+1{{'@actorIndependent' can not be applied to stored properties}} @actorIndependent var globalStoredProperty: Int = 17 struct X { @@ -25,13 +25,13 @@ struct X { set { } } - // expected-error@+1{{'@actorIsolated' can not be applied to stored properties}} + // expected-error@+1{{'@actorIndependent' can not be applied to stored properties}} @actorIndependent static var storedStaticProperty: Int = 17 } class C { - // expected-error@+1{{'@actorIsolated' can only be applied to actor members and global/static variables}} + // expected-error@+1{{'@actorIndependent' can only be applied to actor members and global/static variables}} @actorIndependent var property3: Int { 5 } } @@ -39,7 +39,7 @@ class C { actor class A { var property: Int = 5 - // expected-error@+1{{'@actorIsolated' can not be applied to stored properties}} + // expected-error@+1{{'@actorIndependent' can not be applied to stored properties}} @actorIndependent var property2: Int = 5 @@ -72,6 +72,6 @@ actor class A { @actorIndependent subscript(index: Int) -> String { "\(index)" } - // expected-error@+1{{'@actorIsolated' can only be applied to instance members of actors}} + // expected-error@+1{{'@actorIndependent' can only be applied to instance members of actors}} @actorIndependent static func staticFunc() { } } From 34d22105b88236ea0a37b89c4dd0580eb11ac526 Mon Sep 17 00:00:00 2001 From: Kavon Farvardin Date: Mon, 12 Oct 2020 18:44:37 -0700 Subject: [PATCH 2/3] implemented parsing and typechecking for @actorIndependent(unsafe) [broken] first impl of @actorIndependent in the type checker. [broken] fixed mistake in my parsing code wrt invalid source range [broken] found another spot where ActorIndependent needs custom handling [broken] incomplete set of @actorIndependent(unsafe) tests updates to ActorIndependentUnsafe [fixed] add FIXME plus simple handling of IndependentUnsafe context finished @actorIndependent(unsafe) regression tests added wip serialization / deserialization test focus test to just one actor class round-trip serialize/deserialize test for @actorIndependent serialize -> deserialize -> serialize -> compare to original most of doug's comments addressed robert's comments fix printing bug; add module printing to regression test [nfc] update comment for ActorIsolation::IndependentUnsafe --- include/swift/AST/ActorIsolation.h | 20 +- include/swift/AST/Attr.def | 2 +- include/swift/AST/Attr.h | 23 ++ include/swift/AST/AttrKind.h | 11 + include/swift/AST/DiagnosticsParse.def | 14 +- lib/AST/Attr.cpp | 10 + lib/AST/DiagnosticEngine.cpp | 4 + lib/AST/TypeCheckRequests.cpp | 6 + lib/Parse/ParseDecl.cpp | 53 ++++- lib/Sema/DerivedConformanceActor.cpp | 7 +- lib/Sema/TypeCheckAttr.cpp | 13 +- lib/Sema/TypeCheckConcurrency.cpp | 15 +- lib/Sema/TypeCheckProtocol.cpp | 1 + lib/Serialization/Deserialization.cpp | 8 + lib/Serialization/ModuleFormat.h | 7 +- lib/Serialization/Serialization.cpp | 8 + .../Serialization/attr-actorindependent.swift | 43 ++++ test/attr/actorindependent_unsafe.swift | 211 ++++++++++++++++++ 18 files changed, 425 insertions(+), 31 deletions(-) create mode 100644 test/Serialization/attr-actorindependent.swift create mode 100644 test/attr/actorindependent_unsafe.swift diff --git a/include/swift/AST/ActorIsolation.h b/include/swift/AST/ActorIsolation.h index c5ba69ba3f156..f819345615a60 100644 --- a/include/swift/AST/ActorIsolation.h +++ b/include/swift/AST/ActorIsolation.h @@ -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, @@ -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) { @@ -112,6 +127,7 @@ class ActorIsolation { switch (lhs.kind) { case Independent: + case IndependentUnsafe: case Unspecified: return true; diff --git a/include/swift/AST/Attr.def b/include/swift/AST/Attr.def index 20427b356c81b..ef0eef652711d 100644 --- a/include/swift/AST/Attr.def +++ b/include/swift/AST/Attr.def @@ -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, diff --git a/include/swift/AST/Attr.h b/include/swift/AST/Attr.h index b442cb8cf934a..709c1d62b36e8 100644 --- a/include/swift/AST/Attr.h +++ b/include/swift/AST/Attr.h @@ -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 ); @@ -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 diff --git a/include/swift/AST/AttrKind.h b/include/swift/AST/AttrKind.h index be74f2a3b472e..7ca1a8153fdd0 100644 --- a/include/swift/AST/AttrKind.h +++ b/include/swift/AST/AttrKind.h @@ -79,6 +79,17 @@ enum class InlineKind : uint8_t { enum : unsigned { NumInlineKindBits = countBitsUsed(static_cast(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(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. diff --git a/include/swift/AST/DiagnosticsParse.def b/include/swift/AST/DiagnosticsParse.def index be7c1d83ee465..afc99fd256b2b 100644 --- a/include/swift/AST/DiagnosticsParse.def +++ b/include/swift/AST/DiagnosticsParse.def @@ -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, @@ -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, diff --git a/lib/AST/Attr.cpp b/lib/AST/Attr.cpp index 8920fd7c6c59c..0730b4929b447 100644 --- a/lib/AST/Attr.cpp +++ b/lib/AST/Attr.cpp @@ -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 { @@ -1136,6 +1137,15 @@ StringRef DeclAttribute::getAttrName() const { } llvm_unreachable("Invalid inline kind"); } + case DAK_ActorIndependent: { + switch (cast(this)->getKind()) { + case ActorIndependentKind::Safe: + return "actorIndependent"; + case ActorIndependentKind::Unsafe: + return "actorIndependent(unsafe)"; + } + llvm_unreachable("Invalid actorIndependent kind"); + } case DAK_Optimize: { switch (cast(this)->getMode()) { case OptimizationMode::NoOptimization: diff --git a/lib/AST/DiagnosticEngine.cpp b/lib/AST/DiagnosticEngine.cpp index 7116f3517ac3f..4da5833525b55 100644 --- a/lib/AST/DiagnosticEngine.cpp +++ b/lib/AST/DiagnosticEngine.cpp @@ -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; diff --git a/lib/AST/TypeCheckRequests.cpp b/lib/AST/TypeCheckRequests.cpp index 5626e803ce416..7ea74ca376f4c 100644 --- a/lib/AST/TypeCheckRequests.cpp +++ b/lib/AST/TypeCheckRequests.cpp @@ -1495,6 +1495,7 @@ bool ActorIsolation::requiresSubstitution() const { switch (kind) { case ActorInstance: case Independent: + case IndependentUnsafe: case Unspecified: return false; @@ -1508,6 +1509,7 @@ ActorIsolation ActorIsolation::subst(SubstitutionMap subs) const { switch (kind) { case ActorInstance: case Independent: + case IndependentUnsafe: case Unspecified: return *this; @@ -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; diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index f4a7b9632070f..cd719bc0beafc 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/lib/Parse/ParseDecl.cpp @@ -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; } @@ -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; } @@ -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); @@ -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, @@ -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; } @@ -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); diff --git a/lib/Sema/DerivedConformanceActor.cpp b/lib/Sema/DerivedConformanceActor.cpp index 27b26bfecc0d9..ad826a7734a01 100644 --- a/lib/Sema/DerivedConformanceActor.cpp +++ b/lib/Sema/DerivedConformanceActor.cpp @@ -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( diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp index 111a28cf17f3c..78814b1a5da46 100644 --- a/lib/Sema/TypeCheckAttr.cpp +++ b/lib/Sema/TypeCheckAttr.cpp @@ -290,10 +290,17 @@ class AttributeChecker : public AttributeVisitor { 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::actorindependent_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. diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index db405f7095e0c..379add233a823 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -501,6 +501,7 @@ ActorIsolationRestriction ActorIsolationRestriction::forDeclaration( } case ActorIsolation::Independent: + case ActorIsolation::IndependentUnsafe: // Actor-independent have no restrictions on their access. return forUnrestricted(); @@ -758,6 +759,7 @@ void swift::checkActorIsolation(const Expr *expr, const DeclContext *dc) { switch (auto isolation = swift::getActorIsolation(value)) { case ActorIsolation::ActorInstance: case ActorIsolation::Independent: + case ActorIsolation::IndependentUnsafe: case ActorIsolation::Unspecified: return isolation; @@ -803,7 +805,8 @@ void swift::checkActorIsolation(const Expr *expr, const DeclContext *dc) { return ActorIsolation::forUnspecified(); } - return ActorIsolation::forIndependent(); + // At module scope, actor independence with safety is assumed. + return ActorIsolation::forIndependent(ActorIndependentKind::Safe); } /// Check a reference to an entity within a global actor. @@ -833,6 +836,7 @@ void swift::checkActorIsolation(const Expr *expr, const DeclContext *dc) { } case ActorIsolation::Independent: + case ActorIsolation::IndependentUnsafe: ctx.Diags.diagnose( loc, diag::global_actor_from_independent_context, value->getDescriptiveKind(), value->getName(), globalActor); @@ -933,6 +937,7 @@ void swift::checkActorIsolation(const Expr *expr, const DeclContext *dc) { } break; + case ActorIsolation::IndependentUnsafe: case ActorIsolation::Unspecified: // Okay break; @@ -1028,7 +1033,7 @@ static Optional getIsolationFromAttributes(Decl *decl) { // If the declaration is explicitly marked @actorIndependent, report it as // independent. if (independentAttr) { - return ActorIsolation::forIndependent(); + return ActorIsolation::forIndependent(independentAttr->getKind()); } // If the declaration is marked with a global actor, report it as being @@ -1105,6 +1110,7 @@ static Optional getIsolationFromWitnessedRequirements( llvm_unreachable("protocol requirements cannot be actor instances"); case ActorIsolation::Independent: + case ActorIsolation::IndependentUnsafe: // We only need one @actorIndependent. if (sawActorIndependent) return true; @@ -1168,8 +1174,11 @@ ActorIsolation ActorIsolationRequest::evaluate( // inferred, so that (e.g.) it will be printed and serialized. ASTContext &ctx = value->getASTContext(); switch (inferred) { + // FIXME: if the context is 'unsafe', is it fine to infer the 'safe' one? + case ActorIsolation::IndependentUnsafe: case ActorIsolation::Independent: - value->getAttrs().add(new (ctx) ActorIndependentAttr(true)); + value->getAttrs().add(new (ctx) ActorIndependentAttr( + ActorIndependentKind::Safe, /*IsImplicit=*/true)); break; case ActorIsolation::GlobalActor: { diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp index 01cf489a25a6b..7873b0ad7696d 100644 --- a/lib/Sema/TypeCheckProtocol.cpp +++ b/lib/Sema/TypeCheckProtocol.cpp @@ -2715,6 +2715,7 @@ bool ConformanceChecker::checkActorIsolation( } case ActorIsolation::Independent: + case ActorIsolation::IndependentUnsafe: case ActorIsolation::Unspecified: break; } diff --git a/lib/Serialization/Deserialization.cpp b/lib/Serialization/Deserialization.cpp index 0d853f04053b9..e9a2b2d030b14 100644 --- a/lib/Serialization/Deserialization.cpp +++ b/lib/Serialization/Deserialization.cpp @@ -4136,6 +4136,14 @@ llvm::Error DeclDeserializer::deserializeDeclAttributes() { break; } + case decls_block::ActorIndependent_DECL_ATTR: { + unsigned kind; + serialization::decls_block::ActorIndependentDeclAttrLayout::readRecord( + scratch, kind); + Attr = new (ctx) ActorIndependentAttr((ActorIndependentKind)kind); + break; + } + case decls_block::Optimize_DECL_ATTR: { unsigned kind; serialization::decls_block::OptimizeDeclAttrLayout::readRecord( diff --git a/lib/Serialization/ModuleFormat.h b/lib/Serialization/ModuleFormat.h index 5c35093f2a9b8..d7257e3f490d4 100644 --- a/lib/Serialization/ModuleFormat.h +++ b/lib/Serialization/ModuleFormat.h @@ -56,7 +56,7 @@ const uint16_t SWIFTMODULE_VERSION_MAJOR = 0; /// describe what change you made. The content of this comment isn't important; /// it just ensures a conflict if two people change the module format. /// Don't worry about adhering to the 80-column limit for this line. -const uint16_t SWIFTMODULE_VERSION_MINOR = 582; // sil specialize attribute module parameter +const uint16_t SWIFTMODULE_VERSION_MINOR = 583; // @actorIndependent safe/unsafe attribute /// A standard hash seed used for all string hashes in a serialized module. /// @@ -1807,6 +1807,11 @@ namespace decls_block { BCFixed<2> // inline value >; + using ActorIndependentDeclAttrLayout = BCRecordLayout< + ActorIndependent_DECL_ATTR, + BCFixed<1> // unsafe flag + >; + using OptimizeDeclAttrLayout = BCRecordLayout< Optimize_DECL_ATTR, BCFixed<2> // optimize value diff --git a/lib/Serialization/Serialization.cpp b/lib/Serialization/Serialization.cpp index 5c81c6b5855d6..dc881ddb61732 100644 --- a/lib/Serialization/Serialization.cpp +++ b/lib/Serialization/Serialization.cpp @@ -2294,6 +2294,14 @@ class Serializer::DeclSerializer : public DeclVisitor { return; } + case DAK_ActorIndependent: { + auto *theAttr = cast(DA); + auto abbrCode = S.DeclTypeAbbrCodes[ActorIndependentDeclAttrLayout::Code]; + ActorIndependentDeclAttrLayout::emitRecord(S.Out, S.ScratchRecord, + abbrCode, (unsigned)theAttr->getKind()); + return; + } + case DAK_Optimize: { auto *theAttr = cast(DA); auto abbrCode = S.DeclTypeAbbrCodes[OptimizeDeclAttrLayout::Code]; diff --git a/test/Serialization/attr-actorindependent.swift b/test/Serialization/attr-actorindependent.swift new file mode 100644 index 0000000000000..a7a26c7d6ad6f --- /dev/null +++ b/test/Serialization/attr-actorindependent.swift @@ -0,0 +1,43 @@ +// RUN: %empty-directory(%t) +// RUN: %target-swift-frontend -enable-experimental-concurrency -emit-module-path %t/a.swiftmodule -module-name a %s +// RUN: llvm-bcanalyzer -dump %t/a.swiftmodule | %FileCheck -check-prefix BC-CHECK %s +// RUN: %target-swift-ide-test -print-module -module-to-print a -source-filename x -I %t | %FileCheck -check-prefix MODULE-CHECK %s +// RUN: %target-swift-frontend -enable-experimental-concurrency -emit-module-path %t/b.swiftmodule -module-name a %t/a.swiftmodule +// RUN: cmp -s %t/a.swiftmodule %t/b.swiftmodule + +/////////// +// This test checks for correct serialization & deserialization of +// @actorIndependent and @actorIndependent(unsafe) + +// look for correct annotation after first deserialization's module print: + +// MODULE-CHECK: actor class UnsafeCounter { +// MODULE-CHECK-NEXT: @actorIndependent(unsafe) var storage: Int +// MODULE-CHECK-NEXT: @actorIndependent var count: Int +// MODULE-CHECK-NEXT: var actorCount: Int +// MODULE-CHECK-NEXT: init() +// MODULE-CHECK-NEXT: } + +// and look for unsafe and safe versions of decl in BC dump: + +// BC-CHECK-NOT: UnknownCode +// BC-CHECK: +// BC-CHECK: + + +actor class UnsafeCounter { + + @actorIndependent(unsafe) + private var storage : Int = 0 + + @actorIndependent + var count : Int { + get { storage } + set { storage = newValue } + } + + var actorCount : Int { + get { storage } + set { storage = newValue } + } +} diff --git a/test/attr/actorindependent_unsafe.swift b/test/attr/actorindependent_unsafe.swift new file mode 100644 index 0000000000000..b33da2ab0165e --- /dev/null +++ b/test/attr/actorindependent_unsafe.swift @@ -0,0 +1,211 @@ +// RUN: %target-swift-frontend -typecheck -verify %s -enable-experimental-concurrency + +////////////////////////// +/// Cases that only work because of @actorIndependent(unsafe) +////////////////////////// + +////////////////////////// +// 1 -- basic unsafe methods / properties accessing var member without annotation +actor class Actor1 { + var counter : Int = 42 + + @actorIndependent(unsafe) + func aMethod() { + counter += 1 + } + + @actorIndependent(unsafe) + var computedProp : Int { counter / 2 } +} + +let a1 = Actor1() +let _ = a1.aMethod() +let _ = a1.computedProp == a1.computedProp + + +////////////////////////// +// 2 -- more unsafe methods / properties accessing var member without annotation +actor class WeatherActor1 { + var tempCelsius : Double = 5.0 + + var tempFahrenheit : Double { + get { (1.8 * tempCelsius) + 32.0 } + set { tempCelsius = (newValue - 32.0) / 1.8 } + } + + @actorIndependent(unsafe) + var tempCelsiusUnsafe : Double { + get { tempCelsius } + set { tempCelsius = newValue } + } + + @actorIndependent + var tempCelsiusPretendSafe : Double { + get { tempCelsiusUnsafe } + set { tempCelsiusUnsafe = newValue } + } + + @actorIndependent(unsafe) + var tempCelsiusUnsafe2 : Double { + get { tempCelsius } + set { tempCelsius = newValue } + } + + @actorIndependent(unsafe) + func getTempFahrenheitUnsafe() -> Double { + return tempFahrenheit + } + + @actorIndependent(unsafe) + func setTempFahrenheitUnsafe(_ newValue : Double) { + tempFahrenheit = newValue + } +} + +let wa1 = WeatherActor1() +let _ = wa1.setTempFahrenheitUnsafe(wa1.getTempFahrenheitUnsafe()) +wa1.tempCelsiusUnsafe = wa1.tempCelsiusUnsafe + 1 +wa1.tempCelsiusPretendSafe = wa1.tempCelsiusUnsafe +wa1.tempCelsiusUnsafe2 = wa1.tempCelsiusUnsafe2 + 2 + + +////////////////////////// +// 3 -- basic actorIndependent accessing actorIndependent(unsafe) member +actor class Actor2 { + + @actorIndependent(unsafe) + var counter : Int = 42 + + @actorIndependent + func aMethod() { + counter += 1 + } + + @actorIndependent + var computedProp : Int { counter / 2 } +} + +let a2 = Actor2() +let _ = a2.aMethod() +let _ = a2.computedProp + + +////////////////////////// +// 4 -- more actorIndependent accessing actorIndependent(unsafe) member +actor class WeatherActor2 { + @actorIndependent(unsafe) + var tempCelsius : Double = 5.0 + + @actorIndependent + var tempFahrenheit : Double { + get { (1.8 * tempCelsius) + 32.0 } + set { tempCelsius = (newValue - 32.0) / 1.8 } + } + + @actorIndependent + var tempCelsiusUnsafe : Double { + get { tempCelsius } + set { tempCelsius = newValue } + } + + @actorIndependent + var tempCelsiusUnsafe2 : Double { + get { tempCelsiusUnsafe } + set { tempCelsiusUnsafe = newValue } + } + + @actorIndependent + func getTempFahrenheitUnsafe() -> Double { + return tempFahrenheit + } + + @actorIndependent + func setTempFahrenheitUnsafe(_ newValue : Double) { + tempFahrenheit = newValue + } +} + +let wa2 = WeatherActor2() +let _ = wa2.setTempFahrenheitUnsafe(wa2.getTempFahrenheitUnsafe()) +wa2.tempCelsiusUnsafe = wa2.tempCelsiusUnsafe + 1 +wa2.tempCelsiusUnsafe2 = wa2.tempCelsiusUnsafe2 + 2 + + +////////////////////////// +// 5 -- even more actorIndependent accessing actorIndependent(unsafe) member +actor class WeatherActor3 { + + @actorIndependent(unsafe) + var tempCelsius : Double = 5.0 + + @actorIndependent(unsafe) + var tempFahrenheit : Double { + get { (1.8 * tempCelsius) + 32.0 } + set { tempCelsius = (newValue - 32.0) / 1.8 } + } + + subscript(info : String) -> Double { + get { + switch info { + case "f", "fahrenheit": + return tempFahrenheit + + case "c", "celsius": + return tempCelsius + + default: + print("setter for unknown weather information: " + info) + return 0.0 + } + } + set { + switch info { + case "f", "fahrenheit": + tempFahrenheit = newValue + + case "c", "celsius": + tempCelsius = newValue + + default: + print("getter for unknown weather information: " + info) + } + } + } +} + +let wa3 = WeatherActor3() +wa3.tempFahrenheit += 3 +wa3.tempCelsius -= 1 + + +////////////////////////// +// 6 -- accesses to static members + +actor class Actor3 { + @actorIndependent(unsafe) + static var pi : Double = 3.0 + + @actorIndependent(unsafe) + static var e : Double = 2.0 + + // expected-error@+1{{'@actorIndependent' can not be applied to stored properties}} + @actorIndependent static var pi_2 : Double = 9.0 + + static var e_2 : Double = 4.0 +} + +let _ = Actor3.pi + Actor3.e + + +////////////////////////// +// 7 -- accesses to global vars + +@actorIndependent(unsafe) +var time = 1.12 + +actor class Actor4 { + var currentTime : Double { + get { time } + set { time = newValue } + } +} From 0752121cc5faa5c57739aebaf6ea57ab72959378 Mon Sep 17 00:00:00 2001 From: Kavon Farvardin Date: Wed, 14 Oct 2020 12:27:04 -0700 Subject: [PATCH 3/3] add regression tests for SR-13735 --- test/attr/actorindependent.swift | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/attr/actorindependent.swift b/test/attr/actorindependent.swift index 84abf990d6c33..944355f331adc 100644 --- a/test/attr/actorindependent.swift +++ b/test/attr/actorindependent.swift @@ -75,3 +75,22 @@ actor class A { // expected-error@+1{{'@actorIndependent' can only be applied to instance members of actors}} @actorIndependent static func staticFunc() { } } + +actor class FromProperty { + // expected-note@+3{{mutable state is only available within the actor instance}} + // expected-note@+2{{mutable state is only available within the actor instance}} + // expected-note@+1{{mutable state is only available within the actor instance}} + var counter : Int = 0 + + // expected-error@+2{{actor-isolated property 'counter' can not be referenced from an '@actorIndependent' context}} + @actorIndependent + var halfCounter : Int { counter / 2 } + + @actorIndependent + var ticks : Int { + // expected-error@+1{{actor-isolated property 'counter' can not be referenced from an '@actorIndependent' context}} + get { counter } + // expected-error@+1{{actor-isolated property 'counter' can not be referenced from an '@actorIndependent' context}} + set { counter = newValue } + } +} \ No newline at end of file