Skip to content

Commit 9951d4b

Browse files
rjmccallrjmccall
committed
Fix the emission of closures into reabstracted contexts with
variadic-tuple results. There are three parts to this. First, fix the emission of indirect result parameters to do a proper abstraction-pattern-aware traversal of tuple patterns. There was a FIXME here and everything. Second, fix the computation of substituted abstraction patterns to properly handle vanishing tuples. The previous code was recursively destructuring tuples, but only when it saw a tuple as the substituted type, which of course breaks on vanishing tuples. Finally, fix the emission of returns into vanishing tuple patterns by allowing the code to not produce a TupleInitialization when the tuple pattern vanishes. We should always get a singleton element initializer in this case. Fixes rdar://109843932, plus a closely-related test case for vanishing tuples that I added myself.
1 parent f7a8e04 commit 9951d4b

File tree

5 files changed

+168
-35
lines changed

5 files changed

+168
-35
lines changed

‎include/swift/SIL/AbstractionPatternGenerators.h

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,22 @@ class TupleElementGenerator {
291291
}
292292
}
293293

294+
/// Like `getSubstTypes`, but uses a different and possibly
295+
/// non-canonical tuple type.
296+
TupleEltTypeArrayRef getSubstTypes(Type ncSubstType) const {
297+
assert(!isFinished());
298+
if (!origTupleVanishes) {
299+
return ncSubstType->castTo<TupleType>()
300+
->getElementTypes().slice(substEltIndex,
301+
numSubstEltsForOrigElt);
302+
} else if (numSubstEltsForOrigElt == 0) {
303+
return TupleEltTypeArrayRef();
304+
} else {
305+
scratchSubstElt = TupleTypeElt(ncSubstType);
306+
return TupleEltTypeArrayRef(scratchSubstElt);
307+
}
308+
}
309+
294310
/// Call this to finalize the traversal and assert that it was done
295311
/// properly.
296312
void finish() {

‎lib/SIL/IR/AbstractionPattern.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2064,6 +2064,9 @@ class SubstFunctionTypePatternVisitor
20642064
if (auto gp = handleTypeParameter(pattern, t, level))
20652065
return gp;
20662066

2067+
if (pattern.isTuple())
2068+
return visitTuplePattern(t, pattern);
2069+
20672070
return CanTypeVisitor::visit(t, pattern);
20682071
}
20692072

@@ -2312,11 +2315,15 @@ class SubstFunctionTypePatternVisitor
23122315
TC.Context, ppt->getBaseType(), substArgs));
23132316
}
23142317

