8000 Make `InferIsolatedConformances` and `StrictMemorySafety` migratable features by DougGregor · Pull Request #81703 · swiftlang/swift · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Make InferIsolatedConformances and StrictMemorySafety migratable features #81703

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 7 commits into from
May 22, 2025
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/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -8619,6 +8619,9 @@ GROUPED_ERROR(isolated_conformance_with_sendable_simple,IsolatedConformances,
GROUPED_ERROR(isolated_conformance_wrong_domain,IsolatedConformances,none,
"%0 conformance of %1 to %2 cannot be used in %3 context",
(ActorIsolation, Type, DeclName, ActorIsolation))
GROUPED_WARNING(isolated_conformance_will_become_nonisolated,IsolatedConformances,none,
"conformance of %0 to %1 should be marked 'nonisolated' to retain its behavior with upcoming feature 'InferIsolatedConformances'",
(const ValueDecl *, const ValueDecl *))

//===----------------------------------------------------------------------===//
// MARK: @_inheritActorContext
Expand Down
10 changes: 8 additions & 2 deletions include/swift/Basic/Features.def
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,11 @@
#endif
#endif

#ifndef MIGRATABLE_OPTIONAL_LANGUAGE_FEATURE
#define MIGRATABLE_OPTIONAL_LANGUAGE_FEATURE(FeatureName, SENumber, Name) \
OPTIONAL_LANGUAGE_FEATURE(FeatureName, SENumber, #Name)
#endif

#ifndef UPCOMING_FEATURE
#define UPCOMING_FEATURE(FeatureName, SENumber, Version) \
LANGUAGE_FEATURE(FeatureName, SENumber, #FeatureName)
Expand Down Expand Up @@ -283,14 +288,14 @@ UPCOMING_FEATURE(GlobalActorIsolatedTypesUsability, 0434, 6)
MIGRATABLE_UPCOMING_FEATURE(ExistentialAny, 335, 7)
UPCOMING_FEATURE(InternalImportsByDefault, 409, 7)
UPCOMING_FEATURE(MemberImportVisibility, 444, 7)
UPCOMING_FEATURE(InferIsolatedConformances, 470, 7)
MIGRATABLE_UPCOMING_FEATURE(InferIsolatedConformances, 470, 7)
MIGRATABLE_UPCOMING_FEATURE(NonisolatedNonsendingByDefault, 461, 7)

// Optional language features / modes

/// Diagnose uses of language constructs and APIs that can violate memory
/// safety.
OPTIONAL_LANGUAGE_FEATURE(StrictMemorySafety, 458, "Strict memory safety")
MIGRATABLE_OPTIONAL_LANGUAGE_FEATURE(StrictMemorySafety, 458, "Strict memory safety")

// Experimental features

Expand Down Expand Up @@ -521,6 +526,7 @@ EXPERIMENTAL_FEATURE(ModuleSelector, false)
#undef UPCOMING_FEATURE
#undef MIGRATABLE_UPCOMING_FEATURE
#undef MIGRATABLE_EXPERIMENTAL_FEATURE
#undef MIGRATABLE_OPTIONAL_LANGUAGE_FEATURE
#undef BASELINE_LANGUAGE_FEATURE
#undef OPTIONAL_LANGUAGE_FEATURE
#undef CONDITIONALLY_SUPPRESSIBLE_EXPERIMENTAL_FEATURE
Expand Down
4 changes: 3 additions & 1 deletion include/swift/Basic/LangOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -872,7 +872,9 @@ namespace swift {
FeatureState getFeatureState(Feature feature) const;

/// Returns whether the given feature is enabled.
bool hasFeature(Feature feature) const;
///
/// If allowMigration is set, also returns true when the feature has been enabled for migration.
bool hasFeature(Feature feature, bool allowMigration = false) const;

/// Returns whether a feature with the given name is enabled. Returns
/// `false` if a feature by this name is not known.
Expand Down
4 changes: 4 additions & 0 deletions include/swift/Option/Options.td
Original file line number Diff line number Diff line change
Expand Up @@ -1018,6 +1018,10 @@ def strict_memory_safety : Flag<["-"], "strict-memory-safety">,
Flags<[FrontendOption, ModuleInterfaceOptionIgnorable,
SwiftAPIDigesterOption, SwiftSynthesizeInterfaceOption]>,
HelpText<"Enable strict memory safety checking">;
def strict_memory_safety_migrate : Flag<["-"], "strict-memory-safety:migrate">,
Flags<[FrontendOption, ModuleInterfaceOptionIgnorable,
SwiftAPIDigesterOption, SwiftSynthesizeInterfaceOption]>,
HelpText<"Enable migration to strict memory safety checking">;

def Rpass_EQ : Joined<["-"], "Rpass=">,
Flags<[FrontendOption]>,
Expand Down
3 changes: 3 additions & 0 deletions lib/Basic/Feature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ bool Feature::isMigratable() const {
switch (kind) {
#define MIGRATABLE_UPCOMING_FEATURE(FeatureName, SENumber, Version)
#define MIGRATABLE_EXPERIMENTAL_FEATURE(FeatureName, AvailableInProd)
#define MIGRATABLE_OPTIONAL_LANGUAGE_FEATURE(FeatureName, SENumber, Name)
#define LANGUAGE_FEATURE(FeatureName, SENumber, Description) \
case Feature::FeatureName:
#include "swift/Basic/Features.def"
Expand All @@ -82,6 +83,8 @@ bool Feature::isMigratable() const {
case Feature::FeatureName:
#define MIGRATABLE_EXPERIMENTAL_FEATURE(FeatureName, AvailableInProd) \
case Feature::FeatureName:
#define MIGRATABLE_OPTIONAL_LANGUAGE_FEATURE(FeatureName, SENumber, Name) \
case Feature::FeatureName:
#include "swift/Basic/Features.def"
return true;
}
Expand Down
8 changes: 6 additions & 2 deletions lib/Basic/LangOptions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -335,13 +335,17 @@ LangOptions::FeatureState LangOptions::getFeatureState(Feature feature) const {
return state;
}

bool LangOptions::hasFeature(Feature feature) const {
if (featureStates.getState(feature).isEnabled())
bool LangOptions::hasFeature(Feature feature, bool allowMigration) const {
auto state = featureStates.getState(feature);
if (state.isEnabled())
return true;

if (auto version = feature.getLanguageVersion())
return isSwiftVersionAtLeast(*version);

if (allowMigration && state.isEnabledForMigration())
return true;

return false;
}

Expand Down
66 changes: 66 additions & 0 deletions lib/Basic/SupportedFeatures.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include <array>
#include <vector>

#include "swift/AST/DiagnosticGroups.h"
#include "swift/Basic/Feature.h"
#include "swift/Frontend/Frontend.h"

Expand All @@ -22,11 +23,58 @@ using namespace swift;

namespace swift {
namespace features {

/// The subset of diagnostic groups (called categories by the diagnostic machinery) whose diagnostics should be
/// considered to be part of the migration for this feature.
///
/// When making a feature migratable, ensure that all of the warnings that are used to drive the migration are
/// part of a diagnostic group, and put that diagnostic group into the list for that feature here.
static std::vector<DiagGroupID> migratableCategories(Feature feature) {
switch (feature) {
case Feature::InnerKind::ExistentialAny:
return { DiagGroupID::ExistentialAny };
case Feature::InnerKind::InferIsolatedConformances:
return { DiagGroupID::IsolatedConformances };
case Feature::InnerKind::NonisolatedNonsendingByDefault:
return { DiagGroupID::NonisolatedNonsendingByDefault };
case Feature::InnerKind::StrictMemorySafety:
return { DiagGroupID::StrictMemorySafety };

// Provide unreachable cases for all of the non-migratable features.
#define LANGUAGE_FEATURE(FeatureName, SENumber, Description) case Feature::FeatureName:
#define MIGRATABLE_UPCOMING_FEATURE(FeatureName, SENumber, Version)
#define MIGRATABLE_EXPERIMENTAL_FEATURE(FeatureName, AvailableInProd)
#define MIGRATABLE_OPTIONAL_LANGUAGE_FEATURE(FeatureName, SENumber, Name)
#include "swift/Basic/Features.def"
llvm_unreachable("Not a migratable feature");
}
}

/// For optional language features, return the flag name used by the compiler to enable the feature. For all others,
/// returns an empty optional.
static std::optional<std::string_view> optionalFlagName(Feature feature) {
switch (feature) {
case Feature::StrictMemorySafety:
return "-strict-memory-safety";

#define LANGUAGE_FEATURE(FeatureName, SENumber, Description) case Feature::FeatureName:
#define OPTIONAL_LANGUAGE_FEATURE(FeatureName, SENumber, Description)
#include "swift/Basic/Features.def"
return std::nullopt;
}
}

/// Print information about what features upcoming/experimental are
/// supported by the compiler.
/// The information includes whether a feature is adoptable and for
/// upcoming features - what is the first mode it's introduced.
void printSupportedFeatures(llvm::raw_ostream &out) {
std::array optional{
#define LANGUAGE_FEATURE(FeatureName, SENumber, Description)
#define OPTIONAL_LANGUAGE_FEATURE(FeatureName, SENumber, Description) Feature::FeatureName,
#include "swift/Basic/Features.def"
};

std::array upcoming{
#define LANGUAGE_FEATURE(FeatureName, SENumber, Description)
#define UPCOMING_FEATURE(FeatureName, SENumber, Version) Feature::FeatureName,
Expand All @@ -50,14 +98,32 @@ void printSupportedFeatures(llvm::raw_ostream &out) {
out << "{ \"name\": \"" << feature.getName() << "\"";
if (feature.isMigratable()) {
out << ", \"migratable\": true";

auto categories = migratableCategories(feature);
out << ", \"categories\": [";
llvm::interleave(categories, [&out](DiagGroupID diagGroupID) {
out << "\"" << getDiagGroupInfoByID(diagGroupID).name << "\"";
}, [&out] {
out << ", ";
});
out << "]";
}
if (auto version = feature.getLanguageVersion()) {
out << ", \"enabled_in\": \"" << *version << "\"";
}

if (auto flagName = optionalFlagName(feature)) {
out << ", \"flag_name\": \"" << *flagName << "\"";
}

out << " }";
};

out << " \"features\": {\n";
out << " \"optional\": [\n";
llvm::interleave(optional, printFeature, [&out] { out << ",\n"; });
out << "\n ],\n";

out << " \"upcoming\": [\n";
llvm::interleave(upcoming, printFeature, [&out] { out << ",\n"; });
out << "\n ],\n";
Expand Down
3 changes: 2 additions & 1 deletion lib/Driver/ToolChains.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,8 @@ void ToolChain::addCommonFrontendArgs(const OutputInfo &OI,
options::OPT_disable_experimental_feature,
options::OPT_enable_upcoming_feature,
options::OPT_disable_upcoming_feature});
inputArgs.AddLastArg(arguments, options::OPT_strict_memory_safety);
inputArgs.AddLastArg(arguments, options::OPT_strict_memory_safety,
options::OPT_strict_memory_safety_migrate);
inputArgs.AddLastArg(arguments, options::OPT_warn_implicit_overrides);
inputArgs.AddLastArg(arguments, options::OPT_typo_correction_limit);
inputArgs.AddLastArg(arguments, options::OPT_enable_app_extension);
Expand Down
2 changes: 2 additions & 0 deletions lib/Frontend/CompilerInvocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -996,6 +996,8 @@ static bool ParseEnabledFeatureArgs(LangOptions &Opts, ArgList &Args,

if (Args.hasArg(OPT_strict_memory_safety))
Opts.enableFeature(Feature::StrictMemorySafety);
else if (Args.hasArg(OPT_strict_memory_safety_migrate))
Opts.enableFeature(Feature::StrictMemorySafety, /*forMigration=*/true);

return HadError;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ deriveBodyDistributed_doInvokeOnReturn(AbstractFunctionDecl *afd, void *arg) {
new (C) DeclRefExpr(ConcreteDeclRef(returnTypeParam),
dloc, implicit))}));

if (C.LangOpts.hasFeature(Feature::StrictMemorySafety))
if (C.LangOpts.hasFeature(Feature::StrictMemorySafety, /*allowMigration=*/true))
resultLoadCall = new (C) UnsafeExpr(sloc, resultLoadCall, Type(), true);

auto resultPattern = NamedPattern::createImplicit(C, resultVar);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ deriveBodyRawRepresentable_raw(AbstractFunctionDecl *toRawDecl, void *) {
auto *argList = ArgumentList::forImplicitCallTo(functionRef->getName(),
{selfRef, typeExpr}, C);
Expr *call = CallExpr::createImplicit(C, functionRef, argList);
if (C.LangOpts.hasFeature(Feature::StrictMemorySafety))
if (C.LangOpts.hasFeature(Feature::StrictMemorySafety, /*allowMigration=*/true))
call = UnsafeExpr::createImplicit(C, SourceLoc(), call);
auto *returnStmt = ReturnStmt::createImplicit(C, call);
auto body = BraceStmt::create(C, SourceLoc(), ASTNode(returnStmt),
Expand Down
2 changes: 1 addition & 1 deletion lib/Sema/TypeCheckDeclOverride.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2257,7 +2257,7 @@ static bool checkSingleOverride(ValueDecl *override, ValueDecl *base) {
diagnoseOverrideForAvailability(override, base);
}

if (ctx.LangOpts.hasFeature(Feature::StrictMemorySafety)) {
if (ctx.LangOpts.hasFeature(Feature::StrictMemorySafety, /*allowMigration=*/true)) {
// If the override is unsafe but the base declaration is not, then the
// inheritance itself is unsafe.
auto subs = SubstitutionMap::getOverrideSubstitutions(base, override);
Expand Down
3 changes: 2 additions & 1 deletion lib/Sema/TypeCheckDeclPrimary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2413,7 +2413,8 @@ class DeclChecker : public DeclVisitor<DeclChecker> {

// If strict memory safety checking is enabled, check the storage
// of the nominal type.
if (Ctx.LangOpts.hasFeature(Feature::StrictMemorySafety) &&
if (Ctx.LangOpts.hasFeature(
Feature::StrictMemorySafety, /*allowMigration=*/true) &&
!isa<ProtocolDecl>(nominal)) {
checkUnsafeStorage(nominal);
}
Expand Down
5 changes: 3 additions & 2 deletions lib/Sema/TypeCheckEffects.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4554,7 +4554,8 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>
if (classification.hasUnsafe()) {
// If there is no such effect, complain.
if (S->getUnsafeLoc().isInvalid() &&
Ctx.LangOpts.hasFeature(Feature::StrictMemorySafety)) {
Ctx.LangOpts.hasFeature(Feature::StrictMemorySafety,
/*allowMigration=*/true)) {
auto insertionLoc = S->getPattern()->getStartLoc();
Ctx.Diags.diagnose(S->getForLoc(), diag::for_unsafe_without_unsafe)
.fixItInsert(insertionLoc, "unsafe ");
Expand Down Expand Up @@ -4801,7 +4802,7 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>

void diagnoseUncoveredUnsafeSite(
const Expr *anchor, ArrayRef<UnsafeUse> unsafeUses) {
if (!Ctx.LangOpts.hasFeature(Feature::StrictMemorySafety))
if (!Ctx.LangOpts.hasFeature(Feature::StrictMemorySafety, /*allowMigration=*/true))
return;

const auto &[loc, insertText] = getFixItForUncoveredSite(anchor, "unsafe");
Expand Down
46 changes: 45 additions & 1 deletion lib/Sema/TypeCheckProtocol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2646,7 +2646,8 @@ checkIndividualConformance(NormalProtocolConformance *conformance) {
// If we're enforcing strict memory safety and this conformance hasn't
// opted out, look for safe/unsafe witness mismatches.
if (conformance->getExplicitSafety() == ExplicitSafety::Unspecified &&
Context.LangOpts.hasFeature(Feature::StrictMemorySafety)) {
Context.LangOpts.hasFeature(Feature::StrictMemorySafety,
/*allowMigration=*/true)) {
// Collect all of the unsafe uses for this conformance.
SmallVector<UnsafeUse, 2> unsafeUses;
for (auto requirement: Proto->getMembers()) {
Expand Down Expand Up @@ -6675,6 +6676,49 @@ void TypeChecker::checkConformancesInContext(IterableDeclContext *idc) {
}
}

// If we are migrating to InferIsolatedConformances, and the
// nominal type is global-actor-isolated, look for conformances
// that are nonisolated but were not explicitly marked as such.
// These conformances will need to be marked 'nonisolated' to
// retain their current behavior.
if (Context.LangOpts
.getFeatureState(Feature::InferIsolatedConformances)
.isEnabledForMigration() &&
getActorIsolation(const_cast<NominalTypeDecl *>(nominal))
.isGlobalActor()) {
for (auto conformance : conformances) {
auto normal = dyn_cast<NormalProtocolConformance>(conformance);
if (!normal)
continue;

// Explicit nonisolated and @preconcurrency suppress this.
auto options = normal->getOptions();
if (options.contains(ProtocolConformanceFlags::Nonisolated) ||
options.contains(ProtocolConformanceFlags::Preconcurrency))
continue;

// Only consider conformances that were explicitly written in the source.
if (normal->getSourceKind() != ConformanceEntryKind::Explicit)
continue;

// Only consider conformances to non-marker, nonisolated protocols.
auto proto = normal->getProtocol();
if (proto->isMarkerProtocol() || getActorIsolation(proto).isActorIsolated())
continue;

// Only nonisolated conformances can be affected.
if (!conformance->getIsolation().isNonisolated())
continue;

auto nameLoc = normal->getProtocolNameLoc();
if (nameLoc.isValid()) {
Context.Diags.diagnose(
nameLoc, diag::isolated_conformance_will_become_nonisolated, nominal, proto)
.fixItInsert(nameLoc, "nonisolated ");
}
}
}

if (Context.TypeCheckerOpts.DebugGenericSignatures &&
!conformances.empty()) {
// Now that they're filled out, print out information about the conformances
Expand Down
33 changes: 33 additions & 0 deletions test/Concurrency/isolated_conformance_migrate.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// RUN: %target-swift-frontend -typecheck -verify -target %target-swift-5.1-abi-triple -swift-version 6 -enable-upcoming-feature InferIsolatedConformances:migrate %s

// REQUIRES: concurrency
// REQUIRES: swift_feature_InferIsolatedConformances

protocol P { }
protocol Q: P { }

struct S: P { }

struct S2: Q { }

@MainActor
struct MA1: P { }
// expected-warning@-1{{conformance of 'MA1' to 'P' should be marked 'nonisolated' to retain its behavior with upcoming feature 'InferIsolatedConformances'}}{{13-13=nonisolated }}

@MainActor
struct MA2: Q { }
// expected-warning@-1{{conformance of 'MA2' to 'Q' should be marked 'nonisolated' to retain its behavior with upcoming feature 'InferIsolatedConformances'}}{{13-13=nonisolated }}

@MainActor
struct MA3 { }

extension MA3: P, Q { }
// expected-warning@-1{{conformance of 'MA3' to 'P' should be marked 'nonisolated' to retain its behavior with upcoming feature 'InferIsolatedConformances'}}{{16-16=nonisolated }}
// expected-warning@-2{{conformance of 'MA3' to 'Q' should be marked 'nonisolated' to retain its behavior with upcoming feature 'InferIsolatedConformances'}}{{19-19=nonisolated }}

@MainActor
struct MA4: @MainActor P { }

@MainActor
struct MA5: nonisolated P { }

7 changes: 5 additions & 2 deletions test/Frontend/print-supported-features.swift
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
// RUN: %target-swift-frontend -print-supported-features | %FileCheck %s

// CHECK: "features": {
// CHECK-NEXT: "optional": [
// CHECK: { "name": "StrictMemorySafety", "migratable": true, "categories": ["StrictMemorySafety"], "flag_name": "-strict-memory-safety" }
// CHECK-NEXT: ],
// CHECK-NEXT: "upcoming": [
// CHECK: { "name": "{{.*}}"{{, "migratable": true}}, "enabled_in": "{{.*}}" }
// CHECK: { "name": "InferIsolatedConformances", "migratable": true, "categories": ["IsolatedConformances"], "enabled_in": "7" },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We really need to come up with a test that checks the structure here too...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we have Foundation around, we could mock up some Codable structs/enums and make sure it deserializes

// CHECK: ],
// CHECK-NEXT: "experimental": [
// CHECK: "experimental": [
// CHECK: { "name": "{{.*}}" }
// CHECK: ]
// CHECK: }
Loading
0