[AST] Walk ErrorExpr's original expr in ASTWalker

We set an original expression on ErrorExpr for cases where we have
something semantically invalid that doesn't fit into the AST, but is
still something that the user has explicitly written. For example
this is how we represent unresolved dots without member names (`x.`).
We still want to type-check the underlying expression though since
it can provide useful diagnostics and allows semantic functionality
such as completion and cursor info to work correctly.

rdar://130771574

Author: hamishknight

include/swift/AST/Expr.h

@@ -610,7 +610,8 @@ class ErrorExpr : public Expr {
 
   SourceRange getSourceRange() const { return Range; }
   Expr *getOriginalExpr() const { return OriginalExpr; }
-  
+  void setOriginalExpr(Expr *newExpr) { OriginalExpr = newExpr; }
+
   static bool classof(const Expr *E) {
     return E->getKind() == ExprKind::Error;
   }

lib/AST/ASTWalker.cpp

@@ -616,7 +616,15 @@ class Traversal : public ASTVisitor<Traversal, Expr*, Stmt*,
     }                                                      \
   } while (false)
 
-  Expr *visitErrorExpr(ErrorExpr *E) { return E; }
+  Expr *visitErrorExpr(ErrorExpr *E) {
+    if (auto *origExpr = E->getOriginalExpr()) {
+      auto *newOrigExpr = doIt(origExpr);
+      if (!newOrigExpr)
+        return nullptr;
+      E->setOriginalExpr(newOrigExpr);
+    }
+    return E;
+  }
   Expr *visitCodeCompletionExpr(CodeCompletionExpr *E) {
     if (Expr *baseExpr = E->getBase()) {
       Expr *newBaseExpr = doIt(baseExpr);

lib/IDE/Formatting.cpp

@@ -587,15 +587,6 @@ class RangeWalker: protected ASTWalker {
   }
 
   PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
-    // Walk through error expressions.
-    if (auto *EE = dyn_cast<ErrorExpr>(E)) {
-      if (auto *OE = EE->getOriginalExpr()) {
-        llvm::SaveAndRestore<ASTWalker::ParentTy>(Parent, EE);
-        OE->walk(*this);
-      }
-      return Action::Continue(E);
-    }
-
     if (E->isImplicit())
       return Action::Continue(E);
 
@@ -1491,17 +1482,6 @@ class FormatWalker : public ASTWalker {
       }
     }
 
-    // Walk through error expressions.
-    if (auto *EE = dyn_cast<ErrorExpr>(E)) {
-      if (Action.shouldVisitChildren()) {
-        if (auto *OE = EE->getOriginalExpr()) {
-          llvm::SaveAndRestore<ASTWalker::ParentTy>(Parent, EE);
-          OE->walk(*this);
-        }
-        return Action::SkipNode(E);
-      }
-    }
-
     // FIXME: We ought to be able to use Action::VisitChildrenIf here, but we'd
     // need to ensure the AST is walked in source order (currently not the case
     // for things like postfix operators).

lib/Sema/CompletionContextFinder.cpp

@@ -49,11 +49,6 @@ CompletionContextFinder::walkToExprPre(Expr *E) {
 
   if (auto *Error = dyn_cast<ErrorExpr>(E)) {
     Contexts.push_back({ContextKind::ErrorExpression, E});
-    if (auto *OrigExpr = Error->getOriginalExpr()) {
-      OrigExpr->walk(*this);
-      if (hasCompletionExpr())
-        return Action::Stop();
-    }
   }
 
   if (auto *CCE = dyn_cast<CodeCompletionExpr>(E)) {

lib/Sema/PreCheckTarget.cpp

@@ -1252,7 +1252,8 @@ class PreCheckTarget final : public ASTWalker {
         }
 
         if (isa<UnresolvedDotExpr>(parentExpr) ||
-            isa<MemberRefExpr>(parentExpr)) {
+            isa<MemberRefExpr>(parentExpr) ||
+            isa<ErrorExpr>(parentExpr)) {
           return true;
         } else if (auto *SE = dyn_cast<SubscriptExpr>(parentExpr)) {
           // 'super[]' is valid, but 'x[super]' is not.

test/Constraints/diagnostics.swift

@@ -1578,3 +1578,10 @@ func testAddMemberVsRemoveCall() {
   let b = Foo_74617()
   let c = (a + b).bar() // expected-error {{cannot call value of non-function type 'Float'}} {{22-24=}}
 }
+
+// Make sure we can still type-check the closure.
+do {
+  _ = {
+    let x: String = 0 // expected-error {{cannot convert value of type 'Int' to specified type 'String'}}
+  }. // expected-error {{expected member name following '.'}}
+}

test/Parse/recovery.swift

@@ -550,7 +550,8 @@ func exprPostfix2() {
 
 class ExprSuper {
   init() {
-    super. // expected-error {{expected member name following '.'}} 
+    super. // expected-error {{expected member name following '.'}}
+    // expected-error@-1 {{'super' cannot be used in class 'ExprSuper' because it has no superclass}}
   }
 }
 

test/SourceKit/CursorInfo/rdar130771574.swift

@@ -0,0 +1,10 @@
+struct S {
+  var x: Int
+}
+
+// Make sure we can still resolve 'x' even with the trailing dot.
+func foo(_ s: S) {
+  s.x.
+  // RUN: %sourcekitd-test -req=cursor -pos=%(line-1):5 %s -- %s -module-name main | %FileCheck %s
+}
+// CHECK: s:4main1SV1xSivp

test/StringProcessing/Parse/forward-slash-regex.swift

@@ -445,6 +445,7 @@ _ = ^/"/"
 _ = ^/"[/"
 // expected-error@-1 {{'^' is not a prefix unary operator}}
 // expected-error@-2 {{unterminated string literal}}
+// expected-error@-3 {{cannot parse regular expression: expected custom character class members}}
 
 _ = (^/)("/")
 

validation-test/IDE/crashers/dc8881dce3e968b.swift renamed to validation-test/IDE/crashers_fixed/dc8881dce3e968b.swift

@@ -1,4 +1,4 @@
 // {"kind":"complete","original":"6384b7fc","signature":"swift::TypeChecker::typeCheckForCodeCompletion(swift::constraints::SyntacticElementTarget&, bool, llvm::function_ref<void (swift::constraints::Solution const&)>)","signatureAssert":"Assertion failed: (fallback->E != expr), function typeCheckForCodeCompletion"}
-// 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
 func a -> b
 a > (c > a #^^# )'