Skip to content

Commit

Permalink
Merge pull request #74172 from DougGregor/preconcurrency-conformance-…
Browse files Browse the repository at this point in the history 
…no-sendable

Don't check sendability of witnesses for `@preconcurrency` conformances
  • Loading branch information
@DougGregor
DougGregor committed Jun 7, 2024
2 parents 73b0ad3 + 1aaba1f commit c5cfe5b
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 38 deletions.
61 changes: 33 additions & 28 deletions lib/Sema/TypeCheckProtocol.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3090,7 +3090,8 @@ static bool hasExplicitGlobalActorAttr(ValueDecl *decl) {

std::optional<ActorIsolation>
ConformanceChecker::checkActorIsolation(ValueDecl *requirement,
ValueDecl *witness) {
ValueDecl *witness,
bool &usesPreconcurrency) {

// Determine the isolation of the requirement itself.
auto requirementIsolation = getActorIsolation(requirement);
Expand Down Expand Up @@ -3136,9 +3137,16 @@ ConformanceChecker::checkActorIsolation(ValueDecl *requirement,
return std::nullopt;

case ActorReferenceResult::ExitsActorToNonisolated:
diagnoseNonSendableTypesInReference(
/*base=*/nullptr, getDeclRefInContext(witness),
DC, loc, SendableCheckReason::Conformance);
if (!isPreconcurrency) {
diagnoseNonSendableTypesInReference(
/*base=*/nullptr, getDeclRefInContext(witness),
DC, loc, SendableCheckReason::Conformance);
} else {
// We depended on @preconcurrency since we were exiting an isolation
// domain.
usesPreconcurrency = true;
}

return std::nullopt;
case ActorReferenceResult::EntersActor:
// Handled below.
Expand Down Expand Up @@ -3220,19 +3228,24 @@ ConformanceChecker::checkActorIsolation(ValueDecl *requirement,
witness->getAttrs().hasAttribute<NonisolatedAttr>())
return std::nullopt;

// Check that the results of the witnessing method are sendable
diagnoseNonSendableTypesInReference(
/*base=*/nullptr, getDeclRefInContext(witness),
DC, loc, SendableCheckReason::Conformance,
getActorIsolation(witness), FunctionCheckKind::Results);

// If this requirement is a function, check that its parameters are Sendable as well
if (isa<AbstractFunctionDecl>(requirement)) {
if (!isPreconcurrency) {
// Check that the results of the witnessing method are sendable
diagnoseNonSendableTypesInReference(
/*base=*/nullptr, getDeclRefInContext(requirement),
requirement->getInnermostDeclContext(), requirement->getLoc(),
SendableCheckReason::Conformance, getActorIsolation(witness),
FunctionCheckKind::Params, loc);
/*base=*/nullptr, getDeclRefInContext(witness),
DC, loc, SendableCheckReason::Conformance,
getActorIsolation(witness), FunctionCheckKind::Results);

// If this requirement is a function, check that its parameters are Sendable as well
if (isa<AbstractFunctionDecl>(requirement)) {
diagnoseNonSendableTypesInReference(
/*base=*/nullptr, getDeclRefInContext(requirement),
requirement->getInnermostDeclContext(), requirement->getLoc(),
SendableCheckReason::Conformance, getActorIsolation(witness),
FunctionCheckKind::Params, loc);
}
} else {
// We depended on @preconcurrency to suppress Sendable checking.
usesPreconcurrency = true;
}

// If the witness is accessible across actors, we don't need to consider it
Expand Down Expand Up @@ -4955,7 +4968,7 @@ hasInvalidTypeInConformanceContext(const ValueDecl *requirement,
}

void ConformanceChecker::resolveValueWitnesses() {
bool usesPreconcurrencyConformance = false;
bool usesPreconcurrency = false;

for (auto *requirement : Proto->getProtocolRequirements()) {
// Associated type requirements handled elsewhere.
Expand All @@ -4974,16 +4987,8 @@ void ConformanceChecker::resolveValueWitnesses() {

// Check actor isolation. If we need to enter into the actor's
// isolation within the witness thunk, record that.
if (auto enteringIsolation = checkActorIsolation(requirement, witness)) {
// Only @preconcurrency conformances allow entering isolation
// when neither requirement nor witness are async by adding a
// runtime precondition that witness is always called on the
// expected executor.
if (Conformance->isPreconcurrency()) {
usesPreconcurrencyConformance =
!isAsyncDecl(requirement) && !isAsyncDecl(witness);
}

if (auto enteringIsolation = checkActorIsolation(requirement, witness,
usesPreconcurrency)) {
Conformance->overrideWitness(
requirement,
Conformance->getWitnessUncached(requirement)
Expand Down Expand Up @@ -5134,7 +5139,7 @@ void ConformanceChecker::resolveValueWitnesses() {
}
}

if (Conformance->isPreconcurrency() && !usesPreconcurrencyConformance) {
if (Conformance->isPreconcurrency() && !usesPreconcurrency) {
auto diag = DC->getASTContext().Diags.diagnose(
Conformance->getLoc(), diag::preconcurrency_conformance_not_used,
Proto->getDeclaredInterfaceType());
Expand Down
6 changes: 5 additions & 1 deletion lib/Sema/TypeCheckProtocol.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,11 +130,15 @@ class ConformanceChecker : public WitnessChecker {

/// Check that the witness and requirement have compatible actor contexts.
///
/// \param usesPreconcurrency Will be set true if the conformance is
/// @preconcurrency and we made use of that fact.
///
/// \returns the isolation that needs to be enforced to invoke the witness
/// from the requirement, used when entering an actor-isolated synchronous
/// witness from an asynchronous requirement.
std::optional<ActorIsolation> checkActorIsolation(ValueDecl *requirement,
ValueDecl *witness);
ValueDecl *witness,
bool &usesPreconcurrency);

/// Enforce restrictions on non-final classes witnessing requirements
/// involving the protocol 'Self' type.
Expand Down
23 changes: 14 additions & 9 deletions test/Concurrency/preconcurrency_conformances.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,28 +35,36 @@ extension TestPreconcurrencyAttr : @preconcurrency Q { // Ok
}

class NonSendable {}
// expected-note@-1 6 {{class 'NonSendable' does not conform to the 'Sendable' protocol}}

protocol TestSendability {
var x: NonSendable { get }
func test(_: NonSendable?) -> [NonSendable]
}

// Make sure that preconcurrency conformances don't suppress Sendable diagnostics
// Make sure that preconcurrency conformances suppress Sendable diagnostics,
// because @preconcurrency assumes that the witness will always be called
// from within the same isolation domain (with a dynamic check).
@MainActor
struct Value : @preconcurrency TestSendability {
var x: NonSendable { NonSendable() }
// expected-warning@-1 {{non-sendable type 'NonSendable' in conformance of main actor-isolated property 'x' to protocol requirement cannot cross actor boundary}}
// expected-note@-2 2 {{property declared here}}
// expected-note@-1 2 {{property declared here}}

// expected-warning@+2 {{non-sendable type '[NonSendable]' returned by main actor-isolated instance method 'test' satisfying protocol requirement cannot cross actor boundary}}
// expected-warning@+1 {{non-sendable type 'NonSendable?' in parameter of the protocol requirement satisfied by main actor-isolated instance method 'test' cannot cross actor boundary}}
func test(_: NonSendable?) -> [NonSendable] {
// expected-note@-1 2 {{calls to instance method 'test' from outside of its actor context are implicitly asynchronous}}
[]
}
}

// Make sure we don't spuriously say the @preconcurrency is unnecessary.
@MainActor
struct OtherValue : @preconcurrency TestSendability {
var x: NonSendable { NonSendable() }

nonisolated func test(_: NonSendable?) -> [NonSendable] {
[]
}
}

// Make sure that references to actor isolated witness is diagnosed

// expected-note@+1 2 {{add '@MainActor' to make global function 'test(value:)' part of global actor 'MainActor'}}
Expand All @@ -73,10 +81,7 @@ actor MyActor {

extension MyActor : @preconcurrency TestSendability {
var x: NonSendable { NonSendable() }
// expected-warning@-1 {{non-sendable type 'NonSendable' in conformance of actor-isolated property 'x' to protocol requirement cannot cross actor boundary}}

// expected-warning@+2 {{non-sendable type '[NonSendable]' returned by actor-isolated instance method 'test' satisfying protocol requirement cannot cross actor boundary}}
// expected-warning@+1 {{non-sendable type 'NonSendable?' in parameter of the protocol requirement satisfied by actor-isolated instance method 'test' cannot cross actor boundary}}
func test(_: NonSendable?) -> [NonSendable] {
[]
}
Expand Down

0 comments on commit c5cfe5b

Please sign in to comment.