Skip to content

Commit 462542e

Browse files
authored
Merge pull request #64231 from rjmccall/variadic-generic-tuple-fixes
Fix a bunch of problems with SILGen of tuples with pack expansions
2 parents 3211f68 + 239777a commit 462542e

22 files changed

+797
-139
lines changed

include/swift/AST/Types.h

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2456,15 +2456,30 @@ BEGIN_CAN_TYPE_WRAPPER(TupleType, Type)
24562456
return containsPackExpansionTypeImpl(*this);
24572457
}
24582458

2459+
/// Induce a pack type from the elements of this tuple type.
2460+
inline CanTypeWrapper<PackType> getInducedPackType() const;
2461+
24592462
/// Induce a pack type from a range of the elements of this tuple type.
24602463
inline CanTypeWrapper<PackType>
24612464
getInducedPackType(unsigned start, unsigned count) const;
24622465

2466+
/// Induce a formal pack type with the same shape as the elements
2467+
/// of this lowered tuple type, making a best effort to use the same
2468+
/// element types.
2469+
inline CanTypeWrapper<PackType>
2470+
getInducedApproximateFormalPackType() const;
2471+
24632472
private:
24642473
static bool containsPackExpansionTypeImpl(CanTupleType tuple);
24652474

2475+
static CanTypeWrapper<PackType>
2476+
getInducedPackTypeImpl(CanTupleType tuple);
2477+
24662478
static CanTypeWrapper<PackType>
24672479
getInducedPackTypeImpl(CanTupleType tuple, unsigned start, unsigned count);
2480+
2481+
static CanTypeWrapper<PackType>
2482+
getInducedApproximateFormalPackTypeImpl(CanTupleType tuple);
24682483
END_CAN_TYPE_WRAPPER(TupleType, Type)
24692484

24702485
/// UnboundGenericType - Represents a generic type where the type arguments have
@@ -5355,6 +5370,11 @@ class SILPackType final : public TypeBase, public llvm::FoldingSetNode,
53555370
/// as the shape, not a SIL pack type.
53565371
CanTypeWrapper<PackType> getReducedShape() const;
53575372

5373+
/// Construct a formal pack type with the same shape as this
5374+
/// lowered pack type, making a best effort to use the same element
5375+
/// types.
5376+
CanTypeWrapper<PackType> getApproximateFormalPackType() const;
5377+
53585378
bool containsPackExpansionType() const;
53595379

53605380
void Profile(llvm::FoldingSetNodeID &ID) const {
@@ -6841,11 +6861,20 @@ BEGIN_CAN_TYPE_WRAPPER(PackType, Type)
68416861
}
68426862
END_CAN_TYPE_WRAPPER(PackType, Type)
68436863

6864+
inline CanPackType CanTupleType::getInducedPackType() const {
6865+
return getInducedPackTypeImpl(*this);
6866+
}
6867+
68446868
inline CanPackType
68456869
CanTupleType::getInducedPackType(unsigned start, unsigned end) const {
68466870
return getInducedPackTypeImpl(*this, start, end);
68476871
}
68486872

6873+
inline CanPackType
6874+
CanTupleType::getInducedApproximateFormalPackType() const {
6875+
return getInducedApproximateFormalPackTypeImpl(*this);
6876+
}
6877+
68496878
/// PackExpansionType - The interface type of the explicit expansion of a
68506879
/// corresponding set of variadic generic parameters.
68516880
///

include/swift/SIL/AbstractionPattern.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1333,6 +1333,8 @@ class AbstractionPattern {
13331333
llvm_unreachable("bad kind");
13341334
}
13351335

