Skip to content

Commit 68fb3b1

Browse files
varungandhi-applevarungandhi-apple
authored
Merge pull request #26357 from varungandhi-apple/vg-fix-nested-exhaustiveness-check
Correctly distinguish between tuple fields and associated values in patterns
2 parents 414af34 + 881de88 commit 68fb3b1

File tree

8 files changed

+377
-52
lines changed

8 files changed

+377
-52
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3393,6 +3393,18 @@ ERROR(pattern_type_mismatch_context,none,
33933393

33943394
ERROR(tuple_pattern_in_non_tuple_context,none,
33953395
"tuple pattern cannot match values of the non-tuple type %0", (Type))
3396+
WARNING(matching_pattern_with_many_assoc_values, none,
3397+
"cannot match several associated values at once, "
3398+
"implicitly tupling the associated values and trying to match that "
3399+
"instead", ())
3400+
WARNING(matching_tuple_pattern_with_many_assoc_values,none,
3401+
"a tuple pattern cannot match several associated values at once, "
3402+
"implicitly tupling the associated values and trying to match "
3403+
"that instead", ())
3404+
WARNING(matching_many_patterns_with_tupled_assoc_value,none,
3405+
"the enum case has a single tuple as an associated value, but "
3406+
"there are several patterns here, implicitly tupling the patterns "
3407+
"and trying to match that instead", ())
33963408
ERROR(closure_argument_list_tuple,none,
33973409
"contextual closure type %0 expects %1 argument%s1, "
33983410
"but %2 %select{were|was}3 used in closure body", (Type, unsigned, unsigned, bool))

‎include/swift/AST/PatternNodes.def

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,17 +32,19 @@
3232
#define LAST_PATTERN(Id)
3333
#endif
3434

35-
PATTERN(Paren, Pattern)
36-
PATTERN(Tuple, Pattern)
37-
PATTERN(Named, Pattern)
38-
PATTERN(Any, Pattern)
39-
PATTERN(Typed, Pattern)
40-
PATTERN(Var, Pattern)
41-
REFUTABLE_PATTERN(Is, Pattern)
42-
REFUTABLE_PATTERN(EnumElement, Pattern)
43-
REFUTABLE_PATTERN(OptionalSome, Pattern)
44-
REFUTABLE_PATTERN(Bool, Pattern)
45-
REFUTABLE_PATTERN(Expr, Pattern)
35+
// Metavars: x (variable), pat (pattern), e (expression)
36+
PATTERN(Paren, Pattern) // (pat)
37+
PATTERN(Tuple, Pattern) // (pat1, ..., patN), N >= 1
38+
PATTERN(Named, Pattern) // let pat, var pat
39+
PATTERN(Any, Pattern) // _
40+
PATTERN(Typed, Pattern) // pat : type
41+
PATTERN(Var, Pattern) // x
42+
REFUTABLE_PATTERN(Is, Pattern) // x is myclass
43+
REFUTABLE_PATTERN(EnumElement, Pattern) // .mycase(pat1, ..., patN)
44+
// MyType.mycase(pat1, ..., patN)
45+
REFUTABLE_PATTERN(OptionalSome, Pattern) // pat? nil
46+
REFUTABLE_PATTERN(Bool, Pattern) // true, false
47+
REFUTABLE_PATTERN(Expr, Pattern) // e
4648
LAST_PATTERN(Expr)
4749

4850
#undef REFUTABLE_PATTERN

‎lib/Sema/TypeCheckPattern.cpp

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -994,6 +994,46 @@ bool TypeChecker::typeCheckPattern(Pattern *P, DeclContext *dc,
994994
llvm_unreachable("bad pattern kind!");
995995
}
996996

997+
namespace {
998+
999+
/// We need to allow particular matches for backwards compatibility, so we
1000+
/// "repair" the pattern if needed, so that the exhaustiveness checker receives
1001+
/// well-formed input. Also emit diagnostics warning the user to fix their code.
1002+
///
1003+
/// See SR-11160 and SR-11212 for more discussion.
1004+
//
1005+
// type ~ (T1, ..., Tn) (n >= 2)
1006+
// 1a. pat ~ ((P1, ..., Pm)) (m >= 2)
1007+
// 1b. pat
1008+
// type ~ ((T1, ..., Tn)) (n >= 2)
1009+
// 2. pat ~ (P1, ..., Pm) (m >= 2)
1010+
void implicitlyUntuplePatternIfApplicable(TypeChecker *TC,
1011+
Pattern *&enumElementInnerPat,
1012+
Type enumPayloadType) {
1013+
if (auto *tupleType = dyn_cast<TupleType>(enumPayloadType.getPointer())) {
1014+
if (tupleType->getNumElements() >= 2
1015+
&& enumElementInnerPat->getKind() == PatternKind::Paren) {
1016+
auto *semantic = enumElementInnerPat->getSemanticsProvidingPattern();
1017+
if (auto *tuplePattern = dyn_cast<TuplePattern>(semantic)) {
1018+
if (tuplePattern->getNumElements() >= 2) {
1019+
TC->diagnose(tuplePattern->getLoc(),
1020+
diag::matching_tuple_pattern_with_many_assoc_values);
1021+
enumElementInnerPat = semantic;
1022+
}
1023+
} else {
1024+
TC->diagnose(enumElementInnerPat->getLoc(),
1025+
diag::matching_pattern_with_many_assoc_values);
1026+
}
1027+
}
1028+
} else if (auto *tupleType = enumPayloadType->getAs<TupleType>()) {
1029+
if (tupleType->getNumElements() >= 2
1030+
&& enumElementInnerPat->getKind() == PatternKind::Tuple)
1031+
TC->diagnose(enumElementInnerPat->getLoc(),
1032+
diag::matching_many_patterns_with_tupled_assoc_value);
1033+
}
1034+
}
1035+
}
1036+
9971037
/// Perform top-down type coercion on the given pattern.
9981038
bool TypeChecker::coercePatternToType(Pattern *&P, TypeResolution resolution,
9991039
Type type,
@@ -1487,6 +1527,9 @@ bool TypeChecker::coercePatternToType(Pattern *&P, TypeResolution resolution,
14871527
auto newSubOptions = subOptions;
14881528
newSubOptions.setContext(TypeResolverContext::EnumPatternPayload);
14891529
newSubOptions |= TypeResolutionFlags::FromNonInferredPattern;
1530+
1531+
::implicitlyUntuplePatternIfApplicable(this, sub, elementType);
1532+
14901533
if (coercePatternToType(sub, resolution, elementType, newSubOptions))
14911534
return true;
14921535
EEP->setSubPattern(sub);

‎lib/Sema/TypeCheckSwitchStmt.cpp

Lines changed: 40 additions & 23 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):
@@ -735,6 +737,30 @@ namespace {
735737
}
736738
}
737739

740+
/// Use this if you're doing getAs<TupleType> on a Type (and it succeeds)
741+
/// to compute the spaces for it. Handy for disambiguating fields
742+
/// that are tuples from associated values.
743+
///
744+
/// .e((a: X, b: X)) -> ((a: X, b: X))
745+
/// vs .f(a: X, b: X) -> (a: X, b: X)
746+
static void getTupleTypeSpaces(Type &outerType,
747+
TupleType *tty,
748+
SmallVectorImpl<Space> &spaces) {
749+
ArrayRef<TupleTypeElt> ttyElts = tty->getElements();
750+
if (isa<ParenType>(outerType.getPointer())) {
751+
// We had an actual tuple!
752+
SmallVector<Space, 4> innerSpaces;
753+
for (auto &elt: ttyElts)
754+
innerSpaces.push_back(Space::forType(elt.getType(), elt.getName()));
755+
spaces.push_back(
756+
Space::forConstructor(tty, Identifier(), innerSpaces));
757+
} else {
758+
// We're looking at the fields of a constructor here.
759+
for (auto &elt: ttyElts)
760+
spaces.push_back(Space::forType(elt.getType(), elt.getName()));
761+
}
762+
};
763+
738764
// Decompose a type into its component spaces.
739765
static void decompose(TypeChecker &TC, const DeclContext *DC, Type tp,
740766
SmallVectorImpl<Space> &arr) {
@@ -748,8 +774,6 @@ namespace {
748774
auto children = E->getAllElements();
749775
std::transform(children.begin(), children.end(),
750776
std::back_inserter(arr), [&](EnumElementDecl *eed) {
751-
SmallVector<Space, 4> constElemSpaces;
752-
753777
// We need the interface type of this enum case but it may
754778
// not have been computed.
755779
if (!eed->hasInterfaceType()) {
@@ -768,17 +792,15 @@ namespace {
768792
return Space();
769793
}
770794

795+
// .e(a: X, b: X) -> (a: X, b: X)
796+
// .f((a: X, b: X)) -> ((a: X, b: X)
771797
auto eedTy = tp->getCanonicalType()
772798
->getTypeOfMember(E->getModuleContext(), eed,
773799
eed->getArgumentInterfaceType());
800+
SmallVector<Space, 4> constElemSpaces;
774801
if (eedTy) {
775802
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-
});
803+
Space::getTupleTypeSpaces(eedTy, TTy, constElemSpaces);
782804
} else if (auto *TTy = dyn_cast<ParenType>(eedTy.getPointer())) {
783805
constElemSpaces.push_back(
784806
Space::forType(TTy->getUnderlyingType(), Identifier()));
@@ -1441,14 +1463,11 @@ namespace {
14411463
// FIXME: SE-0155 makes this case unreachable.
14421464
if (SP->getKind() == PatternKind::Named
14431465
|| SP->getKind() == PatternKind::Any) {
1444-
if (auto *TTy = SP->getType()->getAs<TupleType>()) {
1445-
for (auto ty : TTy->getElements()) {
1446-
conArgSpace.push_back(Space::forType(ty.getType(),
1447-
ty.getName()));
1448-
}
1449-
} else {
1466+
Type outerType = SP->getType();
1467+
if (auto *TTy = outerType->getAs<TupleType>())
1468+
Space::getTupleTypeSpaces(outerType, TTy, conArgSpace);
1469+
else
14501470
conArgSpace.push_back(projectPattern(TC, SP));
1451-
}
14521471
} else if (SP->getKind() == PatternKind::Tuple) {
14531472
Space argTupleSpace = projectPattern(TC, SP);
14541473
// Tuples are modeled as if they are enums with a single, nameless
@@ -1458,9 +1477,7 @@ namespace {
14581477
if (argTupleSpace.isEmpty())
14591478
return Space();
14601479
assert(argTupleSpace.getKind() == SpaceKind::Constructor);
1461-
conArgSpace.insert(conArgSpace.end(),
1462-
argTupleSpace.getSpaces().begin(),
1463-
argTupleSpace.getSpaces().end());
1480+
conArgSpace.push_back(argTupleSpace);
14641481
} else {
14651482
conArgSpace.push_back(projectPattern(TC, SP));
14661483
}

‎test/Compatibility/implicit_tupling_untupling_codegen.swift

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
// RUN: %target-run-simple-swift | %FileCheck %s
2+
3+
// Even though we test that type-checking and exhaustiveness checking work fine
4+
// in the presence of implicit tupling/untupling in exhaustive_switch.swift,
5+
// make sure that the "patched" patterns do not lead to incorrect codegen.
6+
7+
enum Untupled {
8+
case upair(Int, Int)
9+
}
10+
11+
let u_ex = Untupled.upair(1, 2)
12+
13+
func sr11212_content_untupled_pattern_tupled1(u: Untupled) -> (Int, Int) {
14+
switch u { case .upair((let x, let y)): return (x, y) }
15+
}
16+
print(sr11212_content_untupled_pattern_tupled1(u: u_ex))
17+
// CHECK: (1, 2)
18+
19+
func sr11212_content_untupled_pattern_tupled2(u: Untupled) -> (Int, Int) {
20+
switch u { case .upair(let (x, y)): return (x, y) }
21+
}
22+
print(sr11212_content_untupled_pattern_tupled2(u: u_ex))
23+
// CHECK: (1, 2)
24+
25+
func sr11212_content_untupled_pattern_tupled3(u: Untupled) -> (Int, Int) {
26+
switch u { case let .upair((x, y)): return (x, y) }
27+
}
28+
print(sr11212_content_untupled_pattern_tupled3(u: u_ex))
29+
// CHECK: (1, 2)
30+
31+
func sr11212_content_untupled_pattern_untupled1(u: Untupled) -> (Int, Int) {
32+
switch u { case .upair(let x, let y): return (x, y) }
33+
}
34+
print(sr11212_content_untupled_pattern_untupled1(u: u_ex))
35+
// CHECK: (1, 2)
36+
37+
func sr11212_content_untupled_pattern_untupled2(u: Untupled) -> (Int, Int) {
38+
switch u { case let .upair(x, y): return (x, y) }
39+
}
40+
print(sr11212_content_untupled_pattern_untupled2(u: u_ex))
41+
// CHECK: (1, 2)
42+
43+
func sr11212_content_untupled_pattern_ambiguous1(u: Untupled) -> (Int, Int) {
44+
switch u { case .upair(let u_): return u_ }
45+
}
46+
print(sr11212_content_untupled_pattern_ambiguous1(u: u_ex))
47+
// CHECK: (1, 2)
48+
49+
func sr11212_content_untupled_pattern_ambiguous2(u: Untupled) -> (Int, Int) {
50+
switch u { case let .upair(u_): return u_ }
51+
}
52+
print(sr11212_content_untupled_pattern_ambiguous2(u: u_ex))
53+
// CHECK: (1, 2)
54+
55+
enum Tupled {
56+
case tpair((Int, Int))
57+
}
58+
59+
let t_ex = Tupled.tpair((1, 2))
60+
61+
func sr11212_content_tupled_pattern_tupled1(t: Tupled) -> (Int, Int) {
62+
switch t { case .tpair((let x, let y)): return (x, y) }
63+
}
64+
print(sr11212_content_tupled_pattern_tupled1(t: t_ex))
65+
// CHECK: (1, 2)
66+
67+
func sr11212_content_tupled_pattern_tupled2(t: Tupled) -> (Int, Int) {
68+
switch t { case .tpair(let (x, y)): return (x, y) }
69+
}
70+
print(sr11212_content_tupled_pattern_tupled2(t: t_ex))
71+
// CHECK: (1, 2)
72+
73+
func sr11212_content_tupled_pattern_tupled3(t: Tupled) -> (Int, Int) {
74+
switch t { case let .tpair((x, y)): return (x, y) }
75+
}
76+
print(sr11212_content_tupled_pattern_tupled3(t: t_ex))
77+
// CHECK: (1, 2)
78+
79+
func sr11212_content_tupled_pattern_untupled1(t: Tupled) -> (Int, Int) {
80+
switch t { case .tpair(let x, let y): return (x, y) }
81+
}
82+
print(sr11212_content_tupled_pattern_untupled1(t: t_ex))
83+
// CHECK: (1, 2)
84+
85+
func sr11212_content_tupled_pattern_untupled2(t: Tupled) -> (Int, Int) {
86+
switch t { case let .tpair(x, y): return (x, y) }
87+
}
88+
print(sr11212_content_tupled_pattern_untupled2(t: t_ex))
89+
// CHECK: (1, 2)
90+
91+
func sr11212_content_tupled_pattern_ambiguous1(t: Tupled) -> (Int, Int) {
92+
switch t { case .tpair(let t_): return t_ }
93+
}
94+
print(sr11212_content_tupled_pattern_ambiguous1(t: t_ex))
95+
// CHECK: (1, 2)
96+
97+
func sr11212_content_tupled_pattern_ambiguous2(t: Tupled) -> (Int, Int) {
98+
switch t { case let .tpair(t_): return t_ }
99+
}
100+
print(sr11212_content_tupled_pattern_ambiguous2(t: t_ex))
101+
// CHECK: (1, 2)
102+
103+
enum Box<T> {
104+
case box(T)
105+
}
106+
107+
let b_ex : Box<(Int, Int)> = Box.box((1, 2))
108+
109+
func sr11212_content_generic_pattern_tupled1(b: Box<(Int, Int)>) -> (Int, Int) {
110+
switch b { case .box((let x, let y)): return (x, y) }
111+
}
112+
print(sr11212_content_generic_pattern_tupled1(b: b_ex))
113+
// CHECK: (1, 2)
114+
115+
func sr11212_content_generic_pattern_tupled2(b: Box<(Int, Int)>) -> (Int, Int) {
116+
switch b { case .box(let (x, y)): return (x, y) }
117+
}
118+
print(sr11212_content_generic_pattern_tupled2(b: b_ex))
119+
// CHECK: (1, 2)
120+
121+
func sr11212_content_generic_pattern_tupled3(b: Box<(Int, Int)>) -> (Int, Int) {
122+
switch b { case let .box((x, y)): return (x, y) }
123+
}
124+
print(sr11212_content_generic_pattern_tupled3(b: b_ex))
125+
// CHECK: (1, 2)
126+
127+
func sr11212_content_generic_pattern_untupled1(b: Box<(Int, Int)>) -> (Int, Int) {
128+
switch b { case .box(let x, let y): return (x, y) }
129+
}
130+
print(sr11212_content_generic_pattern_untupled1(b: b_ex))
131+
// CHECK: (1, 2)
132+
133+
func sr11212_content_generic_pattern_untupled2(b: Box<(Int, Int)>) -> (Int, Int) {
134+
switch b { case let .box(x, y): return (x, y) }
135+
}
136+
print(sr11212_content_generic_pattern_untupled2(b: b_ex))
137+
// CHECK: (1, 2)
138+
139+
func sr11212_content_generic_pattern_ambiguous1(b: Box<(Int, Int)>) -> (Int, Int) {
140+
switch b { case .box(let b_): return b_ }
141+
}
142+
print(sr11212_content_generic_pattern_ambiguous1(b: b_ex))
143+
// CHECK: (1, 2)
144+
145+
func sr11212_content_generic_pattern_ambiguous2(b: Box<(Int, Int)>) -> (Int, Int) {
146+
switch b { case let .box(b_): return b_ }
147+
}
148+
print(sr11212_content_generic_pattern_ambiguous2(b: b_ex))
149+
// CHECK: (1, 2)

‎test/Parse/matching_patterns.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ enum Voluntary<T> : Equatable {
100100
()
101101

102102
case .Twain(), // expected-error{{tuple pattern has the wrong length for tuple type '(T, T)'}}
103-
.Twain(_),
103+
.Twain(_), // expected-warning {{cannot match several associated values at once, implicitly tupling the associated values and trying to match that instead}}
104104
.Twain(_, _),
105105
.Twain(_, _, _): // expected-error{{tuple pattern has the wrong length for tuple type '(T, T)'}}
106106
()
@@ -143,7 +143,7 @@ case Voluntary<Int>.Mere,
143143
.Mere(_):
144144
()
145145
case .Twain,
146-
.Twain(_),
146+
.Twain(_), // expected-warning {{cannot match several associated values at once, implicitly tupling the associated values and trying to match that instead}}
147147
.Twain(_, _),
148148
.Twain(_, _, _): // expected-error{{tuple pattern has the wrong length for tuple type '(Int, Int)'}}
149149
()

0 commit comments

Comments
 (0)