Skip to content

IRGen: Fix miscompile when a generic parameter is fixed to a tuple containing a pack #81564

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 33 additions & 24 deletions lib/IRGen/Fulfillment.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -178,35 +178,44 @@ bool FulfillmentMap::searchTypeMetadata(IRGenModule &IGM, CanType type,
source, std::move(path), keys);
}

if (auto tupleType = dyn_cast<TupleType>(type)) {
if (tupleType->getNumElements() == 1 &&
isa<PackExpansionType>(tupleType.getElementType(0))) {

bool hadFulfillment = false;
auto packType = tupleType.getInducedPackType();

{
auto argPath = path;
argPath.addTuplePackComponent();
hadFulfillment |= searchTypeMetadataPack(IGM, packType,
isExact, metadataState, source,
std::move(argPath), keys);
}
// TODO: functions
// TODO: metatypes

{
auto argPath = path;
argPath.addTupleShapeComponent();
hadFulfillment |= searchShapeRequirement(IGM, packType, source,
std::move(argPath));
return false;
}

}
/// Metadata fulfillment in a tuple conformance witness thunks.
///
/// \return true if any fulfillments were added by this search.
bool FulfillmentMap::searchTupleTypeMetadata(IRGenModule &IGM, CanTupleType tupleType,
IsExact_t isExact,
MetadataState metadataState,
unsigned source, MetadataPath &&path,
const InterestingKeysCallback &keys) {
if (tupleType->getNumElements() == 1 &&
isa<PackExpansionType>(tupleType.getElementType(0))) {

return hadFulfillment;
bool hadFulfillment = false;
auto packType = tupleType.getInducedPackType();

{
auto argPath = path;
argPath.addTuplePackComponent();
hadFulfillment |= searchTypeMetadataPack(IGM, packType,
isExact, metadataState, source,
std::move(argPath), keys);
}
}

// TODO: functions
// TODO: metatypes
{
auto argPath = path;
argPath.addTupleShapeComponent();
hadFulfillment |= searchShapeRequirement(IGM, packType, source,
std::move(argPath));

}

return hadFulfillment;
}

return false;
}
Expand Down
8 changes: 8 additions & 0 deletions lib/IRGen/Fulfillment.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,14 @@ class FulfillmentMap {
unsigned sourceIndex, MetadataPath &&path,
const InterestingKeysCallback &interestingKeys);

/// Metadata fulfillment in tuple conformance witness thunks.
///
/// \return true if any fulfillments were added by this search.
bool searchTupleTypeMetadata(IRGenModule &IGM, CanTupleType type, IsExact_t isExact,
MetadataState metadataState,
unsigned sourceIndex, MetadataPath &&path,
const InterestingKeysCallback &interestingKeys);

/// Search the given type metadata pack for useful fulfillments.
///
/// \return true if any fulfillments were added by this search.
Expand Down
27 changes: 21 additions & 6 deletions lib/IRGen/GenProto.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,8 @@ class PolymorphicConvention {
CanType type, Args... args);
bool considerType(CanType type, IsExact_t isExact,
unsigned sourceIndex, MetadataPath &&path);
bool considerTupleType(CanTupleType type, IsExact_t isExact,
unsigned sourceIndex, MetadataPath &&path);

/// Testify to generic parameters in the Self type of a protocol
/// witness method.
Expand Down Expand Up @@ -354,6 +356,15 @@ bool PolymorphicConvention::considerType(CanType type, IsExact_t isExact,
std::move(path), callbacks);
}

bool PolymorphicConvention::considerTupleType(CanTupleType type, IsExact_t isExact,
unsigned sourceIndex,
MetadataPath &&path) {
FulfillmentMapCallback callbacks(*this);
return Fulfillments.searchTupleTypeMetadata(IGM, type, isExact,
MetadataState::Complete, sourceIndex,
std::move(path), callbacks);
}

