Skip to content

Commit e1403c6

Browse files
committed
[GSB] Improve handling of layout constraints
This PR addresses TODOs from #8241. - It supports merging for layout constraints, e.g., if both a _Trivial constraint and a _Trivial(64) constraint appear on a type parameter, we keep only _Trivial(64) as a more specific layout constraint. We do a similar thing for ref-counted/native-ref-counted. The overall idea is to keep the more specific of two compatible layout constraints. - The presence of a superclass constraint implies a layout constraint, e.g., a superclass constraint implies _Class or _NativeClass
1 parent 4e22629 commit e1403c6

File tree

5 files changed

+208
-3
lines changed

5 files changed

+208
-3
lines changed

include/swift/AST/LayoutConstraint.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,10 @@ class LayoutConstraint {
259259

260260
LayoutConstraintInfo *operator->() const { return Ptr; }
261261

262+
/// Merge these two constraints and return a more specific one
263+
/// or fail if they’re incompatible and return an unknown constraint.
264+
LayoutConstraint merge(LayoutConstraint Other);
265+
262266
explicit operator bool() const { return Ptr != 0; }
263267

264268
void dump() const;

lib/AST/GenericSignatureBuilder.cpp

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2079,9 +2079,14 @@ bool GenericSignatureBuilder::addLayoutRequirement(
20792079
equivClass->layoutConstraints.push_back({PAT, Layout, Source});
20802080

20812081
// Update the layout in the equivalence class, if we didn't have one already.
2082-
// FIXME: Layouts can probably be merged sensibly.
20832082
if (!equivClass->layout)
20842083
equivClass->layout = Layout;
2084+
else {
2085+
// Try to merge layout constraints.
2086+
auto mergedLayout = equivClass->layout.merge(Layout);
2087+
if (mergedLayout->isKnownLayout() && mergedLayout != equivClass->layout)
2088+
equivClass->layout = mergedLayout;
2089+
}
20852090

20862091
return false;
20872092
}
@@ -2120,6 +2125,16 @@ bool GenericSignatureBuilder::updateSuperclass(
21202125
if (!equivClass->superclass) {
21212126
equivClass->superclass = superclass;
21222127
updateSuperclassConformances();
2128+
// Presence of a superclass constraint implies a _Class layout
2129+
// constraint.
2130+
auto layoutReqSource = source->viaSuperclass(*this, nullptr);
2131+
addLayoutRequirement(T,
2132+
LayoutConstraint::getLayoutConstraint(
2133+
superclass->getClassOrBoundGenericClass()->isObjC()
2134+
? LayoutConstraintKind::Class
2135+
: LayoutConstraintKind::NativeClass,
2136+
getASTContext()),
2137+
layoutReqSource);
21232138
return false;
21242139
}
21252140

@@ -3459,7 +3474,9 @@ void GenericSignatureBuilder::checkLayoutConstraints(
34593474
return constraint.value == equivClass->layout;
34603475
},
34613476
[&](LayoutConstraint layout) {
3462-
if (layout == equivClass->layout)
3477+
// If the layout constraints are mergable, i.e. compatible,
3478+
// it is a redundancy.
3479+
if (layout.merge(equivClass->layout)->isKnownLayout())
34633480
return ConstraintRelation::Redundant;
34643481

34653482
return ConstraintRelation::Conflicting;

lib/AST/LayoutConstraint.cpp

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,168 @@ void *LayoutConstraintInfo::operator new(size_t bytes, const ASTContext &ctx,
140140

141141
SourceRange LayoutConstraintLoc::getSourceRange() const { return getLoc(); }
142142

143+
#define MERGE_LOOKUP(lhs, rhs) \
144+
mergeTable[int(lhs)][int(rhs)]
145+
146+
// Shortcut for spelling the whole enumerator.
147+
#define E(X) LayoutConstraintKind::X
148+
#define MERGE_CONFLICT LayoutConstraintKind::UnknownLayout
149+
150+
/// This is a 2-D table defining the merging rules for layout constraints.
151+
/// It is indexed by means of LayoutConstraintKind.
152+
/// mergeTable[i][j] tells the kind of a layout constraint is the result
153+
/// of merging layout constraints of kinds 'i' and 'j'.
154+
///
155+
/// Some of the properties of the merge table, where C is any type of layout
156+
/// constraint:
157+
/// UnknownLayout x C -> C
158+
/// C x UnknownLayout -> C
159+
/// C x C -> C
160+
/// mergeTable[i][j] == mergeTable[j][i], i.e. the table is symmetric.
161+
/// mergeTable[i][j] == UnknownLayout if merging layout constraints of kinds i
162+
/// and j would result in a conflict.
163+
static LayoutConstraintKind mergeTable[unsigned(E(LastLayout)) +
164+
1][unsigned(E(LastLayout)) + 1] = {
165+
// Initialize the row for UnknownLayout.
166+
{E(/* UnknownLayout */ UnknownLayout),
167+
E(/* TrivialOfExactSize */ TrivialOfExactSize),
168+
E(/* TrivialOfAtMostSize */ TrivialOfAtMostSize), E(/* Trivial */ Trivial),
169+
E(/* Class */ Class), E(/* NativeClass */ NativeClass),
170+
E(/* RefCountedObject*/ RefCountedObject),
171+
E(/* NativeRefCountedObject */ NativeRefCountedObject)},
172+
173+
// Initiaze the row for TrivialOfExactSize.
174+
{E(/* UnknownLayout */ TrivialOfExactSize),
175+
E(/* TrivialOfExactSize */ TrivialOfExactSize), MERGE_CONFLICT,
176+
E(/* Trivial */ TrivialOfExactSize), MERGE_CONFLICT, MERGE_CONFLICT,
177+
MERGE_CONFLICT, MERGE_CONFLICT},
178+
179+
// Initiaze the row for TrivialOfAtMostSize.
180+
{E(/* UnknownLayout */ TrivialOfAtMostSize), MERGE_CONFLICT,
181+
E(/* TrivialOfAtMostSize */ TrivialOfAtMostSize),
182+
E(/* Trivial */ TrivialOfAtMostSize), MERGE_CONFLICT, MERGE_CONFLICT,
183+
MERGE_CONFLICT, MERGE_CONFLICT},
184+
185+
// Initiaze the row for Trivial.
186+
{E(/* UnknownLayout */ Trivial),
187+
E(/* TrivialOfExactSize */ TrivialOfExactSize),
188+
E(/* TrivialOfAtMostSize */ TrivialOfAtMostSize), E(/* Trivial */ Trivial),
189+
MERGE_CONFLICT, MERGE_CONFLICT, MERGE_CONFLICT, MERGE_CONFLICT},
190+
191+
// Initiaze the row for Class.
192+
{E(/* UnknownLayout*/ Class), MERGE_CONFLICT, MERGE_CONFLICT,
193+
MERGE_CONFLICT, E(/* Class */ Class), E(/* NativeClass */ NativeClass),
194+
E(/* RefCountedObject */ Class),
195+
E(/* NativeRefCountedObject */ NativeClass)},
196+
197+
// Initiaze the row for NativeClass.
198+
{E(/* UnknownLayout */ NativeClass), MERGE_CONFLICT, MERGE_CONFLICT,
199+
MERGE_CONFLICT, E(/* Class */ NativeClass),
200+
E(/* NativeClass */ NativeClass), E(/* RefCountedObject */ NativeClass),
201+
E(/* NativeRefCountedObject */ NativeClass)},
202+
203+
// Initiaze the row for RefCountedObject.
204+
{E(/* UnknownLayout */ RefCountedObject), MERGE_CONFLICT, MERGE_CONFLICT,
205+
MERGE_CONFLICT, E(/* Class */ Class), E(/* NativeClass */ NativeClass),
206+
E(/* RefCountedObject */ RefCountedObject),
207+
E(/* NativeRefCountedObject */ NativeRefCountedObject)},
208+
209+
// Initiaze the row for NativeRefCountedObject.
210+
{E(/* UnknownLayout */ NativeRefCountedObject), MERGE_CONFLICT,
211+
MERGE_CONFLICT, MERGE_CONFLICT, E(/* Class */ NativeClass),
212+
E(/* NativeClass */ NativeClass),
213+
E(/* RefCountedObject */ NativeRefCountedObject),
214+
E(/* NativeRefCountedObject*/ NativeRefCountedObject)},
215+
};
216+
217+
#undef E
218+
219+
// Fixed-size trivial constraint can be combined with AtMostSize trivial
220+
// constraint into a fixed-size trivial constraint, if
221+
// fixed_size_layout.size <= at_most_size_layout.size and
222+
// their alignment requirements are not contradicting.
223+
//
224+
// Only merges if LHS would become the result of the merge.
225+
static LayoutConstraint
226+
mergeKnownSizeTrivialConstraints(LayoutConstraint LHS, LayoutConstraint RHS) {
227+
assert(LHS->isKnownSizeTrivial() && RHS->isKnownSizeTrivial());
228+
229+
// LHS should be fixed-size.
230+
if (!LHS->isFixedSizeTrivial())
231+
return LayoutConstraint::getUnknownLayout();
232+
233+
// RHS should be at-most-size.
234+
if (RHS->isFixedSizeTrivial())
235+
return LayoutConstraint::getUnknownLayout();
236+
237+
// Check that sizes are compatible, i.e.
238+
// fixed_size_layout.size <= at_most_size_layout.size
239+
if (LHS->getMaxTrivialSizeInBits() > RHS->getMaxTrivialSizeInBits())
240+
return LayoutConstraint::getUnknownLayout();
241+
242+
// Check alignments
243+
244+
// Quick exit if at_most_size_layout does not care about the alignment.
245+
if (!RHS->getAlignment())
246+
return LHS;
247+
248+
// Check if fixed_size_layout.alignment is a multple of
249+
// at_most_size_layout.alignment.
250+
if (LHS->getAlignment() && LHS->getAlignment() % RHS->getAlignment() == 0)
251+
return LHS;
252+
253+
return LayoutConstraint::getUnknownLayout();
254+
}
255+
256+
LayoutConstraint
257+
LayoutConstraint::merge(LayoutConstraint Other) {
258+
auto Self = *this;
259+
260+
// If both constraints are the same, they are always equal.
261+
if (Self == Other)
262+
return Self;
263+
264+
// TrivialOfExactSize and TrivialOfAtMostSize are a special case,
265+
// because not only their kind, but their parameters need to be compared as
266+
// well.
267+
if (Self->isKnownSizeTrivial() && Other->isKnownSizeTrivial()) {
268+
// If we got here, it means that the Self == Other check above has failed.
269+
// And that could only happen if either the kinds are different or
270+
// size/alignment parameters are different.
271+
272+
// Try to merge fixed-size constraint with an at-most-size constraint,
273+
// if possible.
274+
LayoutConstraint MergedKnownSizeTrivial;
275+
MergedKnownSizeTrivial = mergeKnownSizeTrivialConstraints(Self, Other);
276+
if (MergedKnownSizeTrivial->isKnownLayout())
277+
return MergedKnownSizeTrivial;
278+
279+
MergedKnownSizeTrivial = mergeKnownSizeTrivialConstraints(Other, Self);
280+
if (MergedKnownSizeTrivial->isKnownLayout())
281+
return MergedKnownSizeTrivial;
282+
283+
return LayoutConstraint::getUnknownLayout();
284+
}
285+
286+
// Lookup in the mergeTable if this combination of layouts can be merged.
287+
auto mergeKind = MERGE_LOOKUP(Self->getKind(), Other->getKind());
288+
// The merge table should be symmetric.
289+
assert(mergeKind == MERGE_LOOKUP(Other->getKind(), Self->getKind()));
290+
291+
// Merge is not possible, report a conflict.
292+
if (mergeKind == LayoutConstraintKind::UnknownLayout)
293+
return LayoutConstraint::getUnknownLayout();
294+
295+
if (Self->getKind() == mergeKind)
296+
return Self;
297+
298+
if (Other->getKind() == mergeKind)
299+
return Other;
300+
301+
// The result of the merge is not equal to any of the input constraints, e.g.
302+
// Class x NativeRefCountedObject -> NativeClass.
303+
return LayoutConstraint::getLayoutConstraint(mergeKind);
304+
}
143305

144306
LayoutConstraint
145307
LayoutConstraint::getLayoutConstraint(LayoutConstraintKind Kind) {

test/Generics/superclass_constraint.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ extension P2 where Self.T : C {
7676
// CHECK: superclassConformance1
7777
// CHECK: Requirements:
7878
// CHECK-NEXT: τ_0_0 : C [τ_0_0: Explicit @ {{.*}}:11]
79+
// CHECK-NEXT: τ_0_0 : _NativeClass [τ_0_0: Explicit @ {{.*}}:11 -> Superclass]
7980
// CHECK-NEXT: τ_0_0 : P3 [τ_0_0: Explicit @ {{.*}}:11 -> Superclass (C: P3)]
8081
// CHECK: Canonical generic signature: <τ_0_0 where τ_0_0 : C>
8182
func superclassConformance1<T>(t: T)
@@ -87,6 +88,7 @@ func superclassConformance1<T>(t: T)
8788
// CHECK: superclassConformance2
8889
// CHECK: Requirements:
8990
// CHECK-NEXT: τ_0_0 : C [τ_0_0: Explicit @ {{.*}}:11]
91+
// CHECK-NEXT: τ_0_0 : _NativeClass [τ_0_0: Explicit @ {{.*}}:11 -> Superclass]
9092
// CHECK-NEXT: τ_0_0 : P3 [τ_0_0: Explicit @ {{.*}}:11 -> Superclass (C: P3)]
9193
// CHECK: Canonical generic signature: <τ_0_0 where τ_0_0 : C>
9294
func superclassConformance2<T>(t: T)
@@ -100,6 +102,7 @@ class C2 : C, P4 { }
100102
// CHECK: superclassConformance3
101103
// CHECK: Requirements:
102104
// CHECK-NEXT: τ_0_0 : C2 [τ_0_0: Explicit @ {{.*}}:46]
105+
// CHECK-NEXT: τ_0_0 : _NativeClass [τ_0_0: Explicit @ {{.*}}:46 -> Superclass]
103106
// CHECK-NEXT: τ_0_0 : P4 [τ_0_0: Explicit @ {{.*}}:61 -> Superclass (C2: P4)]
104107
// CHECK: Canonical generic signature: <τ_0_0 where τ_0_0 : C2>
105108
func superclassConformance3<T>(t: T) where T : C, T : P4, T : C2 {}

test/attr/attr_specialize.swift

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,15 +160,34 @@ func funcWithForbiddenSpecializeRequirement<T>(_ t: T) {
160160
@_specialize(where T: _Trivial(32), T: _Trivial(64), T: _Trivial, T: _RefCountedObject)
161161
// expected-error@-1{{generic parameter 'T' has conflicting layout constraints '_Trivial(64)' and '_Trivial(32)'}}
162162
// expected-error@-2{{generic parameter 'T' has conflicting layout constraints '_RefCountedObject' and '_Trivial(32)'}}
163-
// expected-error@-3{{generic parameter 'T' has conflicting layout constraints '_Trivial' and '_Trivial(32)'}}
163+
// expected-warning@-3{{redundant layout constraint 'T' : '_Trivial'}}
164164
// expected-note@-4 3{{layout constraint constraint 'T' : '_Trivial(32)' written here}}
165+
@_specialize(where T: _Trivial, T: _Trivial(64))
166+
// expected-warning@-1{{redundant layout constraint 'T' : '_Trivial'}}
167+
// expected-note@-2 1{{layout constraint constraint 'T' : '_Trivial(64)' written here}}
168+
@_specialize(where T: _RefCountedObject, T: _NativeRefCountedObject)
169+
// expected-warning@-1{{redundant layout constraint 'T' : '_RefCountedObject'}}
170+
// expected-note@-2 1{{layout constraint constraint 'T' : '_NativeRefCountedObject' written here}}
165171
@_specialize(where Array<T> == Int) // expected-error{{Only requirements on generic parameters are supported by '_specialize' attribute}}
166172
// expected-error@-1{{generic signature requires types 'Array<T>' and 'Int' to be the same}}
167173
@_specialize(where T.Element == Int) // expected-error{{Only requirements on generic parameters are supported by '_specialize' attribute}}
168174
public func funcWithComplexSpecializeRequirements<T: ProtocolWithDep>(t: T) -> Int {
169175
return 55555
170176
}
171177

178+
public protocol Proto: class {
179+
}
180+
181+
@_specialize(where T: _RefCountedObject)
182+
// expected-warning@-1{{redundant layout constraint 'T' : '_RefCountedObject'}}
183+
@_specialize(where T: _Trivial)
184+
// expected-error@-1{{generic parameter 'T' has conflicting layout constraints '_Trivial' and '_NativeClass'}}
185+
@_specialize(where T: _Trivial(64))
186+
// expected-error@-1{{generic parameter 'T' has conflicting layout constraints '_Trivial(64)' and '_NativeClass'}}
187+
public func funcWithABaseClassRequirement<T>(t: T) -> Int where T: C1 {
188+
return 44444
189+
}
190+
172191
public struct S1 {
173192
}
174193

0 commit comments

Comments
 (0)