Skip to content

Commit 8fcd4e6

Browse files
authored
Merge pull request #23311 from theblixguy/fix/throw-in-defer
[Diag] [Typechecker] Diagnose throw in defer properly
2 parents 155c6a9 + 731c516 commit 8fcd4e6

File tree

3 files changed

+66
-3
lines changed

3 files changed

+66
-3
lines changed

lib/Sema/TypeCheckError.cpp

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -892,6 +892,9 @@ class Context {
892892

893893
/// The pattern of a catch.
894894
CatchGuard,
895+
896+
/// A defer body
897+
DeferBody
895898
};
896899

897900
private:
@@ -917,6 +920,7 @@ class Context {
917920

918921
Kind TheKind;
919922
bool DiagnoseErrorOnTry = false;
923+
bool isInDefer = false;
920924
DeclContext *RethrowsDC = nullptr;
921925
InterpolatedStringLiteralExpr *InterpolatedString = nullptr;
922926

@@ -959,6 +963,12 @@ class Context {
959963
D->getInterfaceType(), D->hasImplicitSelfDecl() ? 2 : 1));
960964
}
961965

966+
static Context forDeferBody() {
967+
Context result(Kind::DeferBody);
968+
result.isInDefer = true;
969+
return result;
970+
}
971+
962972
static Context forInitializer(Initializer *init) {
963973
if (isa<DefaultArgumentInitializer>(init)) {
964974
return Context(Kind::DefaultArgument);
@@ -1029,13 +1039,20 @@ class Context {
10291039
}
10301040

10311041
static void diagnoseThrowInIllegalContext(TypeChecker &TC, ASTNode node,
1032-
StringRef description) {
1042+
StringRef description,
1043+
bool throwInDefer = false) {
10331044
if (auto *e = node.dyn_cast<Expr*>())
10341045
if (isa<ApplyExpr>(e)) {
10351046
TC.diagnose(e->getLoc(), diag::throwing_call_in_illegal_context,
10361047
description);
10371048
return;
10381049
}
1050+
1051+
if (throwInDefer) {
1052+
// Return because this would've already been diagnosed in TypeCheckStmt.
1053+
return;
1054+
}
1055+
10391056
TC.diagnose(node.getStartLoc(), diag::throw_in_illegal_context,
10401057
description);
10411058
}
@@ -1196,6 +1213,9 @@ class Context {
11961213
case Kind::CatchGuard:
11971214
diagnoseThrowInIllegalContext(TC, E, "a catch guard expression");
11981215
return;
1216+
case Kind::DeferBody:
1217+
diagnoseThrowInIllegalContext(TC, E, "a defer body", isInDefer);
1218+
return;
11991219
}
12001220
llvm_unreachable("bad context kind");
12011221
}
@@ -1223,6 +1243,7 @@ class Context {
12231243
case Kind::DefaultArgument:
12241244
case Kind::CatchPattern:
12251245
case Kind::CatchGuard:
1246+
case Kind::DeferBody:
12261247
assert(!DiagnoseErrorOnTry);
12271248
// Diagnosed at the call sites.
12281249
return;
@@ -1670,7 +1691,10 @@ void TypeChecker::checkFunctionErrorHandling(AbstractFunctionDecl *fn) {
16701691
PrettyStackTraceDecl debugStack("checking error handling for", fn);
16711692
#endif
16721693

1673-
CheckErrorCoverage checker(*this, Context::forFunction(fn));
1694+
auto isDeferBody = isa<FuncDecl>(fn) && cast<FuncDecl>(fn)->isDeferBody();
1695+
auto context =
1696+
isDeferBody ? Context::forDeferBody() : Context::forFunction(fn);
1697+
CheckErrorCoverage checker(*this, context);
16741698

16751699
// If this is a debugger function, suppress 'try' marking at the top level.
16761700
if (fn->getAttrs().hasAttribute<LLDBDebuggerFunctionAttr>())

lib/Sema/TypeCheckStmt.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -545,6 +545,12 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
545545
}
546546

547547
Stmt *visitThrowStmt(ThrowStmt *TS) {
548+
// If the throw is in a defer, then it isn't valid.
549+
if (isInDefer()) {
550+
TC.diagnose(TS->getThrowLoc(), diag::jump_out_of_defer, "throw");
551+
return nullptr;
552+
}
553+
548554
// Coerce the operand to the exception type.
549555
auto E = TS->getSubExpr();
550556

test/stmt/statements.swift

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -356,13 +356,46 @@ func test_defer(_ a : Int) {
356356

357357
class SomeTestClass {
358358
var x = 42
359-
359+
360360
func method() {
361361
defer { x = 97 } // self. not required here!
362362
// expected-warning@-1 {{'defer' statement before end of scope always executes immediately}}{{5-10=do}}
363363
}
364364
}
365365

366+
enum DeferThrowError: Error {
367+
case someError
368+
}
369+
370+
func throwInDefer() {
371+
defer { throw DeferThrowError.someError } // expected-error {{'throw' cannot transfer control out of a defer statement}}
372+
print("Foo")
373+
}
374+
375+
func throwingFuncInDefer1() throws {
376+
defer { try throwingFunctionCalledInDefer() } // expected-error {{errors cannot be thrown out of a defer body}}
377+
print("Bar")
378+
}
379+
380+
func throwingFuncInDefer2() throws {
381+
defer { throwingFunctionCalledInDefer() } // expected-error {{errors cannot be thrown out of a defer body}}
382+
print("Bar")
383+
}
384+
385+
func throwingFuncInDefer3() {
386+
defer { try throwingFunctionCalledInDefer() } // expected-error {{errors cannot be thrown out of a defer body}}
387+
print("Bar")
388+
}
389+
390+
func throwingFuncInDefer4() {
391+
defer { throwingFunctionCalledInDefer() } // expected-error {{errors cannot be thrown out of a defer body}}
392+
print("Bar")
393+
}
394+
395+
func throwingFunctionCalledInDefer() throws {
396+
throw DeferThrowError.someError
397+
}
398+
366399
class SomeDerivedClass: SomeTestClass {
367400
override init() {
368401
defer {

0 commit comments

Comments
 (0)