void PolymorphicConvention::considerWitnessSelf(CanSILFunctionType fnType) {
CanType selfTy = fnType->getSelfInstanceType(
IGM.getSILModule(), IGM.getMaximalTypeExpansionContext());
Expand All @@ -362,13 +373,17 @@ void PolymorphicConvention::considerWitnessSelf(CanSILFunctionType fnType) {
// First, bind type metadata for Self.
Sources.emplace_back(MetadataSource::Kind::SelfMetadata, selfTy);

if (selfTy->is<GenericTypeParamType>()) {
// The Self type is abstract, so we can fulfill its metadata from
// the Self metadata parameter.
addSelfMetadataFulfillment(selfTy);
}
if (auto tupleTy = dyn_cast<TupleType>(selfTy)) {
considerTupleType(tupleTy, IsInexact, Sources.size() - 1, MetadataPath());
} else {
if (isa<GenericTypeParamType>(selfTy)) {
// The Self type is abstract, so we can fulfill its metadata from
// the Self metadata parameter.
addSelfMetadataFulfillment(selfTy);
}

considerType(selfTy, IsInexact, Sources.size() - 1, MetadataPath());
considerType(selfTy, IsInexact, Sources.size() - 1, MetadataPath());
}

// The witness table for the Self : P conformance can be
// fulfilled from the Self witness table parameter.
Expand Down
19 changes: 14 additions & 5 deletions lib/IRGen/LocalTypeData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -735,11 +735,20 @@ void LocalTypeDataCache::addAbstractForTypeMetadata(IRGenFunction &IGF,
// Look for anything at all that's fulfilled by this. If we don't find
// anything, stop.
FulfillmentMap fulfillments;
if (!fulfillments.searchTypeMetadata(IGF.IGM, type, isExact,
metadata.getStaticLowerBoundOnState(),
/*source*/ 0, MetadataPath(),
callbacks)) {
return;
if (auto tupleType = dyn_cast<TupleType>(type)) {
if (!fulfillments.searchTupleTypeMetadata(IGF.IGM, tupleType, isExact,
metadata.getStaticLowerBoundOnState(),
/*source*/ 0, MetadataPath(),
callbacks)) {
return;
}
} else {
if (!fulfillments.searchTypeMetadata(IGF.IGM, type, isExact,
metadata.getStaticLowerBoundOnState(),
/*source*/ 0, MetadataPath(),
callbacks)) {
return;
}
}

addAbstractForFulfillments(IGF, std::move(fulfillments),
Expand Down
13 changes: 13 additions & 0 deletions test/IRGen/variadic_generic_fulfillment_tuple.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// RUN: %target-swift-frontend -emit-ir %s -target %target-swift-5.9-abi-triple | %FileCheck %s

struct G<T> {
var t: T

// Make sure we *don't* try to fulfill the pack from the tuple, because we cannot
// distinguish the one-element case that way.

// CHECK-LABEL: define {{.*}} void @"$s34variadic_generic_fulfillment_tuple1GV20fixOuterGenericParamyACyqd__qd__Qp_tGqd__qd__Qp_t_tRvd__qd__qd__Qp_tRszlF"(ptr noalias sret(%swift.opaque) %0, ptr noalias %1, {{i32|i64}} %2, ptr %"each U", ptr noalias swiftself %3)
func fixOuterGenericParam<each U>(_ t: T) -> G<T> where T == (repeat each U) {
return G<T>(t: t)
}
}
14 changes: 14 additions & 0 deletions test/Interpreter/variadic_generic_tuples.swift
Original file line number Diff line number Diff line change
Expand Up @@ -85,5 +85,19 @@ tuples.test("labels") {
expectEqual("(label: Swift.Int, Swift.String)", _typeName(oneElementLabeledTuple(t: String.self)))
}

// https://github.com/swiftlang/swift/issues/78191
tuples.test("fulfillment") {
struct S<T> {
let t: T

func f<each A, each B>(_ b: S<(repeat each B)>) -> S<(repeat each A, repeat each B)>
where T == (repeat each A) {
return S<(repeat each A, repeat each B)>(t: (repeat each t, repeat each b.t))
}
}

let s = S(t: "hello")
expectEqual("S<(String, Int, Int)>(t: (\"hello\", 1, 2))", String(describing: s.f(S(t: (1, 2)))))
}

runAllTests()