From f089ba9464fc164f6dc4ce01dafb1ec2f545912c Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 13 Oct 2020 15:17:13 -0700 Subject: [PATCH 1/6] [Concurrency] Propagation of actor constraints. Implement propagation rules for global actor constraints, which can come from: * Enclosing extension or type * Superclass of a class * Overridden declaration * Requirement witnessed by a declaration * Storage declaration for an accessor --- include/swift/AST/ActorIsolation.h | 2 + lib/Sema/TypeCheckConcurrency.cpp | 250 +++++++++++++++--- test/Concurrency/global_actor_inference.swift | 90 +++++++ .../actor/global_actor_conformance.swift | 5 +- 4 files changed, 305 insertions(+), 42 deletions(-) create mode 100644 test/Concurrency/global_actor_inference.swift diff --git a/include/swift/AST/ActorIsolation.h b/include/swift/AST/ActorIsolation.h index 2aec57cdf20d4..78961df373191 100644 --- a/include/swift/AST/ActorIsolation.h +++ b/include/swift/AST/ActorIsolation.h @@ -84,6 +84,8 @@ class ActorIsolation { operator Kind() const { return getKind(); } + bool isUnspecified() const { return kind == Unspecified; } + ClassDecl *getActor() const { assert(getKind() == ActorInstance); return actor; diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index 87afeb4734f04..4a7ba10daea93 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -971,65 +971,237 @@ void swift::checkActorIsolation(const Expr *expr, const DeclContext *dc) { const_cast(expr)->walk(walker); } -ActorIsolation ActorIsolationRequest::evaluate( - Evaluator &evaluator, ValueDecl *value) const { +/// Determine actor isolation solely from attributes. +/// +/// \returns the actor isolation determined from attributes alone (with no +/// inference rules). Returns \c None if there were no attributes on this +/// declaration. +static Optional getIsolationFromAttributes(Decl *decl) { // Look up attributes on the declaration that can affect its actor isolation. // If any of them are present, use that attribute. - auto independentAttr = value->getAttrs().getAttribute(); - auto globalActorAttr = value->getGlobalActorAttr(); + auto independentAttr = decl->getAttrs().getAttribute(); + auto globalActorAttr = decl->getGlobalActorAttr(); unsigned numIsolationAttrs = (independentAttr ? 1 : 0) + (globalActorAttr ? 1 : 0); - if (numIsolationAttrs > 0) { - // Only one such attribute is valid. - if (numIsolationAttrs > 1) { - value->diagnose( - diag::actor_isolation_multiple_attr, value->getDescriptiveKind(), - value->getName(), independentAttr->getAttrName(), - globalActorAttr->second->getName().str()) - .highlight(independentAttr->getRangeWithAt()) - .highlight(globalActorAttr->first->getRangeWithAt()); - } + if (numIsolationAttrs == 0) + return None; - // If the declaration is explicitly marked @actorIndependent, report it as - // independent. - if (independentAttr) { - return ActorIsolation::forIndependent(); + // Only one such attribute is valid. + if (numIsolationAttrs > 1) { + DeclName name; + if (auto value = dyn_cast(decl)) { + name = value->getName(); + } else if (auto ext = dyn_cast(decl)) { + if (auto selfTypeDecl = ext->getSelfNominalTypeDecl()) + name = selfTypeDecl->getName(); } - // If the declaration is marked with a global actor, report it as being - // part of that global actor. - if (globalActorAttr) { - TypeResolutionOptions options(TypeResolverContext::None); - TypeResolution resolver = TypeResolution::forInterface( - value->getInnermostDeclContext(), options, nullptr); - Type globalActorType = resolver.resolveType( - globalActorAttr->first->getTypeRepr(), nullptr); - if (!globalActorType || globalActorType->hasError()) - return ActorIsolation::forUnspecified(); + decl->diagnose( + diag::actor_isolation_multiple_attr, decl->getDescriptiveKind(), + name, independentAttr->getAttrName(), + globalActorAttr->second->getName().str()) + .highlight(independentAttr->getRangeWithAt()) + .highlight(globalActorAttr->first->getRangeWithAt()); + } + + // If the declaration is explicitly marked @actorIndependent, report it as + // independent. + if (independentAttr) { + return ActorIsolation::forIndependent(); + } + + // If the declaration is marked with a global actor, report it as being + // part of that global actor. + if (globalActorAttr) { + TypeResolutionOptions options(TypeResolverContext::None); + TypeResolution resolver = TypeResolution::forInterface( + decl->getInnermostDeclContext(), options, nullptr); + Type globalActorType = resolver.resolveType( + globalActorAttr->first->getTypeRepr(), nullptr); + if (!globalActorType || globalActorType->hasError()) + return ActorIsolation::forUnspecified(); + + return ActorIsolation::forGlobalActor(globalActorType); + } + + llvm_unreachable("Forgot about an attribute?"); +} + +/// Infer isolation from witnessed protocol requirements. +static Optional getIsolationFromWitnessedRequirements( + ValueDecl *value) { + auto dc = value->getDeclContext(); + auto idc = dyn_cast_or_null(dc->getAsDecl()); + if (!idc) + return None; + + // Walk through each of the conformances in this context, collecting any + // requirements that have actor isolation. + auto conformances = evaluateOrDefault( + dc->getASTContext().evaluator, + LookupAllConformancesInContextRequest{idc}, { }); + using IsolatedRequirement = + std::tuple; + SmallVector isolatedRequirements; + for (auto conformance : conformances) { + auto protocol = conformance->getProtocol(); + for (auto found : protocol->lookupDirect(value->getName())) { + if (!isa(found->getDeclContext())) + continue; + + auto requirement = dyn_cast(found); + if (!requirement || isa(requirement)) + continue; + + auto requirementIsolation = getActorIsolation(requirement); + if (requirementIsolation.isUnspecified()) + continue; + + auto witness = conformance->getWitnessDecl(requirement); + if (witness != value) + continue; - return ActorIsolation::forGlobalActor(globalActorType); + isolatedRequirements.push_back( + IsolatedRequirement{conformance, requirementIsolation, requirement}); } + } + + // Filter out duplicate actors. + SmallPtrSet globalActorTypes; + bool sawActorIndependent = false; + isolatedRequirements.erase( + std::remove_if(isolatedRequirements.begin(), isolatedRequirements.end(), + [&](IsolatedRequirement &isolated) { + auto isolation = std::get<1>(isolated); + switch (isolation) { + case ActorIsolation::ActorInstance: + llvm_unreachable("protocol requirements cannot be actor instances"); + + case ActorIsolation::Independent: + // We only need one @actorIndependent. + if (sawActorIndependent) + return true; + + sawActorIndependent = true; + return false; + + case ActorIsolation::Unspecified: + return true; + + case ActorIsolation::GlobalActor: { + // Substitute into the global actor type. + auto conformance = std::get<0>(isolated); + auto requirementSubs = SubstitutionMap::getProtocolSubstitutions( + conformance->getProtocol(), dc->getSelfTypeInContext(), + ProtocolConformanceRef(conformance)); + Type globalActor = isolation.getGlobalActor().subst(requirementSubs); + if (!globalActorTypes.insert(globalActor->getCanonicalType()).second) + return true; + + // Update the global actor type, now that we've done this substitution. + std::get<1>(isolated) = ActorIsolation::forGlobalActor(globalActor); + return false; + } + } + }), + isolatedRequirements.end()); + + if (isolatedRequirements.size() != 1) + return None; + + return std::get<1>(isolatedRequirements.front()); +} + +ActorIsolation ActorIsolationRequest::evaluate( + Evaluator &evaluator, ValueDecl *value) const { + // If this declaration has one of the actor isolation attributes, report + // that. + if (auto isolationFromAttr = getIsolationFromAttributes(value)) { + return *isolationFromAttr; + } + + // Determine the default isolation for this declaration, which may still be + // overridden by other inference rules. + ActorIsolation defaultIsolation = ActorIsolation::forUnspecified(); + + // Check for instance members of actor classes, which are part of + // actor-isolated state. + auto classDecl = value->getDeclContext()->getSelfClassDecl(); + if (classDecl && classDecl->isActor() && value->isInstanceMember()) { + defaultIsolation = ActorIsolation::forActorInstance(classDecl); + } - llvm_unreachable("Forgot about an attribute?"); + // Disable inference of actor attributes outside of normal Swift source files. + if (auto sourceFile = value->getDeclContext()->getParentSourceFile()) { + switch (sourceFile->Kind) { + case SourceFileKind::Interface: + case SourceFileKind::SIL: + return defaultIsolation; + + case SourceFileKind::Library: + case SourceFileKind::Main: + // Attempt inference below. + break; + } + } else { + return defaultIsolation; } + // Function used when returning an inferred isolation. + auto inferredIsolation = [&](ActorIsolation inferred) { + return inferred; + }; + // If the declaration overrides another declaration, it must have the same // actor isolation. if (auto overriddenValue = value->getOverriddenDecl()) { if (auto isolation = getActorIsolation(overriddenValue)) - return isolation; + return inferredIsolation(isolation); } - // Check for instance members of actor classes, which are part of - // actor-isolated state. - auto classDecl = value->getDeclContext()->getSelfClassDecl(); - if (classDecl && classDecl->isActor() && value->isInstanceMember()) { - // Part of the actor's isolated state. - return ActorIsolation::forActorInstance(classDecl); + // If the declaration witnesses a protocol requirement that is isolated, + // use that. + if (auto witnessedIsolation = getIsolationFromWitnessedRequirements(value)) { + return inferredIsolation(*witnessedIsolation); + } + + // If the declaration is a class with a superclass that has specified + // isolation, use that. + if (auto classDecl = dyn_cast(value)) { + if (auto superclassDecl = classDecl->getSuperclassDecl()) { + auto superclassIsolation = getActorIsolation(superclassDecl); + if (!superclassIsolation.isUnspecified()) + return inferredIsolation(superclassIsolation); + } + } + + // If this is an accessor, use the actor isolation of its storage + // declaration. + if (auto accessor = dyn_cast(value)) { + auto storageIsolation = getActorIsolation(accessor->getStorage()); + if (!storageIsolation.isUnspecified()) + return inferredIsolation(storageIsolation); + } + + // If the declaration is in an extension that has one of the isolation + // attributes, use that. + if (auto ext = dyn_cast(value->getDeclContext())) { + if (auto isolationFromAttr = getIsolationFromAttributes(ext)) { + return inferredIsolation(*isolationFromAttr); + } + } + + // If the declaration is in a nominal type (or extension thereof) that + // has isolation, use that. + if (auto selfTypeDecl = value->getDeclContext()->getSelfNominalTypeDecl()) { + auto selfTypeIsolation = getActorIsolation(selfTypeDecl); + if (!selfTypeIsolation.isUnspecified()) { + return inferredIsolation(selfTypeIsolation); + } } - // Everything else is unspecified. - return ActorIsolation::forUnspecified(); + // Default isolation for this member. + return defaultIsolation; } ActorIsolation swift::getActorIsolation(ValueDecl *value) { diff --git a/test/Concurrency/global_actor_inference.swift b/test/Concurrency/global_actor_inference.swift new file mode 100644 index 0000000000000..8fcbe36011835 --- /dev/null +++ b/test/Concurrency/global_actor_inference.swift @@ -0,0 +1,90 @@ +// RUN: %target-typecheck-verify-swift -enable-experimental-concurrency +// REQUIRES: concurrency + +import _Concurrency + +actor class SomeActor { } + +@globalActor +struct SomeGlobalActor { + static let shared = SomeActor() +} + +@globalActor +struct OtherGlobalActor { + static let shared = SomeActor() +} + +// ---------------------------------------------------------------------- +// Global actor inference for protocols +// ---------------------------------------------------------------------- + +@SomeGlobalActor +protocol P1 { + func method() +} + +protocol P2 { + @SomeGlobalActor func method1() + func method2() +} + + +class C1: P1 { + func method() { } // expected-note{{only asynchronous methods can be used outside the actor instance}} + + @OtherGlobalActor func testMethod() { + method() // expected-error{{instance method 'method()' isolated to global actor 'SomeGlobalActor' can not be referenced from different global actor 'OtherGlobalActor'}} + } +} + +class C2: P2 { + func method1() { } // expected-note{{only asynchronous methods can be used outside the actor instance}} + func method2() { } + + @OtherGlobalActor func testMethod() { + method1() // expected-error{{instance method 'method1()' isolated to global actor 'SomeGlobalActor' can not be referenced from different global actor 'OtherGlobalActor'}} + method2() // okay + } +} + +// ---------------------------------------------------------------------- +// Global actor inference for classes and extensions +// ---------------------------------------------------------------------- +@SomeGlobalActor class C3 { + func method1() { } // expected-note{{only asynchronous methods can be used outside the actor instance}} +} + +extension C3 { + func method2() { } // expected-note{{only asynchronous methods can be used outside the actor instance}} +} + +class C4: C3 { + func method3() { } // expected-note{{only asynchronous methods can be used outside the actor instance}} +} + +extension C4 { + func method4() { } // expected-note{{only asynchronous methods can be used outside the actor instance}} +} + +class C5 { + func method1() { } +} + +@SomeGlobalActor extension C5 { + func method2() { } // expected-note{{only asynchronous methods can be used outside the actor instance}} +} + +@OtherGlobalActor func testGlobalActorInference(c3: C3, c4: C4, c5: C5) { + // Propagation via class annotation + c3.method1() // expected-error{{instance method 'method1()' isolated to global actor 'SomeGlobalActor' can not be referenced from different global actor 'OtherGlobalActor'}} + c3.method2() // expected-error{{instance method 'method2()' isolated to global actor 'SomeGlobalActor' can not be referenced from different global actor 'OtherGlobalActor'}} + + // Propagation via subclassing + c4.method3() // expected-error{{instance method 'method3()' isolated to global actor 'SomeGlobalActor' can not be referenced from different global actor 'OtherGlobalActor'}} + c4.method4() // expected-error{{instance method 'method4()' isolated to global actor 'SomeGlobalActor' can not be referenced from different global actor 'OtherGlobalActor'}} + + // Propagation in an extension. + c5.method1() // OK: no propagation + c5.method2() // expected-error{{instance method 'method2()' isolated to global actor 'SomeGlobalActor' can not be referenced from different global actor 'OtherGlobalActor'}} +} diff --git a/test/decl/class/actor/global_actor_conformance.swift b/test/decl/class/actor/global_actor_conformance.swift index 8144892e12769..9bbdf4196a074 100644 --- a/test/decl/class/actor/global_actor_conformance.swift +++ b/test/decl/class/actor/global_actor_conformance.swift @@ -18,7 +18,7 @@ struct GenericGlobalActor { protocol P1 { associatedtype Assoc - @GlobalActor func method1() // expected-note{{declared here}} + @GlobalActor func method1() @GenericGlobalActor func method2() // expected-note{{declared here}} @GenericGlobalActor func method3() func method4() // expected-note{{declared here}} @@ -33,8 +33,7 @@ protocol P2 { class C1 : P1, P2 { typealias Assoc = String - // FIXME: This will be inferred - func method1() { } // expected-error{{instance method 'method1()' must be isolated to the global actor 'GlobalActor' to satisfy corresponding requirement from protocol 'P1'}}{{3-3=@GlobalActor}} + func method1() { } @GenericGlobalActor func method2() { } // expected-error{{instance method 'method2()' isolated to global actor 'GenericGlobalActor' can not satisfy corresponding requirement from protocol 'P1' isolated to global actor 'GenericGlobalActor'}} @GenericGlobalActorfunc method3() { } From 2f7ff6aa398dfe65f09fb2e44bc5969fdebbc11b Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 13 Oct 2020 21:41:03 -0700 Subject: [PATCH 2/6] [Concurrency] Allow ActorIsolation in diagnostic messages. ActorIsolation is rendered as a descriptive phrase before an entity, e.g, "actor-independent" or "global actor 'UIActor'-isolated" when used in diagnostics. --- include/swift/AST/ActorIsolation.h | 1 + include/swift/AST/DiagnosticEngine.h | 14 ++++++++++++++ lib/AST/DiagnosticEngine.cpp | 20 ++++++++++++++++++++ 3 files changed, 35 insertions(+) diff --git a/include/swift/AST/ActorIsolation.h b/include/swift/AST/ActorIsolation.h index 78961df373191..7856ed8133fc0 100644 --- a/include/swift/AST/ActorIsolation.h +++ b/include/swift/AST/ActorIsolation.h @@ -16,6 +16,7 @@ #ifndef SWIFT_AST_ACTORISOLATIONSTATE_H #define SWIFT_AST_ACTORISOLATIONSTATE_H +#include "swift/AST/Type.h" #include "llvm/ADT/Hashing.h" namespace llvm { diff --git a/include/swift/AST/DiagnosticEngine.h b/include/swift/AST/DiagnosticEngine.h index 338f146922b5e..3ec8f3648a08a 100644 --- a/include/swift/AST/DiagnosticEngine.h +++ b/include/swift/AST/DiagnosticEngine.h @@ -18,6 +18,7 @@ #ifndef SWIFT_BASIC_DIAGNOSTICENGINE_H #define SWIFT_BASIC_DIAGNOSTICENGINE_H +#include "swift/AST/ActorIsolation.h" #include "swift/AST/DeclNameLoc.h" #include "swift/AST/DiagnosticConsumer.h" #include "swift/AST/TypeLoc.h" @@ -93,6 +94,7 @@ namespace swift { DeclAttribute, VersionTuple, LayoutConstraint, + ActorIsolation, }; namespace diag { @@ -122,6 +124,7 @@ namespace swift { const DeclAttribute *DeclAttributeVal; llvm::VersionTuple VersionVal; LayoutConstraint LayoutConstraintVal; + ActorIsolation ActorIsolationVal; }; public: @@ -209,6 +212,12 @@ namespace swift { DiagnosticArgument(LayoutConstraint L) : Kind(DiagnosticArgumentKind::LayoutConstraint), LayoutConstraintVal(L) { } + + DiagnosticArgument(ActorIsolation AI) + : Kind(DiagnosticArgumentKind::ActorIsolation), + ActorIsolationVal(AI) { + } + /// Initializes a diagnostic argument using the underlying type of the /// given enum. template< @@ -299,6 +308,11 @@ namespace swift { assert(Kind == DiagnosticArgumentKind::LayoutConstraint); return LayoutConstraintVal; } + + ActorIsolation getAsActorIsolation() const { + assert(Kind == DiagnosticArgumentKind::ActorIsolation); + return ActorIsolationVal; + } }; struct DiagnosticFormatOptions { diff --git a/lib/AST/DiagnosticEngine.cpp b/lib/AST/DiagnosticEngine.cpp index 9ab4e08c42620..7116f3517ac3f 100644 --- a/lib/AST/DiagnosticEngine.cpp +++ b/lib/AST/DiagnosticEngine.cpp @@ -660,6 +660,26 @@ static void formatDiagnosticArgument(StringRef Modifier, Out << FormatOpts.OpeningQuotationMark << Arg.getAsLayoutConstraint() << FormatOpts.ClosingQuotationMark; break; + case DiagnosticArgumentKind::ActorIsolation: + switch (auto isolation = Arg.getAsActorIsolation()) { + case ActorIsolation::ActorInstance: + Out << "actor-isolated"; + break; + + case ActorIsolation::GlobalActor: + Out << "global actor " << FormatOpts.OpeningQuotationMark + << isolation.getGlobalActor().getString() + << FormatOpts.ClosingQuotationMark << "-isolated"; + break; + + case ActorIsolation::Independent: + Out << "actor-independent"; + break; + + case ActorIsolation::Unspecified: + Out << "non-actor-isolated"; + break; + } } } From 18fd4be17a6a4307e46eeeae5d5220e3b9179c23 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 13 Oct 2020 21:41:59 -0700 Subject: [PATCH 3/6] [Concurrency] Check actor isolation consistency for overrides & subclasses. Both overriding declarations and subclasses must have the actor isolation as their overridden declarations or superclasses, respectively. Enforce this, ensuring that we're also doing the appropriate substitutions. --- include/swift/AST/ActorIsolation.h | 8 ++ include/swift/AST/DiagnosticsSema.def | 7 ++ lib/AST/TypeCheckRequests.cpp | 23 ++++++ lib/Sema/TypeCheckConcurrency.cpp | 74 ++++++++++++++++++- lib/Sema/TypeCheckConcurrency.h | 7 ++ lib/Sema/TypeCheckDeclPrimary.cpp | 8 +- test/Concurrency/global_actor_inference.swift | 60 ++++++++++++++- 7 files changed, 182 insertions(+), 5 deletions(-) diff --git a/include/swift/AST/ActorIsolation.h b/include/swift/AST/ActorIsolation.h index 7856ed8133fc0..c5ba69ba3f156 100644 --- a/include/swift/AST/ActorIsolation.h +++ b/include/swift/AST/ActorIsolation.h @@ -25,6 +25,7 @@ class raw_ostream; namespace swift { class ClassDecl; +class SubstitutionMap; class Type; /// Determine whether the given types are (canonically) equal, declared here @@ -97,6 +98,13 @@ class ActorIsolation { return globalActor; } + /// Determine whether this isolation will require substitution to be + /// evaluated. + bool requiresSubstitution() const; + + /// Substitute into types within the actor isolation. + ActorIsolation subst(SubstitutionMap subs) const; + friend bool operator==(const ActorIsolation &lhs, const ActorIsolation &rhs) { if (lhs.kind != rhs.kind) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 6a3d213a2314f..5bced0c6ac09a 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -4262,6 +4262,13 @@ ERROR(actor_isolation_multiple_attr,none, "%0 %1 has multiple actor-isolation attributes ('%2' and '%3')", (DescriptiveDeclKind, DeclName, StringRef, StringRef)) +ERROR(actor_isolation_override_mismatch,none, + "%0 %1 %2 has different actor isolation from %3 overridden declaration", + (ActorIsolation, DescriptiveDeclKind, DeclName, ActorIsolation)) +ERROR(actor_isolation_superclass_mismatch,none, + "%0 class %1 has different actor isolation from %2 superclass %3", + (ActorIsolation, Identifier, ActorIsolation, Identifier)) + //------------------------------------------------------------------------------ // MARK: Type Check Types //------------------------------------------------------------------------------ diff --git a/lib/AST/TypeCheckRequests.cpp b/lib/AST/TypeCheckRequests.cpp index 8b774caaea0d2..b72542686e9c4 100644 --- a/lib/AST/TypeCheckRequests.cpp +++ b/lib/AST/TypeCheckRequests.cpp @@ -1447,6 +1447,29 @@ void CustomAttrTypeRequest::cacheResult(Type value) const { attr->setType(value); } +bool ActorIsolation::requiresSubstitution() const { + switch (kind) { + case ActorInstance: + case Independent: + case Unspecified: + return false; + + case GlobalActor: + return getGlobalActor()->hasTypeParameter(); + } +} + +ActorIsolation ActorIsolation::subst(SubstitutionMap subs) const { + switch (kind) { + case ActorInstance: + case Independent: + case Unspecified: + return *this; + + case GlobalActor: + return forGlobalActor(getGlobalActor().subst(subs)); + } +} void swift::simple_display( llvm::raw_ostream &out, const ActorIsolation &state) { diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index 4a7ba10daea93..815f548c694e2 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -1155,8 +1155,16 @@ ActorIsolation ActorIsolationRequest::evaluate( // If the declaration overrides another declaration, it must have the same // actor isolation. if (auto overriddenValue = value->getOverriddenDecl()) { - if (auto isolation = getActorIsolation(overriddenValue)) - return inferredIsolation(isolation); + if (auto isolation = getActorIsolation(overriddenValue)) { + SubstitutionMap subs; + if (auto env = value->getInnermostDeclContext() + ->getGenericEnvironmentOfContext()) { + subs = SubstitutionMap::getOverrideSubstitutions( + overriddenValue, value, subs); + } + + return inferredIsolation(isolation.subst(subs)); + } } // If the declaration witnesses a protocol requirement that is isolated, @@ -1210,3 +1218,65 @@ ActorIsolation swift::getActorIsolation(ValueDecl *value) { ctx.evaluator, ActorIsolationRequest{value}, ActorIsolation::forUnspecified()); } + +void swift::checkOverrideActorIsolation(ValueDecl *value) { + if (isa(value)) + return; + + auto overridden = value->getOverriddenDecl(); + if (!overridden) + return; + + // Determine the actor isolation of this declaration. + auto isolation = getActorIsolation(value); + + // Determine the actor isolation of the overridden function.= + auto overriddenIsolation = getActorIsolation(overridden); + + if (overriddenIsolation.requiresSubstitution()) { + SubstitutionMap subs; + if (auto env = value->getInnermostDeclContext() + ->getGenericEnvironmentOfContext()) { + subs = SubstitutionMap::getOverrideSubstitutions(overridden, value, subs); + overriddenIsolation = overriddenIsolation.subst(subs); + } + } + + // If the isolation matches, we're done. + if (isolation == overriddenIsolation) + return; + + // Isolation mismatch. Diagnose it. + value->diagnose( + diag::actor_isolation_override_mismatch, isolation, + value->getDescriptiveKind(), value->getName(), overriddenIsolation); + overridden->diagnose(diag::overridden_here); +} + +void swift::checkSubclassActorIsolation(ClassDecl *classDecl) { + auto superclassDecl = classDecl->getSuperclassDecl(); + if (!superclassDecl) + return; + + auto isolation = getActorIsolation(classDecl); + auto superclassIsolation = getActorIsolation(superclassDecl); + + if (superclassIsolation.requiresSubstitution()) { + Type superclassType = classDecl->getSuperclass(); + if (!superclassType) + return; + + SubstitutionMap subs = superclassType->getMemberSubstitutionMap( + classDecl->getModuleContext(), classDecl); + superclassIsolation = superclassIsolation.subst(subs); + } + + if (isolation == superclassIsolation) + return; + + // Diagnose mismatch. + classDecl->diagnose( + diag::actor_isolation_superclass_mismatch, isolation, + classDecl->getName(), superclassIsolation, superclassDecl->getName()); + superclassDecl->diagnose(diag::decl_declared_here, superclassDecl->getName()); +} diff --git a/lib/Sema/TypeCheckConcurrency.h b/lib/Sema/TypeCheckConcurrency.h index 5a4869dbae150..ffe4b10f31f77 100644 --- a/lib/Sema/TypeCheckConcurrency.h +++ b/lib/Sema/TypeCheckConcurrency.h @@ -144,6 +144,13 @@ class ActorIsolationRestriction { operator Kind() const { return kind; }; }; +/// Check that the actor isolation of an override matches that of its +/// overridden declaration. +void checkOverrideActorIsolation(ValueDecl *value); + +/// Check that the actor isolation of a class matches that of its superclass. +void checkSubclassActorIsolation(ClassDecl *classDecl); + } // end namespace swift #endif /* SWIFT_SEMA_TYPECHECKCONCURRENCY_H */ diff --git a/lib/Sema/TypeCheckDeclPrimary.cpp b/lib/Sema/TypeCheckDeclPrimary.cpp index adc9c837df6c3..da39bf794a641 100644 --- a/lib/Sema/TypeCheckDeclPrimary.cpp +++ b/lib/Sema/TypeCheckDeclPrimary.cpp @@ -21,6 +21,7 @@ #include "DerivedConformances.h" #include "TypeChecker.h" #include "TypeCheckAccess.h" +#include "TypeCheckConcurrency.h" #include "TypeCheckDecl.h" #include "TypeCheckAvailability.h" #include "TypeCheckObjC.h" @@ -1382,12 +1383,17 @@ class DeclChecker : public DeclVisitor { (void) VD->getFormalAccess(); // Compute overrides. - (void) VD->getOverriddenDecls(); + if (!VD->getOverriddenDecls().empty()) + checkOverrideActorIsolation(VD); // Check whether the member is @objc or dynamic. (void) VD->isObjC(); (void) VD->isDynamic(); + // For a class, check actor isolation. + if (auto classDecl = dyn_cast(VD)) + checkSubclassActorIsolation(classDecl); + // If this is a member of a nominal type, don't allow it to have a name of // "Type" or "Protocol" since we reserve the X.Type and X.Protocol // expressions to mean something builtin to the language. We *do* allow diff --git a/test/Concurrency/global_actor_inference.swift b/test/Concurrency/global_actor_inference.swift index 8fcbe36011835..94ddddee62c39 100644 --- a/test/Concurrency/global_actor_inference.swift +++ b/test/Concurrency/global_actor_inference.swift @@ -15,6 +15,11 @@ struct OtherGlobalActor { static let shared = SomeActor() } +@globalActor +struct GenericGlobalActor { + static var shared: SomeActor { SomeActor() } +} + // ---------------------------------------------------------------------- // Global actor inference for protocols // ---------------------------------------------------------------------- @@ -25,11 +30,10 @@ protocol P1 { } protocol P2 { - @SomeGlobalActor func method1() + @SomeGlobalActor func method1() // expected-note {{'method1()' declared here}} func method2() } - class C1: P1 { func method() { } // expected-note{{only asynchronous methods can be used outside the actor instance}} @@ -88,3 +92,55 @@ class C5 { c5.method1() // OK: no propagation c5.method2() // expected-error{{instance method 'method2()' isolated to global actor 'SomeGlobalActor' can not be referenced from different global actor 'OtherGlobalActor'}} } + +protocol P3 { + @OtherGlobalActor func method1() // expected-note{{'method1()' declared here}} + func method2() +} + +class C6: P2, P3 { + func method1() { } + // expected-error@-1{{instance method 'method1()' must be isolated to the global actor 'SomeGlobalActor' to satisfy corresponding requirement from protocol 'P2'}} + // expected-error@-2{{instance method 'method1()' must be isolated to the global actor 'OtherGlobalActor' to satisfy corresponding requirement from protocol 'P3'}} + func method2() { } + + func testMethod() { + method1() // okay: no inference + method2() // okay: no inference + } +} + +// ---------------------------------------------------------------------- +// Global actor checking for overrides +// ---------------------------------------------------------------------- +actor class GenericSuper { + @GenericGlobalActor func method() { } + + @GenericGlobalActor func method2() { } // expected-note {{overridden declaration is here}} + @GenericGlobalActor func method3() { } // expected-note {{overridden declaration is here}} + @GenericGlobalActor func method4() { } + @GenericGlobalActor func method5() { } +} + +actor class GenericSub : GenericSuper<[T]> { + override func method() { } // expected-note{{only asynchronous methods can be used outside the actor instance; do you want to add 'async'?}} + + @GenericGlobalActor override func method2() { } // expected-error{{global actor 'GenericGlobalActor'-isolated instance method 'method2()' has different actor isolation from global actor 'GenericGlobalActor<[T]>'-isolated overridden declaration}} + @actorIndependent override func method3() { } // expected-error{{actor-independent instance method 'method3()' has different actor isolation from global actor 'GenericGlobalActor<[T]>'-isolated overridden declaration}} + + @OtherGlobalActor func testMethod() { + method() // expected-error{{instance method 'method()' isolated to global actor 'GenericGlobalActor<[T]>' can not be referenced from different global actor 'OtherGlobalActor'}} + } +} + +// ---------------------------------------------------------------------- +// Global actor checking for supeclasses +// ---------------------------------------------------------------------- +struct Container { + @GenericGlobalActor class Superclass { } // expected-note{{'Superclass' declared here}} +} + +struct OtherContainer { + @GenericGlobalActor<[U]> class Subclass1 : Container<[U]>.Superclass { } + @GenericGlobalActor class Subclass2 : Container<[U]>.Superclass { } // expected-error{{global actor 'GenericGlobalActor'-isolated class 'Subclass2' has different actor isolation from global actor 'GenericGlobalActor<[U]>'-isolated superclass 'Superclass'}} +} From 11cf3ceffe20ea7d32678c5d83347735cea94341 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 13 Oct 2020 22:37:24 -0700 Subject: [PATCH 4/6] [Concurrency] Eliminate actor isolation checking for subclasses. Subclasses inherit the global actor from their superclass by default, but it's okay to change it---it's just a default that can be overridden on a per-member basis anyway. --- include/swift/AST/DiagnosticsSema.def | 3 -- lib/Sema/TypeCheckConcurrency.cpp | 28 ------------------- lib/Sema/TypeCheckConcurrency.h | 3 -- lib/Sema/TypeCheckDeclPrimary.cpp | 4 --- test/Concurrency/global_actor_inference.swift | 7 +++-- 5 files changed, 4 insertions(+), 41 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 5bced0c6ac09a..fe4fc96f29b4c 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -4265,9 +4265,6 @@ ERROR(actor_isolation_multiple_attr,none, ERROR(actor_isolation_override_mismatch,none, "%0 %1 %2 has different actor isolation from %3 overridden declaration", (ActorIsolation, DescriptiveDeclKind, DeclName, ActorIsolation)) -ERROR(actor_isolation_superclass_mismatch,none, - "%0 class %1 has different actor isolation from %2 superclass %3", - (ActorIsolation, Identifier, ActorIsolation, Identifier)) //------------------------------------------------------------------------------ // MARK: Type Check Types diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index 815f548c694e2..7c479f05762f9 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -1252,31 +1252,3 @@ void swift::checkOverrideActorIsolation(ValueDecl *value) { value->getDescriptiveKind(), value->getName(), overriddenIsolation); overridden->diagnose(diag::overridden_here); } - -void swift::checkSubclassActorIsolation(ClassDecl *classDecl) { - auto superclassDecl = classDecl->getSuperclassDecl(); - if (!superclassDecl) - return; - - auto isolation = getActorIsolation(classDecl); - auto superclassIsolation = getActorIsolation(superclassDecl); - - if (superclassIsolation.requiresSubstitution()) { - Type superclassType = classDecl->getSuperclass(); - if (!superclassType) - return; - - SubstitutionMap subs = superclassType->getMemberSubstitutionMap( - classDecl->getModuleContext(), classDecl); - superclassIsolation = superclassIsolation.subst(subs); - } - - if (isolation == superclassIsolation) - return; - - // Diagnose mismatch. - classDecl->diagnose( - diag::actor_isolation_superclass_mismatch, isolation, - classDecl->getName(), superclassIsolation, superclassDecl->getName()); - superclassDecl->diagnose(diag::decl_declared_here, superclassDecl->getName()); -} diff --git a/lib/Sema/TypeCheckConcurrency.h b/lib/Sema/TypeCheckConcurrency.h index ffe4b10f31f77..9191f815650f1 100644 --- a/lib/Sema/TypeCheckConcurrency.h +++ b/lib/Sema/TypeCheckConcurrency.h @@ -148,9 +148,6 @@ class ActorIsolationRestriction { /// overridden declaration. void checkOverrideActorIsolation(ValueDecl *value); -/// Check that the actor isolation of a class matches that of its superclass. -void checkSubclassActorIsolation(ClassDecl *classDecl); - } // end namespace swift #endif /* SWIFT_SEMA_TYPECHECKCONCURRENCY_H */ diff --git a/lib/Sema/TypeCheckDeclPrimary.cpp b/lib/Sema/TypeCheckDeclPrimary.cpp index da39bf794a641..5fd674b6d9775 100644 --- a/lib/Sema/TypeCheckDeclPrimary.cpp +++ b/lib/Sema/TypeCheckDeclPrimary.cpp @@ -1390,10 +1390,6 @@ class DeclChecker : public DeclVisitor { (void) VD->isObjC(); (void) VD->isDynamic(); - // For a class, check actor isolation. - if (auto classDecl = dyn_cast(VD)) - checkSubclassActorIsolation(classDecl); - // If this is a member of a nominal type, don't allow it to have a name of // "Type" or "Protocol" since we reserve the X.Type and X.Protocol // expressions to mean something builtin to the language. We *do* allow diff --git a/test/Concurrency/global_actor_inference.swift b/test/Concurrency/global_actor_inference.swift index 94ddddee62c39..178368324eecc 100644 --- a/test/Concurrency/global_actor_inference.swift +++ b/test/Concurrency/global_actor_inference.swift @@ -134,13 +134,14 @@ actor class GenericSub : GenericSuper<[T]> { } // ---------------------------------------------------------------------- -// Global actor checking for supeclasses +// Global actor inference for superclasses // ---------------------------------------------------------------------- struct Container { - @GenericGlobalActor class Superclass { } // expected-note{{'Superclass' declared here}} + @GenericGlobalActor class Superclass { } } struct OtherContainer { + // Okay to change the global actor in a subclass. @GenericGlobalActor<[U]> class Subclass1 : Container<[U]>.Superclass { } - @GenericGlobalActor class Subclass2 : Container<[U]>.Superclass { } // expected-error{{global actor 'GenericGlobalActor'-isolated class 'Subclass2' has different actor isolation from global actor 'GenericGlobalActor<[U]>'-isolated superclass 'Superclass'}} + @GenericGlobalActor class Subclass2 : Container<[U]>.Superclass { } } From a5b15ed6309ce9c893204bec9ba8e9a2d19d9b66 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 13 Oct 2020 22:40:51 -0700 Subject: [PATCH 5/6] [Concurrency] Substitute into superclass global actors when inheriting them. --- lib/Sema/TypeCheckConcurrency.cpp | 13 ++++++++++++- test/Concurrency/global_actor_inference.swift | 10 ++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index 7c479f05762f9..11d1a5e26c975 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -1178,8 +1178,19 @@ ActorIsolation ActorIsolationRequest::evaluate( if (auto classDecl = dyn_cast(value)) { if (auto superclassDecl = classDecl->getSuperclassDecl()) { auto superclassIsolation = getActorIsolation(superclassDecl); - if (!superclassIsolation.isUnspecified()) + if (!superclassIsolation.isUnspecified()) { + if (superclassIsolation.requiresSubstitution()) { + Type superclassType = classDecl->getSuperclass(); + if (!superclassType) + return ActorIsolation::forUnspecified(); + + SubstitutionMap subs = superclassType->getMemberSubstitutionMap( + classDecl->getModuleContext(), classDecl); + superclassIsolation = superclassIsolation.subst(subs); + } + return inferredIsolation(superclassIsolation); + } } } diff --git a/test/Concurrency/global_actor_inference.swift b/test/Concurrency/global_actor_inference.swift index 178368324eecc..1db013712493a 100644 --- a/test/Concurrency/global_actor_inference.swift +++ b/test/Concurrency/global_actor_inference.swift @@ -138,10 +138,20 @@ actor class GenericSub : GenericSuper<[T]> { // ---------------------------------------------------------------------- struct Container { @GenericGlobalActor class Superclass { } + @GenericGlobalActor<[T]> class Superclass2 { } } struct OtherContainer { // Okay to change the global actor in a subclass. @GenericGlobalActor<[U]> class Subclass1 : Container<[U]>.Superclass { } @GenericGlobalActor class Subclass2 : Container<[U]>.Superclass { } + + // Ensure that substitutions work properly when inheriting. + class Subclass3 : Container<(U, V)>.Superclass2 { + func method() { } // expected-note{{only asynchronous methods can be used outside the actor instance}} + + @OtherGlobalActor func testMethod() { + method() // expected-error{{instance method 'method()' isolated to global actor 'GenericGlobalActor<[(U, V)]>' can not be referenced from different global actor 'OtherGlobalActor'}} + } + } } From 3a651a61b7868c6b13a0cb90eaf1a5be92762a7c Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 13 Oct 2020 22:57:06 -0700 Subject: [PATCH 6/6] [Concurrency] Fix circular reference on isolation propagation. --- lib/Sema/TypeCheckConcurrency.cpp | 17 +++++++++-------- .../single-file-private/AnyObject.swift | 14 ++++++++++++++ test/decl/class/circular_inheritance.swift | 16 ++++++++-------- 3 files changed, 31 insertions(+), 16 deletions(-) diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index 11d1a5e26c975..474512979861c 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -1035,6 +1035,9 @@ static Optional getIsolationFromWitnessedRequirements( if (!idc) return None; + if (dc->getSelfProtocolDecl()) + return None; + // Walk through each of the conformances in this context, collecting any // requirements that have actor isolation. auto conformances = evaluateOrDefault( @@ -1167,6 +1170,12 @@ ActorIsolation ActorIsolationRequest::evaluate( } } + // If this is an accessor, use the actor isolation of its storage + // declaration. + if (auto accessor = dyn_cast(value)) { + return getActorIsolation(accessor->getStorage()); + } + // If the declaration witnesses a protocol requirement that is isolated, // use that. if (auto witnessedIsolation = getIsolationFromWitnessedRequirements(value)) { @@ -1194,14 +1203,6 @@ ActorIsolation ActorIsolationRequest::evaluate( } } - // If this is an accessor, use the actor isolation of its storage - // declaration. - if (auto accessor = dyn_cast(value)) { - auto storageIsolation = getActorIsolation(accessor->getStorage()); - if (!storageIsolation.isUnspecified()) - return inferredIsolation(storageIsolation); - } - // If the declaration is in an extension that has one of the isolation // attributes, use that. if (auto ext = dyn_cast(value->getDeclContext())) { diff --git a/test/Incremental/Verifier/single-file-private/AnyObject.swift b/test/Incremental/Verifier/single-file-private/AnyObject.swift index 2323db8859d4d..896bd76c3b7a3 100644 --- a/test/Incremental/Verifier/single-file-private/AnyObject.swift +++ b/test/Incremental/Verifier/single-file-private/AnyObject.swift @@ -72,3 +72,17 @@ func lookupOnAnyObject(object: AnyObject) { // expected-provides {{lookupOnAnyOb // expected-member {{Swift.CVarArg.someMethod}} // expected-member {{Swift.CustomStringConvertible.someMethod}} // expected-member {{Swift.CustomDebugStringConvertible.someMethod}} +// expected-member {{Swift.Equatable.someMember}} +// expected-member{{Swift.CustomDebugStringConvertible.init}} +// expected-member{{Swift.CVarArg.someMember}} +// expected-member{{Foundation._KeyValueCodingAndObservingPublishing.someMember}} +// expected-member{{Swift.Equatable.init}} +// expected-member{{Swift.Hashable.init}} +// expected-member{{Swift.CVarArg.init}} +// expected-member{{Foundation._KeyValueCodingAndObserving.someMember}} +// expected-member{{Foundation._KeyValueCodingAndObservingPublishing.init}} +// expected-member{{Swift.CustomDebugStringConvertible.someMember}} +// expected-member{{Swift.CustomStringConvertible.someMember}} +// expected-member{{Swift.CustomStringConvertible.init}} +// expected-member{{Swift.Hashable.someMember}} +// expected-member{{Foundation._KeyValueCodingAndObserving.init}} diff --git a/test/decl/class/circular_inheritance.swift b/test/decl/class/circular_inheritance.swift index 1ce97f8f216b6..103ceb4679ba5 100644 --- a/test/decl/class/circular_inheritance.swift +++ b/test/decl/class/circular_inheritance.swift @@ -4,14 +4,14 @@ // RUN: not %target-swift-frontend -typecheck -debug-cycles %s -build-request-dependency-graph -output-request-graphviz %t.dot -stats-output-dir %t/stats-dir 2> %t.cycles // RUN: %FileCheck -check-prefix CHECK-DOT %s < %t.dot -class Left // expected-error {{'Left' inherits from itself}} expected-note {{through reference here}} +class Left // expected-error {{'Left' inherits from itself}} expected-note 2{{through reference here}} : Right.Hand { // expected-note {{through reference here}} - class Hand {} + class Hand {} // expected-note {{through reference here}} } -class Right // expected-note {{through reference here}} expected-note{{class 'Right' declared here}} +class Right // expected-note 2 {{through reference here}} expected-note{{class 'Right' declared here}} : Left.Hand { // expected-note {{through reference here}} - class Hand {} + class Hand {} // expected-error {{circular reference}} } class C : B { } // expected-error{{'C' inherits from itself}} @@ -30,15 +30,15 @@ class Outer { class Inner : Outer {} } -class Outer2 // expected-error {{'Outer2' inherits from itself}} expected-note {{through reference here}} +class Outer2 // expected-error {{'Outer2' inherits from itself}} expected-note 2 {{through reference here}} : Outer2.Inner { // expected-note {{through reference here}} - class Inner {} + class Inner {} // expected-error{{circular reference}} } -class Outer3 // expected-error {{'Outer3' inherits from itself}} expected-note {{through reference here}} +class Outer3 // expected-error {{'Outer3' inherits from itself}} expected-note 2 {{through reference here}} : Outer3.Inner { // expected-note {{through reference here}} - class Inner {} + class Inner {} // expected-error{{circular reference}} } // CHECK-DOT: digraph Dependencies