Skip to content

Commit 315d59b

Browse files
ultramiraculousultramiraculous
committed
[CSSimplify] Parameter pack wrapping logic incorrectly considers tuple LValueTypes to not be tuples (swiftlang#85962)
In swiftlang#65125 (and beyond) `matchTypes`, has logic to attempt to wrap an incoming parameter in a tuple under certain conditions that might help with type expansion. In the case the incoming type was backed by a `var`, it would be wrapped by an `LValueType` then be subsequently mis-diagnosed as not-a-tuple. More details in swiftlang#85924 , this this is also the cause of (and fix for) swiftlang#85837 as well...
1 parent 3439f81 commit 315d59b

File tree

3 files changed

+347
-0
lines changed

3 files changed

+347
-0
lines changed

‎lib/Sema/CSSimplify.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7393,6 +7393,9 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
73937393
// Notable exceptions here are: `Any` which doesn't require wrapping and
73947394
// would be handled by an existential promotion in cases where it's allowed,
73957395
// and `Optional<T>` which would be handled by optional injection.
7396+
//
7397+
// `LValueType`s are also ignored at this stage to avoid accidentally wrapping them. If they
7398+
// are valid wrapping targets, they will be tuple-wrapped after the lvalue is converted.
73967399
if (isTupleWithUnresolvedPackExpansion(origType1) ||
73977400
isTupleWithUnresolvedPackExpansion(origType2)) {
73987401
auto isTypeVariableWrappedInOptional = [](Type type) {
@@ -7403,6 +7406,7 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
74037406
};
74047407
if (isa<TupleType>(desugar1) != isa<TupleType>(desugar2) &&
74057408
!isa<InOutType>(desugar1) && !isa<InOutType>(desugar2) &&
7409+
!isa<LValueType>(desugar1) && !isa<LValueType>(desugar2) &&
74067410
!isTypeVariableWrappedInOptional(desugar1) &&
74077411
!isTypeVariableWrappedInOptional(desugar2) &&
74087412
!desugar1->isAny() &&

‎test/Constraints/pack_expansion_types.swift‎

Lines changed: 316 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,3 +300,319 @@ func test_one_element_tuple_vs_non_tuple_matching() {
300300
test(V<Int>.self) // Ok
301301
}
302302
}
303+
304+
// Ensure correct behavior of lvalue tuple parameters
305+
306+
/**
307+
Previously `var`-backed parameters would end up wrapped
308+
in an extraneous tuple level, leading to leading to incorrect
309+
nesting in the output type due to the `LValueType` not being unwrapped.
310+
311+
https://github.com/swiftlang/swift/issues/85924
312+
*/
313+
func test_var_let_tuple_merge_equivalence() {
314+
func merge<each A, each B>(_ a: (repeat each A), _ b: (repeat each B)) -> (repeat each A, repeat each B) {
315+
return (repeat each a, repeat each b)
316+
}
317+
318+
// allLets, TupleFirst
319+
let _: (String, Int, String) = {
320+
let a = ("a", 2) // (String, Int)
321+
let b = "c" // String
322+
return merge(a, b)
323+
}()
324+
325+
// Before #85924 was fixed, this would type as ((String, Int), String)
326+
// varFirst, TupleFirst
327+
let _: (String, Int, String) = {
328+
// expected-warning@+1{{variable 'a' was never mutated; consider changing to 'let' constant}}
329+
var a = ("a", 2) // @lvalue (String, Int)
330+
let b = "c" // String
331+
return merge(a, b)
332+
}()
333+
334+
// varSecond, TupleFirst
335+
let _: (String, Int, String) = {
336+
let a = ("a", 2) // (String, Int)
337+
// expected-warning@+1{{variable 'b' was never mutated; consider changing to 'let' constant}}
338+
var b = "c" // @lvalue String
339+
return merge(a, b)
340+
}()
341+
342+
// allVars, TupleFirst
343+
let _: (String, Int, String) = {
344+
// expected-warning@+1{{variable 'a' was never mutated; consider changing to 'let' constant}}
345+
var a = ("a", 2) // @lvalue (String, Int)
346+
// expected-warning@+1{{variable 'b' was never mutated; consider changing to 'let' constant}}
347+
var b = "c" // @lvalue String
348+
return merge(a, b)
349+
}()
350+
351+
// allLets, TupleSecond
352+
let _: (String, Int, String) = {
353+
let a = "a"
354+
let b = (2, "c")
355+
return merge(a, b)
356+
}()
357+
358+
// varFirst, TupleSecond
359+
let _: (String, Int, String) = {
360+
// expected-warning@+1{{variable 'a' was never mutated; consider changing to 'let' constant}}
361+
var a = "a"
362+
let b = (2, "c")
363+
return merge(a, b)
364+
}()
365+
366+
// varSecond, TupleSecond
367+
let _: (String, Int, String) = {
368+
let a = "a"
369+
// expected-warning@+1{{variable 'b' was never mutated; consider changing to 'let' constant}}
370+
var b = (2, "c")
371+
return merge(a, b)
372+
}()
373+
374+
// allVars, TupleSecond
375+
let _: (String, Int, String) = {
376+
// expected-warning@+1{{variable 'a' was never mutated; consider changing to 'let' constant}}
377+
var a = "a"
378+
// expected-warning@+1{{variable 'b' was never mutated; consider changing to 'let' constant}}
379+
var b = (2, "c")
380+
return merge(a, b)
381+
}()
382+
383+
// allLets, MultiTuple
384+
let _: (String, Int, String) = {
385+
let a = ("a")
386+
let b = (2, "c")
387+
return merge(a, b)
388+
}()
389+
390+
// varFirst, MultiTuple
391+
let _: (String, Int, String) = {
392+
// expected-warning@+1{{variable 'a' was never mutated; consider changing to 'let' constant}}
393+
var a = ("a")
394+
let b = (2, "c")
395+
return merge(a, b)
396+
}()
397+
398+
// varSecond, MultiTuple
399+
let _: (String, Int, String) = {
400+
let a = ("a")
401+
// expected-warning@+1{{variable 'b' was never mutated; consider changing to 'let' constant}}
402+
var b = (2, "c")
403+
return merge(a, b)
404+
}()
405+
406+
// allVars, MultiTuple
407+
let _: (String, Int, String) = {
408+
// expected-warning@+1{{variable 'a' was never mutated; consider changing to 'let' constant}}
409+
var a = ("a")
410+
// expected-warning@+1{{variable 'b' was never mutated; consider changing to 'let' constant}}
411+
var b = (2, "c")
412+
return merge(a, b)
413+
}()
414+
}
415+
416+
func test_var_let_tuple_append_equivalence() {
417+
func append<each A, B>(_ a: (repeat each A), _ b: B) -> (repeat each A, B) {
418+
return (repeat each a, b)
419+
}
420+
421+
// allLets, TupleFirst
422+
let _: (String, Int, String) = {
423+
let a = ("a", 2) // (String, Int)
424+
let b = "c" // String
425+
return append(a, b)
426+
}()
427+
428+
// varFirst, TupleFirst
429+
let _: (String, Int, String) = {
430+
// expected-warning@+1{{variable 'a' was never mutated; consider changing to 'let' constant}}
431+
var a = ("a", 2) // @lvalue (String, Int)
432+
let b = "c" // String
433+
return append(a, b)
434+
}()
435+
436+
// varSecond, TupleFirst
437+
let _: (String, Int, String) = {
438+
let a = ("a", 2) // (String, Int)
439+
// expected-warning@+1{{variable 'b' was never mutated; consider changing to 'let' constant}}
440+
var b = "c" // @lvalue String
441+
return append(a, b)
442+
}()
443+
444+
// allVars, TupleFirst
445+
let _: (String, Int, String) = {
446+
// expected-warning@+1{{variable 'a' was never mutated; consider changing to 'let' constant}}
447+
var a = ("a", 2) // @lvalue (String, Int)
448+
// expected-warning@+1{{variable 'b' was never mutated; consider changing to 'let' constant}}
449+
var b = "c" // @lvalue String
450+
return append(a, b)
451+
}()
452+
453+
// allLets, TupleSecond
454+
let _: (String, (Int, String)) = {
455+
let a = "a"
456+
let b = (2, "c")
457+
return append(a, b)
458+
}()
459+
460+
// varFirst, TupleSecond
461+
let _: (String, (Int, String)) = {
462+
// expected-warning@+1{{variable 'a' was never mutated; consider changing to 'let' constant}}
463+
var a = "a"
464+
let b = (2, "c")
465+
return append(a, b)
466+
}()
467+
468+
// varSecond, TupleSecond
469+
let _: (String, (Int, String)) = {
470+
let a = "a"
471+
// expected-warning@+1{{variable 'b' was never mutated; consider changing to 'let' constant}}
472+
var b = (2, "c")
473+
return append(a, b)
474+
}()
475+
476+
// allVars, TupleSecond
477+
let _: (String, (Int, String)) = {
478+
// expected-warning@+1{{variable 'a' was never mutated; consider changing to 'let' constant}}
479+
var a = "a"
480+
// expected-warning@+1{{variable 'b' was never mutated; consider changing to 'let' constant}}
481+
var b = (2, "c")
482+
return append(a, b)
483+
}()
484+
485+
// allLets, MultiTuple
486+
let _: (String, (Int, String)) = {
487+
let a = ("a")
488+
let b = (2, "c")
489+
return append(a, b)
490+
}()
491+
492+
// varFirst, MultiTuple
493+
let _: (String, (Int, String)) = {
494+
// expected-warning@+1{{variable 'a' was never mutated; consider changing to 'let' constant}}
495+
var a = ("a")
496+
let b = (2, "c")
497+
return append(a, b)
498+
}()
499+
500+
// varSecond, MultiTuple
501+
let _: (String, (Int, String)) = {
502+
let a = ("a")
503+
// expected-warning@+1{{variable 'b' was never mutated; consider changing to 'let' constant}}
504+
var b = (2, "c")
505+
return append(a, b)
506+
}()
507+
508+
// allVars, MultiTuple
509+
let _: (String, (Int, String)) = {
510+
// expected-warning@+1{{variable 'a' was never mutated; consider changing to 'let' constant}}
511+
var a = ("a")
512+
// expected-warning@+1{{variable 'b' was never mutated; consider changing to 'let' constant}}
513+
var b = (2, "c")
514+
return append(a, b)
515+
}()
516+
}
517+
518+
func test_var_let_tuple_prefixOnto_equivalence() {
519+
func prefixOnto<A, each B>(_ a: A, _ b: (repeat each B)) -> (A, repeat each B) {
520+
return (a, repeat each b)
521+
}
522+
523+
// allLets, TupleFirst
524+
let _: ((String, Int), String) = {
525+
let a = ("a", 2)
526+
let b = "c"
527+
return prefixOnto(a, b)
528+
}()
529+
530+
// varFirst, TupleFirst
531+
let _: ((String, Int), String) = {
532+
// expected-warning@+1{{variable 'a' was never mutated; consider changing to 'let' constant}}
533+
var a = ("a", 2)
534+
let b = "c"
535+
return prefixOnto(a, b)
536+
}()
537+
538+
// varSecond, TupleFirst
539+
let _: ((String, Int), String) = {
540+
let a = ("a", 2)
541+
// expected-warning@+1{{variable 'b' was never mutated; consider changing to 'let' constant}}
542+
var b = "c"
543+
return prefixOnto(a, b)
544+
}()
545+
546+
// allVars, TupleFirst
547+
let _: ((String, Int), String) = {
548+
// expected-warning@+1{{variable 'a' was never mutated; consider changing to 'let' constant}}
549+
var a = ("a", 2)
550+
// expected-warning@+1{{variable 'b' was never mutated; consider changing to 'let' constant}}
551+
var b = "c"
552+
return prefixOnto(a, b)
553+
}()
554+
555+
// allLets, TupleSecond
556+
let _: (String, Int, String) = {
557+
let a = "a"
558+
let b = (2, "c")
559+
return prefixOnto(a, b)
560+
}()
561+
562+
// varFirst, TupleSecond
563+
let _: (String, Int, String) = {
564+
// expected-warning@+1{{variable 'a' was never mutated; consider changing to 'let' constant}}
565+
var a = "a"
566+
let b = (2, "c")
567+
return prefixOnto(a, b)
568+
}()
569+
570+
// varSecond, TupleSecond
571+
let _: (String, Int, String) = {
572+
let a = "a"
573+
// expected-warning@+1{{variable 'b' was never mutated; consider changing to 'let' constant}}
574+
var b = (2, "c")
575+
return prefixOnto(a, b)
576+
}()
577+
578+
// allVars, TupleSecond
579+
let _: (String, Int, String) = {
580+
// expected-warning@+1{{variable 'a' was never mutated; consider changing to 'let' constant}}
581+
var a = "a"
582+
// expected-warning@+1{{variable 'b' was never mutated; consider changing to 'let' constant}}
583+
var b = (2, "c")
584+
return prefixOnto(a, b)
585+
}()
586+
587+
// allLets, MultiTuple
588+
let _: (String, Int, String) = {
589+
let a = ("a")
590+
let b = (2, "c")
591+
return prefixOnto(a, b)
592+
}()
593+
594+
// varFirst, MultiTuple
595+
let _: (String, Int, String) = {
596+
// expected-warning@+1{{variable 'a' was never mutated; consider changing to 'let' constant}}
597+
var a = ("a")
598+
let b = (2, "c")
599+
return prefixOnto(a, b)
600+
}()
601+
602+
// varSecond, MultiTuple
603+
let _: (String, Int, String) = {
604+
let a = ("a")
605+
// expected-warning@+1{{variable 'b' was never mutated; consider changing to 'let' constant}}
606+
var b = (2, "c")
607+
return prefixOnto(a, b)
608+
}()
609+
610+
// allVars, MultiTuple
611+
let _: (String, Int, String) = {
612+
// expected-warning@+1 {{variable 'a' was never mutated; consider changing to 'let' constant}}
613+
var a = ("a")
614+
// expected-warning@+1 {{variable 'b' was never mutated; consider changing to 'let' constant}}
615+
var b = (2, "c")
616+
return prefixOnto(a, b)
617+
}()
618+
}

‎test/Sema/issue-85837.swift‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// RUN: %target-typecheck-verify-swift
2+
3+
// Verifies fix for Github Issue #85837
4+
// https://github.com/swiftlang/swift/issues/85837
5+
//
6+
//
7+
8+
@resultBuilder public enum TupleBuilder {
9+
public static func buildPartialBlock<T>(first: T) -> (T) {
10+
return first
11+
}
12+
13+
public static func buildPartialBlock<each A, B>(accumulated: (repeat each A), next: B) -> (repeat each A, B) {
14+
return (repeat each accumulated, next)
15+
}
16+
}
17+
18+
func builder<each A>(@TupleBuilder content: ()->(repeat each A)) -> (repeat each A) {
19+
return content()
20+
}
21+
22+
// Ensure this optimally packs parameters
23+
let built: (String, Int, String) = builder {
24+
"a"
25+
2
26+
"c"
27+
}

0 commit comments

Comments
 (0)