Sink concurrency constraint fix behavior logic into "attempt" functions.

DougGregor · DougGregor · commit db14ab140f61 · 2022-07-08T13:33:14.000-07:00
This is a cleaner pattern for the solver, thanks Pavel!
diff --git a/include/swift/Sema/CSFix.h b/include/swift/Sema/CSFix.h
@@ -799,6 +799,16 @@ class MarkGlobalActorFunction final : public ContextualMismatch {
                                          Type rhs, ConstraintLocator *locator,
                                          FixBehavior fixBehavior);
 
+  /// Try to apply this fix to the given types.
+  ///
+  /// \returns \c true if the fix cannot be applied and the solver must fail,
+  /// or \c false if the fix has been applied and the solver can continue.
+  static bool attempt(ConstraintSystem &cs,
+                      ConstraintKind constraintKind,
+                      FunctionType *fromType,
+                      FunctionType *toType,
+                      ConstraintLocatorBuilder locator);
+
   static bool classof(ConstraintFix *fix) {
     return fix->getKind() == FixKind::MarkGlobalActorFunction;
   }
@@ -850,6 +860,16 @@ class AddSendableAttribute final : public ContextualMismatch {
                                       ConstraintLocator *locator,
                                       FixBehavior fixBehavior);
 
+  /// Try to apply this fix to the given types.
+  ///
+  /// \returns \c true if the fix cannot be applied and the solver must fail,
+  /// or \c false if the fix has been applied and the solver can continue.
+  static bool attempt(ConstraintSystem &cs,
+                      ConstraintKind constraintKind,
+                      FunctionType *fromType,
+                      FunctionType *toType,
+                      ConstraintLocatorBuilder locator);
+
   static bool classof(ConstraintFix *fix) {
     return fix->getKind() == FixKind::AddSendableAttribute;
   }
diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h
@@ -3820,6 +3820,10 @@ class ConstraintSystem {
         });
   }
 
