Merge pull request #13197 from rudkx/rework-iuo

Some small constraint system cleanups.

Author: rudkx

lib/Sema/CSApply.cpp

@@ -7514,9 +7514,6 @@ bool ConstraintSystem::applySolutionFix(Expr *expr,
   }
 
   switch (fix.first.getKind()) {
-  case FixKind::None:
-    llvm_unreachable("no-fix marker should never make it into solution");
-
   case FixKind::ForceOptional: {
     const Expr *unwrapped = affected->getValueProvidingExpr();
     auto type = solution.simplifyType(getType(affected))

lib/Sema/CSGen.cpp

@@ -916,18 +916,6 @@ namespace {
         }
       }
 
-      // If the paren expr has a favored type, and the subExpr doesn't,
-      // propagate downwards. Otherwise, propagate upwards.
-      if (auto parenExpr = dyn_cast<ParenExpr>(expr)) {
-        if (!CS.getFavoredType(parenExpr->getSubExpr())) {
-          CS.setFavoredType(parenExpr->getSubExpr(),
-                            CS.getFavoredType(parenExpr));
-        } else if (!CS.getFavoredType(parenExpr)) {
-          CS.setFavoredType(parenExpr,
-                            CS.getFavoredType(parenExpr->getSubExpr()));
-        }
-      }
-      
       return { true, expr };
     }
 
@@ -1619,10 +1607,6 @@ namespace {
     }
 
     virtual Type visitParenExpr(ParenExpr *expr) {
-      if (auto favoredTy = CS.getFavoredType(expr->getSubExpr())) {
-        CS.setFavoredType(expr, favoredTy);
-      }
-
       auto &ctx = CS.getASTContext();
       auto parenType = CS.getType(expr->getSubExpr())->getInOutObjectType();
       auto parenFlags = ParameterTypeFlags().withInOut(expr->isSemanticallyInOutExpr());

lib/Sema/CSSimplify.cpp

@@ -2395,13 +2395,6 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
         continue;
       }
 
