Skip to content

Commit 3892c55

Browse files
Fixes bug in exhaustive matching check SR-11160.
Also fixes rdar://problem/53312914. The fact that ParenType is being used to distinguish the two cases makes me uncomfortable but I don't have better ideas. Related: - Fixed a bug in pattern projection which I encountered once I fixed SR-11160. This is the 3 line change around conArgSpaces. - Opened SR-11212 for ill-typed patterns that are permitted to compile. Unrelated cleanup: - Removed a redundant switch case in 'isSubspace'. - Added names for referenced papers for faster lookup.
1 parent bbf7e7a commit 3892c55

File tree

2 files changed

+56
-22
lines changed

2 files changed

+56
-22
lines changed

lib/Sema/TypeCheckSwitchStmt.cpp

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,13 @@ namespace {
4747

4848
namespace {
4949

50-
/// The SpaceEngine encapsulates an algorithm for computing the exhaustiveness
51-
/// of a switch statement using an algebra of spaces described by Fengyun Liu
52-
/// and an algorithm for computing warnings for pattern matching by
53-
/// Luc Maranget.
50+
/// The SpaceEngine encapsulates
51+
///
52+
/// 1. An algorithm for computing the exhaustiveness of a switch statement
53+
/// using an algebra of spaces based on Fengyun Liu's
54+
/// "A Generic Algorithm for Checking Exhaustivity of Pattern Matching".
55+
/// 2. An algorithm for computing warnings for pattern matching based on
56+
/// Luc Maranget's "Warnings for pattern matching".
5457
///
5558
/// The main algorithm centers around the computation of the difference and
5659
/// the containment of the "Spaces" given in each case, which reduces the
@@ -298,7 +301,6 @@ namespace {
298301
}
299302

300303
switch (PairSwitch(getKind(), other.getKind())) {
301-
PAIRCASE (SpaceKind::Disjunct, SpaceKind::Empty):
302304
PAIRCASE (SpaceKind::Disjunct, SpaceKind::Type):
303305
PAIRCASE (SpaceKind::Disjunct, SpaceKind::Constructor):
304306
PAIRCASE (SpaceKind::Disjunct, SpaceKind::Disjunct):
@@ -748,8 +750,6 @@ namespace {
748750
auto children = E->getAllElements();
749751
std::transform(children.begin(), children.end(),
750752
std::back_inserter(arr), [&](EnumElementDecl *eed) {
751-
SmallVector<Space, 4> constElemSpaces;
752-
753753
// We need the interface type of this enum case but it may
754754
// not have been computed.
755755
if (!eed->hasInterfaceType()) {
@@ -768,17 +768,28 @@ namespace {
768768
return Space();
769769
}
770770

771+
// .e(a: X, b: X) -> (a: X, b: X)
772+
// .f((a: X, b: X)) -> ((a: X, b: X)
771773
auto eedTy = tp->getCanonicalType()
772774
->getTypeOfMember(E->getModuleContext(), eed,
773775
eed->getArgumentInterfaceType());
776+
SmallVector<Space, 4> constElemSpaces;
774777
if (eedTy) {
775778
if (auto *TTy = eedTy->getAs<TupleType>()) {
776-
// Decompose the payload tuple into its component type spaces.
777-
llvm::transform(TTy->getElements(),
778-
std::back_inserter(constElemSpaces),
779-
[&](TupleTypeElt elt) {
780-
return Space::forType(elt.getType(), elt.getName());
781-
});
779+
if (isa<ParenType>(eedTy.getPointer())) {
780+
// We had an actual tuple!
781+
SmallVector<Space, 4> innerElemSpaces;
782+
for (auto &elt: TTy->getElements())
783+
innerElemSpaces.push_back(
784+
Space::forType(elt.getType(), elt.getName()));
785+
constElemSpaces.push_back(
786+
Space::forConstructor(TTy, Identifier(), innerElemSpaces));
787+
} else {
788+
// We're just looking at fields of a constructor here.
789+
for (auto &elt: TTy->getElements())
790+
constElemSpaces.push_back(
791+
Space::forType(elt.getType(), elt.getName()));
792+
}
782793
} else if (auto *TTy = dyn_cast<ParenType>(eedTy.getPointer())) {
783794
constElemSpaces.push_back(
784795
Space::forType(TTy->getUnderlyingType(), Identifier()));
@@ -1458,9 +1469,7 @@ namespace {
14581469
if (argTupleSpace.isEmpty())
14591470
return Space();
14601471
assert(argTupleSpace.getKind() == SpaceKind::Constructor);
1461-
conArgSpace.insert(conArgSpace.end(),
1462-
argTupleSpace.getSpaces().begin(),
1463-
argTupleSpace.getSpaces().end());
1472+
conArgSpace.push_back(argTupleSpace);
14641473
} else {
14651474
conArgSpace.push_back(projectPattern(TC, SP));
14661475
}

test/Sema/exhaustive_switch.swift

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ enum Result<T> {
4848
}
4949
}
5050

51-
func overParenthesized() {
51+
func parenthesized() {
5252
// SR-7492: Space projection needs to treat extra paren-patterns explicitly.
5353
let x: Result<(Result<Int>, String)> = .Ok((.Ok(1), "World"))
5454
switch x {
@@ -1177,11 +1177,36 @@ func sr10301_as(_ foo: SR10301<String,(Int,Error)>) {
11771177
}
11781178

11791179
func sr11160() {
1180-
switch Optional<(Int, Int)>((5, 6)) { // expected-error {{switch must be exhaustive}}
1181-
// expected-note@-1 {{add missing case: '.some(_, _)'}}
1182-
case let b?: print(b)
1183-
case nil: print(0)
1184-
}
1180+
switch Optional<(Int, Int)>((5, 6)) {
1181+
case let b?: print(b)
1182+
case nil: print(0)
1183+
}
1184+
}
1185+
1186+
enum Z {
1187+
case z1(a: Int)
1188+
case z2(a: Int, b: Int)
1189+
case z3((c: Int, d: Int))
1190+
}
1191+
1192+
func sr11160_extra() {
1193+
switch Z.z1(a: 1) { // expected-error {{switch must be exhaustive}}
1194+
// expected-note@-1 {{add missing case: '.z1(let a)'}}
1195+
case .z2(_, _): ()
1196+
case .z3(_): ()
1197+
}
1198+
1199+
switch Z.z1(a: 1) { // expected-error {{switch must be exhaustive}}
1200+
// expected-note@-1 {{add missing case: '.z2(let a, let b)'}}
1201+
case .z1(_): ()
1202+
case .z3(_): ()
1203+
}
1204+
1205+
switch Z.z1(a: 1) { // expected-error {{switch must be exhaustive}}
1206+
// expected-note@-1 {{add missing case: '.z3((let c, let d))'}}
1207+
case .z1(_): ()
1208+
case .z2(_, _): ()
1209+
}
11851210
}
11861211

11871212
// SR-11212 tests: Some of the tests here rely on compiler bugs related to

0 commit comments

Comments
 (0)