Sema: Split off ConstraintSystem::openUnboundGenericType() from openType()

The openType() function did two things:

- Replace unbound generic types like 'G' with fresh type variables
  applied to bound generic types, like 'G<$T0>'.

- Replace generic parameters with type variables from the replacement
  map.

The two behaviors were mutually exclusive and never used from the
same call site, so split them up into two independent functions.

Also, eliminate ConstraintSystem::openBindingType() since it was only
used in one place and could be expressed in terms of existing functions.

lib/Sema/CSGen.cpp

@@ -1364,7 +1364,7 @@ namespace {
       if (!type || type->hasError()) return Type();
       
       auto locator = CS.getConstraintLocator(E);
-      type = CS.openType(type, locator);
+      type = CS.openUnboundGenericType(type, locator);
       E->getTypeLoc().setType(type, /*validated=*/true);
       return MetatypeType::get(type);
     }
@@ -1927,7 +1927,7 @@ namespace {
         // If a type was explicitly specified, use its opened type.
         if (auto type = param->getTypeLoc().getType()) {
           // FIXME: Need a better locator for a pattern as a base.
-          Type openedType = CS.openType(type, locator);
+          Type openedType = CS.openUnboundGenericType(type, locator);
           param->setType(openedType);
           param->setInterfaceType(openedType);
           continue;
@@ -2000,7 +2000,8 @@ namespace {
       case PatternKind::Typed: {
         auto typedPattern = cast<TypedPattern>(pattern);
         // FIXME: Need a better locator for a pattern as a base.
-        Type openedType = CS.openType(typedPattern->getType(), locator);
+        Type openedType = CS.openUnboundGenericType(typedPattern->getType(),
+                                                    locator);
         if (auto weakTy = openedType->getAs<WeakStorageType>())
           openedType = weakTy->getReferentType();
 
@@ -2496,7 +2497,7 @@ namespace {
         return nullptr;
 
       // Open the type we're casting to.
-      auto toType = CS.openType(expr->getCastTypeLoc().getType(),
+      auto toType = CS.openUnboundGenericType(expr->getCastTypeLoc().getType(),
                                               CS.getConstraintLocator(expr));
       expr->getCastTypeLoc().setType(toType, /*validated=*/true);
 
@@ -2519,7 +2520,7 @@ namespace {
         return nullptr;
 
       // Open the type we're casting to.
-      auto toType = CS.openType(expr->getCastTypeLoc().getType(),
+      auto toType = CS.openUnboundGenericType(expr->getCastTypeLoc().getType(),
                                               CS.getConstraintLocator(expr));
       expr->getCastTypeLoc().setType(toType, /*validated=*/true);
 
@@ -2544,7 +2545,7 @@ namespace {
         return nullptr;
 
       // Open the type we're casting to.
-      auto toType = CS.openType(expr->getCastTypeLoc().getType(),
+      auto toType = CS.openUnboundGenericType(expr->getCastTypeLoc().getType(),
                                               CS.getConstraintLocator(expr));
       expr->getCastTypeLoc().setType(toType, /*validated=*/true);
 
@@ -2564,7 +2565,7 @@ namespace {
 
       // Open up the type we're checking.
       // FIXME: Locator for the cast type?
-      auto toType = CS.openType(expr->getCastTypeLoc().getType(),
+      auto toType = CS.openUnboundGenericType(expr->getCastTypeLoc().getType(),
                                               CS.getConstraintLocator(expr));
       expr->getCastTypeLoc().setType(toType, /*validated=*/true);
 
@@ -2772,7 +2773,7 @@ namespace {
         auto rootObjectTy = resolveTypeReferenceInExpression(rootRepr);
         if (!rootObjectTy || rootObjectTy->hasError())
           return Type();
-        rootObjectTy = CS.openType(rootObjectTy, locator);
+        rootObjectTy = CS.openUnboundGenericType(rootObjectTy, locator);
         CS.addConstraint(ConstraintKind::Bind, root, rootObjectTy,
                          locator);
       }

lib/Sema/CSRanking.cpp

@@ -589,7 +589,7 @@ static bool isDeclAsSpecializedAs(TypeChecker &tc, DeclContext *dc,
                        locator,
                        unused);
 
-        openedType2 = cs.openType(type2, locator, unused);
+        openedType2 = cs.openType(type2, unused);
       }
 
       // Get the type of a reference to the first declaration, swapping in
@@ -611,7 +611,7 @@ static bool isDeclAsSpecializedAs(TypeChecker &tc, DeclContext *dc,
                        locator,
                        replacements);
 
-        openedType1 = cs.openType(type1, locator, replacements);
+        openedType1 = cs.openType(type1, replacements);
       }
 
       for (const auto &replacement : replacements) {

lib/Sema/CSSolver.cpp

@@ -1327,7 +1327,9 @@ static bool tryTypeVariableBindings(
           if (anySolved)
             break;
         }
-        type = cs.openBindingType(type, typeVar->getImpl().getLocator());
+        type = cs.openUnboundGenericType(type,
+                                         typeVar->getImpl().getLocator());
+        type = type->reconstituteSugar(/*recursive=*/false);
       }
 
       // FIXME: We want the locator that indicates where the binding came

lib/Sema/ConstraintSystem.cpp

@@ -348,23 +348,95 @@ ConstraintLocator *ConstraintSystem::getConstraintLocator(
   return getConstraintLocator(anchor, path, builder.getSummaryFlags());
 }
 
-namespace {
+Type ConstraintSystem::openUnboundGenericType(UnboundGenericType *unbound,
-  /// Function object that replaces all occurrences of archetypes and
+                                              ConstraintLocatorBuilder locator,
-  /// dependent types with type variables.
+                                              OpenedTypeMap &replacements) {
-  class ReplaceDependentTypes {
+  auto unboundDecl = unbound->getDecl();
-    ConstraintSystem &cs;
+  if (unboundDecl->isInvalid())
-    ConstraintLocatorBuilder &locator;
+    return ErrorType::get(getASTContext());
-    OpenedTypeMap &replacements;
 
-  public:
+  // If the unbound decl hasn't been validated yet, we have a circular
-    ReplaceDependentTypes(
+  // dependency that isn't being diagnosed properly.
-        ConstraintSystem &cs,
+  if (!unboundDecl->getGenericSignature()) {
-        ConstraintLocatorBuilder &locator,
+    TC.diagnose(unboundDecl, diag::circular_reference);
-        OpenedTypeMap &replacements)
+    return ErrorType::get(unbound);
-      : cs(cs), locator(locator), replacements(replacements) { }
+  }
 
-    Type operator()(Type type) {
+  auto parentTy = unbound->getParent();
-      // Swift only supports rank-1 polymorphism.
+  if (parentTy) {
+    parentTy = openUnboundGenericType(parentTy, locator);
+    unbound = UnboundGenericType::get(unboundDecl, parentTy,
+                                      getASTContext());
+  }
+
+  // Open up the generic type.
+  openGeneric(unboundDecl->getInnermostDeclContext(),
+              unboundDecl->getDeclContext(),
+              unboundDecl->getGenericSignature(),
+              /*skipProtocolSelfConstraint=*/false,
+              locator,
+              replacements);
+
+  if (parentTy) {
+    auto subs = parentTy->getContextSubstitutions(
+      parentTy->getAnyNominal());
+    for (auto pair : subs) {
+      auto found = replacements.find(
+        cast<GenericTypeParamType>(pair.first));
+      assert(found != replacements.end() &&
+             "Missing generic parameter?");
+      addConstraint(ConstraintKind::Equal, found->second, pair.second,
+                    locator);
+    }
+  }
+        
+  // Map the generic parameters to their corresponding type variables.
+  llvm::SmallVector<TypeLoc, 4> arguments;
+  for (auto gp : unboundDecl->getInnermostGenericParamTypes()) {
+    auto found = replacements.find(
+      cast<GenericTypeParamType>(gp->getCanonicalType()));
+    assert(found != replacements.end() &&
+           "Missing generic parameter?");
+    arguments.push_back(TypeLoc::withoutLoc(found->second));
+  }
+
+  // FIXME: For some reason we can end up with unbound->getDecl()
+  // pointing at a generic TypeAliasDecl here. If we find a way to
+  // handle generic TypeAliases elsewhere, this can just become a
+  // call to BoundGenericType::get().
+  return TC.applyUnboundGenericArguments(
+      unbound, unboundDecl,
+      SourceLoc(), DC, arguments,
+      /*options*/TypeResolutionOptions(),
+      /*resolver*/nullptr,
+      /*unsatisfiedDependency*/nullptr);
+}
+
+Type ConstraintSystem::openUnboundGenericType(
+       Type type,
+       ConstraintLocatorBuilder locator) {
+  assert(!type->hasTypeParameter());
+
+  if (!type->hasUnboundGenericType())
+    return type;
+
+  return type.transform([&](Type type) -> Type {
+      if (auto unbound = type->getAs<UnboundGenericType>()) {
+        OpenedTypeMap replacements;
+        return openUnboundGenericType(unbound, locator, replacements);
+      }
+
+      return type;
+    });
+}
+
+Type ConstraintSystem::openType(Type type, OpenedTypeMap &replacements) {
+  assert(!type->hasUnboundGenericType());
+
+  if (!type->hasTypeParameter())
+    return type;
+
+  return type.transform([&](Type type) -> Type {
       assert(!type->is<GenericFunctionType>());
 
       // Replace a generic type parameter with its corresponding type variable.
@@ -375,98 +447,8 @@ namespace {
         return known->second;
       }
 
-      // Replace a dependent member with a fresh type variable and make it a
-      // member of its base type.
-      if (auto dependentMember = type->getAs<DependentMemberType>()) {
-        // Replace type parameters in the base type.
-        if (auto base =
-            ((*this)(dependentMember->getBase()))->getAs<TypeVariableType>()) {
-          return
-            DependentMemberType::get(base, dependentMember->getAssocType());
-        }
-      }
-
-      // Open up unbound generic types, turning them into bound generic
-      // types with type variables for each parameter.
-      if (auto unbound = type->getAs<UnboundGenericType>()) {
-        auto unboundDecl = unbound->getDecl();
-        if (unboundDecl->isInvalid())
-          return ErrorType::get(cs.getASTContext());
-        
-        auto parentTy = unbound->getParent();
-        if (parentTy) {
-          parentTy = parentTy.transform(*this);
-          unbound = UnboundGenericType::get(unboundDecl, parentTy,
-                                            cs.getASTContext());
-        }
-
-        // If the unbound decl hasn't been validated yet, we have a circular
-        // dependency that isn't being diagnosed properly.
-        if (!unboundDecl->getGenericSignature()) {
-          cs.TC.diagnose(unboundDecl, diag::circular_reference);
-          return ErrorType::get(type);
-        }
-
-        OpenedTypeMap unboundReplacements;
-
-        // Open up the generic type.
-        cs.openGeneric(unboundDecl->getInnermostDeclContext(),
-                       unboundDecl->getDeclContext(),
-                       unboundDecl->getGenericSignature(),
-                       /*skipProtocolSelfConstraint=*/false,
-                       locator,
-                       unboundReplacements);
-
-        if (parentTy) {
-          auto subs = parentTy->getContextSubstitutions(
-            parentTy->getAnyNominal());
-          for (auto pair : subs) {
-            auto found = unboundReplacements.find(
-              cast<GenericTypeParamType>(pair.first));
-            assert(found != unboundReplacements.end() &&
-                  "Missing generic parameter?");
-            cs.addConstraint(ConstraintKind::Equal, found->second, pair.second,
-                             locator);
-          }
-        }
-        
-        // Map the generic parameters to their corresponding type variables.
-        llvm::SmallVector<TypeLoc, 4> arguments;
-        for (auto gp : unboundDecl->getInnermostGenericParamTypes()) {
-          auto found = unboundReplacements.find(
-            cast<GenericTypeParamType>(gp->getCanonicalType()));
-          assert(found != unboundReplacements.end() &&
-                 "Missing generic parameter?");
-          arguments.push_back(TypeLoc::withoutLoc(found->second));
-        }
-
-        // FIXME: For some reason we can end up with unbound->getDecl()
-        // pointing at a generic TypeAliasDecl here. If we find a way to
-        // handle generic TypeAliases elsewhere, this can just become a
-        // call to BoundGenericType::get().
-        return cs.TC.applyUnboundGenericArguments(
-                                             unbound, unboundDecl,
-                                             SourceLoc(), cs.DC, arguments,
-                                             /*options*/TypeResolutionOptions(),
-                                             /*resolver*/nullptr,
-                                             /*unsatisfiedDependency*/nullptr);
-      }
-      
       return type;
-    }
+    });
-  };
-} // end anonymous namespace
-
-Type ConstraintSystem::openType(
-       Type startingType,
-       ConstraintLocatorBuilder locator,
-       OpenedTypeMap &replacements) {
-  if (!startingType->hasTypeParameter() &&
-      !startingType->hasUnboundGenericType())
-    return startingType;
-
-  ReplaceDependentTypes replaceDependentTypes(*this, locator, replacements);
-  return startingType.transform(replaceDependentTypes);
 }
 
 /// Remove argument labels from the function type.
@@ -513,15 +495,15 @@ Type ConstraintSystem::openFunctionType(
                 replacements);
 
     // Transform the input and output types.
-    auto inputTy = openType(genericFn->getInput(), locator, replacements);
+    auto inputTy = openType(genericFn->getInput(), replacements);
-    auto resultTy = openType(genericFn->getResult(), locator, replacements);
+    auto resultTy = openType(genericFn->getResult(), replacements);
 
     // Build the resulting (non-generic) function type.
     type = FunctionType::get(inputTy, resultTy,
                              FunctionType::ExtInfo().
                                withThrows(genericFn->throws()));
   } else {
-    type = openType(funcType, locator, replacements);
+    type = openType(funcType, replacements);
   }
 
   return removeArgumentLabels(type, numArgumentLabelsToRemove);
@@ -569,27 +551,6 @@ bool ConstraintSystem::isAnyHashableType(Type type) {
   return false;
 }
 
-Type ConstraintSystem::openBindingType(Type type, 
-                                       ConstraintLocatorBuilder locator) {
-  Type result = openType(type, locator);
-  
-  if (isArrayType(type)) {
-    auto boundStruct = type->getAs<BoundGenericStructType>();
-    if (auto replacement = getTypeChecker().getArraySliceType(
-                             SourceLoc(), boundStruct->getGenericArgs()[0])) {
-      return replacement;
-    }
-  }
-
-  if (auto dict = isDictionaryType(type)) {
-    if (auto replacement = getTypeChecker().getDictionaryType(
-                             SourceLoc(), dict->first, dict->second))
-      return replacement;
-  }
-
-  return result;
-}
-
 Type ConstraintSystem::getFixedTypeRecursive(Type type,
                                              TypeMatchOptions &flags,
                                              bool wantRValue,
@@ -718,13 +679,13 @@ ConstraintSystem::getTypeOfReference(ValueDecl *value,
                                      FunctionRefKind functionRefKind,
                                      ConstraintLocatorBuilder locator,
                                      const DeclRefExpr *base) {
-  OpenedTypeMap replacements;
-
   if (value->getDeclContext()->isTypeContext() && isa<FuncDecl>(value)) {
     // Unqualified lookup can find operator names within nominal types.
     auto func = cast<FuncDecl>(value);
     assert(func->isOperator() && "Lookup should only find operators");
 
+    OpenedTypeMap replacements;
+
     auto openedType = openFunctionType(
             func->getInterfaceType()->castTo<AnyFunctionType>(),
             /*numArgumentLabelsToRemove=*/0,
@@ -734,6 +695,9 @@ ConstraintSystem::getTypeOfReference(ValueDecl *value,
             /*skipProtocolSelfConstraint=*/false);
     auto openedFnType = openedType->castTo<FunctionType>();
 
+    // If we opened up any type variables, record the replacements.
+    recordOpenedTypes(locator, replacements);
+
     // If this is a method whose result type is dynamic Self, replace
     // DynamicSelf with the actual object type.
     if (!func->getDeclContext()->getAsProtocolOrProtocolExtensionContext()) {
@@ -749,9 +713,6 @@ ConstraintSystem::getTypeOfReference(ValueDecl *value,
       openedFnType = openedType->castTo<FunctionType>();
     }
 
-    // If we opened up any type variables, record the replacements.
-    recordOpenedTypes(locator, replacements);
-
     // The reference implicitly binds 'self'.
     return { openedType, openedFnType->getResult() };
   }
@@ -764,10 +725,7 @@ ConstraintSystem::getTypeOfReference(ValueDecl *value,
                                                       isSpecialized);
 
     // Open the type.
-    type = openType(type, locator, replacements);
+    type = openUnboundGenericType(type, locator);
-
-    // If we opened up any type variables, record the replacements.
-    recordOpenedTypes(locator, replacements);
 
     // Module types are not wrapped in metatypes.
     if (type->is<ModuleType>())
@@ -801,6 +759,8 @@ ConstraintSystem::getTypeOfReference(ValueDecl *value,
 
   // Adjust the type of the reference.
   if (auto funcType = valueType->getAs<AnyFunctionType>()) {
+    OpenedTypeMap replacements;
+
     valueType =
       openFunctionType(
           funcType,
@@ -811,12 +771,13 @@ ConstraintSystem::getTypeOfReference(ValueDecl *value,
           value->getInnermostDeclContext(),
           value->getDeclContext(),
           /*skipProtocolSelfConstraint=*/false);
-  } else {
-    valueType = openType(valueType, locator, replacements);
-  }
 
     // If we opened up any type variables, record the replacements.
     recordOpenedTypes(locator, replacements);
+  } else {
+    assert(!valueType->hasUnboundGenericType() &&
+           !valueType->hasTypeParameter());
+  }
 
   return { valueType, valueType };
 }
@@ -923,8 +884,6 @@ void ConstraintSystem::openGeneric(
     (void) result;
   }
 
-  ReplaceDependentTypes replaceDependentTypes(*this, locator, replacements);
-
   // Remember that any new constraints generated by opening this generic are
   // due to the opening.
   locatorPtr = getConstraintLocator(
@@ -936,7 +895,7 @@ void ConstraintSystem::openGeneric(
   for (auto req : sig->getRequirements()) {
   switch (req.getKind()) {
     case RequirementKind::Conformance: {
-      auto subjectTy = req.getFirstType().transform(replaceDependentTypes);
+      auto subjectTy = openType(req.getFirstType(), replacements);
       auto proto = req.getSecondType()->castTo<ProtocolType>();
       auto protoDecl = proto->getDecl();
 
@@ -953,7 +912,7 @@ void ConstraintSystem::openGeneric(
     }
 
     case RequirementKind::Layout: {
-      auto subjectTy = req.getFirstType().transform(replaceDependentTypes);
+      auto subjectTy = openType(req.getFirstType(), replacements);
       auto layoutConstraint = req.getLayoutConstraint();
 
       if (layoutConstraint->isClass())
@@ -967,15 +926,15 @@ void ConstraintSystem::openGeneric(
     }
 
     case RequirementKind::Superclass: {
-      auto subjectTy = req.getFirstType().transform(replaceDependentTypes);
+      auto subjectTy = openType(req.getFirstType(), replacements);
-      auto boundTy = req.getSecondType().transform(replaceDependentTypes);
+      auto boundTy = openType(req.getSecondType(), replacements);
       addConstraint(ConstraintKind::Subtype, subjectTy, boundTy, locatorPtr);
       break;
     }
 
     case RequirementKind::SameType: {
-      auto firstTy = req.getFirstType().transform(replaceDependentTypes);
+      auto firstTy = openType(req.getFirstType(), replacements);
-      auto secondTy = req.getSecondType().transform(replaceDependentTypes);
+      auto secondTy = openType(req.getSecondType(), replacements);
       addConstraint(ConstraintKind::Bind, firstTy, secondTy, locatorPtr);
       break;
     }
@@ -1102,10 +1061,10 @@ ConstraintSystem::getTypeOfMemberReference(
                 locator, replacements);
 
     // Open up the type of the member.
-    openedType = openType(openedType, locator, replacements);
+    openedType = openType(openedType, replacements);
 
     // Determine the object type of 'self'.
-    auto selfTy = openType(outerDC->getSelfInterfaceType(), locator,
+    auto selfTy = openType(outerDC->getSelfInterfaceType(),
                            replacements);
 
     // If self is a struct, properly qualify it based on our base

lib/Sema/ConstraintSystem.h

@@ -1881,29 +1881,33 @@ public:
   /// \brief Coerce the given expression to an rvalue, if it isn't already.
   Expr *coerceToRValue(Expr *expr);
 
-  /// \brief "Open" the given type by replacing any occurrences of generic
+  /// \brief "Open" the given unbound type by introducing fresh type
-  /// parameter types and dependent member types with fresh type variables.
+  /// variables for generic parameters and constructing a bound generic
+  /// type from these type variables.
+  ///
+  /// \param unbound The type to open.
+  ///
+  /// \returns The opened type.
+  Type openUnboundGenericType(UnboundGenericType *unbound,
+                              ConstraintLocatorBuilder locator,
+                              OpenedTypeMap &replacements);
+
+  /// \brief "Open" the given type by replacing any occurrences of unbound
+  /// generic types with bound generic types with fresh type variables as
+  /// generic arguments.
   ///
   /// \param type The type to open.
   ///
   /// \returns The opened type.
-  Type openType(Type type, ConstraintLocatorBuilder locator) {
+  Type openUnboundGenericType(Type type, ConstraintLocatorBuilder locator);
-    OpenedTypeMap replacements;
-    return openType(type, locator, replacements);
-  }
 
   /// \brief "Open" the given type by replacing any occurrences of generic
   /// parameter types and dependent member types with fresh type variables.
   ///
   /// \param type The type to open.
   ///
-  /// \param replacements The mapping from opened types to the type
-  /// variables to which they were opened.
-  ///
   /// \returns The opened type, or \c type if there are no archetypes in it.
-  Type openType(Type type,
+  Type openType(Type type, OpenedTypeMap &replacements);
-                ConstraintLocatorBuilder locator,
-                OpenedTypeMap &replacements);
 
   /// \brief "Open" the given function type.
   ///
@@ -1933,18 +1937,6 @@ public:
       DeclContext *outerDC,
       bool skipProtocolSelfConstraint);
 
-  /// \brief "Open" the given binding type by replacing any occurrences of
-  /// archetypes (including those implicit in unbound generic types) with
-  /// fresh type variables.
-  ///
-  /// This variant of \c openType() tweaks the result from \c openType() to
-  /// prefer arrays to slices.
-  /// FIXME: This is a bit of a hack.
-  ///
-  /// \param type The type to open.
-  /// \returns The opened type, or \c type if there are no archetypes in it.
-  Type openBindingType(Type type, ConstraintLocatorBuilder locator);
-
   /// Open the generic parameter list and its requirements, creating
   /// type variables for each of the type parameters.
   void openGeneric(DeclContext *innerDC,