Skip to content

Commit ae1c023

Browse files
committed
Fix effects checking for rethrowing functions involving parameter packs
Effects checking for rethrowing functions was using unsubstituted interface types for the parameter list. When a rethrows function has parameter pack arguments, the interface type may have a different number of parameters from the resulting argument list, causing the effects checking to bail out early. This has been wrong since the introduction of parameter packs, but recent refactoring of effects checking ended up promoting this bug to a compiler crash. Fix the bug, and make sure we don't crash if the effects checker hits an issue here. Fixes rdar://116740385.
1 parent ffa5ba7 commit ae1c023

File tree

2 files changed

+35
-3
lines changed

2 files changed

+35
-3
lines changed

lib/Sema/TypeCheckEffects.cpp

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1173,14 +1173,21 @@ class ApplyClassifier {
11731173
// because it only counts for rethrows/reasync purposes if it
11741174
// lines up with a throws/async function parameter in the
11751175
// original type.
1176-
auto *origType = fnRef.getType()->getAs<AnyFunctionType>();
1177-
if (!origType) {
1176+
Type fnInterfaceType = fnRef.getType();
1177+
if (!fnInterfaceType) {
11781178
result.merge(Classification::forInvalidCode());
11791179
return;
11801180
}
11811181

11821182
// Use the most significant result from the arguments.
1183-
auto params = origType->getParams();
1183+
auto *fnSubstType = fnInterfaceType.subst(fnRef.getSubstitutions())
1184+
->getAs<AnyFunctionType>();
1185+
if (!fnSubstType) {
1186+
result.merge(Classification::forInvalidCode());
1187+
return;
1188+
}
1189+
1190+
auto params = fnSubstType->getParams();
11841191
if (params.size() != args->size()) {
11851192
result.merge(Classification::forInvalidCode());
11861193
return;
@@ -2914,6 +2921,9 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>
29142921

29152922
auto asyncKind = classification.getConditionalKind(EffectKind::Async);
29162923
E->setNoAsync(asyncKind == ConditionalEffectKind::None);
2924+
} else {
2925+
E->setThrows(ThrownErrorDestination());
2926+
E->setNoAsync(true);
29172927
}
29182928

29192929
// If current apply expression did not type-check, don't attempt

test/decl/func/rethrows.swift

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -659,3 +659,25 @@ protocol P1 {
659659
func open(p: any P1, s: any Sequence) throws {
660660
_ = p.test(s).map(\.id)
661661
}
662+
663+
// Rethrows checking and parameter packs, oh my.
664+
func rethrowsWithParameterPacks<each Arg>(_ arguments: repeat each Arg, body: () throws -> Void) rethrows { }
665+
666+
enum MyError: Error {
667+
case fail
668+
}
669+
670+
func testRethrowsWithParameterPacks() throws {
671+
try rethrowsWithParameterPacks { throw MyError.fail }
672+
rethrowsWithParameterPacks { }
673+
674+
try rethrowsWithParameterPacks(1) { throw MyError.fail }
675+
rethrowsWithParameterPacks(1) { }
676+
677+
try rethrowsWithParameterPacks(1, "hello") { throw MyError.fail }
678+
rethrowsWithParameterPacks(1, "hello") { }
679+
680+
rethrowsWithParameterPacks { throw MyError.fail }
681+
// expected-error@-1{{call can throw but is not marked with 'try'}}
682+
// expected-note@-2{{call is to 'rethrows' function, but argument function can throw}}
683+
}

0 commit comments

Comments
 (0)