Skip to content

[6.2] Make InferIsolatedConformances and StrictMemorySafety migratable features #81744

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/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -8545,6 +8545,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 @@ -522,6 +527,7 @@ EXPERIMENTAL_FEATURE(CopyBlockOptimization, true)
#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
41 changes: 39 additions & 2 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,6 +23,33 @@ 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");
}
}

/// Print information about what features upcoming/experimental are
/// supported by the compiler.
/// The information includes whether a feature is adoptable and for
Expand Down Expand Up @@ -50,9 +78,18 @@ 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;
out << ", \"enabled_in\": \"" << *version << "\"";
}
out << " }";
};
Expand All @@ -71,4 +108,4 @@ void printSupportedFeatures(llvm::raw_ostream &out) {
}

} // end namespace features
} // end namespace swift
} // end namespace swift
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 @@ -965,6 +965,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
2 changes: 1 addition & 1 deletion lib/Sema/CSApply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8172,7 +8172,7 @@ Expr *ExprRewriter::convertLiteralInPlace(
Diag<> brokenProtocolDiag, Diag<> brokenBuiltinProtocolDiag) {
// If coercing a literal to an unresolved type, we don't try to look up the
// witness members, just do it.
if (type->is<UnresolvedType>()) {
if (type->is<UnresolvedType>() || type->is<ErrorType>()) {
cs.setType(literal, type);
return literal;
}
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 @@ -2262,7 +2262,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 @@ -2430,7 +2430,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
18 changes: 6 additions & 12 deletions lib/Sema/TypeCheckEffects.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1187,8 +1187,7 @@ class Classification {
Classification result;
bool considerAsync = !onlyEffect || *onlyEffect == EffectKind::Async;
bool considerThrows = !onlyEffect || *onlyEffect == EffectKind::Throws;
bool considerUnsafe = (!onlyEffect || *onlyEffect == EffectKind::Unsafe) &&
ctx.LangOpts.hasFeature(Feature::StrictMemorySafety);
bool considerUnsafe = (!onlyEffect || *onlyEffect == EffectKind::Unsafe);

// If we're tracking "unsafe" effects, compute them here.
if (considerUnsafe) {
Expand Down Expand Up @@ -1710,8 +1709,7 @@ class ApplyClassifier {
!fnType->isAsync() &&
!E->isImplicitlyAsync() &&
!hasAnyConformances &&
(fnRef.getExplicitSafety() == ExplicitSafety::Safe ||
!ctx.LangOpts.hasFeature(Feature::StrictMemorySafety))) {
fnRef.getExplicitSafety() == ExplicitSafety::Safe) {
return Classification();
}

Expand Down Expand Up @@ -1932,7 +1930,6 @@ class ApplyClassifier {
// If the safety of the callee is unspecified, check the safety of the
// arguments specifically.
if (hasUnspecifiedSafety &&
ctx.LangOpts.hasFeature(Feature::StrictMemorySafety) &&
!(assumedSafeArguments && assumedSafeArguments->contains(E))) {
classifyApplyEffect(EffectKind::Unsafe);
}
Expand Down Expand Up @@ -4553,13 +4550,13 @@ 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 ");
}
} else if (S->getUnsafeLoc().isValid() &&
Ctx.LangOpts.hasFeature(Feature::StrictMemorySafety)) {
} else if (S->getUnsafeLoc().isValid()) {
// Extraneous "unsafe" on the sequence.
Ctx.Diags.diagnose(S->getUnsafeLoc(), diag::no_unsafe_in_unsafe_for)
.fixItRemove(S->getUnsafeLoc());
Expand Down Expand Up @@ -4604,9 +4601,6 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>
}

void diagnoseRedundantUnsafe(UnsafeExpr *E) const {
if (!Ctx.LangOpts.hasFeature(Feature::StrictMemorySafety))
return;

// Silence this warning in the expansion of the _SwiftifyImport macro.
// This is a hack because it's tricky to determine when to insert "unsafe".
unsigned bufferID =
Expand Down Expand Up @@ -4880,7 +4874,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 @@ -2650,7 +2650,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 @@ -6668,6 +6669,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
7 changes: 0 additions & 7 deletions lib/Sema/TypeCheckUnsafe.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -341,10 +341,6 @@ bool swift::enumerateUnsafeUses(ArrayRef<ProtocolConformanceRef> conformances,
if (conformance.isInvalid())
continue;

ASTContext &ctx = conformance.getProtocol()->getASTContext();
if (!ctx.LangOpts.hasFeature(Feature::StrictMemorySafety))
return false;

if (!conformance.hasEffect(EffectKind::Unsafe))
continue;

Expand Down Expand Up @@ -413,9 +409,6 @@ bool swift::isUnsafeInConformance(const ValueDecl *requirement,

void swift::diagnoseUnsafeType(ASTContext &ctx, SourceLoc loc, Type type,
llvm::function_ref<void(Type)> diagnose) {
if (!ctx.LangOpts.hasFeature(Feature::StrictMemorySafety))
return;

if (!type->isUnsafe())
return;

Expand Down
Loading