Merge pull request #63994 from hamishknight/context-switch

Resolves #63750

Author: hamishknight

lib/Sema/CSSyntacticElement.cpp

@@ -1785,8 +1785,11 @@ class SyntacticElementSolutionApplication
       }
     }
 
+    // Note we perform a limited exhaustiveness check if we weren't able to
+    // apply the solution, as the subject and patterns may not be well-formed.
     TypeChecker::checkSwitchExhaustiveness(
-        switchStmt, context.getAsDeclContext(), limitExhaustivityChecks);
+        switchStmt, context.getAsDeclContext(),
+        /*limited*/ limitExhaustivityChecks || hadError);
 
     return switchStmt;
   }

lib/Sema/TypeCheckDeclPrimary.cpp

@@ -438,7 +438,7 @@ static void checkForEmptyOptionSet(const VarDecl *VD) {
 }
 
 template<typename T>
-static void diagnoseDuplicateDecls(const T &decls) {
+static void diagnoseDuplicateDecls(T &&decls) {
   llvm::SmallDenseMap<DeclBaseName, const ValueDecl *> names;
   for (auto *current : decls) {
     if (!current->hasName() || current->isImplicit())
@@ -453,6 +453,14 @@ static void diagnoseDuplicateDecls(const T &decls) {
                           current->getName()), [&]() {
         other->diagnose(diag::invalid_redecl_prev, other->getName());
       });
+
+      // Mark the decl as invalid, unless it's a GenericTypeParamDecl, which is
+      // expected to maintain its type of GenericTypeParamType.
+      // This is needed to avoid emitting a duplicate diagnostic when running
+      // redeclaration checking in the case where the VarDecl is part of the
+      // enclosing context, e.g `let (x, x) = (0, 0)`.
+      if (!isa<GenericTypeParamDecl>(current))
+        current->setInvalid();
     }
   }
 }

lib/Sema/TypeCheckPattern.cpp

@@ -629,6 +629,8 @@ Pattern *TypeChecker::resolvePattern(Pattern *P, DeclContext *DC,
                                      bool isStmtCondition) {
   P = ResolvePattern(DC).visit(P);
 
+  TypeChecker::diagnoseDuplicateBoundVars(P);
+
   // If the entire pattern is "(pattern_expr (type_expr SomeType))", then this
   // is an invalid pattern.  If it were actually a value comparison (with ~=)
   // then the metatype would have had to be spelled with "SomeType.self".  What

lib/Sema/TypeCheckStmt.cpp

@@ -865,8 +865,6 @@ bool TypeChecker::typeCheckStmtConditionElement(StmtConditionElement &elt,
   }
   elt.setPattern(pattern);
 
-  TypeChecker::diagnoseDuplicateBoundVars(pattern);
-
   // Check the pattern, it allows unspecified types because the pattern can
   // provide type information.
   auto contextualPattern = ContextualPattern::forRawPattern(pattern, dc);
@@ -1427,8 +1425,6 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
     if (TypeChecker::typeCheckForEachBinding(DC, S))
       return nullptr;
 
-    TypeChecker::diagnoseDuplicateBoundVars(S->getPattern());
-
     // Type-check the body of the loop.
     auto sourceFile = DC->getParentSourceFile();
     checkLabeledStmtShadowing(getASTContext(), sourceFile, S);
@@ -1495,13 +1491,11 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
     }
     labelItem.setPattern(pattern, /*resolved=*/true);
 
