Skip to content

Commit 490edfa

Browse files
authored
Merge pull request #81564 from slavapestov/fix-issue-78191
IRGen: Fix miscompile when a generic parameter is fixed to a tuple containing a pack
2 parents c8eba86 + f17baef commit 490edfa

File tree

6 files changed

+103
-35
lines changed

6 files changed

+103
-35
lines changed

lib/IRGen/Fulfillment.cpp

Lines changed: 33 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -178,35 +178,44 @@ bool FulfillmentMap::searchTypeMetadata(IRGenModule &IGM, CanType type,
178178
source, std::move(path), keys);
179179
}
180180

181-
if (auto tupleType = dyn_cast<TupleType>(type)) {
182-
if (tupleType->getNumElements() == 1 &&
183-
isa<PackExpansionType>(tupleType.getElementType(0))) {
184-
185-
bool hadFulfillment = false;
186-
auto packType = tupleType.getInducedPackType();
187-
188-
{
189-
auto argPath = path;
190-
argPath.addTuplePackComponent();
191-
hadFulfillment |= searchTypeMetadataPack(IGM, packType,
192-
isExact, metadataState, source,
193-
std::move(argPath), keys);
194-
}
181+
// TODO: functions
182+
// TODO: metatypes
195183

196-
{
197-
auto argPath = path;
198-
argPath.addTupleShapeComponent();
199-
hadFulfillment |= searchShapeRequirement(IGM, packType, source,
200-
std::move(argPath));
184+
return false;
185+
}
201186

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

204-
return hadFulfillment;
198+
bool hadFulfillment = false;
199+
auto packType = tupleType.getInducedPackType();
200+
201+
{
202+
auto argPath = path;
203+
argPath.addTuplePackComponent();
204+
hadFulfillment |= searchTypeMetadataPack(IGM, packType,
205+
isExact, metadataState, source,
206+
std::move(argPath), keys);
205207
}
206-
}
207208

208-
// TODO: functions
209-
// TODO: metatypes
209+
{
210+
auto argPath = path;
211+
argPath.addTupleShapeComponent();
212+
hadFulfillment |= searchShapeRequirement(IGM, packType, source,
213+
std::move(argPath));
214+
215+
}
216+
217+
return hadFulfillment;
218+
}
210219

211220
return false;
212221
}

lib/IRGen/Fulfillment.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,14 @@ class FulfillmentMap {
102102
unsigned sourceIndex, MetadataPath &&path,
103103
const InterestingKeysCallback &interestingKeys);
104104

105+
/// Metadata fulfillment in tuple conformance witness thunks.
106+
///
107+
/// \return true if any fulfillments were added by this search.
108+
bool searchTupleTypeMetadata(IRGenModule &IGM, CanTupleType type, IsExact_t isExact,
109+
MetadataState metadataState,
110+
unsigned sourceIndex, MetadataPath &&path,
111+
const InterestingKeysCallback &interestingKeys);
112+
105113
/// Search the given type metadata pack for useful fulfillments.
106114
///
107115
/// \return true if any fulfillments were added by this search.

lib/IRGen/GenProto.cpp

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,8 @@ class PolymorphicConvention {
163163
CanType type, Args... args);
164164
bool considerType(CanType type, IsExact_t isExact,
165165
unsigned sourceIndex, MetadataPath &&path);
166+
bool considerTupleType(CanTupleType type, IsExact_t isExact,
167+
unsigned sourceIndex, MetadataPath &&path);
166168

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