1336+
bool doesTupleContainPackExpansionType() const;
1337+
13361338
static AbstractionPattern
13371339
projectTupleElementType(const AbstractionPattern *base, size_t index) {
13381340
return base->getTupleElementType(index);

include/swift/SIL/SILBuilder.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1675,6 +1675,9 @@ class SILBuilder {
16751675
SILValue Operand,
16761676
unsigned FieldNo,
16771677
SILType ResultTy) {
1678+
assert(!Operand->getType().castTo<TupleType>().containsPackExpansionType()
1679+
&& "tuples with pack expansions must be indexed with "
1680+
"tuple_pack_element_addr");
16781681
return insert(new (getModule()) TupleElementAddrInst(
16791682
getSILDebugLocation(Loc), Operand, FieldNo, ResultTy));
16801683
}

include/swift/SIL/SILType.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -531,6 +531,14 @@ class SILType {
531531
return SILType(castTo<TupleType>().getElementType(index), getCategory());
532532
}
533533

534+
/// Given that this is a pack expansion type, return the lowered type
535+
/// of the pattern type. The result will have the same value category
536+
/// as the base type.
537+
SILType getPackExpansionPatternType() const {
538+
return SILType(castTo<PackExpansionType>().getPatternType(),
539+
getCategory());
540+
}
541+
534542
/// Return the immediate superclass type of this type, or null if
535543
/// it's the most-derived type.
536544
SILType getSuperclass() const {

lib/AST/ParameterPack.cpp

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -478,9 +478,76 @@ bool SILPackType::containsPackExpansionType() const {
478478
return false;
479479
}
480480

481+
CanPackType
482+
CanTupleType::getInducedPackTypeImpl(CanTupleType tuple) {
483+
return getInducedPackTypeImpl(tuple, 0, tuple->getNumElements());
484+
}
485+
481486
CanPackType
482487
CanTupleType::getInducedPackTypeImpl(CanTupleType tuple, unsigned start, unsigned count) {
483488
assert(start + count <= tuple->getNumElements() && "range out of range");
484489
auto &ctx = tuple->getASTContext();
485490
return CanPackType::get(ctx, tuple.getElementTypes().slice(start, count));
486491
}
492+
493+
static CanType getApproximateFormalElementType(const ASTContext &ctx,
494+
CanType loweredEltType) {
495+
CanType formalEltType = TupleType::getEmpty(ctx);
496+
if (auto expansion = dyn_cast<PackExpansionType>(loweredEltType))
497+
formalEltType = CanPackExpansionType::get(formalEltType,
498+
expansion.getCountType());
499+
return formalEltType;
500+
}
501+
502+
template <class Collection>
503+
static CanPackType getApproximateFormalPackType(const ASTContext &ctx,
504+
Collection loweredEltTypes) {
505+
// Build an array of formal element types, but be lazy about it:
506+
// use the original array unless we see an element type that doesn't
507+
// work as a legal format type.
508+
Optional<SmallVector<CanType, 4>> formalEltTypes;
509+
for (auto i : indices(loweredEltTypes)) {
510+
auto loweredEltType = loweredEltTypes[i];
511+
bool isLegal = loweredEltType->isLegalFormalType();
512+
513+
// If the type isn't legal as a formal type, substitute the empty
514+
// tuple type (or an invariant expansion of it over the count type).
515+
CanType formalEltType = loweredEltType;
516+
if (!isLegal) {
517+
formalEltType = getApproximateFormalElementType(ctx, loweredEltType);
518+
}
519+
520+
// If we're already building an array, unconditionally append to it.
521+
// Otherwise, if the type isn't legal, build the array up to this
522+
// point and then append. Otherwise, we're still being lazy.
523+
if (formalEltTypes) {
524+
formalEltTypes->push_back(formalEltType);
525+
} else if (!isLegal) {
526+
formalEltTypes.emplace();
527+
formalEltTypes->reserve(loweredEltTypes.size());
528+
formalEltTypes->append(loweredEltTypes.begin(),
529+
loweredEltTypes.begin() + i);
530+
formalEltTypes->push_back(formalEltType);
531+
}
532+
533+
assert(isLegal || formalEltTypes.hasValue());
534+
}
535+
536+
// Use the array we built if we made one (if we ever saw a non-legal
537+
// element type).
538+
if (formalEltTypes) {
539+
return CanPackType::get(ctx, *formalEltTypes);
540+
} else {
541+
return CanPackType::get(ctx, loweredEltTypes);
542+
}
543+
}
544+
545+
CanPackType SILPackType::getApproximateFormalPackType() const {
546+
return ::getApproximateFormalPackType(getASTContext(), getElementTypes());
547+
}
548+
549+
CanPackType
550+
CanTupleType::getInducedApproximateFormalPackTypeImpl(CanTupleType tuple) {
551+
return ::getApproximateFormalPackType(tuple->getASTContext(),
552+
tuple.getElementTypes());
553+
}

lib/SIL/IR/AbstractionPattern.cpp

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -430,6 +430,40 @@ AbstractionPattern::getTupleElementType(unsigned index) const {
430430
llvm_unreachable("bad kind");
431431
}
432432

433+
bool AbstractionPattern::doesTupleContainPackExpansionType() const {
434+
switch (getKind()) {
435+
case Kind::Invalid:
436+
llvm_unreachable("querying invalid abstraction pattern!");
437+
case Kind::Opaque:
438+
case Kind::PartialCurriedObjCMethodType:
439+
case Kind::CurriedObjCMethodType:
440+
case Kind::CFunctionAsMethodType:
441+
case Kind::CurriedCFunctionAsMethodType:
442+
case Kind::PartialCurriedCFunctionAsMethodType:
443+
case Kind::ObjCMethodType:
444+
case Kind::CXXMethodType:
445+
case Kind::CurriedCXXMethodType:
446+
case Kind::PartialCurriedCXXMethodType:
447+
case Kind::OpaqueFunction:
448+
case Kind::OpaqueDerivativeFunction:
449+
llvm_unreachable("pattern is not a tuple");
450+
case Kind::Tuple: {
451+
for (auto &elt : llvm::makeArrayRef(OrigTupleElements,
452+
getNumTupleElements_Stored())) {
453+
if (elt.isPackExpansion())
454+
return true;
455+
}
456+
return true;
457+
}
458+
case Kind::ObjCCompletionHandlerArgumentsType:
459+
case Kind::Type:
460+
case Kind::Discard:
461+
case Kind::ClangType:
462+
return cast<TupleType>(getType()).containsPackExpansionType();
463+
}
464+
llvm_unreachable("bad kind");
465+
}
466+
433467
static CanType getCanPackElementType(CanType type, unsigned index) {
434468
return cast<PackType>(type).getElementType(index);
435469
}

lib/SIL/IR/SILFunctionType.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1705,7 +1705,7 @@ class DestructureInputs {
17051705
packElts.push_back(eltTy.getASTType());
17061706
});
17071707

1708-
bool indirect = origType.arePackElementsPassedIndirectly(TC);
1708+
bool indirect = origEltType.arePackElementsPassedIndirectly(TC);
17091709
SILPackType::ExtInfo extInfo(/*address*/ indirect);
17101710
auto packTy = SILPackType::get(TC.Context, extInfo, packElts);
17111711

lib/SIL/Verifier/SILVerifier.cpp

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3097,6 +3097,9 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
30973097

30983098
void checkTupleInst(TupleInst *TI) {
30993099
CanTupleType ResTy = requireObjectType(TupleType, TI, "Result of tuple");
3100+
require(!ResTy.containsPackExpansionType(),
3101+
"tuple instruction cannot be used with tuples containing "
3102+
"pack expansions");
31003103

31013104
require(TI->getElements().size() == ResTy->getNumElements(),
31023105
"Tuple field count mismatch!");
@@ -3288,6 +3291,10 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
32883291
void checkTupleExtractInst(TupleExtractInst *EI) {
32893292
CanTupleType operandTy = requireObjectType(TupleType, EI->getOperand(),
32903293
"Operand of tuple_extract");
3294+
require(!operandTy.containsPackExpansionType(),
3295+
"tuple_extract cannot be used with tuples containing "
3296+
"pack expansions");
3297+
32913298
require(EI->getType().isObject(),
32923299
"result of tuple_extract must be object");
32933300

@@ -3343,21 +3350,21 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
33433350
}
33443351

33453352
void checkTupleElementAddrInst(TupleElementAddrInst *EI) {
3346-
SILType operandTy = EI->getOperand()->getType();
3347-
require(operandTy.isAddress(),
3348-
"must derive element_addr from address");
33493353
require(EI->getType().isAddress(),
33503354
"result of tuple_element_addr must be address");
3351-
require(operandTy.is<TupleType>(),
3352-
"must derive tuple_element_addr from tuple");
3353-
3354-
ArrayRef<TupleTypeElt> fields = operandTy.castTo<TupleType>()->getElements();
3355-
require(EI->getFieldIndex() < fields.size(),
3356-
"invalid field index for element_addr instruction");
3355+
SILType operandTy = EI->getOperand()->getType();
3356+
auto tupleType = requireAddressType(TupleType, operandTy,
3357+
"operand of tuple_element_addr must be the address of a tuple");
3358+
require(!tupleType.containsPackExpansionType(),
3359+
"tuple_element_addr cannot be used with tuples containing "
3360+
"pack expansions");
3361+
3362+
require(EI->getFieldIndex() < tupleType->getNumElements(),
3363+
"invalid field index for tuple_element_addr instruction");
33573364
if (EI->getModule().getStage() != SILStage::Lowered) {
33583365
requireSameType(
33593366
EI->getType().getASTType(),
3360-
CanType(fields[EI->getFieldIndex()].getType()),
3367+
tupleType.getElementType(EI->getFieldIndex()),
33613368
"type of tuple_element_addr does not match type of element");
33623369
}
33633370
}

lib/SILGen/Cleanup.cpp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -420,3 +420,27 @@ ManagedValue CleanupCloner::clone(SILValue value) const {
420420
}
421421
return SGF.emitManagedRValueWithCleanup(value);
422422
}
423+
424+
ManagedValue
425+
CleanupCloner::cloneForTuplePackExpansionComponent(SILValue tupleAddr,
426+
CanPackType inducedPackType,
427+
unsigned componentIndex) const {
428+
if (isLValue) {
429+
return ManagedValue::forLValue(tupleAddr);
430+
}
431+
432+
if (!hasCleanup) {
433+
return ManagedValue::forUnmanaged(tupleAddr);
434+
}
435+
436+
assert(!writebackBuffer.has_value());
437+
auto expansionTy = tupleAddr->getType().getTupleElementType(componentIndex);
438+
if (expansionTy.getPackExpansionPatternType().isTrivial(SGF.F))
439+
return ManagedValue::forUnmanaged(tupleAddr);
440+
441+
auto cleanup =
442+
SGF.enterPartialDestroyRemainingTupleCleanup(tupleAddr, inducedPackType,
443+
componentIndex,
444+
/*start at */ SILValue());
445+
return ManagedValue(tupleAddr, cleanup);
446+
}

lib/SILGen/Cleanup.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,10 @@ class CleanupCloner {
338338

339339
ManagedValue clone(SILValue value) const;
340340

341+
ManagedValue cloneForTuplePackExpansionComponent(SILValue value,
342+
CanPackType inducedPackType,
343+
unsigned componentIndex) const;
344+
341345
static void
342346
getClonersForRValue(SILGenFunction &SGF, const RValue &rvalue,
343347
SmallVectorImpl<CleanupCloner> &resultingCloners);

0 commit comments

Comments
 (0)