+  /// Determine whether the callee for the given locator is marked as
+  /// `@preconcurrency`.
+  bool hasPreconcurrencyCallee(ConstraintLocatorBuilder locator);
+
   /// Determine whether the given declaration is unavailable from the
   /// current context.
   bool isDeclUnavailable(const Decl *D,
diff --git a/lib/Sema/CSFix.cpp b/lib/Sema/CSFix.cpp
@@ -17,6 +17,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "CSDiagnostics.h"
+#include "TypeCheckConcurrency.h"
 #include "swift/AST/Expr.h"
 #include "swift/AST/ParameterList.h"
 #include "swift/AST/Type.h"
@@ -247,6 +248,38 @@ bool MarkGlobalActorFunction::diagnose(const Solution &solution,
   return failure.diagnose(asNote);
 }
 
+/// The fix behavior to apply to a concurrency-related diagnostic.
+static Optional<FixBehavior>
+getConcurrencyFixBehavior(
+    ConstraintSystem &cs, ConstraintKind constraintKind,
+    ConstraintLocatorBuilder locator, bool forSendable) {
+  // We can only handle the downgrade for conversions.
+  switch (constraintKind) {
+  case ConstraintKind::Conversion:
+  case ConstraintKind::ArgumentConversion:
+    break;
+
+  default:
+    if (!cs.shouldAttemptFixes())
+      return None;
+
+    return FixBehavior::Error;
+  }
+
+  // For a @preconcurrency callee outside of a strict concurrency
+  // context, ignore.
+  if (cs.hasPreconcurrencyCallee(locator) &&
+      !contextRequiresStrictConcurrencyChecking(
+          cs.DC, GetClosureType{cs}, ClosureIsolatedByPreconcurrency{cs}))
+    return FixBehavior::Suppress;
+
+  // Otherwise, warn until Swift 6.
+  if (!cs.getASTContext().LangOpts.isSwiftVersionAtLeast(6))
+    return FixBehavior::DowngradeToWarning;
+
+  return FixBehavior::Error;
+}
+
 MarkGlobalActorFunction *
 MarkGlobalActorFunction::create(ConstraintSystem &cs, Type lhs, Type rhs,
                                 ConstraintLocator *locator,
@@ -259,6 +292,23 @@ MarkGlobalActorFunction::create(ConstraintSystem &cs, Type lhs, Type rhs,
       cs, lhs, rhs, locator, fixBehavior);
 }
 
+bool MarkGlobalActorFunction::attempt(ConstraintSystem &cs,
+                                      ConstraintKind constraintKind,
+                                      FunctionType *fromType,
+                                      FunctionType *toType,
+                                      ConstraintLocatorBuilder locator) {
+  auto fixBehavior = getConcurrencyFixBehavior(
+      cs, constraintKind, locator, /*forSendable=*/false);
+  if (!fixBehavior)
+    return true;
+
+  auto *fix = MarkGlobalActorFunction::create(
+      cs, fromType, toType, cs.getConstraintLocator(locator),
+      *fixBehavior);
+
+  return cs.recordFix(fix);
+}
+
 bool AddSendableAttribute::diagnose(const Solution &solution,
                                       bool asNote) const {
   AttributedFuncToTypeConversionFailure failure(
@@ -280,6 +330,22 @@ AddSendableAttribute::create(ConstraintSystem &cs,
   return new (cs.getAllocator()) AddSendableAttribute(
       cs, fromType, toType, locator, fixBehavior);
 }
+
+bool AddSendableAttribute::attempt(ConstraintSystem &cs,
+                                   ConstraintKind constraintKind,
+                                   FunctionType *fromType,
+                                   FunctionType *toType,
+                                   ConstraintLocatorBuilder locator) {
+  auto fixBehavior = getConcurrencyFixBehavior(
+      cs, constraintKind, locator, /*forSendable=*/true);
+  if (!fixBehavior)
+    return true;
+
+  auto *fix = AddSendableAttribute::create(
+      cs, fromType, toType, cs.getConstraintLocator(locator), *fixBehavior);
+  return cs.recordFix(fix);
+}
+
 bool RelabelArguments::diagnose(const Solution &solution, bool asNote) const {
   LabelingFailure failure(solution, getLocator(), getLabels());
   return failure.diagnose(asNote);
diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp
@@ -2655,14 +2655,10 @@ static bool fixExtraneousArguments(ConstraintSystem &cs,
       /*impact=*/numExtraneous * 2);
 }
 
-bool hasPreconcurrencyCallee(ConstraintSystem *cs,
-                             ConstraintLocatorBuilder locator) {
-  if (cs->getASTContext().isSwiftVersionAtLeast(6))
-    // Swift 6 mode does not reduce errors to warnings.
-    return false;
-
-  auto calleeLocator = cs->getCalleeLocator(cs->getConstraintLocator(locator));
-  auto calleeOverload = cs->findSelectedOverloadFor(calleeLocator);
+bool ConstraintSystem::hasPreconcurrencyCallee(
+    ConstraintLocatorBuilder locator) {
+  auto calleeLocator = getCalleeLocator(getConstraintLocator(locator));
+  auto calleeOverload = findSelectedOverloadFor(calleeLocator);
   if (!calleeOverload || !calleeOverload->choice.isDecl())
     return false;
 
@@ -2716,48 +2712,12 @@ ConstraintSystem::matchFunctionTypes(FunctionType *func1, FunctionType *func2,
       increaseScore(SK_SyncInAsync);
   }
 
-  /// The behavior limit to apply to a concurrency check.
-  auto getConcurrencyFixBehavior = [&](
-        bool forSendable
-      ) -> Optional<FixBehavior> {
-        // We can only handle the downgrade for conversions.
-        switch (kind) {
-        case ConstraintKind::Conversion:
-        case ConstraintKind::ArgumentConversion:
-          break;
-
-        default:
-          if (!shouldAttemptFixes())
-            return None;
-
-          return FixBehavior::Error;
-        }
-
-        // For a @preconcurrency callee outside of a strict concurrency
-        // context, ignore.
-        if (hasPreconcurrencyCallee(this, locator) &&
-            !contextRequiresStrictConcurrencyChecking(DC, GetClosureType{*this}, ClosureIsolatedByPreconcurrency{*this}))
-          return FixBehavior::Suppress;
-
-        // Otherwise, warn until Swift 6.
-        if (!getASTContext().LangOpts.isSwiftVersionAtLeast(6))
-          return FixBehavior::DowngradeToWarning;
-
-        return FixBehavior::Error;
-  };
-
   // A @Sendable function can be a subtype of a non-@Sendable function.
   if (func1->isSendable() != func2->isSendable()) {
     // Cannot add '@Sendable'.
     if (func2->isSendable() || kind < ConstraintKind::Subtype) {
-      if (auto fixBehavior = getConcurrencyFixBehavior(true)) {
-        auto *fix = AddSendableAttribute::create(
-            *this, func1, func2, getConstraintLocator(locator), *fixBehavior);
-        if (recordFix(fix))
-          return getTypeMatchFailure(locator);
-      } else {
+      if (AddSendableAttribute::attempt(*this, kind, func1, func2, locator))
         return getTypeMatchFailure(locator);
-      }
     }
   }
 
@@ -2785,16 +2745,8 @@ ConstraintSystem::matchFunctionTypes(FunctionType *func1, FunctionType *func2,
         return getTypeMatchFailure(locator);
     } else if (func1->getGlobalActor() && !func2->isAsync()) {
       // Cannot remove a global actor from a synchronous function.
-      if (auto fixBehavior = getConcurrencyFixBehavior(false)) {
-        auto *fix = MarkGlobalActorFunction::create(
-            *this, func1, func2, getConstraintLocator(locator),
-            *fixBehavior);
-
-        if (recordFix(fix))
-          return getTypeMatchFailure(locator);
-      } else {
+      if (MarkGlobalActorFunction::attempt(*this, kind, func1, func2, locator))
         return getTypeMatchFailure(locator);
-      }
     } else if (kind < ConstraintKind::Subtype) {
       return getTypeMatchFailure(locator);
     }
@@ -9839,7 +9791,7 @@ bool ConstraintSystem::resolveClosure(TypeVariableType *typeVar,
   auto *inferredClosureType = getClosureType(closure);
 
   // Note if this closure is isolated by preconcurrency.
-  if (hasPreconcurrencyCallee(this, locator))
+  if (hasPreconcurrencyCallee(locator))
     preconcurrencyClosures.insert(closure);
 
   // Let's look through all optionals associated with contextual
