diff --git a/include/swift/AST/ActorIsolation.h b/include/swift/AST/ActorIsolation.h index 0f1707f0dc0..adcceb8e909 100644 --- a/include/swift/AST/ActorIsolation.h +++ b/include/swift/AST/ActorIsolation.h @@ -26,8 +26,10 @@ class raw_ostream; namespace swift { class DeclContext; class ModuleDecl; +class VarDecl; class NominalTypeDecl; class SubstitutionMap; +class AbstractFunctionDecl; /// Determine whether the given types are (canonically) equal, declared here /// to avoid having to include Types.h. @@ -36,6 +38,11 @@ bool areTypesEqual(Type type1, Type type2); /// Determine whether the given type is suitable as a concurrent value type. bool isSendableType(ModuleDecl *module, Type type); +/// Determines if the 'let' can be read from anywhere within the given module, +/// regardless of the isolation or async-ness of the context in which +/// the var is read. +bool isLetAccessibleAnywhere(const ModuleDecl *fromModule, VarDecl *let); + /// Describes the actor isolation of a given declaration, which determines /// the actors with which it can interact. class ActorIsolation { @@ -169,6 +176,9 @@ ActorIsolation getActorIsolation(ValueDecl *value); /// Determine how the given declaration context is isolated. ActorIsolation getActorIsolationOfContext(DeclContext *dc); +/// Determines whether this function's body uses flow-sensitive isolation. +bool usesFlowSensitiveIsolation(AbstractFunctionDecl const *fn); + void simple_display(llvm::raw_ostream &out, const ActorIsolation &state); } // end namespace swift diff --git a/include/swift/AST/DiagnosticsSIL.def b/include/swift/AST/DiagnosticsSIL.def index 8ee3b8ab078..fcc23a9f0ed 100644 --- a/include/swift/AST/DiagnosticsSIL.def +++ b/include/swift/AST/DiagnosticsSIL.def @@ -168,19 +168,6 @@ ERROR(variable_defer_use_uninit,none, ERROR(self_closure_use_uninit,none, "'self' captured by a closure before all members were initialized", ()) -ERROR(self_disallowed_nonisolated_actor_init,none, - "%select{|this use of }0actor 'self' cannot %2 %select{from|in}0 " - "%select{a non-isolated, designated|a global-actor isolated}1 initializer", - (bool, bool, StringRef)) -ERROR(self_disallowed_plain_actor_init,none, - "%select{|this use of }0actor 'self' can only %1 %select{from|in}0 an async initializer", - (bool, StringRef)) -NOTE(actor_convenience_init,none, - "convenience initializers allow non-isolated use of 'self' once " - "initialized", - ()) - - ERROR(variable_addrtaken_before_initialized,none, @@ -625,6 +612,16 @@ WARNING(warning_int_to_fp_inexact, none, "'%1' is not exactly representable as %0; it becomes '%2'", (Type, StringRef, StringRef)) +// Flow-isolation diagnostics +ERROR(isolated_after_nonisolated, none, + "cannot access %1 %2 here in %select{non-isolated initializer|deinitializer}0", + (bool, DescriptiveDeclKind, DeclName)) +NOTE(nonisolated_blame, none, "after %1%2 %3, " + "only non-isolated properties of 'self' can be accessed from " + "%select{this init|a deinit}0", (bool, StringRef, StringRef, DeclName)) +ERROR(non_sendable_from_deinit,none, + "cannot access %1 %2 with a non-sendable type %0 from non-isolated deinit", + (Type, DescriptiveDeclKind, DeclName)) // Yield usage errors ERROR(return_before_yield, none, "accessor must yield before returning",()) diff --git a/include/swift/SILOptimizer/PassManager/Passes.def b/include/swift/SILOptimizer/PassManager/Passes.def index 9d70700e78a..1b98211ba96 100644 --- a/include/swift/SILOptimizer/PassManager/Passes.def +++ b/include/swift/SILOptimizer/PassManager/Passes.def @@ -224,6 +224,8 @@ PASS(EmitDFDiagnostics, "dataflow-diagnostics", "Emit SIL Diagnostics") PASS(EscapeAnalysisDumper, "escapes-dump", "Dump Escape Analysis Results") +PASS(FlowIsolation, "flow-isolation", + "Enforces flow-sensitive actor isolation rules") PASS(FunctionOrderPrinter, "function-order-printer", "Print Function Order for Testing") PASS(FunctionSignatureOpts, "function-signature-opts", diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 4f737af56b5..d1296928859 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -1985,6 +1985,45 @@ static bool isPolymorphic(const AbstractStorageDecl *storage) { return false; } +/// Returns true iff a defer's storage access kind should always +/// match the access kind of its immediately enclosing function. +/// +/// In Swift 5 and earlier, this was not true, meaning that property observers, +/// etc, would be invoked in initializers or deinitializers if a property access +/// happens within a defer, but not when outside the defer. +static bool deferMatchesEnclosingAccess(const FuncDecl *defer) { + assert(defer->isDeferBody()); + + // In Swift 6+, then yes. + if (defer->getASTContext().isSwiftVersionAtLeast(6)) + return true; + + // If the defer is part of a function that is a member of an actor or + // concurrency-aware type, then yes. + if (auto *deferContext = defer->getParent()) { + if (auto *funcContext = deferContext->getParent()) { + if (auto *type = funcContext->getSelfNominalTypeDecl()) { + if (type->isAnyActor()) + return true; + + switch (getActorIsolation(type)) { + case swift::ActorIsolation::Unspecified: + case swift::ActorIsolation::GlobalActorUnsafe: + break; + + case swift::ActorIsolation::ActorInstance: + case swift::ActorIsolation::DistributedActorInstance: + case swift::ActorIsolation::Independent: + case swift::ActorIsolation::GlobalActor: + return true; + } + } + } + } + + return false; +} + static bool isDirectToStorageAccess(const DeclContext *UseDC, const VarDecl *var, bool isAccessOnSelf) { if (!var->hasStorage()) @@ -1994,6 +2033,11 @@ static bool isDirectToStorageAccess(const DeclContext *UseDC, if (AFD == nullptr) return false; + // Check if this is a function representing a defer. + if (auto *func = dyn_cast(AFD)) + if (func->isDeferBody() && deferMatchesEnclosingAccess(func)) + return isDirectToStorageAccess(func->getParent(), var, isAccessOnSelf); + // The property reference is for immediate class, not a derived class. if (AFD->getParent()->getSelfNominalTypeDecl() != var->getDeclContext()->getSelfNominalTypeDecl()) diff --git a/lib/SILOptimizer/Mandatory/CMakeLists.txt b/lib/SILOptimizer/Mandatory/CMakeLists.txt index 6b95bf092d6..927259804c5 100644 --- a/lib/SILOptimizer/Mandatory/CMakeLists.txt +++ b/lib/SILOptimizer/Mandatory/CMakeLists.txt @@ -14,6 +14,7 @@ target_sources(swiftSILOptimizer PRIVATE DiagnoseStaticExclusivity.cpp DiagnoseUnreachable.cpp Differentiation.cpp + FlowIsolation.cpp IRGenPrepare.cpp LexicalLifetimeEliminator.cpp LowerHopToActor.cpp diff --git a/lib/SILOptimizer/Mandatory/DIMemoryUseCollector.cpp b/lib/SILOptimizer/Mandatory/DIMemoryUseCollector.cpp index 682bf303548..9ba07958796 100644 --- a/lib/SILOptimizer/Mandatory/DIMemoryUseCollector.cpp +++ b/lib/SILOptimizer/Mandatory/DIMemoryUseCollector.cpp @@ -450,11 +450,10 @@ ConstructorDecl *DIMemoryObjectInfo::getActorInitSelf() const { // is it for an actor? if (decl->isAnyActor()) if (auto *silFn = MemoryInst->getFunction()) - // is it a designated initializer? + // are we in a constructor? if (auto *ctor = dyn_cast_or_null( silFn->getDeclContext()->getAsDecl())) - if (ctor->isDesignatedInit()) - return ctor; + return ctor; return nullptr; } diff --git a/lib/SILOptimizer/Mandatory/DIMemoryUseCollector.h b/lib/SILOptimizer/Mandatory/DIMemoryUseCollector.h index c812a32d9eb..82af88211d7 100644 --- a/lib/SILOptimizer/Mandatory/DIMemoryUseCollector.h +++ b/lib/SILOptimizer/Mandatory/DIMemoryUseCollector.h @@ -169,7 +169,7 @@ public: } /// Returns the initializer if the memory use is 'self' and appears in an - /// actor's designated initializer. Otherwise, returns nullptr. + /// actor's initializer. Otherwise, returns nullptr. ConstructorDecl *getActorInitSelf() const; /// True if this memory object is the 'self' of a derived class initializer. diff --git a/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp b/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp index d9a936461c8..f89386bbfb6 100644 --- a/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp +++ b/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp @@ -386,14 +386,6 @@ namespace { using BlockStates = BasicBlockData; - enum class ActorInitKind { - None, // not an actor init - Plain, // synchronous initializer - PlainAsync, // asynchronous initializer - GlobalActorIsolated, // isolated to global-actor (any async-ness) - NonIsolated // explicitly 'nonisolated' (any async-ness) - }; - /// LifetimeChecker - This is the main heavy lifting for definite /// initialization checking of a memory object. class LifetimeChecker { @@ -504,28 +496,6 @@ namespace { bool diagnoseReturnWithoutInitializingStoredProperties( const SILInstruction *Inst, SILLocation loc, const DIMemoryUse &Use); - /// Returns true iff TheMemory involves 'self' in a restricted kind of - /// actor initializer. If a non-null Kind pointer was passed in, - /// then the specific kind of restricted actor initializer will be - /// written out. Otherwise, the None initializer kind will be written out. - bool isRestrictedActorInitSelf(ActorInitKind *kind = nullptr) const; - - /// Emits a diagnostic to flag an illegal use of self in an actor init. - /// The message is tuned based on the input arguments. - /// \param ProblemDesc fills in the blank for "cannot ___ in an actor init" - /// \param suggestConvenienceInit if true, will emit a note about - /// convenience inits - /// \param specifyThisUse if true, precedes the message with "this use of" - /// to be more precise about the problem. - void reportIllegalUseForActorInit(const DIMemoryUse &Use, - ActorInitKind ActorKind, - StringRef ProblemDesc, - bool suggestConvenienceInit, - bool specifyThisUse) const; - - void handleLoadUseFailureForActorInit(const DIMemoryUse &Use, - ActorInitKind ActorKind) const; - void handleLoadUseFailure(const DIMemoryUse &Use, bool SuperInitDone, bool FailedSelfUse); @@ -930,14 +900,31 @@ static bool isFailableInitReturnUseOfEnum(EnumInst *EI); void LifetimeChecker::injectActorHops() { auto ctor = TheMemory.getActorInitSelf(); - // Must be `self` within an actor's designated initializer. + // Must be `self` within an actor's initializer. if (!ctor) return; - // If the initializer has restricted use of `self`, then no hops are needed. - if (isRestrictedActorInitSelf()) + // Must not be an init that uses flow-sensitive isolation. + if (usesFlowSensitiveIsolation(ctor)) return; + // Must be an async initializer. + if (!ctor->hasAsync()) + return; + + // Must be an initializer that is isolated to self. + switch (getActorIsolation(ctor)) { + case ActorIsolation::ActorInstance: + case ActorIsolation::DistributedActorInstance: + break; + + case ActorIsolation::Unspecified: + case ActorIsolation::Independent: + case ActorIsolation::GlobalActorUnsafe: + case ActorIsolation::GlobalActor: + return; + } + // Even if there are no stored properties to initialize, we still need a hop. // We insert this directly after the mark_uninitialized instruction, so // that it happens as early as `self` is available.` @@ -1149,13 +1136,9 @@ void LifetimeChecker::doIt() { void LifetimeChecker::handleLoadUse(const DIMemoryUse &Use) { bool IsSuperInitComplete, FailedSelfUse; - ActorInitKind ActorKind = ActorInitKind::None; // If the value is not definitively initialized, emit an error. if (!isInitializedAtUse(Use, &IsSuperInitComplete, &FailedSelfUse)) return handleLoadUseFailure(Use, IsSuperInitComplete, FailedSelfUse); - // Check if it involves 'self' in a restricted actor init. - if (isRestrictedActorInitSelf(&ActorKind)) - return handleLoadUseFailureForActorInit(Use, ActorKind); } static void replaceValueMetatypeInstWithMetatypeArgument( @@ -1466,7 +1449,6 @@ static FullApplySite findApply(SILInstruction *I, bool &isSelfParameter) { void LifetimeChecker::handleInOutUse(const DIMemoryUse &Use) { bool IsSuperInitDone, FailedSelfUse; - ActorInitKind ActorKind = ActorInitKind::None; // inout uses are generally straight-forward: the memory must be initialized // before the "address" is passed as an l-value. @@ -1485,12 +1467,6 @@ void LifetimeChecker::handleInOutUse(const DIMemoryUse &Use) { return; } - // 'self' cannot be passed 'inout' from some kinds of actor initializers. - if (isRestrictedActorInitSelf(&ActorKind)) - reportIllegalUseForActorInit(Use, ActorKind, "be passed 'inout'", - /*suggestConvenienceInit=*/false, - /*specifyThisUse=*/false); - // One additional check: 'let' properties may never be passed inout, because // they are only allowed to have their initial value set, not a subsequent // overwrite. @@ -1657,17 +1633,9 @@ void LifetimeChecker::handleEscapeUse(const DIMemoryUse &Use) { // The value must be fully initialized at all escape points. If not, diagnose // the error. bool SuperInitDone, FailedSelfUse, FullyUninitialized; - ActorInitKind ActorKind = ActorInitKind::None; if (isInitializedAtUse(Use, &SuperInitDone, &FailedSelfUse, &FullyUninitialized)) { - - // no escaping uses of 'self' are allowed in restricted actor inits. - if (isRestrictedActorInitSelf(&ActorKind)) - reportIllegalUseForActorInit(Use, ActorKind, "be captured by a closure", - /*suggestConvenienceInit=*/true, - /*specifyThisUse=*/false); - return; } @@ -2051,141 +2019,6 @@ bool LifetimeChecker::diagnoseReturnWithoutInitializingStoredProperties( return true; } -bool LifetimeChecker::isRestrictedActorInitSelf(ActorInitKind *kind) const { - - auto result = [&](ActorInitKind k, bool isRestricted) -> bool { - if (kind) - *kind = k; - return isRestricted; - }; - - // Determines whether the constructor was explicitly marked as `nonisolated` - auto isExplicitlyNonIsolated = [] (ConstructorDecl const* ctor) -> bool { - if (auto *attr = ctor->getAttrs().getAttribute()) - return !attr->isImplicit(); - return false; - }; - - // Currently: being synchronous, or having an explicitly specified isolation, - // means the actor's self is restricted within the init. - if (auto *ctor = TheMemory.getActorInitSelf()) { - auto isolation = getActorIsolation(ctor); - switch (isolation.getKind()) { - case ActorIsolation::Unspecified: - assert(ctor->isObjC() && "unexpected kind of actor ctor isolation"); - break; - - case ActorIsolation::Independent: - if (isExplicitlyNonIsolated(ctor)) - return result(ActorInitKind::NonIsolated, true); - break; - - case ActorIsolation::ActorInstance: - case ActorIsolation::DistributedActorInstance: - break; // handle these below. - - case ActorIsolation::GlobalActor: - case ActorIsolation::GlobalActorUnsafe: - return result(ActorInitKind::GlobalActorIsolated, true); - }; - - if (ctor->hasAsync()) - return result(ActorInitKind::PlainAsync, false); - else - return result(ActorInitKind::Plain, true); - } - - return result(ActorInitKind::None, false); -} - -void LifetimeChecker::reportIllegalUseForActorInit( - const DIMemoryUse &Use, - ActorInitKind ActorKind, - StringRef ProblemDesc, - bool suggestConvenienceInit, - bool specifyThisUse) const { - - ConstructorDecl *asyncNonIsoCtor = nullptr; - - switch(ActorKind) { - case ActorInitKind::None: - case ActorInitKind::PlainAsync: - llvm::report_fatal_error("this actor init is never problematic!"); - - case ActorInitKind::Plain: - diagnose(Module, Use.Inst->getLoc(), diag::self_disallowed_plain_actor_init, - specifyThisUse, ProblemDesc) - .warnUntilSwiftVersion(6); - break; - - case ActorInitKind::NonIsolated: - if (auto *ctor = TheMemory.getActorInitSelf()) - if (ctor->hasAsync()) - asyncNonIsoCtor = ctor; - - LLVM_FALLTHROUGH; - - case ActorInitKind::GlobalActorIsolated: { - bool isGlobalInstead = ActorKind == ActorInitKind::GlobalActorIsolated; - auto diag = diagnose(Module, Use.Inst->getLoc(), - diag::self_disallowed_nonisolated_actor_init, specifyThisUse, - isGlobalInstead, ProblemDesc); - - // Emit a fix-it to remove `nonisolated`. - // For an async init, it would fix their program. - if (asyncNonIsoCtor) { - auto attr = asyncNonIsoCtor->getAttrs().getAttribute(); - diag.fixItRemove(attr->getRange()); - } - - diag.warnUntilSwiftVersion(6); - break; - } - }; - - if (suggestConvenienceInit) - diagnose(Module, Use.Inst->getLoc(), diag::actor_convenience_init); -} - -void LifetimeChecker::handleLoadUseFailureForActorInit( - const DIMemoryUse &Use, - ActorInitKind ActorKind) const { - assert(TheMemory.isAnyInitSelf()); - SILInstruction *Inst = Use.Inst; - - // While enum instructions have no side-effects, they do wrap-up `self` - // such that it can escape our analysis. So, enum instruction uses are only - // acceptable if the enum is part of a specific pattern of usage that is - // generated for a failable initializer. - if (auto *enumInst = dyn_cast(Inst)) { - if (isFailableInitReturnUseOfEnum(enumInst)) - return; - - // Otherwise, if the use has no possibility of side-effects, then its OK. - } else if (!Inst->mayHaveSideEffects()) { - return; - - // While loads can have side-effects, we are not concerned by them here. - } else if (isa(Inst)) { - return; - } - - // Skip autogenerated instructions. - if (Use.Inst->getLoc().isAutoGenerated()) - return; - - // Everything else is disallowed! - - // We cannot easily determine which argument in the call the use of 'self' - // appears in. If we could, then we could determine whether the callee - // is 'isolated' to that parameter, in order to avoid suggesting a convenience - // init in those cases. For now, just always suggest it if it's an apply. - bool suggestConvenience = isa(Inst); - - reportIllegalUseForActorInit(Use, ActorKind, "appear", suggestConvenience, - /*specifyThisUse=*/true); -} - /// Check and diagnose various failures when a load use is not fully /// initialized. /// diff --git a/lib/SILOptimizer/Mandatory/FlowIsolation.cpp b/lib/SILOptimizer/Mandatory/FlowIsolation.cpp new file mode 100644 index 00000000000..0063719b3ee --- /dev/null +++ b/lib/SILOptimizer/Mandatory/FlowIsolation.cpp @@ -0,0 +1,861 @@ +//===-- FlowIsolation.cpp - Enforces flow-sensitive actor isolation rules -===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2021 - 2022 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +#define DEBUG_TYPE "flow-isolation" + +#include "swift/AST/ActorIsolation.h" +#include "swift/AST/DiagnosticsSIL.h" +#include "swift/SIL/ApplySite.h" +#include "swift/SIL/BitDataflow.h" +#include "swift/SIL/BasicBlockBits.h" +#include "swift/SIL/DebugUtils.h" +#include "swift/SILOptimizer/PassManager/Transforms.h" + +using namespace swift; + +namespace { + +class AnalysisInfo; + +// MARK: utilities + +static SILFunction* getCallee(SILInstruction *someInst) { + if (auto apply = ApplySite::isa(someInst)) + if (SILFunction *callee = apply.getCalleeFunction()) + return callee; + return nullptr; +} + +// Represents the state of isolation for `self` during the flow-analysis, +// at entry and exit to a block. The states are part of a semi-lattice, +// where the extra top element represents a conflict in isolation: +// +// T = "top" +// / \ +// Iso NonIso +// \ / +// B = "bottom" +// +// While we will be talking about isolated vs nonisolated uses, the only +// isolated uses that we consider are stored property accesses. +struct State { + // Each state kind, as an integer, is its position in any bit vectors. + enum Kind { + Isolated = 0, + Nonisolated = 1 + }; + + // Number of states, excluding Top or Bottom, in this flow problem. + static constexpr unsigned NumStates = 2; +}; + +/// a ritzy wrapper for a SILInstruction* that includes a high-level description +//struct NonIsolatedUse { +// // NOTE: these enum values correspond to messages in diagnostics +// enum Kind { +// Uncategorized = 0, +// SelfCopy = 1, +// InvokeFunc = 2, +// }; +// +// SILInstruction* inst; +// Kind kind; +// +// NonIsolatedUse() : inst(nullptr), kind(Uncategorized) {} +// NonIsolatedUse(SILInstruction *i, Kind c) : inst(i), kind(c) {} +// NonIsolatedUse(std::pair const& other) { +// inst = other.first; +// kind = other.second; +// } +// +// /// Returns an ID corresponding to a selector in a diagnostic suitable +// /// for describing this use kind. +// unsigned descriptionID() const { +// return (unsigned)kind; +// } +// +// /// Returns an identifier describing the subject of the use kind. +// DeclName descriptionSubject() const { +// switch (kind) { +// // subject is 'self' +// case SelfCopy: +// case Uncategorized: { +// auto &ctx = inst->getModule().getASTContext(); +// return ctx.Id_self; +// break; +// } +// +// // subject is the callee function +// case MethodCall: +// case ExplicitArgInCall: { +// auto apply = ApplySite::isa(inst); +// if (auto *fn = apply.getReferencedFunctionOrNull()) +// if (auto *declCxt = fn->getDeclContext()) +// if (auto *astDecl = dyn_cast(declCxt->getAsDecl())) +// return astDecl->getName(); +// } +// }; +// } +// +// operator bool() const { +// return inst; +// } +// +// void dump() const LLVM_ATTRIBUTE_USED { +// inst->dump(); +// llvm::dbgs() << "(" << descriptionID() << ")\n"; +// } +//}; + +/// Information gathered for analysis that is specific to a block. +struct Info { + using UseSet = SmallPtrSet; + + /// Records all nonisolated uses of `self` in the block, and their kind of + /// use to aid diagnostics. + UseSet nonisolatedUses; + + /// Records all stored property uses based on `self` in the block. + /// These are the only isolated uses that we care about. + UseSet propertyUses; + + Info() : nonisolatedUses(), propertyUses() {} + + // Diagnoses all property uses as being an error. + void diagnoseAll(AnalysisInfo &info, bool forDeinit, + SILInstruction *blame = nullptr); + + /// Returns the block corresponding to this information. + SILBasicBlock* getBlock() const { + if (!propertyUses.empty()) + return (*(propertyUses.begin()))->getParent(); + + if (!nonisolatedUses.empty()) + return (*(nonisolatedUses.begin()))->getParent(); + + // I only expect to call this when there's a use, so to save memory + // we compute the corresponding block from its stored uses. + assert(false && "no uses to determine block"); + return nullptr; + } + + SILInstruction* firstPropertyUse() const { + auto *blk = getBlock(); + + for (auto &inst : *blk) { + if (propertyUses.count(&inst)) + return &inst; + } + + assert(false && "no first property use found!"); + return nullptr; + } + + bool hasNonisolatedUse() const { + return !nonisolatedUses.empty(); + } + + bool hasPropertyUse() const { + return !propertyUses.empty(); + } + + void dump() const LLVM_ATTRIBUTE_USED { + llvm::dbgs() << "nonisolatedUses:\n"; + for (auto const *i : nonisolatedUses) + i->dump(); + + llvm::dbgs() << "propertyUses:\n"; + for (auto const *i : propertyUses) + i->dump(); + } +}; + +/// \returns true iff the function is a deinit, or a defer of a deinit. +static bool isWithinDeinit(SILFunction *fn) { + auto *astFn = fn->getDeclContext()->getAsDecl(); + + if (auto *funcDecl = dyn_cast(astFn)) + if (funcDecl->isDeferBody()) + astFn = funcDecl->getParent()->getAsDecl(); + + return isa(astFn); +} + +/// Carries the state of analysis for an entire SILFunction. +class AnalysisInfo : public BasicBlockData { +private: + /// Isolation state at the start of the entry block to this function. + /// This should always be `isolated`, unless if this is a `defer`. + State::Kind startingIsolation = State::Isolated; + +public: + + // The deferBlocks information is shared between all blocks of + // this analysis information's function. + llvm::SmallMapVector< SILFunction*, + std::unique_ptr, 8> deferBlocks; + + // Only computed after calling solve() + BitDataflow flow; + + /// This value represents the outgoing isolation state of the function if + /// a normal return is reached, along with the block that returns normally. + /// Only computed after calling solve(), where it remains None if the function + /// doesn't return normally. + Optional> normalReturn = None; + + /// indicates whether the SILFunction is (or contained in) a deinit. + bool forDeinit; + + AnalysisInfo(SILFunction *fn) : BasicBlockData(fn), + flow(fn, State::NumStates) { + forDeinit = isWithinDeinit(fn); + } + + // analyzes the function for uses of `self`. + void analyze(const SILArgument* selfParam); + + // Solves the data-flow problem, assuming analysis has been performed. + void solve(); + + // Verifies uses in this function, assuming solving has been performed. + void verifyIsolation(); + + /// Finds an appropriate instruction that can be blamed for introducing a + /// source of `nonisolation` in a control-flow path leading the given + /// instruction. Preferring the closest block. Use for diagnostics. + /// \param start an instruction that can be reached by a `nonisolated` + /// use in the CFG. + /// \returns an instruction that can be used for blame in a diagnostic. + SILInstruction *findNonisolatedBlame(SILInstruction *start); + + void diagnoseEntireFunction(SILInstruction* blame) { + assert(blame); + for (auto bnd : *this) + bnd.data.diagnoseAll(*this, forDeinit, blame); + } + + /// Does this function have a nonisolated use? + bool hasNonisolatedUse() const { + for (auto const& bnd : *this) + if (bnd.data.hasNonisolatedUse()) + return true; + + return false; + } + + /// Does this function have a property use? + bool hasPropertyUse() const { + for (auto const& bnd : *this) + if (bnd.data.hasPropertyUse()) + return true; + + return false; + } + + /// Do we have sub-analysis information for this function, as a defer body? + bool haveDeferInfo(SILFunction *someFn) { + assert(someFn); + return deferBlocks.count(someFn) > 0; + } + + AnalysisInfo& getOrCreateDeferInfo(SILFunction* someFn) { + assert(someFn); + + if (haveDeferInfo(someFn)) + return *(deferBlocks[someFn]); + + // otherwise, insert fresh info and retry. + deferBlocks.insert({someFn, std::make_unique(someFn)}); + return getOrCreateDeferInfo(someFn); + } + + /// Records an incoming isolation kind to this function from a call-site. + /// \returns true iff the start state has changed from isolated to nonisolated + bool setNonisolatedStart() { + // once we enter the nonisolated state, nothing will change that. + if (startingIsolation == State::Nonisolated) + return false; + + startingIsolation = State::Nonisolated; + return true; + } + + /// Test whether the incoming isolation kind was set to nonisolated. + bool hasNonisolatedStart() const { + return startingIsolation == State::Nonisolated; + } + + /// Records that the instruction accesses an isolated property. + void markPropertyUse(SILInstruction *i) { + LLVM_DEBUG(llvm::dbgs() << "marking as isolated: " << *i); + auto &blockData = this->operator[](i->getParent()); + blockData.propertyUses.insert(i); + } + + /// Records that the instruction causes 'self' to become nonisolated. + void markNonIsolated(SILInstruction *i) { + LLVM_DEBUG(llvm::dbgs() << "marking as non-isolated: " << *i); + auto &blockData = this->operator[](i->getParent()); + blockData.nonisolatedUses.insert(i); + } + + void dump() const LLVM_ATTRIBUTE_USED { + llvm::dbgs() << "analysis-info for " << getFunction()->getName() << "\n"; + for (auto const& bnd : *this) { + llvm::dbgs() << "bb" << bnd.block.getDebugID() << "\n"; + bnd.data.dump(); + } + llvm::dbgs() << "flow-problem state:\n"; + flow.dump(); + + // print the defer information in a different color, if supported. + llvm::WithColor color(llvm::dbgs(), raw_ostream::BLUE); + for (auto const& entry : deferBlocks) + entry.second->dump(); + } +}; + +// MARK: diagnostics + +SILInstruction *AnalysisInfo::findNonisolatedBlame(SILInstruction* startInst) { + assert(startInst); + + SILBasicBlock* firstBlk = startInst->getParent(); + assert(firstBlk->getParent() == getFunction()); + + // workList for breadth-first search to find one of the closest blocks. + std::deque workList; + BasicBlockSet visited(getFunction()); + + // seed the search + workList.push_back(firstBlk); + SILBasicBlock::reverse_iterator cursor = startInst->getReverseIterator(); + + while (!workList.empty()) { + auto *block = workList.front(); + workList.pop_front(); + auto &state = flow[block]; + + // if this block doesn't have exiting nonisolation, then there's no + // way we'll find nonisolation in a predecessor. + assert(state.exitSet[State::Nonisolated] && "nonisolation is unreachable!"); + + // If this is the first time we're scanning the start block, then leave + // the cursor alone and do a partial scan. If the block is part of + // a cycle, we want to scan the block entirely on the second visit, so we + // do not count this as a visit. + if (startInst) { + startInst = nullptr; // make sure second visit scans entirely. + } else { + cursor = block->rbegin(); + visited.insert(block); + } + + // does this block generate non-isolation? + if (state.genSet[State::Nonisolated]) { + auto &data = this->operator[](block); + assert(!data.nonisolatedUses.empty()); + + // scan from the cursor backwards in this block. + while (cursor != block->rend()) { + auto *inst = &*cursor; + cursor++; + + if (data.nonisolatedUses.count(inst)) { + return inst; + } + } + } + + for (auto *pred : block->getPredecessorBlocks()) { + // skip visited + if (visited.contains(pred)) + continue; + + // skip blocks that do not contribute nonisolation. + if (flow[pred].exitSet[State::Nonisolated] == false) + continue; + + workList.push_back(pred); + } + } + + llvm_unreachable("failed to find nonisolated blame."); +} + +ValueDecl *findCallee(ApplySite &apply) { + SILValue callee = apply.getCalleeOrigin(); + + auto check = [](ValueDecl *decl) -> ValueDecl* { + // if this is an accessor, then return the storage instead. + if (auto accessor = dyn_cast(decl)) + return accessor->getStorage(); + + return decl; + }; + + if (auto *methInst = dyn_cast(callee)) + return check(methInst->getMember().getDecl()); + + if (auto *funcInst = dyn_cast(callee)) { + auto *refFunc = funcInst->getInitiallyReferencedFunction(); + if (auto *declCxt = refFunc->getDeclContext()) + if (auto *absFn = dyn_cast(declCxt->getAsDecl())) + return check(absFn); + } + + return nullptr; +} + +StringRef verbForInvoking(ValueDecl *value) { + // Only computed properties need a different verb. + if (isa(value)) + return "accessing "; + + return "calling "; +} + +/// For a specific note diagnostic that describes the blamed instruction for +/// introducing non-isolation, this function produces the values needed +/// to describe it to the user. Thus, the implementation of this function is +/// closely tied to that diagnostic. +std::tuple +describe(SILInstruction *blame) { + auto &ctx = blame->getModule().getASTContext(); + + // check if it's a call-like thing. + if (auto apply = ApplySite::isa(blame)) { + // NOTE: we can't use ApplySite::getCalleeFunction because it is overly + // precise in finding the specific corresponding SILFunction. We only care + // about describing the referenced AST decl, since that's all programmers + // know. + ValueDecl *callee = findCallee(apply); + + // if we have no callee info, all we know is it's a call involving self. + if (!callee) + return {"a call involving", "", ctx.Id_self}; + + return { + verbForInvoking(callee), + callee->getDescriptiveKindName(callee->getDescriptiveKind()), + callee->getName() + }; + } + + // handle non-call blames + switch (blame->getKind()) { + case SILInstructionKind::CopyValueInst: + return {"making a copy of", "", ctx.Id_self}; + default: + return {"this use of", "", ctx.Id_self}; + } +} + +/// Emits errors for all isolated uses of `self` in the given block. +/// \param blame the instruction to blame for introducing non-isolation. +/// If not provided, a suitable instruction will be automatically found using a +/// search. +/// \param info the AnalysisInfo corresponding to the function containing this +/// block. +void Info::diagnoseAll(AnalysisInfo &info, bool forDeinit, + SILInstruction* blame) { + if (propertyUses.empty()) + return; + + // Blame that is valid for the first property use is valid for all uses + // in this block. + if (!blame) + blame = info.findNonisolatedBlame(firstPropertyUse()); + + // if needed, find the blame inside of the defer callee. + if (auto *callee = getCallee(blame)) { + if (info.haveDeferInfo(callee)) { + auto &defer = info.getOrCreateDeferInfo(callee); + assert(defer.normalReturn && "noreturn defer should never be blamed!"); + + auto *retBlk = defer.normalReturn->first; + blame = defer.findNonisolatedBlame(retBlk->getTerminator()); + } + } + + auto *fn = info.getFunction(); + auto &diag = fn->getASTContext().Diags; + + SILLocation blameLoc = blame->getDebugLocation().getLocation(); + + for (auto *use : propertyUses) { + // If the illegal use is a call to a defer, then recursively diagnose + // all of the defer's uses, if this is the first time encountering it. + if (auto *callee = getCallee(use)) { + assert(info.haveDeferInfo(callee) && "non-defer call as a property use?"); + auto &defer = info.getOrCreateDeferInfo(callee); + if (defer.setNonisolatedStart()) { + defer.diagnoseEntireFunction(blame); + } + continue; + } + + assert(isa(use) && "only expecting one kind of instr."); + + SILLocation illegalLoc = use->getDebugLocation().getLocation(); + VarDecl *var = cast(use)->getField(); + + diag.diagnose(illegalLoc.getSourceLoc(), diag::isolated_after_nonisolated, + forDeinit, var->getDescriptiveKind(), var->getName()) + .highlight(illegalLoc.getSourceRange()) + .warnUntilSwiftVersion(6); + + // after , ... can't use self anymore, etc ... + // example: + // after calling function 'hello()', ... + StringRef verb; + StringRef adjective; + DeclName subject; + std::tie(verb, adjective, subject) = describe(blame); + + diag.diagnose(blameLoc.getSourceLoc(), diag::nonisolated_blame, + forDeinit, verb, adjective, subject) + .highlight(blameLoc.getSourceRange()); + } +} + +// MARK: analysis + +/// \returns true iff the access is concurrency-safe in a nonisolated context +/// without an await. +static bool accessIsConcurrencySafe(ModuleDecl *module, + RefElementAddrInst *inst) { + VarDecl *var = inst->getField(); + + // must be accessible from nonisolated and Sendable + return isLetAccessibleAnywhere(module, var) + && isSendableType(module, var->getType()); +} + +/// \returns true iff the ref_element_addr instruction is only used +/// to deinitialize the referenced element. +static bool onlyDeinitAccess(RefElementAddrInst *inst) { + if (auto operand = inst->getSingleUse()) { + if (auto *access = dyn_cast(operand->getUser())) { + return access->getAccessKind() == SILAccessKind::Deinit; + } + } + return false; +} + +/// Checks that the accessed element conforms to Sendable; emitting a +/// diagnostic if it is not Sendable. The diagnostic assumes that the access +/// is happening in a deinit that uses flow-isolation. +/// \returns true iff a diagnostic was emitted for this reference. +static bool diagnoseNonSendableFromDeinit(ModuleDecl *module, + RefElementAddrInst *inst) { + VarDecl *var = inst->getField(); + Type ty = var->getType(); + + if (isSendableType(module, ty)) + return false; + + auto &diag = var->getASTContext().Diags; + + diag.diagnose(inst->getLoc().getSourceLoc(), diag::non_sendable_from_deinit, + ty, var->getDescriptiveKind(), var->getName()) + .warnUntilSwiftVersion(6); + + return true; +} + +/// Analyzes a function for uses of `self` and records the kinds of isolation +/// required. +/// \param selfParam the parameter of \c getFunction() that should be +/// treated as \c self +void AnalysisInfo::analyze(const SILArgument *selfParam) { + assert(selfParam && "analyzing a function with no self?"); + + ModuleDecl *module = getFunction()->getModule().getSwiftModule(); + + // Use a worklist to track the uses left to be searched. + SmallVector worklist; + + // Seed with direct users of `self` + worklist.append(selfParam->use_begin(), selfParam->use_end()); + + while (!worklist.empty()) { + Operand *operand = worklist.pop_back_val(); + SILInstruction *user = operand->getUser(); + + // First, check if this is an apply that involves `self` + if (auto apply = ApplySite::isa(user)) { + + // Check if the callee is a function representing a defer block. + if (SILFunction *callee = apply.getCalleeFunction()) { + if (auto *dc = callee->getDeclContext()) { + if (auto *decl = dyn_cast_or_null(dc->getAsDecl())) { + if (decl->isDeferBody()) { + + // If we need to analyze the defer first, do so. + if (!haveDeferInfo(callee)) { + // NOTE: the defer function is not like a method, because it + // doesn't satisfy hasSelfParam(). + auto const* calleeSelfParam = + callee->getArgument(apply.getAppliedArgIndex(*operand)); + + // Recursion depth is bounded by the lexical nesting of + // defer blocks in the input program. + auto &defer = getOrCreateDeferInfo(callee); + defer.analyze(calleeSelfParam); + defer.solve(); + } + + auto const& defer = getOrCreateDeferInfo(callee); + + // A defer effectively has one exit block, because it can't throw. + // Otherwise, it may never return (e.g., fatalError). + // So, we say that this instruction generates nonisolation only + // if it can return normally, and if it does, it carries + // nonisolation. + if (defer.normalReturn) { + if (defer.normalReturn->second == State::Nonisolated) { + markNonIsolated(user); + } + } + + // If the defer body has any stored property uses, we record that + // in the parent by declaring this call-site being a property use. + if (defer.hasPropertyUse()) + markPropertyUse(user); + + continue; + } + } + } + } + + // For all other call-sites, uses of `self` are nonisolated. + markNonIsolated(user); + continue; + } + + // Handle non-ApplySite instructions. + switch (user->getKind()) { + // Look for a property access. + // Sadly, formal accesses are not always emitted by SILGen, particularly, + // within the initializers we care about. So we rely on ref_element_addr. + case SILInstructionKind::RefElementAddrInst: { + RefElementAddrInst *refInst = cast(user); + + // skip auto-generated deinit accesses. + if (onlyDeinitAccess(refInst)) + continue; + + // skip known-safe accesses. + if (accessIsConcurrencySafe(module, refInst)) + continue; + + // emit a diagnostic and skip if it's non-sendable in a deinit + if (forDeinit && diagnoseNonSendableFromDeinit(module, refInst)) + continue; + + markPropertyUse(user); + break; + } + + // Look through certian kinds of single-value instructions. + case SILInstructionKind::CopyValueInst: + // TODO: If we had some actual escape analysis information, we could + // avoid marking a trivial copy as a nonisolated use, since it doesn't + // actually escape the function. We have to be conservative here + // and assume it might. + markNonIsolated(user); + break; + + case SILInstructionKind::BeginAccessInst: + case SILInstructionKind::BeginBorrowInst: { + auto *svi = cast(user); + worklist.append(svi->use_begin(), svi->use_end()); + break; + } + + default: + // don't follow this instruction. + LLVM_DEBUG(llvm::dbgs() << DEBUG_TYPE << " def-use walk skipping: " + << *user); + break; + } + } +} + +/// Initialize and solve the dataflow problem, assuming the entry block starts +/// isolated. +void AnalysisInfo::solve() { + SILFunction *fn = getFunction(); + SILBasicBlock *returnBlk = nullptr; + + // NOTE: if the starting isolation is nonisolated, the solution is trivial. + // Since we don't expect calls to solve in that situation, I haven't + // implemented that + + // Initialize the forward dataflow problem. + for (auto pair : flow) { + SILBasicBlock *blk = &(pair.block); + auto &data = pair.data; + + // record the return block + if (isa(blk->getTerminator())) { + assert(returnBlk == nullptr); // should only be one! + returnBlk = blk; + } + + // Set everything to Bottom. + data.entrySet.reset(); + data.genSet.reset(); + data.killSet.reset(); + data.exitSet.reset(); + + if (blk == fn->getEntryBlock()) + data.entrySet.set(startingIsolation); + + // A nonisolated use "kills" isolation and generates nonisolation. + if (this->operator[](blk).hasNonisolatedUse()) { + data.killSet.set(State::Isolated); + data.genSet.set(State::Nonisolated); + } + } + + // Solve using a union so that Top represents a conflict. + flow.solveForwardWithUnion(); + + // If this function can return normally, update the outgoing isolation + // in that case. This is needed to implement `defer`. + if (returnBlk) { + auto &returnInfo = flow[returnBlk]; + if (returnInfo.exitSet[State::Nonisolated]) + normalReturn = std::make_pair(returnBlk, State::Nonisolated); + else + normalReturn = std::make_pair(returnBlk, State::Isolated); + } +} + +/// Enforces isolation rules, given the flow and block-local information. +void AnalysisInfo::verifyIsolation() { + // go through all the blocks. + for (auto entry : *this) { + auto &block = entry.block; + auto &data = entry.data; + auto &flowInfo = flow[&block]; + + // If the block has no isolated uses, then skip it. + if (data.propertyUses.empty()) + continue; + + // If flow-analysis determined that we might be `nonisolated` coming + // into this block, then all isolated uses in this block are invalid. + if (flowInfo.entrySet[State::Nonisolated]) { + data.diagnoseAll(*this, forDeinit); + continue; + } + + // Otherwise, we must be starting off isolated. + assert(flowInfo.entrySet[State::Isolated]); + + // If this block doesn't introduce nonisolation, then we can skip it. + if (data.nonisolatedUses.empty()) { + // make sure flow analysis agrees. + assert(flowInfo.exitSet[State::Nonisolated] == 0); + continue; + } + + // Finally, we must scan the block to determine which isolated uses + // are illegal. If isolated uses appear after nonisolated ones, then + // that is an error. So, our strategy is to remove the valid isolated + // uses, until we find the first nonisolated use. Then, we can simply + // diagnose the remaining uses. + SILInstruction *nonisolatedUse = nullptr; + auto current = block.begin(); + while (current != block.end()) { + SILInstruction *inst = &*current; + + auto result = data.nonisolatedUses.find(inst); + if (result != data.nonisolatedUses.end()) { + nonisolatedUse = *result; + break; + } + + data.propertyUses.erase(inst); + current++; + } + + assert(nonisolatedUse && "should have found a use!"); + data.diagnoseAll(*this, forDeinit, nonisolatedUse); + } + + // recursively verify isolation of defer functions. + for (auto &entry : deferBlocks) { + // skip those with nonisolated start, since we've already diagnosed those. + if (entry.second->hasNonisolatedStart()) + continue; + + entry.second->verifyIsolation(); + } +} + +// MARK: high-level setup + +/// Performs flow-sensitive actor-isolation checking on the given SILFunction. +void checkFlowIsolation(SILFunction *fn) { + assert(fn->hasSelfParam() && "cannot analyze without a self param!"); + + // Step 1 -- Analyze uses of `self` within the function. + AnalysisInfo info(fn); + info.analyze(fn->getSelfArgument()); + + // Step 2 -- Initialize and solve the dataflow problem. + info.solve(); + + LLVM_DEBUG(info.dump()); + + // Step 3 -- With the information gathered, check for flow-isolation issues. + info.verifyIsolation(); +} + +/// The FlowIsolation pass performs flow-sensitive actor-isolation checking in the body of actor member +/// functions that treat `self` as `nonisolated` after the first `nonisolated` use. This pass uses a simple +/// forward dataflow analysis to track these changes and emits diagnostics if an isolated use of `self` appears +/// when `self` may be `nonisolated` at that point in the function. +class FlowIsolation : public SILFunctionTransform { + + /// The entry point to the checker. + void run() override { + SILFunction *fn = getFunction(); + + // Don't rerun diagnostics on deserialized functions. + if (fn->wasDeserializedCanonical()) + return; + + // Look for functions that use flow-isolation. + if (auto *dc = fn->getDeclContext()) + if (auto *afd = dyn_cast_or_null(dc->getAsDecl())) + if (usesFlowSensitiveIsolation(afd)) + checkFlowIsolation(fn); + + return; + } + +}; // class + +} // anonymous namespace + +/// This pass is known to depend on the following passes having run before it: +/// - NoReturnFolding +SILTransform *swift::createFlowIsolation() { + return new FlowIsolation(); +} diff --git a/lib/SILOptimizer/PassManager/PassPipeline.cpp b/lib/SILOptimizer/PassManager/PassPipeline.cpp index 4ee2fe60740..aaba113a2b9 100644 --- a/lib/SILOptimizer/PassManager/PassPipeline.cpp +++ b/lib/SILOptimizer/PassManager/PassPipeline.cpp @@ -122,6 +122,8 @@ static void addMandatoryDiagnosticOptPipeline(SILPassPipelinePlan &P) { P.addNoReturnFolding(); addDefiniteInitialization(P); + P.addFlowIsolation(); + // Automatic differentiation: canonicalize all differentiability witnesses // and `differentiable_function` instructions. P.addDifferentiation(); diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index e4a5df8f6e8..7b19f05b755 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -87,6 +87,71 @@ void swift::addAsyncNotes(AbstractFunctionDecl const* func) { } } +static bool requiresFlowIsolation(ActorIsolation typeIso, + ConstructorDecl const *ctor) { + assert(ctor->isDesignatedInit()); + + auto ctorIso = getActorIsolation(const_cast(ctor)); + + // Regardless of async-ness, a mismatch in isolation means we need to be + // flow-sensitive. + if (typeIso != ctorIso) + return true; + + // Otherwise, if it's an actor instance, then it depends on async-ness. + switch (typeIso.getKind()) { + case ActorIsolation::GlobalActor: + case ActorIsolation::GlobalActorUnsafe: + case ActorIsolation::Unspecified: + case ActorIsolation::Independent: + return false; + + case ActorIsolation::ActorInstance: + case ActorIsolation::DistributedActorInstance: + return !(ctor->hasAsync()); // need flow-isolation for non-async. + }; +} + +bool swift::usesFlowSensitiveIsolation(AbstractFunctionDecl const *fn) { + if (!fn) + return false; + + // Only designated constructors or destructors use this kind of isolation. + if (auto const* ctor = dyn_cast(fn)) { + if (!ctor->isDesignatedInit()) + return false; + } else if (!isa(fn)) { + return false; + } + + auto *dc = fn->getDeclContext(); + + // Must be part of a nominal type. + auto *nominal = dc->getSelfNominalTypeDecl(); + assert(nominal && "init/deinit not part of a nominal?"); + + // If it's part of an actor type, then its deinit and some of its inits use + // flow-isolation. + if (nominal->isAnyActor()) { + if (isa(fn)) + return true; + + return requiresFlowIsolation(ActorIsolation::forActorInstance(nominal), + cast(fn)); + } + + // Otherwise, the type must be isolated to a global actor. + auto nominalIso = getActorIsolation(nominal); + if (!nominalIso.isGlobalActor()) + return false; + + // if it's a deinit, then it's flow-isolated. + if (isa(fn)) + return true; + + return requiresFlowIsolation(nominalIso, cast(fn)); +} + bool IsActorRequest::evaluate( Evaluator &evaluator, NominalTypeDecl *nominal) const { // Protocols are actors if they inherit from `Actor`. @@ -313,6 +378,32 @@ Type swift::getExplicitGlobalActor(ClosureExpr *closure) { return globalActor; } +/// A 'let' declaration is safe across actors if it is either +/// nonisolated or it is accessed from within the same module. +static bool varIsSafeAcrossActors(const ModuleDecl *fromModule, + VarDecl *var, + ActorIsolation &varIsolation) { + // must be immutable + if (!var->isLet()) + return false; + + // if nonisolated, it's OK + if (varIsolation == ActorIsolation::Independent) + return true; + + // Otherwise, if it's in the same module then it's OK too. + if (fromModule == var->getDeclContext()->getParentModule()) + return true; + + return false; +} + +bool swift::isLetAccessibleAnywhere(const ModuleDecl *fromModule, + VarDecl *let) { + auto isolation = getActorIsolation(let); + return varIsSafeAcrossActors(fromModule, let, isolation); +} + /// Determine the isolation rules for a given declaration. ActorIsolationRestriction ActorIsolationRestriction::forDeclaration( ConcreteDeclRef declRef, const DeclContext *fromDC, bool fromExpression) { @@ -366,18 +457,12 @@ ActorIsolationRestriction ActorIsolationRestriction::forDeclaration( auto isolation = getActorIsolation(cast(decl)); - // 'let' declarations are immutable, so they can be accessed across + // 'let' declarations are immutable, so some of them can be accessed across // actors. bool isAccessibleAcrossActors = false; if (auto var = dyn_cast(decl)) { - // A 'let' declaration is accessible across actors if it is either - // nonisolated or it is accessed from within the same module. - if (var->isLet() && - (isolation == ActorIsolation::Independent || - var->getDeclContext()->getParentModule() == - fromDC->getParentModule())) { + if (varIsSafeAcrossActors(fromDC->getParentModule(), var, isolation)) isAccessibleAcrossActors = true; - } } // A function that provides an asynchronous context has no restrictions @@ -2580,9 +2665,53 @@ namespace { }); } - static bool isConvenienceInit(AbstractFunctionDecl const* fn) { - if (auto ctor = dyn_cast_or_null(fn)) - return ctor->isConvenienceInit(); + /// To support flow-isolation, some member accesses in inits / deinits + /// must be permitted, despite the isolation of 'self' not being + /// correct in Sema. + /// + /// \param refCxt the context in which the member reference happens. + /// \param baseActor the actor referenced in the base of the member access. + /// \param member the declaration corresponding to the accessed member. + /// + /// \returns true iff the member access is permitted in Sema because it will + /// be verified later by flow-isolation. + bool checkedByFlowIsolation(DeclContext const *refCxt, + ReferencedActor &baseActor, + ValueDecl const *member) { + + // base of member reference must be `self` + if (!baseActor.isActorSelf()) + return false; + + // Must be directly in an init/deinit that uses flow-isolation, + // or a defer within such a functions. + // + // NOTE: once flow-isolation can analyze calls to arbitrary local + // functions, we should be using isActorInitOrDeInitContext instead + // of this ugly loop. + AbstractFunctionDecl const* fnDecl = nullptr; + while (true) { + fnDecl = dyn_cast_or_null(refCxt->getAsDecl()); + if (!fnDecl) + return false; + + // go up one level if this context is a defer. + if (auto *d = dyn_cast(fnDecl)) { + if (d->isDeferBody()) { + refCxt = refCxt->getParent(); + continue; + } + } + break; + } + + if (!usesFlowSensitiveIsolation(fnDecl)) + return false; + + // Stored properties are definitely OK. + if (auto *var = dyn_cast(member)) + if (var->hasStorage() && var->isInstanceMember()) + return true; return false; } @@ -2650,13 +2779,17 @@ namespace { if (isolatedActor) return false; + // For now, cross-actor self reference to a member is OK from an actor's + // init or deinit. Later, flow-isolation will check unsafe references. + if (isActorInitOrDeInitContext(getDeclContext())) + return false; + // If we have a distributed actor that might be remote, check that // we are referencing a properly-distributed member. bool performDistributedChecks = isolation.getActorType()->isDistributedActor() && !isolatedActor.isPotentiallyIsolated && - !isa(member) && - !isActorInitOrDeInitContext(getDeclContext()); + !isa(member); if (performDistributedChecks) { if (auto access = checkDistributedAccess(memberLoc, member, context)){ // This is a distributed access, so mark it as throwing or @@ -2679,12 +2812,12 @@ namespace { if (isolatedActor) return false; - // An instance member of an actor can be referenced from an actor's - // designated initializer or deinitializer. - if (isolatedActor.isActorSelf() && member->isInstanceMember()) - if (auto fn = isActorInitOrDeInitContext(getDeclContext())) - if (!isConvenienceInit(fn)) - return false; + // Some initializers and deinitializers have special permission to + // access an isolated member on `self`. If that case applies, then we + // can skip checking. + if (checkedByFlowIsolation(getDeclContext(), isolatedActor, + member)) + return false; // An escaping partial application of something that is part of // the actor's isolated state is never permitted. @@ -3698,8 +3831,9 @@ bool HasIsolatedSelfRequest::evaluate( } if (auto ctor = dyn_cast(value)) { - // In an actor's convenience or synchronous init, self is not isolated. - if (ctor->isConvenienceInit() || !ctor->hasAsync()) + // When no other isolation applies to an actor's constructor, + // then it is isolated only if it is async. + if (!ctor->hasAsync()) return false; } diff --git a/test/Concurrency/actor_definite_init.swift b/test/Concurrency/actor_definite_init.swift index b0fa51622bc..4cec6a96e27 100644 --- a/test/Concurrency/actor_definite_init.swift +++ b/test/Concurrency/actor_definite_init.swift @@ -53,11 +53,9 @@ actor Convenient { init(throwyDesignated val: Int) throws { guard val > 0 else { throw BogusError.blah } self.x = 10 - say(msg: "hello?") // expected-warning {{this use of actor 'self' can only appear in an async initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} + say(msg: "hello?") - Task { self } // expected-warning {{actor 'self' can only be captured by a closure from an async initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} + Task { self } } init(asyncThrowyDesignated val: Int) async throws { @@ -114,104 +112,38 @@ actor MyActor { _ = self.x self.y = self.x - Task { self } // expected-warning{{actor 'self' can only be captured by a closure from an async initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} + Task { self } // expected-note 2 {{after making a copy of 'self', only non-isolated properties of 'self' can be accessed from this init}} - self.helloWorld() // expected-warning{{this use of actor 'self' can only appear in an async initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} + self.x = randomInt() // expected-warning {{cannot access property 'x' here in non-isolated initializer}} - callMethod(self) // expected-warning{{this use of actor 'self' can only appear in an async initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} + callMethod(self) // expected-note 5 {{after calling global function 'callMethod', only non-isolated properties of 'self' can be accessed from this init}} - passInout(&self.x) // expected-warning{{actor 'self' can only be passed 'inout' from an async initializer}} + passInout(&self.x) // expected-warning {{cannot access property 'x' here in non-isolated initializer}} - self.x = self.y - self.x = randomInt() + if c { + // expected-warning@+2 {{cannot access property 'y' here in non-isolated initializer}} + // expected-warning@+1 {{cannot access property 'x' here in non-isolated initializer}} + self.x = self.y + } + + // expected-warning@+2 {{cannot access property 'y' here in non-isolated initializer}} + // expected-warning@+1 {{cannot access property 'x' here in non-isolated initializer}} (_, _) = (self.x, self.y) - _ = self.x == 0 + _ = self.x == 0 // expected-warning {{cannot access property 'x' here in non-isolated initializer}} - self.hax = self // expected-warning{{this use of actor 'self' can only appear in an async initializer}} - _ = self.hax + while c { + // expected-warning@+2 {{cannot access property 'hax' here in non-isolated initializer}} + // expected-note@+1 2 {{after making a copy of 'self', only non-isolated properties of 'self' can be accessed from this init}} + self.hax = self + _ = self.hax // expected-warning {{cannot access property 'hax' here in non-isolated initializer}} + } - _ = computedProp // expected-warning{{this use of actor 'self' can only appear in an async initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} - - computedProp = 1 // expected-warning{{this use of actor 'self' can only appear in an async initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} - - Task { // expected-warning {{actor 'self' can only be captured by a closure from an async initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} + Task { _ = await self.hax await self.helloWorld() } - } - init?(i1_nil c: Bool) { - self.x = 0 - guard c else { return nil } - self.y = self.x - - Task { self } // expected-warning{{actor 'self' can only be captured by a closure from an async initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} - - self.helloWorld() // expected-warning{{this use of actor 'self' can only appear in an async initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} - - callMethod(self) // expected-warning{{this use of actor 'self' can only appear in an async initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} - - passInout(&self.x) // expected-warning{{actor 'self' can only be passed 'inout' from an async initializer}} - - self.x = self.y - - self.hax = self // expected-warning{{this use of actor 'self' can only appear in an async initializer}} - _ = self.hax - - _ = computedProp // expected-warning{{this use of actor 'self' can only appear in an async initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} - - computedProp = 1 // expected-warning{{this use of actor 'self' can only appear in an async initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} - - Task { // expected-warning {{actor 'self' can only be captured by a closure from an async initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} - _ = await self.hax - await self.helloWorld() - } - } - - init!(i1_boom c: Bool) { - self.x = 0 - guard c else { return nil } - self.y = self.x - - Task { self } // expected-warning{{actor 'self' can only be captured by a closure from an async initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} - - self.helloWorld() // expected-warning{{this use of actor 'self' can only appear in an async initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} - - callMethod(self) // expected-warning{{this use of actor 'self' can only appear in an async initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} - - passInout(&self.x) // expected-warning{{actor 'self' can only be passed 'inout' from an async initializer}} - - self.x = self.y - - self.hax = self // expected-warning{{this use of actor 'self' can only appear in an async initializer}} - _ = self.hax - - _ = computedProp // expected-warning{{this use of actor 'self' can only appear in an async initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} - - computedProp = 1 // expected-warning{{this use of actor 'self' can only appear in an async initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} - - Task { // expected-warning {{actor 'self' can only be captured by a closure from an async initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} - _ = await self.hax - await self.helloWorld() - } + { _ = self }() } @MainActor @@ -219,33 +151,11 @@ actor MyActor { self.x = 0 self.y = self.x - Task { self } // expected-warning{{actor 'self' cannot be captured by a closure from a global-actor isolated initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} + Task { self } // expected-note {{after making a copy of 'self', only non-isolated properties of 'self' can be accessed from this init}} - self.helloWorld() // expected-warning{{this use of actor 'self' cannot appear in a global-actor isolated initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} + callMethod(self) - callMethod(self) // expected-warning{{this use of actor 'self' cannot appear in a global-actor isolated initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} - - passInout(&self.x) // expected-warning{{actor 'self' cannot be passed 'inout' from a global-actor isolated initializer}} - - self.x = self.y - - self.hax = self // expected-warning{{this use of actor 'self' cannot appear in a global-actor isolated initializer}} - _ = self.hax - - _ = computedProp // expected-warning{{this use of actor 'self' cannot appear in a global-actor isolated initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} - - computedProp = 1 // expected-warning{{this use of actor 'self' cannot appear in a global-actor isolated initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} - - Task { // expected-warning {{actor 'self' cannot be captured by a closure from a global-actor isolated initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} - _ = await self.hax - await self.helloWorld() - } + passInout(&self.x) // expected-warning {{cannot access property 'x' here in non-isolated initializer}} } init(i3 c: Bool) async { @@ -270,42 +180,21 @@ actor MyActor { _ = self.hax self.helloWorld() } + + { _ = self }() } @MainActor init(i4 c: Bool) async { - self.x = 0 - self.y = self.x + self.x = 0 + self.y = self.x - Task { self } // expected-warning{{actor 'self' cannot be captured by a closure from a global-actor isolated initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} + Task { self } // expected-note {{after making a copy of 'self', only non-isolated properties of 'self' can be accessed from this init}} - self.helloWorld() // expected-warning{{this use of actor 'self' cannot appear in a global-actor isolated initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} + callMethod(self) - callMethod(self) // expected-warning{{this use of actor 'self' cannot appear in a global-actor isolated initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} - - passInout(&self.x) // expected-warning{{actor 'self' cannot be passed 'inout' from a global-actor isolated initializer}} - - self.x = self.y - - self.hax = self // expected-warning{{this use of actor 'self' cannot appear in a global-actor isolated initializer}} - _ = self.hax - - _ = computedProp // expected-warning{{this use of actor 'self' cannot appear in a global-actor isolated initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} - - computedProp = 1 // expected-warning{{this use of actor 'self' cannot appear in a global-actor isolated initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} - - Task { // expected-warning {{actor 'self' cannot be captured by a closure from a global-actor isolated initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} - _ = await self.hax - await self.helloWorld() - } + passInout(&self.x) // expected-warning {{cannot access property 'x' here in non-isolated initializer}} } - } @@ -315,10 +204,9 @@ actor X { init(v1 start: Int) { self.counter = start - Task { await self.setCounter(start + 1) } // expected-warning {{actor 'self' can only be captured by a closure from an async initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} + Task { await self.setCounter(start + 1) } // expected-note {{after making a copy of 'self', only non-isolated properties of 'self' can be accessed from this init}} - if self.counter != start { + if self.counter != start { // expected-warning {{cannot access property 'counter' here in non-isolated initializer}} fatalError("where's my protection?") } } @@ -343,10 +231,10 @@ actor EscapeArtist { init(attempt1: Bool) { self.x = 0 - globalVar = self // expected-warning {{this use of actor 'self' can only appear in an async initializer}} + globalVar = self // expected-note {{after making a copy of 'self', only non-isolated properties of 'self' can be accessed from this init}} Task { await globalVar!.isolatedMethod() } - if self.x == 0 { + if self.x == 0 { // expected-warning {{cannot access property 'x' here in non-isolated initializer}} fatalError("race detected.") } } @@ -354,11 +242,11 @@ actor EscapeArtist { init(attempt2: Bool) { self.x = 0 - let wrapped: EscapeArtist? = .some(self) // expected-warning {{this use of actor 'self' can only appear in an async initializer}} + let wrapped: EscapeArtist? = .some(self) // expected-note {{after making a copy of 'self', only non-isolated properties of 'self' can be accessed from this init}} let selfUnchained = wrapped! Task { await selfUnchained.isolatedMethod() } - if self.x == 0 { + if self.x == 0 { // expected-warning {{cannot access property 'x' here in non-isolated initializer}} fatalError("race detected.") } } @@ -366,9 +254,7 @@ actor EscapeArtist { init(attempt3: Bool) { self.x = 0 - // expected-warning@+2 {{variable 'unchainedSelf' was never mutated; consider changing to 'let' constant}} - // expected-warning@+1 {{this use of actor 'self' can only appear in an async initializer}} - var unchainedSelf = self + let unchainedSelf = self unchainedSelf.nonisolated() } @@ -378,17 +264,15 @@ actor EscapeArtist { let unchainedSelf = self - unchainedSelf.nonisolated() // expected-warning {{this use of actor 'self' can only appear in an async initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} + unchainedSelf.nonisolated() - let _ = { unchainedSelf.nonisolated() } // expected-warning {{actor 'self' can only be captured by a closure from an async initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} + let _ = { unchainedSelf.nonisolated() }() } init(attempt5: Bool) { self.x = 0 - let box = CardboardBox(item: self) // expected-warning {{this use of actor 'self' can only appear in an async initializer}} + let box = CardboardBox(item: self) box.item.nonisolated() } @@ -397,8 +281,7 @@ actor EscapeArtist { func fn() { self.nonisolated() } - fn() // expected-warning {{this use of actor 'self' can only appear in an async initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} + fn() } func isolatedMethod() { x += 1 } @@ -407,46 +290,44 @@ actor EscapeArtist { @available(SwiftStdlib 5.5, *) actor Ahmad { - func f() {} + nonisolated func f() {} + var prop: Int = 0 init(v1: Void) { - Task.detached { await self.f() } // expected-warning {{actor 'self' can only be captured by a closure from an async initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} - - f() // expected-warning {{this use of actor 'self' can only appear in an async initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} + Task.detached { self.f() } // expected-note {{after making a copy of 'self', only non-isolated properties of 'self' can be accessed from this init}} + f() + prop += 1 // expected-warning {{cannot access property 'prop' here in non-isolated initializer}} } nonisolated init(v2: Void) async { - Task.detached { await self.f() } // expected-warning {{actor 'self' cannot be captured by a closure from a non-isolated, designated initializer}} {{3-15=}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} - - f() // expected-warning {{this use of actor 'self' cannot appear in a non-isolated, designated initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} + Task.detached { self.f() } // expected-note {{after making a copy of 'self', only non-isolated properties of 'self' can be accessed from this init}} + f() + prop += 1 // expected-warning {{cannot access property 'prop' here in non-isolated initializer}} } } @available(SwiftStdlib 5.5, *) actor Rain { var x: Int = 0 - func f() {} + nonisolated func f() {} - init() { - defer { self.f() } // expected-warning {{this use of actor 'self' can only appear in an async initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} + init(_ hollerBack: (Rain) -> () -> Void) { + defer { self.f() } - defer { _ = self.x } // expected-warning {{this use of actor 'self' can only appear in an async initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} + defer { _ = self.x } // expected-warning {{cannot access property 'x' here in non-isolated initializer}} - defer { Task { await self.f() } } // expected-warning {{this use of actor 'self' can only appear in an async initializer}} - // expected-note@-1 {{convenience initializers allow non-isolated use of 'self' once initialized}} + defer { Task { self.f() } } + + defer { _ = hollerBack(self) } // expected-note {{after a call involving 'self', only non-isolated properties of 'self' can be accessed from this init}} } - init() async { + init(_ hollerBack: (Rain) -> () -> Void) async { defer { self.f() } defer { _ = self.x } defer { Task { self.f() } } + + defer { _ = hollerBack(self) } } } diff --git a/test/Concurrency/actor_isolation.swift b/test/Concurrency/actor_isolation.swift index 8c016a4c3a2..8651b28a16c 100644 --- a/test/Concurrency/actor_isolation.swift +++ b/test/Concurrency/actor_isolation.swift @@ -751,15 +751,44 @@ extension SomeClassInActor.ID { // ---------------------------------------------------------------------- @available(SwiftStdlib 5.1, *) actor SomeActorWithInits { - var mutableState: Int = 17 + var mutableState: Int = 17 // expected-note 3 {{mutation of this property is only permitted within the actor}} var otherMutableState: Int + let nonSendable: SomeClass + + // Sema should not complain about referencing non-sendable members + // in an actor init or deinit, as those are diagnosed later by flow-isolation. + init(_ x: SomeClass) { + self.nonSendable = x + } init(i1: Bool) { self.mutableState = 42 self.otherMutableState = 17 - self.isolated() + self.isolated() // expected-error{{actor-isolated instance method 'isolated()' can not be referenced from a non-isolated context}} self.nonisolated() + + defer { + isolated() // expected-error{{actor-isolated instance method 'isolated()' can not be referenced from a non-isolated context}} + mutableState += 1 // okay + nonisolated() + } + + func local() { + isolated() // expected-error{{actor-isolated instance method 'isolated()' can not be referenced from a non-isolated context}} + mutableState += 1 // expected-error{{actor-isolated property 'mutableState' can not be mutated from a non-isolated context}} + nonisolated() + } + local() + + let _ = { + defer { + isolated() // expected-error{{actor-isolated instance method 'isolated()' can not be referenced from a non-isolated context}} + mutableState += 1 // expected-error{{actor-isolated property 'mutableState' can not be mutated from a non-isolated context}} + nonisolated() + } + nonisolated() + }() } init(i2: Bool) async { @@ -778,15 +807,16 @@ actor SomeActorWithInits { convenience init(i4: Bool) async { self.init(i1: i4) - await self.isolated() + self.isolated() self.nonisolated() } - @MainActor init(i5: Bool) { + @MainActor init(i5 x: SomeClass) { self.mutableState = 42 self.otherMutableState = 17 + self.nonSendable = x - self.isolated() + self.isolated() // expected-error{{actor-isolated instance method 'isolated()' can not be referenced from the main actor}} self.nonisolated() } @@ -794,7 +824,7 @@ actor SomeActorWithInits { self.mutableState = 42 self.otherMutableState = 17 - self.isolated() + await self.isolated() self.nonisolated() } @@ -810,8 +840,27 @@ actor SomeActorWithInits { self.nonisolated() } + deinit { + let _ = self.nonSendable // OK only through typechecking, not SIL. - func isolated() { } // expected-note 2 {{calls to instance method 'isolated()' from outside of its actor context are implicitly asynchronous}} + defer { + isolated() // expected-error{{actor-isolated instance method 'isolated()' can not be referenced from a non-isolated context}} + mutableState += 1 // okay + nonisolated() + } + + let _ = { + defer { + isolated() // expected-error{{actor-isolated instance method 'isolated()' can not be referenced from a non-isolated context}} + mutableState += 1 // expected-error{{actor-isolated property 'mutableState' can not be mutated from a non-isolated context}} + nonisolated() + } + nonisolated() + }() + } + + + func isolated() { } // expected-note 9 {{calls to instance method 'isolated()' from outside of its actor context are implicitly asynchronous}} nonisolated func nonisolated() {} } @@ -893,6 +942,27 @@ extension OtherModuleActor { } } +actor CrossModuleFromInitsActor { + init(v1 actor: OtherModuleActor) { + _ = actor.a // expected-error {{actor-isolated property 'a' can not be referenced from a non-isolated context}} + _ = actor.b // okay + + _ = actor.c // expected-error {{actor-isolated property 'c' can not be referenced from a non-isolated context}} + } + + init(v2 actor: OtherModuleActor) async { + _ = actor.a // expected-error{{expression is 'async' but is not marked with 'await'}} + // expected-note@-1{{property access is 'async'}} + _ = await actor.a // okay + _ = actor.b // okay + _ = actor.c // expected-error{{expression is 'async' but is not marked with 'await'}} + // expected-note@-1{{property access is 'async'}} + // expected-warning@-2{{non-sendable type 'SomeClass' in implicitly asynchronous access to actor-isolated property 'c' cannot cross actor boundary}} + _ = await actor.c // expected-warning{{non-sendable type 'SomeClass' in implicitly asynchronous access to actor-isolated property 'c' cannot cross actor boundary}} + _ = await actor.d // okay + } +} + // ---------------------------------------------------------------------- // Actor protocols. @@ -1030,11 +1100,18 @@ func test_invalid_reference_to_actor_member_without_a_call_note() { } } -// Actor isolation and "defer" +// Actor isolation and initializers through typechecking only, not flow-isolation. actor Counter { var counter: Int = 0 - func next() -> Int { + // expected-note@+2{{mutation of this property is only permitted within the actor}} + // expected-note@+1{{property declared here}} + var computedProp : Int { + get { 0 } + set { } + } + + func next() -> Int { // expected-note 2 {{calls to instance method 'next()' from outside of its actor context are implicitly asynchronous}} defer { counter = counter + 1 } @@ -1050,6 +1127,20 @@ actor Counter { return counter } + + init() { + _ = self.next() // expected-error {{actor-isolated instance method 'next()' can not be referenced from a non-isolated context}} + defer { _ = self.next() } // expected-error {{actor-isolated instance method 'next()' can not be referenced from a non-isolated context}} + + _ = computedProp // expected-error {{actor-isolated property 'computedProp' can not be referenced from a non-isolated context}} + computedProp = 1 // expected-error {{actor-isolated property 'computedProp' can not be mutated from a non-isolated context}} + + } + + convenience init(material: Int) async { + self.init() + self.counter = 10 // FIXME: this should work, and also needs to work in definite initialization by injecting hops + } } /// Superclass checks for global actor-qualified class types. diff --git a/test/Concurrency/concurrent_value_checking.swift b/test/Concurrency/concurrent_value_checking.swift index 734be257ef0..1af431dd710 100644 --- a/test/Concurrency/concurrent_value_checking.swift +++ b/test/Concurrency/concurrent_value_checking.swift @@ -1,7 +1,7 @@ // RUN: %target-typecheck-verify-swift -disable-availability-checking -warn-concurrency // REQUIRES: concurrency -class NotConcurrent { } // expected-note 26{{class 'NotConcurrent' does not conform to the 'Sendable' protocol}} +class NotConcurrent { } // expected-note 25{{class 'NotConcurrent' does not conform to the 'Sendable' protocol}} // ---------------------------------------------------------------------- // Sendable restriction on actor operations @@ -29,7 +29,7 @@ actor A2 { } convenience init(delegatingAsync value: NotConcurrent) async { - await self.init(valueAsync: value) // expected-warning{{non-sendable type 'NotConcurrent' passed in call to actor-isolated initializer 'init(valueAsync:)' cannot cross actor boundary}} + await self.init(valueAsync: value) } } diff --git a/test/Concurrency/flow_isolation.swift b/test/Concurrency/flow_isolation.swift new file mode 100644 index 00000000000..a593ce5671e --- /dev/null +++ b/test/Concurrency/flow_isolation.swift @@ -0,0 +1,237 @@ +// RUN: %target-swift-frontend -parse-as-library -O -emit-sil -verify %s + +func randomBool() -> Bool { return false } +func logTransaction(_ i: Int) {} + +enum Color: Error { + case red + case yellow + case blue +} + +func takeNonSendable(_ ns: NonSendableType) {} +func takeSendable(_ s: SendableType) {} + +class NonSendableType { + var x: Int = 0 +} + +struct SendableType: Sendable {} + +struct Money { + var dollars: Int + var euros: Int { + return dollars * 2 + } +} + +func takeBob(_ b: Bob) {} + +actor Bob { + var x: Int + + nonisolated func speak() { } + nonisolated var cherry : String { + get { "black cherry" } + } + + init(v0 initial: Int) { + self.x = 0 + speak() // expected-note {{after calling instance method 'speak()', only non-isolated properties of 'self' can be accessed from this init}} + speak() + self.x = 1 // expected-warning {{cannot access property 'x' here in non-isolated initializer}} + speak() + } + + init(v1 _: Void) { + self.x = 0 + _ = cherry // expected-note {{after accessing property 'cherry', only non-isolated properties of 'self' can be accessed from this init}} + self.x = 1 // expected-warning {{cannot access property 'x' here in non-isolated initializer}} + } + + init(v2 callBack: (Bob) -> Void) { + self.x = 0 + callBack(self) // expected-note {{after a call involving 'self', only non-isolated properties of 'self' can be accessed from this init}} + self.x = 1 // expected-warning {{cannot access property 'x' here in non-isolated initializer}} + } + + init(v3 _: Void) { + self.x = 1 + takeBob(self) // expected-note {{after calling global function 'takeBob', only non-isolated properties of 'self' can be accessed from this init}} + self.x = 1 // expected-warning {{cannot access property 'x' here in non-isolated initializer}} + } + + + + // ensure noniso does not flow out of a defer's unreachable block + init(spice doesNotFlow: Int) { + if true { + defer { + if randomBool() { + speak() + fatalError("oops") + } + } + self.x = 0 + } + self.x = 20 + } +} + +actor Casey { + var money: Money + + nonisolated func speak(_ msg: String) { print("Casey: \(msg)") } + nonisolated func cashUnderMattress() -> Int { return 0 } + + init() { + money = Money(dollars: 100) + defer { logTransaction(money.euros) } // expected-warning {{cannot access property 'money' here in non-isolated initializer}} + self.speak("Yay, I have $\(money.dollars)!") // expected-note {{after calling instance method 'speak', only non-isolated properties of 'self' can be accessed from this init}} + } + + init(with start: Int) { + money = Money(dollars: start) + + if (money.dollars < 0) { + self.speak("Oh no, I'm in debt!") // expected-note 3 {{after calling instance method 'speak', only non-isolated properties of 'self' can be accessed from this init}} + } + logTransaction(money.euros) // expected-warning {{cannot access property 'money' here in non-isolated initializer}} + + // expected-warning@+1 2 {{cannot access property 'money' here in non-isolated initializer}} + money.dollars = money.dollars + 1 + + if randomBool() { + // expected-note@+2 {{after calling instance method 'cashUnderMattress()', only non-isolated properties of 'self' can be accessed from this init}} + // expected-warning@+1 {{cannot access property 'money' here in non-isolated initializer}} + money.dollars += cashUnderMattress() + } + } +} + +actor Demons { + let ns: NonSendableType + + init(_ x: NonSendableType) { + self.ns = x + } + + deinit { + let _ = self.ns // expected-warning {{cannot access property 'ns' with a non-sendable type 'NonSendableType' from non-isolated deinit}} + } +} + +func pass(_ t: T) {} + +actor ExampleFromProposal { + let immutableSendable = SendableType() + var mutableSendable = SendableType() + let nonSendable = NonSendableType() + + init() { + _ = self.immutableSendable // ok + _ = self.mutableSendable // ok + _ = self.nonSendable // ok + + f() // expected-note 2 {{after calling instance method 'f()', only non-isolated properties of 'self' can be accessed from this init}} + + _ = self.immutableSendable // ok + _ = self.mutableSendable // expected-warning {{cannot access property 'mutableSendable' here in non-isolated initializer}} + _ = self.nonSendable // expected-warning {{cannot access property 'nonSendable' here in non-isolated initializer}} + } + + + deinit { + _ = self.immutableSendable // ok + _ = self.mutableSendable // ok + _ = self.nonSendable // expected-warning {{cannot access property 'nonSendable' with a non-sendable type 'NonSendableType' from non-isolated deinit}} + + f() // expected-note {{after calling instance method 'f()', only non-isolated properties of 'self' can be accessed from a deinit}} + + _ = self.immutableSendable // ok + _ = self.mutableSendable // expected-warning {{cannot access property 'mutableSendable' here in deinitializer}} + _ = self.nonSendable // expected-warning {{cannot access property 'nonSendable' with a non-sendable type 'NonSendableType' from non-isolated deinit}} + } + + nonisolated func f() {} +} + +@MainActor +class CheckGAIT1 { + var silly = 10 + var ns = NonSendableType() + + nonisolated func f() {} + + nonisolated init(_ c: Color) { + silly += 0 + repeat { + switch c { + case .red: + continue + case .yellow: + silly += 1 + continue + case .blue: + f() // expected-note {{after calling instance method 'f()', only non-isolated properties of 'self' can be accessed from this init}} + } + break + } while true + silly += 2 // expected-warning {{cannot access property 'silly' here in non-isolated initializer}} + } + + deinit { + _ = ns // expected-warning {{cannot access property 'ns' with a non-sendable type 'NonSendableType' from non-isolated deinit}} + f() // expected-note {{after calling instance method 'f()', only non-isolated properties of 'self' can be accessed from a deinit}} + _ = silly // expected-warning {{cannot access property 'silly' here in deinitializer}} + + } +} + +actor ControlFlowTester { + let ns: NonSendableType = NonSendableType() + let s: SendableType = SendableType() + var count: Int = 0 + + nonisolated func noniso() {} + + // FIXME: since we're allowing let-bound sendable property reads, + // these error messages need tweaking. We may actually want to say + // that "cannot access it while self is nonisolated" and say the it became + // nonisolated from some point. That's probably the most informative explaination. + init(v1: Void) { + noniso() // expected-note {{after calling instance method 'noniso()', only non-isolated properties of 'self' can be accessed from this init}} + takeNonSendable(self.ns) // expected-warning {{cannot access property 'ns' here in non-isolated initializer}} + takeSendable(self.s) + } + + init(v2 c: Color) throws { + if c == .red { + noniso() + throw c + } else if c == .blue { + noniso() + fatalError("soul") + } + count += 1 + } + + init(v3 c: Color) { + do { + noniso() // expected-note 2 {{after calling instance method 'noniso()', only non-isolated properties of 'self' can be accessed from this init}} + + throw c + + } catch Color.blue { + count += 1 // expected-warning {{cannot access property 'count' here in non-isolated initializer}} + } catch { + count += 1 // expected-warning {{cannot access property 'count' here in non-isolated initializer}} + } + } + + + init(v4 c: Color) { + defer { count += 1 } // expected-warning {{cannot access property 'count' here in non-isolated initializer}} + noniso() // expected-note 1 {{after calling instance method 'noniso()', only non-isolated properties of 'self' can be accessed from this init}} + } +} diff --git a/test/SILGen/observers.swift b/test/SILGen/observers.swift index c4dfc53088a..289d87d32d9 100644 --- a/test/SILGen/observers.swift +++ b/test/SILGen/observers.swift @@ -1,5 +1,5 @@ -// RUN: %target-swift-emit-silgen -Xllvm -sil-full-demangle -parse-as-library -disable-objc-attr-requires-foundation-module -enable-objc-interop %s | %FileCheck %s +// RUN: %target-swift-emit-silgen -swift-version 5 -Xllvm -sil-full-demangle -parse-as-library -disable-objc-attr-requires-foundation-module -enable-objc-interop %s | %FileCheck %s var zero: Int = 0 @@ -9,7 +9,7 @@ func takeInt(_ a : Int) {} public struct DidSetWillSetTests { // CHECK-LABEL: sil [ossa] @$s9observers010DidSetWillC5TestsV{{[_0-9a-zA-Z]*}}fC - public init(x : Int) { + @MainActor public init(x : Int) { // Accesses to didset/willset variables are direct in init methods and dtors. a = x a = x @@ -24,6 +24,17 @@ public struct DidSetWillSetTests { // CHECK: [[WRITE:%.*]] = begin_access [modify] [unknown] [[PB_SELF]] // CHECK: [[P2:%.*]] = struct_element_addr [[WRITE]] : $*DidSetWillSetTests, #DidSetWillSetTests.a // CHECK-NEXT: assign %0 to [[P2]] + + // In Swift 5 and earlier, the accesses are _not_ direct within a defer that + // appears in an init/deinit for non-actor types. + defer { a = x } + + // CHECK-LABEL: sil private [ossa] @$s9observers010DidSetWillC5TestsV1xACSi_tcfc6$deferL_yyF + // CHECK-NOT: assign + // CHECK: [[SETTER:%.*]] = function_ref @$s9observers010DidSetWillC5TestsV1aSivs + // CHECK-NEXT: apply [[SETTER]] + // CHECK-NOT: assign + // CHECK: } // end sil function } public var a: Int { @@ -167,6 +178,72 @@ public struct DidSetWillSetTests { } } +actor Pop { + var a: Int = 0 { + didSet {} + } + + init(input: Int) { + defer { a = input } + } + // In Swift 5, a defer in an actor init or deinit matches its enclosing context, + // which is direct-to-storage. + // CHECK-LABEL: sil private [ossa] @$s9observers3PopC5inputACSi_tcfc6$deferL_yyF + // CHECK-NOT: apply + // CHECK: assign {{%.*}} to {{%.*}} : $*Int + // CHECK-NOT: apply + // CHECK: } // end sil function + + deinit { + defer { a = 0 } + } + + // CHECK-LABEL: sil private [ossa] @$s9observers3PopCfd6$deferL_yyF + // CHECK: [[INT:%.*]] = apply {{.*}} : $@convention(method) (Builtin.IntLiteral, @thin Int.Type) -> Int + // CHECK-NOT: apply + // CHECK: assign [[INT]] to {{.*}} : $*Int + // CHECK-NOT: apply + // CHECK: } // end sil function +} + +@MainActor +class SaltNVinegar { + var a: Int = 0 { + didSet {} + } + + init(regular val: Int) { + defer { a = val } + + // In Swift 5, an actor-isolated type has its defers match-up. So, we're + // direct-to-storage here... + // CHECK-LABEL: sil private [ossa] @$s9observers12SaltNVinegarC7regularACSi_tcfc6$deferL_yyF + // CHECK-NOT: apply + // CHECK: assign {{%.*}} to {{%.*}} : $*Int + // CHECK-NOT: apply + // CHECK: } // end sil function + } +} + +@MainActor(unsafe) +class BBQ { + var a: Int = 0 { + didSet {} + } + + init(regular val: Int) { + defer { a = val } + + // ... but if the type is using unsafe global-actor isolation, then we preserve + // the non-direct access in the defer. + // CHECK-LABEL: sil private [ossa] @$s9observers3BBQC7regularACSi_tcfc6$deferL_yyF + // CHECK-NOT: assign + // CHECK: [[SETTER:%.*]] = class_method {{%.*}} : $BBQ, #BBQ.a!setter + // CHECK-NEXT: apply [[SETTER]] + // CHECK-NOT: assign + // CHECK: } // end sil function + } +} // Test global observing properties. diff --git a/test/SILGen/observers_swift6.swift b/test/SILGen/observers_swift6.swift new file mode 100644 index 00000000000..d8fa15e65cf --- /dev/null +++ b/test/SILGen/observers_swift6.swift @@ -0,0 +1,19 @@ +// RUN: %target-swift-emit-silgen -swift-version 6 -Xllvm -sil-full-demangle -parse-as-library -disable-objc-attr-requires-foundation-module -enable-objc-interop %s | %FileCheck %s + +class Kitty { + var age: Int = 5 { + didSet { + } + } + + init(val: Int) { + defer { age = val } + } + + // Access in a defer matches the context in Swift 6, so this defer should be + // direct-to-storage. + // CHECK-LABEL: sil private [ossa] @$s16observers_swift65KittyC3valACSi_tcfc6$deferL_yyF + // CHECK: [[REF:%.+]] = ref_element_addr {{.*}} : $Kitty, #Kitty.age + // CHECK: [[ACCESS:%.+]] = begin_access {{.*}} [[REF]] : $*Int + // CHECK: assign {{.*}} to [[ACCESS]] : $*Int +}