-    TypeChecker::diagnoseDuplicateBoundVars(pattern);
-
     // Otherwise for each variable in the pattern, make sure its type is
     // identical to the initial case decl and stash the previous case decl as
     // the parent of the decl.
     pattern->forEachVariable([&](VarDecl *vd) {
-      if (!vd->hasName())
+      if (!vd->hasName() || vd->isInvalid())
         return;
 
       // We know that prev var decls matches the initial var decl. So if we can

test/Constraints/rdar105781521.swift

@@ -0,0 +1,21 @@
+// RUN: %target-typecheck-verify-swift
+
+// rdar://105781521
+enum MyEnum {
+  case first(String)
+}
+
+func takeClosure(_ x: () -> Void) {}
+
+func test(value: MyEnum) {
+  takeClosure {
+    switch value {
+    case .first(true):
+      // expected-error@-1 {{expression pattern of type 'Bool' cannot match values of type 'String'}}
+      // expected-note@-2 {{overloads for '~=' exist with these partially matching parameter lists: (Substring, String)}}
+      break
+    default:
+      break
+    }
+  }
+}

test/Parse/matching_patterns.swift

@@ -47,7 +47,7 @@ case 1 + (_): // expected-error{{'_' can only appear in a pattern or on the left
 }
 
 switch (x,x) {
-case (var a, var a): // expected-error {{invalid redeclaration of 'a'}} expected-note {{'a' previously declared here}} expected-warning {{variable 'a' was never used; consider replacing with '_' or removing it}} expected-warning {{variable 'a' was never used; consider replacing with '_' or removing it}}
+case (var a, var a): // expected-error {{invalid redeclaration of 'a'}} expected-note {{'a' previously declared here}}
   fallthrough
 case _: // expected-warning {{case is already handled by previous patterns; consider removing it}}
   ()

test/Sema/redeclaration-checking.swift

@@ -63,36 +63,30 @@ func stmtTest() {
   if case (let x, let x)? = n {}
   // expected-note@-1 {{'x' previously declared here}}
   // expected-error@-2 {{invalid redeclaration of 'x'}}
-  // expected-warning@-3 2{{never used}}
 
   for case (let x, let x) in [(Int, Int)]() {}
   // expected-note@-1 {{'x' previously declared here}}
   // expected-error@-2 {{invalid redeclaration of 'x'}}
-  // expected-warning@-3 2{{never used}}
 
   switch n {
   case (let x, let x)?: _ = ()
   // expected-note@-1 {{'x' previously declared here}}
   // expected-error@-2 {{invalid redeclaration of 'x'}}
-  // expected-warning@-3 2{{never used}}
   case nil: _ = ()
   }
 
   while case (let x, let x)? = n {}
   // expected-note@-1 {{'x' previously declared here}}
   // expected-error@-2 {{invalid redeclaration of 'x'}}
-  // expected-warning@-3 2{{never used}}
 
   guard case (let x, let x)? = n else {}
   // expected-note@-1 {{'x' previously declared here}}
   // expected-error@-2 {{invalid redeclaration of 'x'}}
-  // expected-warning@-3 2{{never used}}
 
   do {} catch MyError.error(let x, let x) {}
   // expected-note@-1 {{'x' previously declared here}}
   // expected-error@-2 {{invalid redeclaration of 'x'}}
-  // expected-warning@-3 2{{never used}}
-  // expected-warning@-4 {{unreachable}}
+  // expected-warning@-3 {{unreachable}}
 }
 
 func fullNameTest() {
@@ -111,4 +105,51 @@ protocol SillyProtocol {
 
   subscript(x: Int, x: Int) -> Int { get }
   subscript(a x: Int, b x: Int) -> Int { get }
-}
+}
+
+// https://github.com/apple/swift/issues/63750
+let issue63750 = {
+  for (x,x) in [(0,0)] {}
+  // expected-error@-1 {{invalid redeclaration of 'x'}}
+  // expected-note@-2 {{'x' previously declared here}}
+
+  if case let (x,x) = (0,0) {}
+  // expected-error@-1 {{invalid redeclaration of 'x'}}
+  // expected-note@-2 {{'x' previously declared here}}
+
+  switch (0,0) {
+  case let (x,x):
+    // expected-error@-1 {{invalid redeclaration of 'x'}}
+    // expected-note@-2 {{'x' previously declared here}}
+    ()
+  }
+  // FIXME: We should really say 'let' instead of 'var' in "'var' binding pattern cannot appear in an expression"
+  // https://github.com/apple/swift/issues/63993
+  func bar(_ x: Int) -> Int { x }
+  if case (bar(let x), let x) = (0,0) {}
+  // expected-error@-1 {{'var' binding pattern cannot appear in an expression}}
+  // expected-error@-2 {{invalid redeclaration of 'x'}}
+  // expected-note@-3 {{'x' previously declared here}}
+}
+func issue63750fn() {
+  // Make sure the behavior is consistent with the multi-statement closure case.
+  for (x,x) in [(0,0)] {}
+  // expected-error@-1 {{invalid redeclaration of 'x'}}
+  // expected-note@-2 {{'x' previously declared here}}
+
+  if case let (x,x) = (0,0) {} // expected-warning {{'if' condition is always true}}
+  // expected-error@-1 {{invalid redeclaration of 'x'}}
+  // expected-note@-2 {{'x' previously declared here}}
+
+  switch (0,0) {
+  case let (x,x):
+    // expected-error@-1 {{invalid redeclaration of 'x'}}
+    // expected-note@-2 {{'x' previously declared here}}
+    ()
+  }
+  func bar(_ x: Int) -> Int { x }
+  if case (bar(let x), let x) = (0,0) {}
+  // expected-error@-1 {{'var' binding pattern cannot appear in an expression}}
+  // expected-error@-2 {{invalid redeclaration of 'x'}}
+  // expected-note@-3 {{'x' previously declared here}}
+}

test/expr/closure/closures.swift

@@ -331,7 +331,6 @@ func testCaptureBehavior(_ ptr : SomeClass) {
   let v2 : SomeClass = ptr
 
   doStuff { [weak v1] in v1!.foo() }
-  // expected-warning @+2 {{variable 'v1' was written to, but never read}}
   doStuff { [weak v1,                 // expected-note {{previous}}
              weak v1] in v1!.foo() }  // expected-error {{invalid redeclaration of 'v1'}}
   doStuff { [unowned v2] in v2.foo() }
@@ -340,7 +339,6 @@ func testCaptureBehavior(_ ptr : SomeClass) {
   doStuff { [weak v1, weak v2] in v1!.foo() + v2!.foo() }
 
   let i = 42
-  // expected-warning @+1 {{variable 'i' was never mutated}}
   doStuff { [weak i] in i! }   // expected-error {{'weak' may only be applied to class and class-bound protocol types, not 'Int'}}
 }