Merge pull request swiftlang#9840 from jckarter/already-checked-key-path

Sema: Don't crash when type-checking a KeyPathExpr that's already type-checked.

Author: jckarter

include/swift/AST/Expr.h

@@ -4744,6 +4744,25 @@ class KeyPathExpr : public Expr {
       return getKind() != Kind::Invalid;
     }
 
+    bool isResolved() const {
+      if (!getComponentType())
+        return false;
+      
+      switch (getKind()) {
+      case Kind::Subscript:
+      case Kind::OptionalChain:
+      case Kind::OptionalWrap:
+      case Kind::OptionalForce:
+      case Kind::Property:
+        return true;
+      
+      case Kind::UnresolvedSubscript:
+      case Kind::UnresolvedProperty:
+      case Kind::Invalid:
+        return false;
+      }
+    }
+    
     Expr *getIndexExpr() const {
       switch (getKind()) {
       case Kind::Subscript:

lib/Sema/CSApply.cpp

@@ -3986,6 +3986,20 @@ namespace {
       if (E->getType()->hasError())
         return E;
 
+      // If a component is already resolved, then all of them should be
+      // resolved, and we can let the expression be. This might happen when
+      // re-checking a failed system for diagnostics.
+      if (!E->getComponents().empty()
+          && E->getComponents().front().isResolved()) {
+        assert([&]{
+          for (auto &c : E->getComponents())
+            if (!c.isResolved())
+              return false;
+          return true;
+        }());
+        return E;
+      }
+
       SmallVector<KeyPathExpr::Component, 4> resolvedComponents;
 
       // Resolve each of the components.

lib/Sema/CSGen.cpp

@@ -2757,11 +2757,26 @@ namespace {
         return ErrorType::get(CS.getASTContext());
       }
 
-      // If the key path contained any syntactically invalid components, bail
-      // out.
-      for (auto &c : E->getComponents())
-        if (!c.isValid())
+      for (auto &c : E->getComponents()) {
+        // If the key path contained any syntactically invalid components, bail
+        // out.
+        if (!c.isValid()) {
           return ErrorType::get(CS.getASTContext());
+        }
+        
+        // If a component is already resolved, then all of them should be
+        // resolved, and we can let the expression be. This might happen when
+        // re-checking a failed system for diagnostics.
+        if (c.isResolved()) {
+          assert([&]{
+            for (auto &c : E->getComponents())
+              if (!c.isResolved())
+                return false;
+            return true;
+          }());
+          return E->getType();
+        }
+      }
 
       // For native key paths, traverse the key path components to set up
       // appropriate type relationships at each level.

test/expr/unary/keypath/salvage-with-other-type-errors.swift

@@ -0,0 +1,31 @@
+// RUN: %target-swift-frontend -typecheck -verify %s
+
+// Ensure that key path exprs can tolerate being re-type-checked when necessary
+// to diagnose other errors in adjacent exprs.
+
+struct P<T: K> { }
+
+struct S {
+    init<B>(_ a: P<B>) {
+        fatalError()
+    }
+}
+
+protocol K { }
+
+func + <Object>(lhs: KeyPath<A, Object>, rhs: String) -> P<Object> {
+    fatalError()
+}
+
+// expected-error@+1{{}}
+func + (lhs: KeyPath<A, String>, rhs: String) -> P<String> {
+    fatalError()
+}
+
+struct A {
+    let id: String
+}
+
+extension A: K {
+    static let j = S(\A.id + "id")
+}