[Constraint solver] Perform the "occurs" check properly.

We've been performing the "occurs" check when computing potential
bindings for type variables, but we weren't actually performing the
check for bindings that *must* occur. Perform the occurs check before
binding type variables, which fixes a few crashers and is far more principled.

Note that this obviates the need for tracking the type variables we've
substituted in simplifyType(), so simplify that as well.

Fixes rdar://problem/27879334 / SR-2351.

Author: DougGregor

lib/Sema/CSSimplify.cpp

@@ -1208,6 +1208,17 @@ static bool allowsBridgingFromObjC(TypeChecker &tc, DeclContext *dc,
   return true;
 }
 
+/// Determine whether the given type variables occurs in the given type.
+static bool typeVarOccursInType(TypeVariableType *typeVar, Type type) {
+  SmallVector<TypeVariableType *, 4> typeVars;
+  type->getTypeVariables(typeVars);
+  for (auto referencedTypeVar : typeVars) {
+    if (referencedTypeVar == typeVar) return true;
+  }
+
+  return false;
+}
+
 ConstraintSystem::SolutionKind
 ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
                              TypeMatchOptions flags,
@@ -1287,6 +1298,11 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
       // Provide a fixed type for the type variable.
       bool wantRvalue = kind == ConstraintKind::Equal;
       if (typeVar1) {
+        // Simplify the right-hand type and perform the "occurs" check.
+        type2 = simplifyType(type2);
+        if (typeVarOccursInType(typeVar1, type2))
+          return formUnsolvedResult();
+
         // If we want an rvalue, get the rvalue.
         if (wantRvalue)
           type2 = type2->getRValueType();
@@ -1310,7 +1326,7 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
 
         // A constraint that binds any pointer to a void pointer is
         // ineffective, since any pointer can be converted to a void pointer.
-        if (kind == ConstraintKind::BindToPointerType && desugar2->isVoid()) {
+        if (kind == ConstraintKind::BindToPointerType && type2->isVoid()) {
           // Bind type1 to Void only as a last resort.
           addConstraint(ConstraintKind::Defaultable, typeVar1, type2,
                         getConstraintLocator(locator));
@@ -1327,6 +1343,11 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
         return SolutionKind::Solved;
       }
 
+        // Simplify the left-hand type and perform the "occurs" check.
+        type1 = simplifyType(type1);
+        if (typeVarOccursInType(typeVar2, type1))
+          return formUnsolvedResult();
+
       // If we want an rvalue, get the rvalue.
       if (wantRvalue)
         type1 = type1->getRValueType();
@@ -1344,14 +1365,24 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
 
     case ConstraintKind::BindParam: {
       if (typeVar2 && !typeVar1) {
-        if (auto *iot = dyn_cast<InOutType>(desugar1)) {
+        // Simplify the left-hand type and perform the "occurs" check.
+        type1 = simplifyType(type1);
+        if (typeVarOccursInType(typeVar2, type1))
+          return formUnsolvedResult();
+
+        if (auto *iot = type1->getAs<InOutType>()) {
           assignFixedType(typeVar2, LValueType::get(iot->getObjectType()));
         } else {
           assignFixedType(typeVar2, type1);
         }
         return SolutionKind::Solved;
       } else if (typeVar1 && !typeVar2) {
-        if (auto *lvt = dyn_cast<LValueType>(desugar2)) {
+        // Simplify the right-hand type and perform the "occurs" check.
+        type2 = simplifyType(type2);
+        if (typeVarOccursInType(typeVar1, type2))
+          return formUnsolvedResult();
+
+        if (auto *lvt = type2->getAs<LValueType>()) {
           assignFixedType(typeVar1, InOutType::get(lvt->getObjectType()));
         } else {
           assignFixedType(typeVar1, type2);

lib/Sema/ConstraintSystem.cpp

@@ -1578,17 +1578,12 @@ Type ConstraintSystem::lookThroughImplicitlyUnwrappedOptionalType(Type type) {
   return Type();
 }
 
-Type ConstraintSystem::simplifyType(Type type,
-       llvm::SmallPtrSet<TypeVariableType *, 16> &substituting) {
+Type ConstraintSystem::simplifyType(Type type) {
   return type.transform([&](Type type) -> Type {
     if (auto tvt = dyn_cast<TypeVariableType>(type.getPointer())) {
       tvt = getRepresentative(tvt);
       if (auto fixed = getFixedType(tvt)) {
-        if (substituting.insert(tvt).second) {
-          auto result = simplifyType(fixed, substituting);
-          substituting.erase(tvt);
-          return result;
-        }
+        return simplifyType(fixed);
       }
 
       return tvt;

lib/Sema/ConstraintSystem.h

@@ -1894,10 +1894,7 @@ class ConstraintSystem {
   ///
   /// The resulting types can be compared canonically, so long as additional
   /// type equivalence requirements aren't introduced between comparisons.
-  Type simplifyType(Type type){
-    llvm::SmallPtrSet<TypeVariableType *, 16> substituting;
-   return simplifyType(type, substituting);
-  }
+  Type simplifyType(Type type);
 
   /// Given a ValueMember, UnresolvedValueMember, or TypeMember constraint,
   /// perform a lookup into the specified base type to find a candidate list.
@@ -1913,22 +1910,7 @@ class ConstraintSystem {
                                          ConstraintLocator *memberLocator,
                                          bool includeInaccessibleMembers);
 
-private:
-
-  /// \brief Simplify a type, by replacing type variables with either their
-  /// fixed types (if available) or their representatives.
-  ///
-  /// \param type the type to be simplified.
-  ///
-  /// \param substituting the set of type variables that we're already
-  /// substituting for. These type variables will not be substituted again,
-  /// to avoid infinite recursion.
-  ///
-  /// The resulting types can be compared canonically, so long as additional
-  /// type equivalence requirements aren't introduced between comparisons.
-  Type simplifyType(Type type,
-                    llvm::SmallPtrSet<TypeVariableType *, 16> &substituting);
-  
+private:  
   /// \brief Attempt to simplify the given construction constraint.
   ///
   /// \param valueType The type being constructed.

validation-test/Sema/type_checker_crashers/rdar27879334.swift renamed to validation-test/Sema/type_checker_crashers_fixed/rdar27879334.swift

@@ -1,4 +1,4 @@
-// RUN: not --crash %target-swift-frontend %s -parse
+// RUN: not %target-swift-frontend %s -parse
 
 class N {}
 

validation-test/compiler_crashers/28362-swift-constraints-constraintgraphnode-getadjacency.swift renamed to validation-test/compiler_crashers_fixed/28362-swift-constraints-constraintgraphnode-getadjacency.swift

@@ -5,6 +5,6 @@
 // See http://swift.org/LICENSE.txt for license information
 // See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 
-// RUN: not --crash %target-swift-frontend %s -parse
+// RUN: not %target-swift-frontend %s -parse
 // REQUIRES: asserts
 [1,{[1,{[]

validation-test/compiler_crashers/28400-swift-nominaltypedecl-prepareextensions.swift renamed to validation-test/compiler_crashers_fixed/28400-swift-nominaltypedecl-prepareextensions.swift

@@ -5,8 +5,7 @@
 // See http://swift.org/LICENSE.txt for license information
 // See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 
-// RUN: not --crash %target-swift-frontend %s -parse
-// REQUIRES: asserts
+// RUN: %target-swift-frontend %s -parse
 // Discovered by https://github.com/airspeedswift (airspeedswift)
 
 let s1: Set<Int>? = []