Merge pull request #84220 from hamishknight/times-arrow

[AST] Allow storing original expression in `ErrorTypeRepr`

Author: hamishknight

include/swift/AST/TypeRepr.h

@@ -216,35 +216,29 @@ class alignas(1 << TypeReprAlignInBits) TypeRepr
 /// A TypeRepr for a type with a syntax error.  Can be used both as a
 /// top-level TypeRepr and as a part of other TypeRepr.
 ///
-/// The client can either emit a detailed diagnostic at the construction time
-/// (in the parser) or store a zero-arg diagnostic in this TypeRepr to be
-/// emitted after parsing, during type resolution.
-///
 /// All uses of this type should be ignored and not re-diagnosed.
 class ErrorTypeRepr : public TypeRepr {
   SourceRange Range;
-  std::optional<ZeroArgDiagnostic> DelayedDiag;
 
-  ErrorTypeRepr(SourceRange Range, std::optional<ZeroArgDiagnostic> Diag)
-      : TypeRepr(TypeReprKind::Error), Range(Range), DelayedDiag(Diag) {}
+  /// 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;
 
-public:
-  static ErrorTypeRepr *
-  create(ASTContext &Context, SourceRange Range,
-         std::optional<ZeroArgDiagnostic> DelayedDiag = std::nullopt) {
-    assert((!DelayedDiag || Range) && "diagnostic needs a location");
-    return new (Context) ErrorTypeRepr(Range, DelayedDiag);
-  }
+  ErrorTypeRepr(SourceRange Range, Expr *OriginalExpr)
+      : TypeRepr(TypeReprKind::Error), Range(Range),
+        OriginalExpr(OriginalExpr) {}
 
-  static ErrorTypeRepr *
-  create(ASTContext &Context, SourceLoc Loc = SourceLoc(),
-         std::optional<ZeroArgDiagnostic> DelayedDiag = std::nullopt) {
-    return create(Context, SourceRange(Loc), DelayedDiag);
+public:
+  static ErrorTypeRepr *create(ASTContext &Context, SourceRange Range,
+                               Expr *OriginalExpr = nullptr) {
+    return new (Context) ErrorTypeRepr(Range, OriginalExpr);
   }
 
-  /// If there is a delayed diagnostic stored in this TypeRepr, consumes and
-  /// emits that diagnostic.
-  void dischargeDiagnostic(ASTContext &Context);
+  /// 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/AST/TypeRepr.cpp

@@ -970,15 +970,6 @@ void IntegerTypeRepr::printImpl(ASTPrinter &Printer,
   Printer.printText(getValue());
 }
 
-void ErrorTypeRepr::dischargeDiagnostic(swift::ASTContext &Context) {
-  if (!DelayedDiag)
-    return;
-
-  // Consume and emit the diagnostic.
-  Context.Diags.diagnose(Range.Start, *DelayedDiag).highlight(Range);
-  DelayedDiag = std::nullopt;
-}
-
 // See swift/Basic/Statistic.h for declaration: this enables tracing
 // TypeReprs, is defined here to avoid too much layering violation / circular
 // linkage dependency.

lib/Parse/ParseDecl.cpp

@@ -9935,7 +9935,8 @@ Parser::parseDeclSubscript(SourceLoc StaticLoc,
 
     if (ElementTy.isNull()) {
       // Always set an element type.
-      ElementTy = makeParserResult(ElementTy, ErrorTypeRepr::create(Context));
+      ElementTy = makeParserResult(
+          ElementTy, ErrorTypeRepr::create(Context, getTypeErrorLoc()));
     }
   }
 

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)

lib/Sema/TypeCheckType.cpp

@@ -2766,7 +2766,6 @@ NeverNullType TypeResolver::resolveType(TypeRepr *repr,
 
   switch (repr->getKind()) {
   case TypeReprKind::Error:
-    cast<ErrorTypeRepr>(repr)->dischargeDiagnostic(getASTContext());
     return ErrorType::get(getASTContext());
 
   case TypeReprKind::Attributed:

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}}

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#^^#
 !