Merge pull request #81703 from DougGregor/more-migratable-features

Make `InferIsolatedConformances` and `StrictMemorySafety` migratable features

Author: DougGregor

include/swift/AST/DiagnosticsSema.def

@@ -8617,6 +8617,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

include/swift/Basic/Features.def

@@ -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)
@@ -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
 
@@ -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

include/swift/Basic/LangOptions.h

@@ -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.

include/swift/Option/Options.td

@@ -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]>,

lib/Basic/Feature.cpp

@@ -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"
@@ -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;
   }

lib/Basic/LangOptions.cpp

@@ -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;
 }
 

lib/Basic/SupportedFeatures.cpp

@@ -13,6 +13,7 @@
 #include <array>
 #include <vector>
 
+#include "swift/AST/DiagnosticGroups.h"
 #include "swift/Basic/Feature.h"
 #include "swift/Frontend/Frontend.h"
 
@@ -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,
@@ -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";

lib/Driver/ToolChains.cpp

@@ -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);

lib/Frontend/CompilerInvocation.cpp

@@ -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;
 }

lib/Sema/DerivedConformance/DerivedConformanceDistributedActor.cpp

@@ -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);

lib/Sema/DerivedConformance/DerivedConformanceRawRepresentable.cpp

@@ -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),

lib/Sema/TypeCheckDeclOverride.cpp

@@ -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);

lib/Sema/TypeCheckDeclPrimary.cpp

@@ -2427,7 +2427,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);
         }

lib/Sema/TypeCheckEffects.cpp

@@ -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 ");
@@ -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");

lib/Sema/TypeCheckProtocol.cpp

@@ -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()) {
@@ -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

test/Concurrency/isolated_conformance_migrate.swift

@@ -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 { }
+

test/Frontend/print-supported-features.swift

@@ -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" },
 // CHECK:   ],
-// CHECK-NEXT:   "experimental": [
+// CHECK:   "experimental": [
 // CHECK:     { "name": "{{.*}}" }
 // CHECK:   ]
 // CHECK: }