[AST] Allow storing original expression in ErrorTypeRepr

This is useful for ArrowExpr when the sub-expressions aren't valid
TypeExprs. Rather than throwing away the AST, attach it to the
ErrorTypeRepr to ensure we can still type-check it. This ensures
semantic functionality still works correctly, and fixes a crash where
we'd stop visiting an invalid binding pattern, losing track of the
nested VarDecl.

Author: hamishknight

include/swift/AST/TypeRepr.h

@@ -220,15 +220,26 @@ class alignas(1 << TypeReprAlignInBits) TypeRepr
 class ErrorTypeRepr : public TypeRepr {
   SourceRange Range;
 
-  ErrorTypeRepr(SourceRange Range)
-      : TypeRepr(TypeReprKind::Error), Range(Range) {}
+  /// The original expression that failed to be resolved as a TypeRepr. Like
+  /// ErrorExpr's original expr, this exists to ensure that semantic
+  /// functionality can still work correctly, and is used to ensure we don't
+  /// drop nodes from the AST.
+  Expr *OriginalExpr;
+
+  ErrorTypeRepr(SourceRange Range, Expr *OriginalExpr)
+      : TypeRepr(TypeReprKind::Error), Range(Range),
+        OriginalExpr(OriginalExpr) {}
 
 public:
-  static ErrorTypeRepr *
-  create(ASTContext &Context, SourceRange Range) {
-    return new (Context) ErrorTypeRepr(Range);
+  static ErrorTypeRepr *create(ASTContext &Context, SourceRange Range,
+                               Expr *OriginalExpr = nullptr) {
+    return new (Context) ErrorTypeRepr(Range, OriginalExpr);
   }
 
+  /// Retrieve the original expression that failed to be resolved as a TypeRepr.
+  Expr *getOriginalExpr() const { return OriginalExpr; }
+  void setOriginalExpr(Expr *newExpr) { OriginalExpr = newExpr; }
+
   static bool classof(const TypeRepr *T) {
     return T->getKind() == TypeReprKind::Error;
   }

lib/AST/ASTDumper.cpp

@@ -4619,6 +4619,8 @@ class PrintTypeRepr : public TypeReprVisitor<PrintTypeRepr, void, Label>,
 
   void visitErrorTypeRepr(ErrorTypeRepr *T, Label label) {
     printCommon("type_error", label);
+    if (auto *originalExpr = T->getOriginalExpr())
+      printRec(originalExpr, Label::optional("original_expr"));
   }
 
   void visitAttributedTypeRepr(AttributedTypeRepr *T, Label label) {

lib/AST/ASTWalker.cpp

@@ -2267,6 +2267,12 @@ Pattern *Traversal::visitBoolPattern(BoolPattern *P) {
 
 #pragma mark Type representation traversal
 bool Traversal::visitErrorTypeRepr(ErrorTypeRepr *T) {
+  if (auto *originalExpr = T->getOriginalExpr()) {
+    auto *newExpr = doIt(originalExpr);
+    if (!newExpr)
+      return true;
+    T->setOriginalExpr(newExpr);
+  }
   return false;
 }
 

lib/AST/Pattern.cpp

@@ -221,13 +221,15 @@ namespace {
       return Action::Continue(E);
     }
 
+    PreWalkAction walkToTypeReprPre(TypeRepr *T) override {
+      // ErrorTypeReprs can contain invalid expressions.
+      return Action::Continue();
+    }
+
     // Don't walk into anything else.
     PreWalkResult<Stmt *> walkToStmtPre(Stmt *S) override {
       return Action::SkipNode(S);
     }
-    PreWalkAction walkToTypeReprPre(TypeRepr *T) override {
-      return Action::SkipNode();
-    }
     PreWalkAction walkToParameterListPre(ParameterList *PL) override {
       return Action::SkipNode();
     }

lib/Sema/PreCheckTarget.cpp

@@ -2347,14 +2347,17 @@ TypeExpr *PreCheckTarget::simplifyTypeExpr(Expr *E) {
       return nullptr;
     };
 
+    auto makeErrorTypeRepr = [&](Expr *E) -> ErrorTypeRepr * {
+      return ErrorTypeRepr::create(Ctx, E->getSourceRange(), E);
+    };
+
     TupleTypeRepr *ArgsTypeRepr = extractInputTypeRepr(AE->getArgsExpr());
     if (!ArgsTypeRepr) {
       Ctx.Diags.diagnose(AE->getArgsExpr()->getLoc(),
                          diag::expected_type_before_arrow);
-      auto ArgRange = AE->getArgsExpr()->getSourceRange();
-      auto ErrRepr = ErrorTypeRepr::create(Ctx, ArgRange);
+      auto ErrRepr = makeErrorTypeRepr(AE->getArgsExpr());
       ArgsTypeRepr =
-          TupleTypeRepr::create(Ctx, {ErrRepr}, ArgRange);
+          TupleTypeRepr::create(Ctx, {ErrRepr}, ErrRepr->getSourceRange());
     }
 
     TypeRepr *ThrownTypeRepr = nullptr;
@@ -2367,8 +2370,7 @@ TypeExpr *PreCheckTarget::simplifyTypeExpr(Expr *E) {
     if (!ResultTypeRepr) {
       Ctx.Diags.diagnose(AE->getResultExpr()->getLoc(),
                          diag::expected_type_after_arrow);
-      ResultTypeRepr =
-          ErrorTypeRepr::create(Ctx, AE->getResultExpr()->getSourceRange());
+      ResultTypeRepr = makeErrorTypeRepr(AE->getResultExpr());
     }
 
     auto NewTypeRepr = new (Ctx)

test/Parse/type_expr.swift

@@ -313,13 +313,24 @@ func testFunctionCollectionTypes() {
   _ = [() -> Int]()
   _ = [(Int) -> ()]()
 
-  _ = 2 + () -> Int // expected-error {{expected type before '->'}}
-  _ = () -> (Int, Int).2 // expected-error {{expected type after '->'}}
+  // This is folded as '(2 + ()) -> Int'
+  _ = 2 + () -> Int
+  // expected-error@-1 {{expected type before '->'}}
+  // expected-error@-2 {{cannot convert value of type '()' to expected argument type 'Int'}}
+
+  _ = () -> (Int, Int).2
+  // expected-error@-1 {{expected type after '->'}}
+  // expected-error@-2 {{value of tuple type '(Int, Int)' has no member '2'}}
+
   _ = (Int) -> Int // expected-error {{expected member name or initializer call after type name}} expected-note{{use '.self' to reference the type object}}
 
   _ = @convention(c) () -> Int // expected-error{{expected member name or initializer call after type name}} expected-note{{use '.self' to reference the type object}}
   _ = 1 + (@convention(c) () -> Int).self // expected-error{{cannot convert value of type '(@convention(c) () -> Int).Type' to expected argument type 'Int'}}
-  _ = (@autoclosure () -> Int) -> (Int, Int).2 // expected-error {{expected type after '->'}}
+
+  _ = (@autoclosure () -> Int) -> (Int, Int).2
+  // expected-error@-1 {{expected type after '->'}}
+  // expected-error@-2 {{value of tuple type '(Int, Int)' has no member '2'}}
+
   _ = ((@autoclosure () -> Int) -> (Int, Int)).1 // expected-error {{type '(@autoclosure () -> Int) -> (Int, Int)' has no member '1'}}
   _ = ((inout Int) -> Void).self
 
@@ -331,6 +342,18 @@ func testFunctionCollectionTypes() {
   let _ = [Int throws Int](); // expected-error{{'throws' may only occur before '->'}} expected-error {{consecutive statements on a line must be separated by ';'}}
 }
 
+func testInvalidArrowWithClosure() {
+  _ = { undefined -> undefined2 }
+  // expected-error@-1 {{cannot find 'undefined' in scope}}
+  // expected-error@-2 {{cannot find 'undefined2' in scope}}
+  // expected-error@-3 {{expected type before '->'}}
+  // expected-error@-4 {{expected type after '->'}}
+
+  () -> { let x: Int = "" }
+  // expected-error@-1 {{expected type after '->'}}
+  // expected-error@-2 {{cannot convert value of type 'String' to specified type 'Int'}}
+}
+
 func compositionType() {
   _ = P1 & P2 // expected-error {{expected member name or initializer call after type name}} expected-note{{use '.self'}} {{7-7=(}} {{14-14=).self}}
   _ = any P1 & P1 // expected-error {{expected member name or initializer call after type name}} expected-note{{use '.self'}} {{7-7=(}} {{18-18=).self}}

…-test/IDE/crashers/2681cf587abeb72.swift …IDE/crashers_fixed/2681cf587abeb72.swiftvalidation-test/IDE/crashers/2681cf587abeb72.swift renamed to validation-test/IDE/crashers_fixed/2681cf587abeb72.swift validation-test/IDE/crashers/2681cf587abeb72.swift renamed to validation-test/IDE/crashers_fixed/2681cf587abeb72.swift

@@ -1,4 +1,4 @@
 // {"kind":"complete","original":"2ad6764c","signature":"swift::constraints::Solution::getType(swift::KeyPathExpr const*, unsigned int) const","signatureAssert":"Assertion failed: (hasType(KP, I) && \"Expected type to have been set!\"), function getType"}
-// RUN: not --crash %target-swift-ide-test -code-completion -batch-code-completion -skip-filecheck -code-completion-diagnostics -source-filename %s
+// RUN: %target-swift-ide-test -code-completion -batch-code-completion -skip-filecheck -code-completion-diagnostics -source-filename %s
 0 -> \ a#^^#
 !

…test/IDE/crashers/7361e81dcc17636c.swift …DE/crashers_fixed/7361e81dcc17636c.swiftvalidation-test/IDE/crashers/7361e81dcc17636c.swift renamed to validation-test/IDE/crashers_fixed/7361e81dcc17636c.swift validation-test/IDE/crashers/7361e81dcc17636c.swift renamed to validation-test/IDE/crashers_fixed/7361e81dcc17636c.swift

@@ -1,3 +1,3 @@
 // {"kind":"complete","signature":"swift::constraints::TypeVarRefCollector::walkToStmtPre(swift::Stmt*)","signatureAssert":"Assertion failed: (result), function getClosureType"}
-// RUN: not --crash %target-swift-ide-test -code-completion -batch-code-completion -skip-filecheck -code-completion-diagnostics -source-filename %s
+// RUN: %target-swift-ide-test -code-completion -batch-code-completion -skip-filecheck -code-completion-diagnostics -source-filename %s
 a -> { let b = switch { case return } func c -> #^COMPLETE^#

validation-test/compiler_crashers_2_fixed/26193e19e8973c86.swift

@@ -0,0 +1,4 @@
+// {"kind":"typecheck","original":"01146903","signature":"swift::NamingPatternRequest::evaluate(swift::Evaluator&, swift::VarDecl*) const"}
+// RUN: not %target-swift-frontend -typecheck %s
+switch  {
+case let .a -> b b

validation-test/compiler_crashers_2_fixed/89b5f5bb4b65efe.swift

@@ -0,0 +1,6 @@
+// {"kind":"typecheck","original":"01146903","signature":"(anonymous namespace)::ConstraintWalker::walkToExprPost(swift::Expr*)"}
+// RUN: not %target-swift-frontend -typecheck %s
+switch <#expression#> {
+case let .a -> b:
+  b
+}