From 775191cd946472f47b35ad63953f51426ba7430e Mon Sep 17 00:00:00 2001
From: Holly Borla <hborla@apple.com>
Date: Sat, 15 Jun 2024 20:32:27 -0700
Subject: [PATCH 1/2] [TypeCheckEffects] Diagnose type mismatches for thrown
 errors in contexts that throw `Never`.

Previously, this mistake slipped through the effects checker because it used
a fallback type of `any Error` if a catch node had a null thrown error type.
This change also fixes an issue where thrown error type mismatches were not
diagnosed at all in autoclosures.
---
 lib/Sema/TypeCheckEffects.cpp | 15 +++++++++++++++
 test/stmt/typed_throws.swift  | 26 ++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/lib/Sema/TypeCheckEffects.cpp b/lib/Sema/TypeCheckEffects.cpp
index 61c324b401a55..fedad839001fe 100644
--- a/lib/Sema/TypeCheckEffects.cpp
+++ b/lib/Sema/TypeCheckEffects.cpp
@@ -3010,9 +3010,24 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>
   Type getCaughtErrorTypeAt(SourceLoc loc) {
     auto dc = CurContext.getDeclContext();
     auto module = dc->getParentModule();
+
+    // Autoclosures can't be found via ASTScope lookup.
+    if (CurContext.isAutoClosure()) {
+      auto *closure = dyn_cast<AutoClosureExpr>(CurContext.getDeclContext());
+      if (auto type = closure->getEffectiveThrownType())
+        return *type;
+
+      // Otherwise, the closure does not throw.
+      return Ctx.getNeverType();
+    }
+
     if (CatchNode catchNode = ASTScope::lookupCatchNode(module, loc)) {
       if (auto caughtType = catchNode.getThrownErrorTypeInContext(Ctx))
         return *caughtType;
+
+      // If a catch node returns null for its thrown error type, we're
+      // in a non-throwing context.
+      return Ctx.getNeverType();
     }
 
     // Fall back to the error existential.
diff --git a/test/stmt/typed_throws.swift b/test/stmt/typed_throws.swift
index ab1fb7776c9d8..ecc4c04bd3931 100644
--- a/test/stmt/typed_throws.swift
+++ b/test/stmt/typed_throws.swift
@@ -311,3 +311,29 @@ func testDoCatchInClosure(cond: Bool, x: ThrowingMembers) {
     }
   }
 }
+
+func takesThrowingAutoclosure(_: @autoclosure () throws(MyError) -> Int) {}
+func takesNonThrowingAutoclosure(_: @autoclosure () throws(Never) -> Int) {}
+
+func getInt() throws -> Int { 0 }
+
+func throwingAutoclosures() {
+  takesThrowingAutoclosure(try getInt())
+  // expected-error@-1 {{thrown expression type 'any Error' cannot be converted to error type 'MyError'}}
+
+  takesNonThrowingAutoclosure(try getInt())
+  // expected-error@-1 {{thrown expression type 'any Error' cannot be converted to error type 'Never'}}
+}
+
+func noThrow() throws(Never) {
+  throw MyError.epicFailed
+  // expected-error@-1 {{thrown expression type 'MyError' cannot be converted to error type 'Never'}}
+
+  try doSomething()
+  // expected-error@-1 {{thrown expression type 'MyError' cannot be converted to error type 'Never'}}
+
+  do throws(Never) {
+    throw MyError.epicFailed
+    // expected-error@-1 {{thrown expression type 'MyError' cannot be converted to error type 'Never'}}
+  } catch {}
+}

From 8d6434410b591cbf6f8cfc0c729ab27e8aa15ebc Mon Sep 17 00:00:00 2001
From: Holly Borla <hborla@apple.com>
Date: Tue, 18 Jun 2024 21:31:44 -0700
Subject: [PATCH 2/2] [Parser] Hoist up 'try' exprs out from sequence exprs in
 the parser.

If the left-most sequence expr is a 'try', hoist it up to turn
'(try x) + y' into 'try (x + y)'. This is necessary to do in the
parser because 'try' nodes are represented in the ASTScope tree
to look up catch nodes. The scope tree must be syntactic because
it's constructed before sequence folding happens during preCheckExpr.
Otherwise, catch node lookup would find the incorrect catch node for
'try x + y' at the source location for 'y'.

'try' has restrictions for where it can appear within a sequence
expr. This is still diagnosed in TypeChecker::foldSequence.
---
 lib/Parse/ParseExpr.cpp | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/lib/Parse/ParseExpr.cpp b/lib/Parse/ParseExpr.cpp
index 71a9d38cd0331..e402269c43711 100644
--- a/lib/Parse/ParseExpr.cpp
+++ b/lib/Parse/ParseExpr.cpp
@@ -372,6 +372,23 @@ ParserResult<Expr> Parser::parseExprSequence(Diag<> Message,
   if (SequencedExprs.size() == 1)
     return makeParserResult(SequenceStatus, SequencedExprs[0]);
 
+  // If the left-most sequence expr is a 'try', hoist it up to turn
+  // '(try x) + y' into 'try (x + y)'. This is necessary to do in the
+  // parser because 'try' nodes are represented in the ASTScope tree
+  // to look up catch nodes. The scope tree must be syntactic because
+  // it's constructed before sequence folding happens during preCheckExpr.
+  // Otherwise, catch node lookup would find the incorrect catch node for
+  // 'try x + y' at the source location for 'y'.
+  //
+  // 'try' has restrictions for where it can appear within a sequence
+  // expr. This is still diagnosed in TypeChecker::foldSequence.
+  if (auto *tryEval = dyn_cast<AnyTryExpr>(SequencedExprs[0])) {
+    SequencedExprs[0] = tryEval->getSubExpr();
+    auto *sequence = SequenceExpr::create(Context, SequencedExprs);
+    tryEval->setSubExpr(sequence);
+    return makeParserResult(SequenceStatus, tryEval);
+  }
+
   return makeParserResult(SequenceStatus,
                           SequenceExpr::create(Context, SequencedExprs));
 }