[Sema] Fold SequenceExpr in pre-checking pre-walk

Doing it in the post-walk meant we ended up
walking the children twice, which lead to duplicate
diagnostics and incorrect inference of the
level of application for function references. Move
it to the pre-walk, ensuring that we resolve any
operator references before folding.

Author: hamishknight

lib/Sema/PreCheckExpr.cpp

@@ -1056,9 +1056,6 @@ namespace {
     /// Keep track of acceptable DiscardAssignmentExpr's.
     llvm::SmallPtrSet<DiscardAssignmentExpr*, 2> CorrectDiscardAssignmentExprs;
 
-    /// The current number of nested \c SequenceExprs that we're within.
-    unsigned SequenceExprDepth = 0;
-
     /// The current number of nested \c SingleValueStmtExprs that we're within.
     unsigned SingleValueStmtExprDepth = 0;
 
@@ -1127,6 +1124,16 @@ namespace {
     PreWalkResult<Expr *> walkToExprPre(Expr *expr) override {
       auto &diags = Ctx.Diags;
 
+      // Fold sequence expressions.
+      if (auto *seqExpr = dyn_cast<SequenceExpr>(expr)) {
+        auto result = TypeChecker::foldSequence(seqExpr, DC);
+        result = result->walk(*this);
+        if (!result)
+          return Action::Stop();
+        // Already walked.
+        return Action::SkipNode(result);
+      }
+
       // FIXME(diagnostics): `InOutType` could appear here as a result
       // of successful re-typecheck of the one of the sub-expressions e.g.
       // `let _: Int = { (s: inout S) in s.bar() }`. On the first
@@ -1158,11 +1165,9 @@ namespace {
           return Action::Stop();
 
         // If we're going to recurse, record this expression on the stack.
-        if (recursive) {
-          if (isa<SequenceExpr>(expr))
-            SequenceExprDepth++;
+        if (recursive)
           ExprStack.push_back(expr);
-        }
+
         return Action::VisitNodeIf(recursive, expr);
       };
 
@@ -1292,17 +1297,6 @@ namespace {
       assert(ExprStack.back() == expr);
       ExprStack.pop_back();
 
-      // Fold sequence expressions.
-      if (auto *seqExpr = dyn_cast<SequenceExpr>(expr)) {
-        auto result = TypeChecker::foldSequence(seqExpr, DC);
-        SequenceExprDepth--;
-        result = result->walk(*this);
-        if (!result)
-          return Action::Stop();
-
-        return Action::Continue(result);
-      }
-
       // Type check the type parameters in an UnresolvedSpecializeExpr.
       if (auto *us = dyn_cast<UnresolvedSpecializeExpr>(expr)) {
         if (auto *typeExpr = simplifyUnresolvedSpecializeExpr(us))
@@ -1426,12 +1420,9 @@ namespace {
         return Action::Continue(simplified);
 
       // Diagnose a '_' that isn't on the immediate LHS of an assignment. We
-      // skip diagnostics if we've explicitly marked the expression as valid,
-      // or if we're inside a SequenceExpr (since the whole tree will be
-      // re-checked when we finish folding anyway).
+      // skip diagnostics if we've explicitly marked the expression as valid.
       if (auto *DAE = dyn_cast<DiscardAssignmentExpr>(expr)) {
-        if (!CorrectDiscardAssignmentExprs.count(DAE) &&
-            SequenceExprDepth == 0) {
+        if (!CorrectDiscardAssignmentExprs.count(DAE)) {
           ctx.Diags.diagnose(expr->getLoc(),
                              diag::discard_expr_outside_of_assignment);
           return Action::Stop();
@@ -1688,7 +1679,7 @@ bool PreCheckExpression::possiblyInTypeContext(Expr *E) {
 /// been explicitly marked as correct, and the current AST state allows it.
 bool PreCheckExpression::canSimplifyDiscardAssignmentExpr(
     DiscardAssignmentExpr *DAE) {
-  return !CorrectDiscardAssignmentExprs.count(DAE) && SequenceExprDepth == 0 &&
+  return !CorrectDiscardAssignmentExprs.count(DAE) &&
          possiblyInTypeContext(DAE);
 }
 

lib/Sema/TypeCheckExpr.cpp

@@ -631,6 +631,15 @@ swift::DefaultTypeRequest::evaluate(Evaluator &evaluator,
 }
 
 Expr *TypeChecker::foldSequence(SequenceExpr *expr, DeclContext *dc) {
+  // First resolve any unresolved decl references in operator positions.
+  for (auto i : indices(expr->getElements())) {
+    if (i % 2 == 0)
+      continue;
+    auto *elt = expr->getElement(i);
+    if (auto *UDRE = dyn_cast<UnresolvedDeclRefExpr>(elt))
+      elt = TypeChecker::resolveDeclRefExpr(UDRE, dc);
+    expr->setElement(i, elt);
+  }
   ArrayRef<Expr*> Elts = expr->getElements();
   assert(Elts.size() > 1 && "inadequate number of elements in sequence");
   assert((Elts.size() & 1) == 1 && "even number of elements in sequence");

test/Constraints/argument_matching.swift

@@ -1810,3 +1810,14 @@ func test_extraneous_argument_with_inout() {
   var x: Int = 0
   test(42, &x) // expected-error {{extra argument in call}}
 }
+
+// https://github.com/swiftlang/swift/issues/75527
+struct Issue75527 {
+  func foo(x: Int) {}
+
+  func bar() {
+    typealias Magic<T> = T
+    let fn = Issue75527.foo(self) as Magic
+    fn(0) // Make sure the argument label does not escape here.
+  }
+}

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

@@ -52,7 +52,6 @@ do {
   _=/0/
   // expected-error@-1 {{'_' can only appear in a pattern or on the left side of an assignment}}
   // expected-error@-2 {{cannot find operator '=/' in scope}}
-  // expected-error@-3 {{'/' is not a postfix unary operator}}
 }
 
 // No closing '/' so a prefix operator.

test/expr/primary/super/unsupported.swift

@@ -42,8 +42,7 @@ class Derived: Base {
     // expected-error@+1 {{'super' may be used only to access a superclass member, subscript, or initializer}}
     let _ = self[super]
 
-    // FIXME: Duplicate diagnostic because expression is re-pre-checked after folding initial sequence expression.
-    // expected-error@+1 2 {{'super' may be used only to access a superclass member, subscript, or initializer}}
+    // expected-error@+1 {{'super' may be used only to access a superclass member, subscript, or initializer}}
     _ = (0, super)
 
     func nested() {