-      // If the first thing we found is a fix, add a "don't fix" marker.
-      if (conversionsOrFixes.empty()) {
-        constraints.push_back(
-          Constraint::createFixed(*this, constraintKind, FixKind::None,
-                                  type1, type2, fixedLocator));
-      }
-
       auto fix = *potential.getFix();
       constraints.push_back(
         Constraint::createFixed(*this, constraintKind, fix, type1, type2,
@@ -4193,7 +4186,21 @@ ConstraintSystem::simplifyApplicableFnConstraint(
   ConstraintLocatorBuilder outerLocator =
     getConstraintLocator(anchor, parts, locator.getSummaryFlags());
 
-retry:
+  unsigned unwrapCount = 0;
+  if (shouldAttemptFixes()) {
+    // If we have an optional type, try forcing it to see if that
+    // helps. Note that we only deal with function and metatype types
+    // below, so there is no reason not to attempt to strip these off
+    // immediately.
+    while (auto objectType2 = desugar2->getOptionalObjectType()) {
+      type2 = objectType2;
+      desugar2 = type2->getDesugaredType();
+
+      // Track how many times we do this so that we can record a fix for each.
+      ++unwrapCount;
+    }
+  }
+
   // For a function, bind the output and convert the argument to the input.
   auto func1 = type1->castTo<FunctionType>();
   if (auto func2 = dyn_cast<FunctionType>(desugar2)) {
@@ -4221,6 +4228,11 @@ ConstraintSystem::simplifyApplicableFnConstraint(
           == SolutionKind::Error)
       return SolutionKind::Error;
 
+    // Record any fixes we attempted to get to the correct solution.
+    while (unwrapCount-- > 0)
+      if (recordFix(FixKind::ForceOptional, getConstraintLocator(locator)))
+        return SolutionKind::Error;
+
     return SolutionKind::Solved;
   }
 
@@ -4231,23 +4243,18 @@ ConstraintSystem::simplifyApplicableFnConstraint(
       return formUnsolved();
 
     // Construct the instance from the input arguments.
-    return simplifyConstructionConstraint(instance2, func1, subflags,
+    auto simplified = simplifyConstructionConstraint(instance2, func1, subflags,
                                           /*FIXME?*/ DC,
                                           FunctionRefKind::SingleApply,
                                           getConstraintLocator(outerLocator));
-  }
-
-  if (!shouldAttemptFixes())
-    return SolutionKind::Error;
 
-  // If we're coming from an optional type, unwrap the optional and try again.
-  if (auto objectType2 = desugar2->getOptionalObjectType()) {
-    if (recordFix(FixKind::ForceOptional, getConstraintLocator(locator)))
-      return SolutionKind::Error;
+    // Record any fixes we attempted to get to the correct solution.
+    if (simplified == SolutionKind::Solved)
+      while (unwrapCount-- > 0)
+        if (recordFix(FixKind::ForceOptional, getConstraintLocator(locator)))
+          return SolutionKind::Error;
 
-    type2 = objectType2;
-    desugar2 = type2->getDesugaredType();
-    goto retry;
+    return simplified;
   }
 
   return SolutionKind::Error;
@@ -4729,15 +4736,15 @@ bool ConstraintSystem::recordFix(Fix fix, ConstraintLocatorBuilder locator) {
   }
 
   // Record the fix.
-  if (fix.getKind() != FixKind::None) {
-    // Increase the score. If this would make the current solution worse than
-    // the best solution we've seen already, stop now.
-    increaseScore(SK_Fix);
-    if (worseThanBestSolution())
-      return true;
 
-    Fixes.push_back({fix, getConstraintLocator(locator)});
-  }
+  // Increase the score. If this would make the current solution worse than
+  // the best solution we've seen already, stop now.
+  increaseScore(SK_Fix);
+  if (worseThanBestSolution())
+    return true;
+
+  Fixes.push_back({fix, getConstraintLocator(locator)});
+
   return false;
 }
 
@@ -4746,31 +4753,40 @@ ConstraintSystem::simplifyFixConstraint(Fix fix, Type type1, Type type2,
                                         ConstraintKind matchKind,
                                         TypeMatchOptions flags,
                                         ConstraintLocatorBuilder locator) {
-  if (recordFix(fix, locator))
-    return SolutionKind::Error;
-
   // Try with the fix.
   TypeMatchOptions subflags =
     getDefaultDecompositionOptions(flags) | TMF_ApplyingFix;
   switch (fix.getKind()) {
-  case FixKind::None:
-    return matchTypes(type1, type2, matchKind, subflags, locator);
-
   case FixKind::ForceOptional:
-  case FixKind::OptionalChaining:
+  case FixKind::OptionalChaining: {
     // Assume that '!' was applied to the first type.
-    return matchTypes(type1->getRValueObjectType()->getOptionalObjectType(),
-                      type2, matchKind, subflags, locator);
+    auto result =
+        matchTypes(type1->getRValueObjectType()->getOptionalObjectType(), type2,
+                   matchKind, subflags, locator);
+    if (result == SolutionKind::Solved)
+      if (recordFix(fix, locator))
+        return SolutionKind::Error;
 
+    return result;
+  }
   case FixKind::ForceDowncast:
     // These work whenever they are suggested.
+    if (recordFix(fix, locator))
+      return SolutionKind::Error;
+
     return SolutionKind::Solved;
 
-  case FixKind::AddressOf:
+  case FixKind::AddressOf: {
     // Assume that '&' was applied to the first type, turning an lvalue into
     // an inout.
-    return matchTypes(InOutType::get(type1->getRValueType()), type2,
-                      matchKind, subflags, locator);
+    auto result = matchTypes(InOutType::get(type1->getRValueType()), type2,
+                             matchKind, subflags, locator);
+    if (result == SolutionKind::Solved)
+      if (recordFix(fix, locator))
+        return SolutionKind::Error;
+
+    return result;
+  }
 
   case FixKind::CoerceToCheckedCast:
     llvm_unreachable("handled elsewhere");

lib/Sema/Constraint.cpp

@@ -493,8 +493,6 @@ Type Fix::getTypeArgument(ConstraintSystem &cs) const {
 
 StringRef Fix::getName(FixKind kind) {
   switch (kind) {
-  case FixKind::None:
-    return "prevent fixes";
   case FixKind::ForceOptional:
     return "fix: force optional";
   case FixKind::OptionalChaining:

lib/Sema/Constraint.h

@@ -232,10 +232,6 @@ enum RememberChoice_t : bool {
 /// Describes the kind of fix to apply to the given constraint before
 /// visiting it.
 enum class FixKind : uint8_t {
-  /// No fix, which is used as a placeholder indicating that future processing
-  /// of this constraint should not attempt fixes.
-  None,
-
   /// Introduce a '!' to force an optional unwrap.
   ForceOptional,
 
@@ -264,9 +260,7 @@ class Fix {
   friend class Constraint;
 
 public:
-  Fix() : Kind(FixKind::None), Data(0) { }
-  
-  Fix(FixKind kind) : Kind(kind), Data(0) { 
+  Fix(FixKind kind) : Kind(kind), Data(0) {
     assert(kind != FixKind::ForceDowncast && "Use getForceDowncast()");
   }
 

lib/Sema/ConstraintSystem.h

@@ -90,8 +90,10 @@ class ConstraintLocator;
 
 /// Describes a conversion restriction or a fix.
 struct RestrictionOrFix {
-  ConversionRestrictionKind Restriction;
-  Fix TheFix;
+  union {
+    ConversionRestrictionKind Restriction;
+    Fix TheFix;
+  };
   bool IsRestriction;
 
 public:
@@ -1559,11 +1561,11 @@ class ConstraintSystem {
 
   TypeBase* getFavoredType(Expr *E) {
     assert(E != nullptr);
-    return this->FavoredTypes[E];
+    return this->FavoredTypes[E->getSemanticsProvidingExpr()];
   }
   void setFavoredType(Expr *E, TypeBase *T) {
     assert(E != nullptr);
-    this->FavoredTypes[E] = T;
+    this->FavoredTypes[E->getSemanticsProvidingExpr()] = T;
   }
 
   /// Set the type in our type map for a given expression. The side