2315-
CanType visitTupleType(CanTupleType tuple, AbstractionPattern pattern) {
2318+
/// Visit a tuple pattern. Note that, because of vanishing tuples,
2319+
/// we can't handle this case by matching a tuple type in the
2320+
/// substituted type; we have to check for a tuple pattern in the
2321+
/// top-level visit routine.
2322+
CanType visitTuplePattern(CanType substType, AbstractionPattern pattern) {
23162323
assert(pattern.isTuple());
23172324

23182325
SmallVector<TupleTypeElt, 4> tupleElts;
2319-
pattern.forEachTupleElement(tuple, [&](TupleElementGenerator &elt) {
2326+
pattern.forEachTupleElement(substType, [&](TupleElementGenerator &elt) {
23202327
auto substEltTypes = elt.getSubstTypes();
23212328
CanType eltTy;
23222329
if (!elt.isOrigPackExpansion()) {

‎lib/SILGen/SILGenProlog.cpp

Lines changed: 63 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1566,54 +1566,91 @@ SILValue SILGenFunction::emitGetCurrentExecutor(SILLocation loc) {
15661566
return ExpectedExecutor;
15671567
}
15681568

1569+
static void emitIndirectPackParameter(SILGenFunction &SGF,
1570+
PackType *resultType,
1571+
CanTupleEltTypeArrayRef
1572+
resultTypesInContext,
1573+
AbstractionPattern origExpansionType,
1574+
DeclContext *DC) {
1575+
auto &ctx = SGF.getASTContext();
1576+
1577+
bool indirect =
1578+
origExpansionType.arePackElementsPassedIndirectly(SGF.SGM.Types);
1579+
SmallVector<CanType, 4> packElts;
1580+
for (auto substEltType : resultTypesInContext) {
1581+
auto origComponentType
1582+
= origExpansionType.getPackExpansionComponentType(substEltType);
1583+
CanType loweredEltTy =
1584+
SGF.getLoweredRValueType(origComponentType, substEltType);
1585+
packElts.push_back(loweredEltTy);
1586+
}
1587+
1588+
SILPackType::ExtInfo extInfo(indirect);
1589+
auto packType = SILPackType::get(ctx, extInfo, packElts);
1590+
auto resultSILType = SILType::getPrimitiveAddressType(packType);
1591+
1592+
auto var = new (ctx) ParamDecl(SourceLoc(), SourceLoc(),
1593+
ctx.getIdentifier("$return_value"), SourceLoc(),
1594+
ctx.getIdentifier("$return_value"),
1595+
DC);
1596+
var->setSpecifier(ParamSpecifier::InOut);
1597+
var->setInterfaceType(resultType);
1598+
auto *arg = SGF.F.begin()->createFunctionArgument(resultSILType, var);
1599+
(void)arg;
1600+
}
1601+
15691602
static void emitIndirectResultParameters(SILGenFunction &SGF,
15701603
Type resultType,
15711604
AbstractionPattern origResultType,
15721605
DeclContext *DC) {
1573-
// Expand tuples.
1606+
CanType resultTypeInContext =
1607+
DC->mapTypeIntoContext(resultType)->getCanonicalType();
1608+
1609+
// Tuples in the original result type are expanded.
15741610
if (origResultType.isTuple()) {
1575-
auto tupleType = resultType->castTo<TupleType>();
1576-
for (unsigned i = 0, e = origResultType.getNumTupleElements(); i < e; ++i) {
1577-
emitIndirectResultParameters(SGF, tupleType->getElementType(i),
1578-
origResultType.getTupleElementType(i),
1579-
DC);
1580-
}
1611+
origResultType.forEachTupleElement(resultTypeInContext,
1612+
[&](TupleElementGenerator &elt) {
1613+
auto origEltType = elt.getOrigType();
1614+
auto substEltTypes = elt.getSubstTypes(resultType);
1615+
1616+
// If the original element isn't a pack expansion, pull out the
1617+
// corresponding substituted tuple element and recurse.
1618+
if (!elt.isOrigPackExpansion()) {
1619+
emitIndirectResultParameters(SGF, substEltTypes[0], origEltType, DC);
1620+
return;
1621+
}
1622+
1623+
// Otherwise, bind a pack parameter.
1624+
PackType *resultPackType = [&] {
1625+
SmallVector<Type, 4> packElts(substEltTypes.begin(),
1626+
substEltTypes.end());
1627+
return PackType::get(SGF.getASTContext(), packElts);
1628+
}();
1629+
emitIndirectPackParameter(SGF, resultPackType, elt.getSubstTypes(),
1630+
origEltType, DC);
1631+
});
15811632
return;
15821633
}
15831634

1635+
assert(!resultType->is<PackExpansionType>());
1636+
15841637
// If the return type is address-only, emit the indirect return argument.
15851638
auto &resultTI =
1586-
SGF.SGM.Types.getTypeLowering(origResultType,
1587-
DC->mapTypeIntoContext(resultType),
1639+
SGF.SGM.Types.getTypeLowering(origResultType, resultTypeInContext,
15881640
SGF.getTypeExpansionContext());
15891641

15901642
// The calling convention always uses minimal resilience expansion.
15911643
auto &resultTIConv = SGF.SGM.Types.getTypeLowering(
1592-
DC->mapTypeIntoContext(resultType), TypeExpansionContext::minimal());
1644+
resultTypeInContext, TypeExpansionContext::minimal());
15931645
auto resultConvType = resultTIConv.getLoweredType();
15941646

15951647
auto &ctx = SGF.getASTContext();
15961648

15971649
SILType resultSILType = resultTI.getLoweredType().getAddressType();
15981650

1599-
// FIXME: respect susbtitution properly and collect the appropriate
1600-
// tuple components from resultType that correspond to the
1601-
// pack expansion in origType.
1602-
bool isPackExpansion = resultType->is<PackExpansionType>();
1603-
if (isPackExpansion) {
1604-
resultType = PackType::get(ctx, {resultType});
1605-
1606-
bool indirect =
1607-
origResultType.arePackElementsPassedIndirectly(SGF.SGM.Types);
1608-
SILPackType::ExtInfo extInfo(indirect);
1609-
resultSILType = SILType::getPrimitiveAddressType(
1610-
SILPackType::get(ctx, extInfo, {resultSILType.getASTType()}));
1611-
}
1612-
16131651
// And the abstraction pattern may force an indirect return even if the
16141652
// concrete type wouldn't normally be returned indirectly.
1615-
if (!isPackExpansion &&
1616-
!SILModuleConventions::isReturnedIndirectlyInSIL(resultConvType,
1653+
if (!SILModuleConventions::isReturnedIndirectlyInSIL(resultConvType,
16171654
SGF.SGM.M)) {
16181655
if (!SILModuleConventions(SGF.SGM.M).useLoweredAddresses()
16191656
|| origResultType.getResultConvention(SGF.SGM.Types) != AbstractionPattern::Indirect)

‎lib/SILGen/SILGenStmt.cpp

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -580,11 +580,29 @@ prepareIndirectResultInit(SILGenFunction &SGF, SILLocation loc,
580580
SmallVectorImpl<CleanupHandle> &cleanups) {
581581
// Recursively decompose tuple abstraction patterns.
582582
if (origResultType.isTuple()) {
583-
auto resultTupleType = cast<TupleType>(resultType);
584-
auto tupleInit = new TupleInitialization(resultTupleType);
585-
tupleInit->SubInitializations.reserve(resultTupleType->getNumElements());
583+
// Normally, we build a compound initialization for the tuple. But
584+
// the initialization we build should match the substituted type,
585+
// so if the tuple in the abstraction pattern vanishes under variadic
586+
// substitution, we actually just want to return the initializer
587+
// for the surviving component.
588+
TupleInitialization *tupleInit = nullptr;
589+
SmallVector<InitializationPtr, 1> singletonEltInit;
590+
591+
bool vanishes =
592+
origResultType.getVanishingTupleElementPatternType().hasValue();
593+
if (!vanishes) {
594+
auto resultTupleType = cast<TupleType>(resultType);
595+
tupleInit = new TupleInitialization(resultTupleType);
596+
tupleInit->SubInitializations.reserve(
597+
cast<TupleType>(resultType)->getNumElements());
598+
}
599+
600+
// The list of element initializers to build into.
601+
auto &eltInits = (vanishes
602+
? static_cast<SmallVectorImpl<InitializationPtr> &>(singletonEltInit)
603+
: tupleInit->SubInitializations);
586604

587-
origResultType.forEachTupleElement(resultTupleType,
605+
origResultType.forEachTupleElement(resultType,
588606
[&](TupleElementGenerator &elt) {
589607
if (!elt.isOrigPackExpansion()) {
590608
auto eltInit = prepareIndirectResultInit(SGF, loc, fnTypeForResults,
@@ -594,7 +612,7 @@ prepareIndirectResultInit(SILGenFunction &SGF, SILLocation loc,
594612
directResults,
595613
indirectResultAddrs,
596614
cleanups);
597-
tupleInit->SubInitializations.push_back(std::move(eltInit));
615+
eltInits.push_back(std::move(eltInit));
598616
} else {
599617
assert(allResults[0].isPack());
600618
assert(SGF.silConv.isSILIndirect(allResults[0]));
@@ -604,11 +622,17 @@ prepareIndirectResultInit(SILGenFunction &SGF, SILLocation loc,
604622
indirectResultAddrs = indirectResultAddrs.slice(1);
605623

606624
preparePackResultInit(SGF, loc, elt.getOrigType(), elt.getSubstTypes(),
607-
packAddr,
608-
cleanups, tupleInit->SubInitializations);
625+
packAddr, cleanups, eltInits);
609626
}
610627
});
611628

629+
if (vanishes) {
630+
assert(singletonEltInit.size() == 1);
631+
return std::move(singletonEltInit.front());
632+
}
633+
634+
assert(tupleInit);
635+
assert(eltInits.size() == cast<TupleType>(resultType)->getNumElements());
612636
return InitializationPtr(tupleInit);
613637
}
614638

‎test/SILGen/variadic-generic-reabstraction.swift

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,3 +108,52 @@ func passConcreteClosureToVariadic2(fn: (Int, String) -> Int, x: Int) {
108108
takesVariadicFunction {y in fn(x, y)}
109109
}
110110
*/
111+
112+
// rdar://109843932
113+
// Test that the path where we emit closures naturally at a
114+
// particular abstraction level correctly handles variadic
115+
// expansion in the result type.
116+
func takeClosureWithVariadicResult<each Argument, each Result>(_: (repeat each Argument) -> (repeat each Result)) {}
117+
118+
// CHECK-LABEL: sil {{.*}}@$s4main30testResultReabstractedEmissionyyFSb_SitSi_SbtXEfU_ :
119+
// CHECK-SAME: $@convention(thin) @substituted <each τ_0_0, each τ_0_1> (@pack_guaranteed Pack{repeat each τ_0_0}) -> @pack_out Pack{repeat each τ_0_1} for <Pack{Int, Bool}, Pack{Bool, Int}>
120+
// CHECK: bb0(%0 : $*Pack{Bool, Int}, %1 : $*Pack{Int, Bool}):
121+
// CHECK-NEXT: [[ARG_INDEX_0:%.*]] = scalar_pack_index 0 of $Pack{Int, Bool}
122+
// CHECK-NEXT: [[ARG_ADDR_0:%.*]] = pack_element_get [[ARG_INDEX_0]] of %1 : $*Pack{Int, Bool} as $*Int
123+
// CHECK-NEXT: [[ARG_0:%.*]] = load [trivial] [[ARG_ADDR_0]] : $*Int
124+
// CHECK-NEXT: debug_value [[ARG_0]] :
125+
// CHECK-NEXT: [[ARG_INDEX_1:%.*]] = scalar_pack_index 1 of $Pack{Int, Bool}
126+
// CHECK-NEXT: [[ARG_ADDR_1:%.*]] = pack_element_get [[ARG_INDEX_1]] of %1 : $*Pack{Int, Bool} as $*Bool
127+
// CHECK-NEXT: [[ARG_1:%.*]] = load [trivial] [[ARG_ADDR_1]] : $*Bool
128+
// CHECK-NEXT: debug_value [[ARG_1]] :
129+
// CHECK-NEXT: [[RET_INDEX_0:%.*]] = scalar_pack_index 0 of $Pack{Bool, Int}
130+
// CHECK-NEXT: [[RET_ADDR_0:%.*]] = pack_element_get [[RET_INDEX_0]] of %0 : $*Pack{Bool, Int} as $*Bool
131+
// CHECK-NEXT: [[RET_INDEX_1:%.*]] = scalar_pack_index 1 of $Pack{Bool, Int}
132+
// CHECK-NEXT: [[RET_ADDR_1:%.*]] = pack_element_get [[RET_INDEX_1]] of %0 : $*Pack{Bool, Int} as $*Int
133+
// CHECK-NEXT: store [[ARG_1]] to [trivial] [[RET_ADDR_0]] : $*Bool
134+
// CHECK-NEXT: store [[ARG_0]] to [trivial] [[RET_ADDR_1]] : $*Int
135+
func testResultReabstractedEmission() {
136+
takeClosureWithVariadicResult {
137+
(a: Int, b: Bool) -> (Bool, Int) in (b, a)
138+
}
139+
}
140+
141+
// CHECK-LABEL: sil {{.*}}@$s4main40testResultReabstractedEmission_vanishingyyFSbSi_SbtXEfU_ :
142+
// CHECK-SAME: $@convention(thin) @substituted <each τ_0_0, each τ_0_1> (@pack_guaranteed Pack{repeat each τ_0_0}) -> @pack_out Pack{repeat each τ_0_1} for <Pack{Int, Bool}, Pack{Bool}>
143+
// CHECK: bb0(%0 : $*Pack{Bool}, %1 : $*Pack{Int, Bool}):
144+
// CHECK-NEXT: [[ARG_INDEX_0:%.*]] = scalar_pack_index 0 of $Pack{Int, Bool}
145+
// CHECK-NEXT: [[ARG_ADDR_0:%.*]] = pack_element_get [[ARG_INDEX_0]] of %1 : $*Pack{Int, Bool} as $*Int
146+
// CHECK-NEXT: [[ARG_0:%.*]] = load [trivial] [[ARG_ADDR_0]] : $*Int
147+
// CHECK-NEXT: debug_value [[ARG_0]] :
148+
// CHECK-NEXT: [[ARG_INDEX_1:%.*]] = scalar_pack_index 1 of $Pack{Int, Bool}
149+
// CHECK-NEXT: [[ARG_ADDR_1:%.*]] = pack_element_get [[ARG_INDEX_1]] of %1 : $*Pack{Int, Bool} as $*Bool
150+
// CHECK-NEXT: [[ARG_1:%.*]] = load [trivial] [[ARG_ADDR_1]] : $*Bool
151+
// CHECK-NEXT: debug_value [[ARG_1]] :
152+
// CHECK-NEXT: [[RET_INDEX_0:%.*]] = scalar_pack_index 0 of $Pack{Bool}
153+
// CHECK-NEXT: [[RET_ADDR_0:%.*]] = pack_element_get [[RET_INDEX_0]] of %0 : $*Pack{Bool} as $*Bool
154+
// CHECK-NEXT: store [[ARG_1]] to [trivial] [[RET_ADDR_0]] : $*Bool
155+
func testResultReabstractedEmission_vanishing() {
156+
takeClosureWithVariadicResult {
157+
(a: Int, b: Bool) -> Bool in b
158+
}
159+
}

0 commit comments

Comments
 (0)