Skip to content

[region-isolation] Enable by default when strict concurrency is enabled #72851

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions include/swift/AST/ActorIsolation.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,9 @@ class ActorIsolation {

static ActorIsolation forActorInstanceSelf(ValueDecl *decl);

/// Create an ActorIsolation appropriate for a type that is self.
static ActorIsolation forActorInstanceSelf(NominalTypeDecl *decl);

static ActorIsolation forActorInstanceParameter(NominalTypeDecl *actor,
unsigned parameterIndex) {
return ActorIsolation(ActorInstance, actor, parameterIndex + 1);
Expand Down
5 changes: 5 additions & 0 deletions include/swift/Option/FrontendOptions.td
Original file line number Diff line number Diff line change
Expand Up @@ -1343,4 +1343,9 @@ def disable_experimental_parser_round_trip : Flag<["-"],
"disable-experimental-parser-round-trip">,
HelpText<"Disable round trip through the new swift parser">;

def disable_strict_concurrency_region_based_isolation : Flag<["-"],
"disable-region-based-isolation-with-strict-concurrency">,
HelpText<"Disable region based isolation when running with strict concurrency enabled. Only enabled with asserts">,
Flags<[HelpHidden]>;

} // end let Flags = [FrontendOption, NoDriverOption, HelpHidden]
2 changes: 1 addition & 1 deletion include/swift/SILOptimizer/Analysis/RegionAnalysis.h
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ class regionanalysisimpl::TrackableValueState {
}

ActorIsolation getActorIsolation() const {
return regionInfo.getActorIsolation().value();
return regionInfo.getActorIsolation();
}

void mergeIsolationRegionInfo(SILIsolationInfo newRegionInfo) {
Expand Down
58 changes: 18 additions & 40 deletions include/swift/SILOptimizer/Utils/PartitionUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,20 +110,19 @@ class SILIsolationInfo {
// clang-format off
std::variant<
// Used for actor isolated when we have ActorIsolation info from the AST.
std::optional<ActorIsolation>,
// Used for actor isolation when we infer the actor at the SIL level.
NominalTypeDecl *,
ActorIsolation,
// The task isolated parameter when we find a task isolated value.
SILValue
> data;
// clang-format on

SILIsolationInfo(Kind kind, std::optional<ActorIsolation> actorIsolation)
: kind(kind), data(actorIsolation) {}
SILIsolationInfo(Kind kind, NominalTypeDecl *decl) : kind(kind), data(decl) {}
SILIsolationInfo(ActorIsolation actorIsolation)
: kind(Actor), data(actorIsolation) {}

SILIsolationInfo(Kind kind, SILValue value) : kind(kind), data(value) {}

SILIsolationInfo(Kind kind) : kind(kind), data() {}

public:
SILIsolationInfo() : kind(Kind::Unknown), data() {}

Expand All @@ -146,18 +145,11 @@ class SILIsolationInfo {

void printForDiagnostics(llvm::raw_ostream &os) const;

std::optional<ActorIsolation> getActorIsolation() const {
ActorIsolation getActorIsolation() const {
assert(kind == Actor);
assert(std::holds_alternative<std::optional<ActorIsolation>>(data) &&
assert(std::holds_alternative<ActorIsolation>(data) &&
"Doesn't have an actor isolation?!");
return std::get<std::optional<ActorIsolation>>(data);
}

NominalTypeDecl *getActorInstance() const {
assert(kind == Actor);
assert(std::holds_alternative<NominalTypeDecl *>(data) &&
"Doesn't have an actor instance?!");
return std::get<NominalTypeDecl *>(data);
return std::get<ActorIsolation>(data);
}

SILValue getTaskIsolatedValue() const {
Expand All @@ -167,45 +159,31 @@ class SILIsolationInfo {
return std::get<SILValue>(data);
}

bool hasActorIsolation() const {
return kind == Actor &&
std::holds_alternative<std::optional<ActorIsolation>>(data);
}

bool hasActorInstance() const {
return kind == Actor && std::holds_alternative<NominalTypeDecl *>(data);
}
bool hasActorIsolation() const { return kind == Actor; }

bool hasTaskIsolatedValue() const {
return kind == Task && std::holds_alternative<SILValue>(data);
}

/// If we actually have an actor decl, return that. Otherwise, see if we have
/// an actor isolation if we can find one in there. Returns nullptr if we
/// fail.
NominalTypeDecl *tryInferActorDecl() const;

[[nodiscard]] SILIsolationInfo merge(SILIsolationInfo other) const;

SILIsolationInfo withActorIsolated(ActorIsolation isolation) {
return SILIsolationInfo::getActorIsolated(isolation);
}

static SILIsolationInfo getDisconnected() { return {Kind::Disconnected, {}}; }
static SILIsolationInfo getDisconnected() { return {Kind::Disconnected}; }

static SILIsolationInfo getActorIsolated(ActorIsolation actorIsolation) {
return {Kind::Actor, actorIsolation};
return {actorIsolation};
}

/// Sometimes we may have something that is actor isolated or that comes from
/// a type. First try getActorIsolation and otherwise, just use the type.
static SILIsolationInfo getActorIsolated(NominalTypeDecl *nomDecl) {
auto actorIsolation = swift::getActorIsolation(nomDecl);
if (actorIsolation.isActorIsolated())
return getActorIsolated(actorIsolation);
if (nomDecl->isActor())
return {Kind::Actor, nomDecl};
return SILIsolationInfo();
static SILIsolationInfo getActorIsolated(NominalTypeDecl *typeDecl) {
if (typeDecl->isActor())
return {ActorIsolation::forActorInstanceSelf(typeDecl)};
auto isolation = swift::getActorIsolation(typeDecl);
if (isolation.isGlobalActor())
return {isolation};
return {};
}

static SILIsolationInfo getGlobalActorIsolated(Type globalActorType) {
Expand Down
4 changes: 4 additions & 0 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11370,6 +11370,10 @@ ActorIsolation::forActorInstanceSelf(ValueDecl *decl) {
return ActorIsolation(ActorInstance, dc->getSelfNominalTypeDecl(), 0);
}

ActorIsolation ActorIsolation::forActorInstanceSelf(NominalTypeDecl *selfDecl) {
return ActorIsolation(ActorInstance, selfDecl, 0);
}

NominalTypeDecl *ActorIsolation::getActor() const {
assert(getKind() == ActorInstance ||
getKind() == GlobalActor);
Expand Down
12 changes: 11 additions & 1 deletion lib/DriverTool/sil_opt_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,11 @@ struct SILOptOptions {
llvm::cl::list<std::string> ClangXCC = llvm::cl::list<std::string>(
"Xcc",
llvm::cl::desc("option to pass to clang"));

llvm::cl::opt<bool> DisableRegionBasedIsolationWithStrictConcurrency =
llvm::cl::opt<bool>(
"disable-region-based-isolation-with-strict-concurrency",
llvm::cl::init(false));
};

/// Regular expression corresponding to the value given in one of the
Expand Down Expand Up @@ -701,9 +706,14 @@ int sil_opt_main(ArrayRef<const char *> argv, void *MainAddr) {

Invocation.getLangOptions().UnavailableDeclOptimizationMode =
options.UnavailableDeclOptimization;
if (options.StrictConcurrencyLevel.hasArgStr())
if (options.StrictConcurrencyLevel.hasArgStr()) {
Invocation.getLangOptions().StrictConcurrencyLevel =
options.StrictConcurrencyLevel;
if (options.StrictConcurrencyLevel == StrictConcurrency::Complete &&
!options.DisableRegionBasedIsolationWithStrictConcurrency) {
Invocation.getLangOptions().enableFeature(Feature::RegionBasedIsolation);
}
}

Invocation.getDiagnosticOptions().VerifyMode =
options.VerifyMode ? DiagnosticOptions::Verify : DiagnosticOptions::NoVerify;
Expand Down
10 changes: 10 additions & 0 deletions lib/Frontend/CompilerInvocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1060,6 +1060,16 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
if (Opts.StrictConcurrencyLevel == StrictConcurrency::Complete) {
Opts.enableFeature(Feature::IsolatedDefaultValues);
Opts.enableFeature(Feature::GlobalConcurrency);

// If asserts are enabled, allow for region based isolation to be disabled
// with a flag. This is intended only to be used with tests.
bool enableRegionIsolation = true;
#ifndef NDEBUG
enableRegionIsolation =
!Args.hasArg(OPT_disable_strict_concurrency_region_based_isolation);
#endif
if (enableRegionIsolation)
Opts.enableFeature(Feature::RegionBasedIsolation);
}

Opts.WarnImplicitOverrides =
Expand Down
6 changes: 1 addition & 5 deletions lib/SILOptimizer/Analysis/RegionAnalysis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -624,11 +624,7 @@ void SILIsolationInfo::printForDiagnostics(llvm::raw_ostream &os) const {
os << "disconnected";
return;
case Actor:
if (hasActorIsolation() && getActorIsolation()) {
getActorIsolation()->printForDiagnostics(os);
} else {
os << "actor-isolated";
}
getActorIsolation().printForDiagnostics(os);
return;
case Task:
os << "task-isolated";
Expand Down
58 changes: 6 additions & 52 deletions lib/SILOptimizer/Utils/PartitionUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,7 @@ SILIsolationInfo SILIsolationInfo::get(SILFunctionArgument *arg) {
if (auto functionIsolation = arg->getFunction()->getActorIsolation()) {
if (functionIsolation.isActorIsolated()) {
if (auto *nomDecl = self->getType().getNominalOrBoundGenericNominal()) {
if (auto isolationInfo =
SILIsolationInfo::getActorIsolated(nomDecl)) {
return isolationInfo;
}
return SILIsolationInfo::getActorIsolated(nomDecl);
}
}
}
Expand Down Expand Up @@ -135,23 +132,6 @@ void SILIsolationInfo::print(llvm::raw_ostream &os) const {
}
}

NominalTypeDecl *SILIsolationInfo::tryInferActorDecl() const {
if (hasActorIsolation()) {
auto actorIsolation = getActorIsolation();
if (auto *actor = actorIsolation->getActorOrNullPtr()) {
return actor;
}
return nullptr;
}

if (hasActorInstance()) {
auto actorDecl = getActorInstance();
return actorDecl;
}

return nullptr;
}

SILIsolationInfo SILIsolationInfo::merge(SILIsolationInfo other) const {
// If we are greater than the other kind, then we are further along the
// lattice. We ignore the change.
Expand Down Expand Up @@ -179,21 +159,9 @@ bool SILIsolationInfo::operator==(const SILIsolationInfo &other) const {
return getTaskIsolatedValue() == other.getTaskIsolatedValue();
case Actor:
// First try to use actor isolation if we have them.
if (hasActorIsolation() && other.hasActorIsolation()) {
auto lhsIsolation = getActorIsolation();
auto rhsIsolation = other.getActorIsolation();
if (lhsIsolation && rhsIsolation)
return *lhsIsolation == *rhsIsolation;
}

// Otherwise, try to use the inferred actor decl.
auto *lhsDecl = tryInferActorDecl();
auto *rhsDecl = other.tryInferActorDecl();
if (lhsDecl && rhsDecl)
return lhsDecl == rhsDecl;

// Otherwise, false, they are not equal.
return false;
auto lhsIsolation = getActorIsolation();
auto rhsIsolation = other.getActorIsolation();
return lhsIsolation == rhsIsolation;
}
}

Expand All @@ -207,22 +175,8 @@ void SILIsolationInfo::Profile(llvm::FoldingSetNodeID &id) const {
id.AddPointer(getTaskIsolatedValue());
return;
case Actor:
// We profile in integer cases here so that we can always distinguish in
// between the various cases and the non-case. Just being paranoid.
if (hasActorIsolation()) {
if (auto isolation = getActorIsolation()) {
id.AddInteger(1);
return isolation->Profile(id);
}
}

if (hasActorInstance()) {
id.AddInteger(2);
return id.AddPointer(getActorInstance());
}

id.AddInteger(3);
break;
getActorIsolation().Profile(id);
return;
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// RUN: %target-swift-frontend -disable-availability-checking -debugger-support %s -emit-sil -o /dev/null -verify
// RUN: %target-swift-frontend -disable-availability-checking -debugger-support %s -emit-sil -o /dev/null -verify -strict-concurrency=targeted
// RUN: %target-swift-frontend -disable-availability-checking -debugger-support %s -emit-sil -o /dev/null -verify -strict-concurrency=complete -disable-region-based-isolation-with-strict-concurrency
// RUN: %target-swift-frontend -disable-availability-checking -debugger-support %s -emit-sil -o /dev/null -verify -strict-concurrency=complete
// RUN: %target-swift-frontend -disable-availability-checking -debugger-support %s -emit-sil -o /dev/null -verify -strict-concurrency=complete -enable-upcoming-feature RegionBasedIsolation

// REQUIRES: concurrency
// REQUIRES: asserts
Expand Down
4 changes: 2 additions & 2 deletions test/Concurrency/actor_call_implicitly_async.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// RUN: %target-swift-frontend -disable-availability-checking -strict-concurrency=complete -parse-as-library %s -emit-sil -o /dev/null -verify -verify-additional-prefix complete-
// RUN: %target-swift-frontend -disable-availability-checking -strict-concurrency=complete -parse-as-library %s -emit-sil -o /dev/null -verify -enable-upcoming-feature RegionBasedIsolation
// RUN: %target-swift-frontend -disable-availability-checking -strict-concurrency=complete -parse-as-library %s -emit-sil -o /dev/null -verify -verify-additional-prefix complete- -disable-region-based-isolation-with-strict-concurrency
// RUN: %target-swift-frontend -disable-availability-checking -strict-concurrency=complete -parse-as-library %s -emit-sil -o /dev/null -verify

// REQUIRES: concurrency
// REQUIRES: asserts
Expand Down
8 changes: 4 additions & 4 deletions test/Concurrency/actor_inout_isolation.swift
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify -verify-additional-prefix minimal-targeted-
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify -verify-additional-prefix targeted-complete-tns- -verify-additional-prefix targeted-complete- -verify-additional-prefix minimal-targeted- -strict-concurrency=targeted
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify -verify-additional-prefix targeted-complete-tns- -verify-additional-prefix targeted-complete- -verify-additional-prefix complete-tns- -verify-additional-prefix complete- -strict-concurrency=complete
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify -verify-additional-prefix targeted-complete-tns- -verify-additional-prefix complete-tns- -strict-concurrency=complete -enable-upcoming-feature RegionBasedIsolation
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify -verify-additional-prefix minimal-targeted- -disable-region-based-isolation-with-strict-concurrency
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify -verify-additional-prefix targeted-complete-tns- -verify-additional-prefix targeted-complete- -verify-additional-prefix minimal-targeted- -strict-concurrency=targeted -disable-region-based-isolation-with-strict-concurrency
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify -verify-additional-prefix targeted-complete-tns- -verify-additional-prefix targeted-complete- -verify-additional-prefix complete-tns- -verify-additional-prefix complete- -strict-concurrency=complete -disable-region-based-isolation-with-strict-concurrency
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify -verify-additional-prefix targeted-complete-tns- -verify-additional-prefix complete-tns- -strict-concurrency=complete

// REQUIRES: concurrency
// REQUIRES: asserts
Expand Down
1 change: 0 additions & 1 deletion test/Concurrency/actor_keypath_isolation_swift6.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// RUN: %target-swift-frontend -disable-availability-checking -strict-concurrency=complete -swift-version 6 %s -emit-sil -o /dev/null -verify
// RUN: %target-swift-frontend -disable-availability-checking -strict-concurrency=complete -swift-version 6 %s -emit-sil -o /dev/null -verify -enable-upcoming-feature RegionBasedIsolation

// REQUIRES: concurrency

Expand Down
2 changes: 1 addition & 1 deletion test/Concurrency/async_cancellation.swift
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify -strict-concurrency=targeted
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify -strict-concurrency=complete -disable-region-based-isolation-with-strict-concurrency
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify -strict-concurrency=complete
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify -strict-concurrency=complete -enable-upcoming-feature RegionBasedIsolation

// REQUIRES: concurrency
// REQUIRES: asserts
Expand Down
6 changes: 3 additions & 3 deletions test/Concurrency/async_let_isolation.swift
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify -strict-concurrency=targeted
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify -disable-region-based-isolation-with-strict-concurrency
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify -strict-concurrency=targeted -disable-region-based-isolation-with-strict-concurrency
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify -strict-concurrency=complete -disable-region-based-isolation-with-strict-concurrency
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify -strict-concurrency=complete
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify -strict-concurrency=complete -enable-upcoming-feature RegionBasedIsolation

// REQUIRES: concurrency
// REQUIRES: asserts
Expand Down
2 changes: 1 addition & 1 deletion test/Concurrency/async_sequence_syntax.swift
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify -strict-concurrency=targeted
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify -strict-concurrency=complete -disable-region-based-isolation-with-strict-concurrency
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify -strict-concurrency=complete
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify -strict-concurrency=complete -enable-upcoming-feature RegionBasedIsolation

// REQUIRES: concurrency
// REQUIRES: asserts
Expand Down
2 changes: 1 addition & 1 deletion test/Concurrency/concurrency_warnings.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// RUN: %target-swift-frontend -strict-concurrency=complete -parse-as-library %s -emit-sil -o /dev/null -verify -disable-region-based-isolation-with-strict-concurrency
// RUN: %target-swift-frontend -strict-concurrency=complete -parse-as-library %s -emit-sil -o /dev/null -verify
// RUN: %target-swift-frontend -strict-concurrency=complete -parse-as-library %s -emit-sil -o /dev/null -verify -enable-upcoming-feature RegionBasedIsolation

// REQUIRES: concurrency
// REQUIRES: asserts
Expand Down
4 changes: 2 additions & 2 deletions test/Concurrency/concurrent_value_checking.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// RUN: %target-swift-frontend -disable-availability-checking -strict-concurrency=complete -parse-as-library %s -emit-sil -o /dev/null -verify -verify-additional-prefix complete-
// RUN: %target-swift-frontend -disable-availability-checking -strict-concurrency=complete -parse-as-library %s -emit-sil -o /dev/null -verify -enable-upcoming-feature RegionBasedIsolation
// RUN: %target-swift-frontend -disable-availability-checking -strict-concurrency=complete -parse-as-library %s -emit-sil -o /dev/null -verify -verify-additional-prefix complete- -disable-region-based-isolation-with-strict-concurrency
// RUN: %target-swift-frontend -disable-availability-checking -strict-concurrency=complete -parse-as-library %s -emit-sil -o /dev/null -verify

// REQUIRES: concurrency
// REQUIRES: asserts
Expand Down
2 changes: 1 addition & 1 deletion test/Concurrency/concurrent_value_checking_objc.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// RUN: %target-swift-frontend -disable-availability-checking -strict-concurrency=complete %s -emit-sil -o /dev/null -verify -disable-region-based-isolation-with-strict-concurrency
// RUN: %target-swift-frontend -disable-availability-checking -strict-concurrency=complete %s -emit-sil -o /dev/null -verify
// RUN: %target-swift-frontend -disable-availability-checking -strict-concurrency=complete %s -emit-sil -o /dev/null -verify -enable-upcoming-feature RegionBasedIsolation

// REQUIRES: concurrency
// REQUIRES: objc_interop
Expand Down
Loading