Skip to content

Commit

Permalink
Respect @preconcurrency for instance properties of non-sendable typ…
Browse files Browse the repository at this point in the history
…es in deinit

Instance properties of non-sendable types cannot safely be
accessed within deinitializers. Make sure we respect `@preconcurrency`
when diagnosing these.
  • Loading branch information
DougGregor committed May 17, 2024
1 parent fc85f98 commit c326fd3
Show file tree
Hide file tree
Showing 8 changed files with 57 additions and 27 deletions.
3 changes: 0 additions & 3 deletions include/swift/AST/DiagnosticsSIL.def
Original file line number Diff line number Diff line change
Expand Up @@ -694,9 +694,6 @@ ERROR(isolated_after_nonisolated, none,
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))
ERROR(isolated_property_mutation_in_nonisolated_context,none,
"actor-isolated %kind0 can not be %select{referenced|mutated}1 "
"from a non-isolated context",
Expand Down
4 changes: 4 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -5796,6 +5796,10 @@ ERROR(nonisolated_wrapped_property,none,
"'nonisolated' is not supported on properties with property wrappers",
())

ERROR(non_sendable_from_deinit,none,
"cannot access %1 %2 with a non-sendable type %0 from non-isolated deinit",
(Type, DescriptiveDeclKind, DeclName))

ERROR(actor_instance_property_wrapper,none,
"%0 property in property wrapper type %1 cannot be isolated to "
"the actor instance; consider 'nonisolated'",
Expand Down
9 changes: 9 additions & 0 deletions include/swift/Sema/Concurrency.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@

namespace swift {

class DeclContext;
class SourceFile;
class NominalTypeDecl;
class VarDecl;

/// If any of the imports in this source file was @preconcurrency but there were
/// no diagnostics downgraded or suppressed due to that @preconcurrency, suggest
Expand All @@ -35,6 +37,13 @@ void diagnoseUnnecessaryPreconcurrencyImports(SourceFile &sf);
bool hasExplicitSendableConformance(NominalTypeDecl *nominal,
bool applyModuleDefault = true);

/// Diagnose the use of an instance property of non-sendable type from an
/// nonisolated deinitializer within an actor-isolated type.
///
/// \returns true iff a diagnostic was emitted for this reference.
bool diagnoseNonSendableFromDeinit(
SourceLoc refLoc, VarDecl *var, DeclContext *dc);

} // namespace swift

#endif
29 changes: 8 additions & 21 deletions lib/SILOptimizer/Mandatory/FlowIsolation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "swift/AST/Expr.h"
#include "swift/AST/ActorIsolation.h"
#include "swift/AST/DiagnosticsSIL.h"
#include "swift/Sema/Concurrency.h"
#include "swift/SIL/ApplySite.h"
#include "swift/SIL/BitDataflow.h"
#include "swift/SIL/BasicBlockBits.h"
Expand Down Expand Up @@ -528,31 +529,17 @@ static bool onlyDeinitAccess(RefElementAddrInst *inst) {
/// is happening in a deinit that uses flow-isolation.
/// \returns true iff a diagnostic was emitted for this reference.
static bool diagnoseNonSendableFromDeinit(RefElementAddrInst *inst) {
VarDecl *var = inst->getField();
Type ty = var->getTypeInContext();
DeclContext *dc = inst->getFunction()->getDeclContext();
auto dc = inst->getFunction()->getDeclContext();

// FIXME: we should emit diagnostics in other modes using:
//
// if (!shouldDiagnoseExistingDataRaces(dc))
// return false;
//
// but until we decide how we want to handle isolated state from deinits,
// we're going to limit the noise to complete mode for now.
// For historical reasons, only diagnose this issue in strict mode.
if (dc->getASTContext().LangOpts.StrictConcurrencyLevel
!= StrictConcurrency::Complete)
return false;

if (ty->isSendableType())
!= StrictConcurrency::Complete)
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;
return swift::diagnoseNonSendableFromDeinit(
inst->getLoc().getSourceLoc(),
inst->getField(),
dc);
}

class OperandWorklist {
Expand Down
11 changes: 11 additions & 0 deletions lib/Sema/TypeCheckConcurrency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6830,3 +6830,14 @@ ActorReferenceResult ActorReferenceResult::forReference(

return forEntersActor(declIsolation, options);
}

bool swift::diagnoseNonSendableFromDeinit(
SourceLoc refLoc, VarDecl *var, DeclContext *dc) {
return diagnoseIfAnyNonSendableTypes(var->getTypeInContext(),
SendableCheckContext(dc),
Type(),
SourceLoc(),
refLoc,
diag::non_sendable_from_deinit,
var->getDescriptiveKind(), var->getName());
}
2 changes: 1 addition & 1 deletion test/Concurrency/flow_isolation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ func takeNonSendable(_ ns: NonSendableType) {}
@available(SwiftStdlib 5.1, *)
func takeSendable(_ s: SendableType) {}

class NonSendableType {
class NonSendableType { // expected-note *{{class 'NonSendableType' does not conform to the 'Sendable' protocol}}
var x: Int = 0
func f() {}
}
Expand Down
22 changes: 22 additions & 0 deletions test/Concurrency/predates_concurrency_import_deinit.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// RUN: %empty-directory(%t)
// RUN: %target-swift-frontend -emit-module -emit-module-path %t/StrictModule.swiftmodule -module-name StrictModule -swift-version 6 %S/Inputs/StrictModule.swift
// RUN: %target-swift-frontend -emit-module -emit-module-path %t/NonStrictModule.swiftmodule -module-name NonStrictModule %S/Inputs/NonStrictModule.swift

// RUN: %target-swift-frontend -I %t %s -emit-sil -o /dev/null -verify -strict-concurrency=complete

// REQUIRES: concurrency
// REQUIRES: asserts

@preconcurrency import NonStrictModule
@preconcurrency import StrictModule

@available(SwiftStdlib 5.1, *)
actor ActorWithDeinit {
var ns: NonStrictClass = NonStrictClass()
var ss: StrictStruct = StrictStruct()

deinit {
print(ns)
print(ss) // expected-warning{{cannot access property 'ss' with a non-sendable type 'StrictStruct' from non-isolated deinit}}
}
}
4 changes: 2 additions & 2 deletions test/Concurrency/predates_concurrency_import_swift6.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
// RUN: %target-swift-frontend -emit-module -emit-module-path %t/StrictModule.swiftmodule -module-name StrictModule -swift-version 6 %S/Inputs/StrictModule.swift
// RUN: %target-swift-frontend -emit-module -emit-module-path %t/NonStrictModule.swiftmodule -module-name NonStrictModule %S/Inputs/NonStrictModule.swift

// RUN: %target-swift-frontend -swift-version 6 -I %t %s %s -emit-sil -o /dev/null -verify -parse-as-library
// RUN: %target-swift-frontend -swift-version 6 -I %t %s %s -emit-sil -o /dev/null -verify -enable-upcoming-feature RegionBasedIsolation -parse-as-library
// RUN: %target-swift-frontend -swift-version 6 -I %t %s -emit-sil -o /dev/null -verify -parse-as-library
// RUN: %target-swift-frontend -swift-version 6 -I %t %s -emit-sil -o /dev/null -verify -enable-upcoming-feature RegionBasedIsolation -parse-as-library

@preconcurrency import NonStrictModule
@preconcurrency import StrictModule
Expand Down

0 comments on commit c326fd3

Please sign in to comment.