Skip to content

Commit

Permalink
[Concurrency] Check actor isolation consistency for overrides & subcl…
Browse files Browse the repository at this point in the history
…asses.

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.
  • Loading branch information
DougGregor committed Oct 14, 2020
1 parent 2f7ff6a commit 18fd4be
Show file tree
Hide file tree
Showing 7 changed files with 182 additions and 5 deletions.
8 changes: 8 additions & 0 deletions include/swift/AST/ActorIsolation.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
7 changes: 7 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -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
//------------------------------------------------------------------------------
Expand Down
23 changes: 23 additions & 0 deletions lib/AST/TypeCheckRequests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
74 changes: 72 additions & 2 deletions lib/Sema/TypeCheckConcurrency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -1210,3 +1218,65 @@ ActorIsolation swift::getActorIsolation(ValueDecl *value) {
ctx.evaluator, ActorIsolationRequest{value},
ActorIsolation::forUnspecified());
}

void swift::checkOverrideActorIsolation(ValueDecl *value) {
if (isa<TypeDecl>(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());
}
7 changes: 7 additions & 0 deletions lib/Sema/TypeCheckConcurrency.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
8 changes: 7 additions & 1 deletion lib/Sema/TypeCheckDeclPrimary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -1382,12 +1383,17 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
(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<ClassDecl>(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
Expand Down
60 changes: 58 additions & 2 deletions test/Concurrency/global_actor_inference.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ struct OtherGlobalActor {
static let shared = SomeActor()
}

@globalActor
struct GenericGlobalActor<T> {
static var shared: SomeActor { SomeActor() }
}

// ----------------------------------------------------------------------
// Global actor inference for protocols
// ----------------------------------------------------------------------
Expand All @@ -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}}

Expand Down Expand Up @@ -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<T> {
@GenericGlobalActor<T> func method() { }

@GenericGlobalActor<T> func method2() { } // expected-note {{overridden declaration is here}}
@GenericGlobalActor<T> func method3() { } // expected-note {{overridden declaration is here}}
@GenericGlobalActor<T> func method4() { }
@GenericGlobalActor<T> func method5() { }
}

actor class GenericSub<T> : GenericSuper<[T]> {
override func method() { } // expected-note{{only asynchronous methods can be used outside the actor instance; do you want to add 'async'?}}

@GenericGlobalActor<T> override func method2() { } // expected-error{{global actor 'GenericGlobalActor<T>'-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<T> {
@GenericGlobalActor<T> class Superclass { } // expected-note{{'Superclass' declared here}}
}

struct OtherContainer<U> {
@GenericGlobalActor<[U]> class Subclass1 : Container<[U]>.Superclass { }
@GenericGlobalActor<U> class Subclass2 : Container<[U]>.Superclass { } // expected-error{{global actor 'GenericGlobalActor<U>'-isolated class 'Subclass2' has different actor isolation from global actor 'GenericGlobalActor<[U]>'-isolated superclass 'Superclass'}}
}

0 comments on commit 18fd4be

Please sign in to comment.