Skip to content

Commit 0507b02

Browse files
committed
RequirementMachine: Fix crash-on-invalid with concrete type requirements involving packs
We would crash in some cases, or produce a slightly misleading diagnostic about same-element requirements, which are related but not quite the same. In the fullness of time, we should figure out this corner of the language. Until then, add a new diagnostic since this is really about same-type requirements between concrete types and packs. Fixes rdar://159790557.
1 parent 30fc84c commit 0507b02

File tree

7 files changed

+75
-11
lines changed

7 files changed

+75
-11
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3421,6 +3421,9 @@ ERROR(invalid_shape_requirement,none,
34213421
ERROR(unsupported_same_element,none,
34223422
"same-element requirements are not yet supported",
34233423
())
3424+
ERROR(unsupported_pack_same_type,none,
3425+
"same-type requirements between packs and concrete types are not yet supported",
3426+
())
34243427

34253428
ERROR(requires_generic_params_made_equal,none,
34263429
"same-type requirement makes generic parameters %0 and %1 equivalent",

lib/AST/RequirementMachine/Diagnostics.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,15 @@ bool swift::rewriting::diagnoseRequirementErrors(
226226
break;
227227
}
228228

229+
case RequirementError::Kind::UnsupportedPackSameType: {
230+
if (error.getRequirement().hasError())
231+
break;
232+
233+
ctx.Diags.diagnose(loc, diag::unsupported_pack_same_type);
234+
diagnosedError = true;
235+
break;
236+
}
237+
229238
case RequirementError::Kind::InvalidValueGenericType: {
230239
auto req = error.getRequirement();
231240

lib/AST/RequirementMachine/Diagnostics.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@ struct RequirementError {
5353
RecursiveRequirement,
5454
/// A not-yet-supported same-element requirement, e.g. each T == Int.
5555
UnsupportedSameElement,
56+
/// A not-yet-supported same-type requirement involving packs and concrete
57+
/// types, e.g. (repeat each T) == (Int, repeat each U).
58+
UnsupportedPackSameType,
5659
/// An unexpected value type used in a value generic,
5760
/// e.g. 'let N: String'.
5861
InvalidValueGenericType,
@@ -162,6 +165,10 @@ struct RequirementError {
162165
return {Kind::UnsupportedSameElement, req, loc};
163166
}
164167

168+
static RequirementError forPackSameType(Requirement req, SourceLoc loc) {
169+
return {Kind::UnsupportedPackSameType, req, loc};
170+
}
171+
165172
static RequirementError forInvalidValueGenericType(Type subjectType,
166173
Type constraint,
167174
SourceLoc loc) {

lib/AST/RequirementMachine/RequirementLowering.cpp

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -222,17 +222,15 @@ static void desugarSameTypeRequirement(
222222
break;
223223
}
224224

225-
auto &ctx = firstType->getASTContext();
226-
if (!ctx.LangOpts.hasFeature(Feature::SameElementRequirements)) {
227-
// If one side is a parameter pack, this is a same-element requirement, which
228-
// is not yet supported.
229-
if (firstType->isParameterPack() != secondType->isParameterPack()) {
230-
errors.push_back(RequirementError::forSameElement(
231-
{kind, sugaredFirstType, secondType}, loc));
232-
return true;
233-
}
225+
if (firstType->is<PackExpansionType>() ||
226+
secondType->is<PackExpansionType>()) {
227+
errors.push_back(RequirementError::forPackSameType(
228+
{kind, sugaredFirstType, secondType}, loc));
229+
return true;
234230
}
235231

232+
auto &ctx = firstType->getASTContext();
233+
236234
if (firstType->isValueParameter() || secondType->isValueParameter()) {
237235
// FIXME: If we ever support other value types in the future besides
238236
// 'Int', then we'd want to check their underlying value type to ensure
@@ -265,16 +263,38 @@ static void desugarSameTypeRequirement(
265263
}
266264

267265
if (firstType->isTypeParameter() && secondType->isTypeParameter()) {
266+
if (!ctx.LangOpts.hasFeature(Feature::SameElementRequirements)) {
267+
// If one side is a parameter pack, this is a same-element requirement, which
268+
// is not yet supported.
269+
if (firstType->isParameterPack() != secondType->isParameterPack()) {
270+
errors.push_back(RequirementError::forSameElement(
271+
{kind, sugaredFirstType, secondType}, loc));
272+
return true;
273+
}
274+
}
275+
268276
result.emplace_back(kind, sugaredFirstType, secondType);
269277
return true;
270278
}
271279

272280
if (firstType->isTypeParameter()) {
281+
if (firstType->isParameterPack()) {
282+
errors.push_back(RequirementError::forPackSameType(
283+
{kind, sugaredFirstType, secondType}, loc));
284+
return true;
285+
}
286+
273287
result.emplace_back(kind, sugaredFirstType, secondType);
274288
return true;
275289
}
276290

277291
if (secondType->isTypeParameter()) {
292+
if (secondType->isParameterPack()) {
293+
errors.push_back(RequirementError::forPackSameType(
294+
{kind, sugaredFirstType, secondType}, loc));
295+
return true;
296+
}
297+
278298
result.emplace_back(kind, secondType, sugaredFirstType);
279299
return true;
280300
}

test/ASTGen/diagnostics.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ struct S {
4444
}
4545

4646
struct ExpansionRequirementTest<each T> {}
47-
extension ExpansionRequirementTest where repeat each T == Int {} // expected-error {{same-element requirements are not yet supported}}
47+
extension ExpansionRequirementTest where repeat each T == Int {} // expected-error {{same-type requirements between packs and concrete types are not yet supported}}
4848

4949

5050
#warning("this is a warning") // expected-warning {{this is a warning}}
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// RUN: %target-typecheck-verify-swift
2+
3+
protocol PipelineItem {
4+
associatedtype Input
5+
associatedtype Output
6+
}
7+
8+
// This example will never work; eventually, it will produce a more precise diagnostic.
9+
func bad<First : PipelineItem,
10+
each T : PipelineItem,
11+
Last : PipelineItem>(_ first: First, _ rest: repeat each T, last: Last)
12+
where (First.Output, repeat (each T).Output) == (repeat (each T).Input, Last.Input) {}
13+
// expected-error@-1 2{{same-type requirements between packs and concrete types are not yet supported}}
14+
15+
// This is also invalid.
16+
func bad2<First : PipelineItem,
17+
each T : PipelineItem>(_ first: First, _ rest: repeat each T)
18+
where repeat (each T).Output == (First, repeat (each T).Input) {}
19+
// expected-error@-1 {{same-type requirements between packs and concrete types are not yet supported}}
20+
21+
// This could eventually be made to work.
22+
func good<First : PipelineItem,
23+
each T : PipelineItem>(_ first: First, _ rest: repeat each T)
24+
where (repeat (each T).Output) == (First, repeat (each T).Input) {}
25+
// expected-error@-1 {{generic signature requires types '(repeat (each T).Output)' and '(First, repeat (each T).Input)' to be the same}}

test/Generics/variadic_generic_types.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ struct WithPack<each T> {}
125125
// FIXME: The diagnostics are misleading.
126126

127127
extension WithPack<Int, Int> {}
128-
// expected-error@-1 {{same-element requirements are not yet supported}}
128+
// expected-error@-1 {{same-type requirements between packs and concrete types are not yet supported}}
129129

130130
extension WithPack where (repeat each T) == (Int, Int) {}
131131
// expected-error@-1 {{generic signature requires types '(repeat each T)' and '(Int, Int)' to be the same}}

0 commit comments

Comments
 (0)