[CSClosure] Detect and diagnose conflicting pattern variables in case statements

Avoid mutating case label items while solving, instead let's use types
recorded in the constraint system for each pattern variable and use
that for var reference in the case body. This also helps to detect
and diagnose type conflicts while solving.

Author: xedin

lib/Sema/CSClosure.cpp

@@ -346,6 +346,8 @@ class SyntacticElementConstraintGenerator
         hadError = true;
         return;
       }
+
+      caseItem->setPattern(pattern, /*resolved=*/true);
     }
 
     // Let's generate constraints for pattern + where clause.
@@ -903,8 +905,6 @@ class SyntacticElementConstraintGenerator
       }
     }
 
-    bindSwitchCasePatternVars(context.getAsDeclContext(), caseStmt);
-
     auto *caseLoc = cs.getConstraintLocator(
         locator, LocatorPathElt::SyntacticElement(caseStmt));
 
@@ -928,10 +928,8 @@ class SyntacticElementConstraintGenerator
             locator->castLastElementTo<LocatorPathElt::SyntacticElement>()
                 .asStmt());
 
-        for (auto caseBodyVar : caseStmt->getCaseBodyVariablesOrEmptyArray()) {
-          auto parentVar = caseBodyVar->getParentVarDecl();
-          assert(parentVar && "Case body variables always have parents");
-          cs.setType(caseBodyVar, cs.getType(parentVar));
+        if (recordInferredSwitchCasePatternVars(caseStmt)) {
+          hadError = true;
         }
       }
 
@@ -1058,6 +1056,75 @@ class SyntacticElementConstraintGenerator
         locator->getLastElementAs<LocatorPathElt::SyntacticElement>();
     return parentElt ? parentElt->getElement().isStmt(kind) : false;
   }
+
+  bool recordInferredSwitchCasePatternVars(CaseStmt *caseStmt) {
+    llvm::SmallDenseMap<Identifier, SmallVector<VarDecl *, 2>, 4> patternVars;
+
+    auto recordVar = [&](VarDecl *var) {
+      if (!var->hasName())
+        return;
+      patternVars[var->getName()].push_back(var);
+    };
+
+    for (auto &caseItem : caseStmt->getMutableCaseLabelItems()) {
+      assert(caseItem.isPatternResolved());
+
+      auto *pattern = caseItem.getPattern();
+      pattern->forEachVariable([&](VarDecl *var) { recordVar(var); });
+    }
+
+    for (auto bodyVar : caseStmt->getCaseBodyVariablesOrEmptyArray()) {
+      if (!bodyVar->hasName())
+        continue;
+
+      const auto &variants = patternVars[bodyVar->getName()];
+
+      auto getType = [&](VarDecl *var) {
+        auto type = cs.simplifyType(cs.getType(var));
+        assert(!type->hasTypeVariable());
+        return type;
+      };
+
+      switch (variants.size()) {
+      case 0:
+        break;
+
+      case 1:
+        // If there is only one choice here, let's use it directly.
+        cs.setType(bodyVar, getType(variants.front()));
+        break;
+
+      default: {
+        // If there are multiple choices it could only mean multiple
+        // patterns e.g. `.a(let x), .b(let x), ...:`. Let's join them.
+        Type joinType = getType(variants.front());
+
+        SmallVector<VarDecl *, 2> conflicts;
+        for (auto *var : llvm::drop_begin(variants)) {
+          auto varType = getType(var);
+          // Type mismatch between different patterns.
+          if (!joinType->isEqual(varType))
+            conflicts.push_back(var);
+        }
+
+        if (!conflicts.empty()) {
+          if (!cs.shouldAttemptFixes())
+            return true;
+
+          // dfdf
+          auto *locator = cs.getConstraintLocator(bodyVar);
+          if (cs.recordFix(RenameConflictingPatternVariables::create(
+                  cs, joinType, conflicts, locator)))
+            return true;
+        }
+
+        cs.setType(bodyVar, joinType);
+      }
+      }
+    }
+
+    return false;
+  }
 };
 }
 
@@ -1465,6 +1532,8 @@ class SyntacticElementSolutionApplication
       }
     }
 
+    bindSwitchCasePatternVars(context.getAsDeclContext(), caseStmt);
+
     for (auto *expected : caseStmt->getCaseBodyVariablesOrEmptyArray()) {
       assert(expected->hasName());
       auto prev = expected->getParentVarDecl();

test/expr/closure/multi_statement.swift

@@ -454,3 +454,35 @@ func test_fallthrough_stmt() {
     }
   }()
 }
+
+func test_conflicting_pattern_vars() {
+  enum E {
+  case a(Int, String)
+  case b(String, Int)
+  }
+
+  func fn(_: (E) -> Void) {}
+  func fn<T>(_: (E) -> T) {}
+
+  func test(e: E) {
+    fn {
+      switch $0 {
+      case .a(let x, let y),
+           .b(let x, let y):
+        // expected-error@-1 {{pattern variable bound to type 'String', expected type 'Int'}}
+        // expected-error@-2 {{pattern variable bound to type 'Int', expected type 'String'}}
+        _ = x
+        _ = y
+      }
+    }
+
+    fn {
+      switch $0 {
+      case .a(let x, let y),
+           .b(let y, let x): // Ok
+        _ = x
+        _ = y
+      }
+    }
+  }
+}