359+
bool PolymorphicConvention::considerTupleType(CanTupleType type, IsExact_t isExact,
360+
unsigned sourceIndex,
361+
MetadataPath &&path) {
362+
FulfillmentMapCallback callbacks(*this);
363+
return Fulfillments.searchTupleTypeMetadata(IGM, type, isExact,
364+
MetadataState::Complete, sourceIndex,
365+
std::move(path), callbacks);
366+
}
367+
357368
void PolymorphicConvention::considerWitnessSelf(CanSILFunctionType fnType) {
358369
CanType selfTy = fnType->getSelfInstanceType(
359370
IGM.getSILModule(), IGM.getMaximalTypeExpansionContext());
@@ -362,13 +373,17 @@ void PolymorphicConvention::considerWitnessSelf(CanSILFunctionType fnType) {
362373
// First, bind type metadata for Self.
363374
Sources.emplace_back(MetadataSource::Kind::SelfMetadata, selfTy);
364375

365-
if (selfTy->is<GenericTypeParamType>()) {
366-
// The Self type is abstract, so we can fulfill its metadata from
367-
// the Self metadata parameter.
368-
addSelfMetadataFulfillment(selfTy);
369-
}
376+
if (auto tupleTy = dyn_cast<TupleType>(selfTy)) {
377+
considerTupleType(tupleTy, IsInexact, Sources.size() - 1, MetadataPath());
378+
} else {
379+
if (isa<GenericTypeParamType>(selfTy)) {
380+
// The Self type is abstract, so we can fulfill its metadata from
381+
// the Self metadata parameter.
382+
addSelfMetadataFulfillment(selfTy);
383+
}
370384

371-
considerType(selfTy, IsInexact, Sources.size() - 1, MetadataPath());
385+
considerType(selfTy, IsInexact, Sources.size() - 1, MetadataPath());
386+
}
372387

373388
// The witness table for the Self : P conformance can be
374389
// fulfilled from the Self witness table parameter.

lib/IRGen/LocalTypeData.cpp

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -770,11 +770,20 @@ void LocalTypeDataCache::addAbstractForTypeMetadata(IRGenFunction &IGF,
770770
// Look for anything at all that's fulfilled by this. If we don't find
771771
// anything, stop.
772772
FulfillmentMap fulfillments;
773-
if (!fulfillments.searchTypeMetadata(IGF.IGM, type, isExact,
774-
metadata.getStaticLowerBoundOnState(),
775-
/*source*/ 0, MetadataPath(),
776-
callbacks)) {
777-
return;
773+
if (auto tupleType = dyn_cast<TupleType>(type)) {
774+
if (!fulfillments.searchTupleTypeMetadata(IGF.IGM, tupleType, isExact,
775+
metadata.getStaticLowerBoundOnState(),
776+
/*source*/ 0, MetadataPath(),
777+
callbacks)) {
778+
return;
779+
}
780+
} else {
781+
if (!fulfillments.searchTypeMetadata(IGF.IGM, type, isExact,
782+
metadata.getStaticLowerBoundOnState(),
783+
/*source*/ 0, MetadataPath(),
784+
callbacks)) {
785+
return;
786+
}
778787
}
779788

780789
addAbstractForFulfillments(IGF, std::move(fulfillments),
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// RUN: %target-swift-frontend -emit-ir %s -target %target-swift-5.9-abi-triple | %FileCheck %s
2+
3+
struct G<T> {
4+
var t: T
5+
6+
// Make sure we *don't* try to fulfill the pack from the tuple, because we cannot
7+
// distinguish the one-element case that way.
8+
9+
// 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)
10+
func fixOuterGenericParam<each U>(_ t: T) -> G<T> where T == (repeat each U) {
11+
return G<T>(t: t)
12+
}
13+
}

test/Interpreter/variadic_generic_tuples.swift

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,5 +85,19 @@ tuples.test("labels") {
8585
expectEqual("(label: Swift.Int, Swift.String)", _typeName(oneElementLabeledTuple(t: String.self)))
8686
}
8787

88+
// https://github.com/swiftlang/swift/issues/78191
89+
tuples.test("fulfillment") {
90+
struct S<T> {
91+
let t: T
92+
93+
func f<each A, each B>(_ b: S<(repeat each B)>) -> S<(repeat each A, repeat each B)>
94+
where T == (repeat each A) {
95+
return S<(repeat each A, repeat each B)>(t: (repeat each t, repeat each b.t))
96+
}
97+
}
98+
99+
let s = S(t: "hello")
100+
expectEqual("S<(String, Int, Int)>(t: (\"hello\", 1, 2))", String(describing: s.f(S(t: (1, 2)))))
101+
}
88102

89103
runAllTests()

0 commit comments

Comments
 (0)