Skip to content

Commit 7f8175b

Browse files
slavapestovslavapestov
committed
RequirementMachine: Add two more completion termination checks for concrete type requirements
The concrete nesting limit, which defaults to 30, catches things like A == G<A>. However, with something like A == (A, A), you end up with an exponential problem size before you hit the limit. Add two new limits. The first is the total size of the concrete type, counting all leaves, which defaults to 4000. It can be set with the -requirement-machine-max-concrete-size= frontend flag. The second avoids an assertion in addTypeDifference() which can be hit if a certain counter overflows before any other limit is breached. This also defaults to 4000 and can be set with the -requirement-machine-max-type-differences= frontend flag.
1 parent d22a247 commit 7f8175b

17 files changed

+191
-34
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3441,7 +3441,7 @@ WARNING(associated_type_override_typealias,none,
34413441

34423442
ERROR(requirement_machine_completion_failed,none,
34433443
"cannot build rewrite system for %select{generic signature|protocol}0; "
3444-
"%select{%error|rule count|rule length|concrete nesting}1 limit exceeded",
3444+
"%select{%error|rule count|rule length|concrete type nesting|concrete type size|concrete type difference}1 limit exceeded",
34453445
(unsigned, unsigned))
34463446
NOTE(requirement_machine_completion_rule,none,
34473447
"failed rewrite rule is %0", (StringRef))

‎include/swift/Basic/LangOptions.h

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -580,10 +580,18 @@ namespace swift {
580580
/// algorithm.
581581
unsigned RequirementMachineMaxRuleLength = 12;
582582

583-
/// Maximum concrete type nesting depth for requirement machine property map
584-
/// algorithm.
583+
/// Maximum concrete type nesting depth (when type is viewed as a tree) for
584+
/// requirement machine property map algorithm.
585585
unsigned RequirementMachineMaxConcreteNesting = 30;
586586

587+
/// Maximum concrete type size (total number of nodes in the type tree) for
588+
/// requirement machine property map algorithm.
589+
unsigned RequirementMachineMaxConcreteSize = 4000;
590+
591+
/// Maximum number of "type difference" structures for the requirement machine
592+
/// property map algorithm.
593+
unsigned RequirementMachineMaxTypeDifferences = 4000;
594+
587595
/// Maximum number of attempts to make when splitting concrete equivalence
588596
/// classes.
589597
unsigned RequirementMachineMaxSplitConcreteEquivClassAttempts = 2;

‎include/swift/Option/FrontendOptions.td

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -451,6 +451,14 @@ def requirement_machine_max_concrete_nesting : Joined<["-"], "requirement-machin
451451
Flags<[FrontendOption, HelpHidden, DoesNotAffectIncrementalBuild]>,
452452
HelpText<"Set the maximum concrete type nesting depth before giving up">;
453453

454+
def requirement_machine_max_concrete_size : Joined<["-"], "requirement-machine-max-concrete-size=">,
455+
Flags<[FrontendOption, HelpHidden, DoesNotAffectIncrementalBuild]>,
456+
HelpText<"Set the maximum concrete type total size before giving up">;
457+
458+
def requirement_machine_max_type_differences : Joined<["-"], "requirement-machine-max-type-differences=">,
459+
Flags<[FrontendOption, HelpHidden, DoesNotAffectIncrementalBuild]>,
460+
HelpText<"Set the maximum number of type difference structures allocated before giving up">;
461+
454462
def requirement_machine_max_split_concrete_equiv_class_attempts : Joined<["-"], "requirement-machine-max-split-concrete-equiv-class-attempts=">,
455463
Flags<[FrontendOption, HelpHidden, DoesNotAffectIncrementalBuild]>,
456464
HelpText<"Set the maximum concrete number of attempts at splitting "

‎lib/AST/RequirementMachine/HomotopyReduction.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -291,8 +291,8 @@ RewriteSystem::findRuleToDelete(EliminationPredicate isRedundantRuleFn) {
291291
{
292292
// If both are concrete type requirements, prefer to eliminate the
293293
// one with the more deeply nested type.
294-
unsigned ruleNesting = rule.getNesting();
295-
unsigned otherRuleNesting = otherRule.getNesting();
294+
unsigned ruleNesting = rule.getNestingAndSize().first;
295+
unsigned otherRuleNesting = otherRule.getNestingAndSize().first;
296296

297297
if (ruleNesting != otherRuleNesting) {
298298
if (ruleNesting > otherRuleNesting)

‎lib/AST/RequirementMachine/RequirementMachine.cpp

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,8 @@ RequirementMachine::RequirementMachine(RewriteContext &ctx)
212212
MaxRuleCount = langOpts.RequirementMachineMaxRuleCount;
213213
MaxRuleLength = langOpts.RequirementMachineMaxRuleLength;
214214
MaxConcreteNesting = langOpts.RequirementMachineMaxConcreteNesting;
215+
MaxConcreteSize = langOpts.RequirementMachineMaxConcreteSize;
216+
MaxTypeDifferences = langOpts.RequirementMachineMaxTypeDifferences;
215217
Stats = ctx.getASTContext().Stats;
216218

217219
if (Stats)
@@ -247,6 +249,18 @@ void RequirementMachine::checkCompletionResult(CompletionResult result) const {
247249
out << "Rewrite system exceeded concrete type nesting depth limit\n";
248250
dump(out);
249251
});
252+
253+
case CompletionResult::MaxConcreteSize:
254+
ABORT([&](auto &out) {
255+
out << "Rewrite system exceeded concrete type size limit\n";
256+
dump(out);
257+
});
258+
259+
case CompletionResult::MaxTypeDifferences:
260+
ABORT([&](auto &out) {
261+
out << "Rewrite system exceeded concrete type difference limit\n";
262+
dump(out);
263+
});
250264
}
251265
}
252266

@@ -496,16 +510,26 @@ RequirementMachine::computeCompletion(RewriteSystem::ValidityPolicy policy) {
496510
return std::make_pair(CompletionResult::MaxRuleLength,
497511
ruleCount + i);
498512
}
499-
if (newRule.getNesting() > MaxConcreteNesting + System.getDeepestInitialRule()) {
513+
auto nestingAndSize = newRule.getNestingAndSize();
514+
if (nestingAndSize.first > MaxConcreteNesting + System.getMaxNestingOfInitialRule()) {
500515
return std::make_pair(CompletionResult::MaxConcreteNesting,
501516
ruleCount + i);
502517
}
518+
if (nestingAndSize.second > MaxConcreteSize + System.getMaxSizeOfInitialRule()) {
519+
return std::make_pair(CompletionResult::MaxConcreteSize,
520+
ruleCount + i);
521+
}
503522
}
504523

505524
if (System.getLocalRules().size() > MaxRuleCount) {
506525
return std::make_pair(CompletionResult::MaxRuleCount,
507526
System.getRules().size() - 1);
508527
}
528+
529+
if (System.getTypeDifferenceCount() > MaxTypeDifferences) {
530+
return std::make_pair(CompletionResult::MaxTypeDifferences,
531+
System.getRules().size() - 1);
532+
}
509533
}
510534
}
511535

‎lib/AST/RequirementMachine/RequirementMachine.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@ class RequirementMachine final {
6868
unsigned MaxRuleCount;
6969
unsigned MaxRuleLength;
7070
unsigned MaxConcreteNesting;
71+
unsigned MaxConcreteSize;
72+
unsigned MaxTypeDifferences;
7173

7274
UnifiedStatsReporter *Stats;
7375

‎lib/AST/RequirementMachine/RewriteSystem.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ RewriteSystem::RewriteSystem(RewriteContext &ctx)
3434
Frozen = 0;
3535
RecordLoops = 0;
3636
LongestInitialRule = 0;
37-
DeepestInitialRule = 0;
37+
MaxNestingOfInitialRule = 0;
38+
MaxSizeOfInitialRule = 0;
3839
}
3940

4041
RewriteSystem::~RewriteSystem() {
@@ -90,7 +91,12 @@ void RewriteSystem::initialize(
9091

9192
for (const auto &rule : getLocalRules()) {
9293
LongestInitialRule = std::max(LongestInitialRule, rule.getDepth());
93-
DeepestInitialRule = std::max(DeepestInitialRule, rule.getNesting());
94+
95+
auto nestingAndSize = rule.getNestingAndSize();
96+
MaxNestingOfInitialRule = std::max(MaxNestingOfInitialRule,
97+
nestingAndSize.first);
98+
MaxSizeOfInitialRule = std::max(MaxSizeOfInitialRule,
99+
nestingAndSize.second);
94100
}
95101
}
96102

‎lib/AST/RequirementMachine/RewriteSystem.h

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,13 @@ enum class CompletionResult {
5050
MaxRuleLength,
5151

5252
/// Maximum concrete type nesting depth exceeded.
53-
MaxConcreteNesting
53+
MaxConcreteNesting,
54+
55+
/// Maximum concrete type size exceeded.
56+
MaxConcreteSize,
57+
58+
/// Maximum type difference count exceeded.
59+
MaxTypeDifferences,
5460
};
5561

5662
/// A term rewrite system for working with types in a generic signature.
@@ -107,13 +113,16 @@ class RewriteSystem final {
107113
/// identities among rewrite rules discovered while resolving critical pairs.
108114
unsigned RecordLoops : 1;
109115

110-
/// The length of the longest initial rule, used for the MaxRuleLength
111-
/// completion non-termination heuristic.
116+
/// The length of the longest initial rule, for the MaxRuleLength limit.
112117
unsigned LongestInitialRule : 16;
113118

114-
/// The most deeply nested concrete type appearing in an initial rule, used
115-
/// for the MaxConcreteNesting completion non-termination heuristic.
116-
unsigned DeepestInitialRule : 16;
119+
/// The most deeply nested concrete type appearing in an initial rule,
120+
/// for the MaxConcreteNesting limit.
121+
unsigned MaxNestingOfInitialRule : 16;
122+
123+
/// The largest concrete type by total tree node count that appears in an
124+
/// initial rule, for the MaxConcreteSize limit.
125+
unsigned MaxSizeOfInitialRule : 16;
117126

118127
public:
119128
explicit RewriteSystem(RewriteContext &ctx);
@@ -143,8 +152,12 @@ class RewriteSystem final {
143152
return LongestInitialRule;
144153
}
145154

146-
unsigned getDeepestInitialRule() const {
147-
return DeepestInitialRule;
155+
unsigned getMaxNestingOfInitialRule() const {
156+
return MaxNestingOfInitialRule;
157+
}
158+
159+
unsigned getMaxSizeOfInitialRule() const {
160+
return MaxSizeOfInitialRule;
148161
}
149162

150163
ArrayRef<const ProtocolDecl *> getProtocols() const {
@@ -311,6 +324,10 @@ class RewriteSystem final {
311324
std::optional<unsigned> &lhsDifferenceID,
312325
std::optional<unsigned> &rhsDifferenceID);
313326

327+
unsigned getTypeDifferenceCount() const {
328+
return Differences.size();
329+
}
330+
314331
const TypeDifference &getTypeDifference(unsigned index) const;
315332

316333
void processTypeDifference(const TypeDifference &difference,

‎lib/AST/RequirementMachine/Rule.cpp

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,12 @@ bool Rule::isDerivedFromConcreteProtocolTypeAliasRule() const {
221221
return true;
222222
}
223223

224-
/// Returns the length of the left hand side.
224+
/// Returns the maximum among the length of the left-hand side,
225+
/// and the length of any substitution terms that appear in a
226+
/// property symbol at the end of the left-hand side.
227+
///
228+
/// This is a measure of the complexity of the rule, which stops
229+
/// completion from running forever.
225230
unsigned Rule::getDepth() const {
226231
auto result = LHS.size();
227232

@@ -234,37 +239,41 @@ unsigned Rule::getDepth() const {
234239
return result;
235240
}
236241

237-
/// Returns the nesting depth of the concrete symbol at the end of the
238-
/// left hand side, or 0 if there isn't one.
239-
unsigned Rule::getNesting() const {
242+
/// Returns the complexity of the concrete type in the property symbol
243+
/// at the end of the left-hand side, if there is one.
244+
///
245+
/// This is a measure of the complexity of the rule, which stops
246+
/// completion from running forever.
247+
std::pair<unsigned, unsigned>
248+
Rule::getNestingAndSize() const {
240249
if (LHS.back().hasSubstitutions()) {
241250
auto type = LHS.back().getConcreteType();
242251

243252
struct Walker : TypeWalker {
244253
unsigned Nesting = 0;
245254
unsigned MaxNesting = 0;
255+
unsigned Size = 0;
246256

247257
Action walkToTypePre(Type ty) override {
258+
++Size;
248259
++Nesting;
249-
MaxNesting = std::max(Nesting, MaxNesting);
250-
260+
MaxNesting = std::max(MaxNesting, Nesting);
251261
return Action::Continue;
252262
}
253263

254264
Action walkToTypePost(Type ty) override {
255265
--Nesting;
256-
257266
return Action::Continue;
258267
}
259268
};
260269

261270
Walker walker;
262271
type.walk(walker);
263272

264-
return walker.MaxNesting;
273+
return std::make_pair(walker.MaxNesting, walker.Size);
265274
}
266275

267-
return 0;
276+
return std::make_pair(0, 0);
268277
}
269278

270279
/// Linear order on rules; compares LHS followed by RHS.

‎lib/AST/RequirementMachine/Rule.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ class Rule final {
224224

225225
unsigned getDepth() const;
226226

227-
unsigned getNesting() const;
227+
std::pair<unsigned, unsigned> getNestingAndSize() const;
228228

229229
std::optional<int> compare(const Rule &other, RewriteContext &ctx) const;
230230

‎lib/Frontend/CompilerInvocation.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1769,6 +1769,28 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
17691769
}
17701770
}
17711771

1772+
if (const Arg *A = Args.getLastArg(OPT_requirement_machine_max_concrete_size)) {
1773+
unsigned limit;
1774+
if (StringRef(A->getValue()).getAsInteger(10, limit)) {
1775+
Diags.diagnose(SourceLoc(), diag::error_invalid_arg_value,
1776+
A->getAsString(Args), A->getValue());
1777+
HadError = true;
1778+
} else {
1779+
Opts.RequirementMachineMaxConcreteSize = limit;
1780+
}
1781+
}
1782+
1783+
if (const Arg *A = Args.getLastArg(OPT_requirement_machine_max_type_differences)) {
1784+
unsigned limit;
1785+
if (StringRef(A->getValue()).getAsInteger(10, limit)) {
1786+
Diags.diagnose(SourceLoc(), diag::error_invalid_arg_value,
1787+
A->getAsString(Args), A->getValue());
1788+
HadError = true;
1789+
} else {
1790+
Opts.RequirementMachineMaxTypeDifferences = limit;
1791+
}
1792+
}
1793+
17721794
if (const Arg *A = Args.getLastArg(OPT_requirement_machine_max_split_concrete_equiv_class_attempts)) {
17731795
unsigned limit;
17741796
if (StringRef(A->getValue()).getAsInteger(10, limit)) {

‎test/Constraints/same_types.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ protocol P2 {
284284
// CHECK-LABEL: same_types.(file).structuralSameTypeRecursive1@
285285
// CHECK-NEXT: Generic signature: <T, U>
286286

287-
// expected-error@+2 {{cannot build rewrite system for generic signature; concrete nesting limit exceeded}}
287+
// expected-error@+2 {{cannot build rewrite system for generic signature; concrete type nesting limit exceeded}}
288288
// expected-note@+1 {{τ_0_0.[P2:Assoc1].[concrete: ((((((((((((((((((((((((((((((((τ_0_0.[P2:Assoc1], τ_0_1), τ_0_1), τ_0_1), τ_0_1), τ_0_1), τ_0_1), τ_0_1), τ_0_1), τ_0_1), τ_0_1), τ_0_1), τ_0_1), τ_0_1), τ_0_1), τ_0_1), τ_0_1), τ_0_1), τ_0_1), τ_0_1), τ_0_1), τ_0_1), τ_0_1), τ_0_1), τ_0_1), τ_0_1), τ_0_1), τ_0_1), τ_0_1), τ_0_1), τ_0_1), τ_0_1), τ_0_1)] => τ_0_0.[P2:Assoc1]}}
289289
func structuralSameTypeRecursive1<T: P2, U>(_: T, _: U)
290290
where T.Assoc1 == Tuple2<T.Assoc1, U>

‎test/Generics/infinite_concrete_type.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22

33
class G<T> {}
44

5-
protocol P1 { // expected-error {{cannot build rewrite system for protocol; concrete nesting limit exceeded}}
6-
// expected-note@-1 {{failed rewrite rule is [P1:A].[concrete: G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<[P1].A>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>] => [P1:A]}}
5+
protocol P1 { // expected-error {{cannot build rewrite system for protocol; concrete type difference limit exceeded}}
6+
// expected-note@-1 {{failed rewrite rule is [P1:B].[superclass: G<G<G<G<G<G<G<G<G<G<G<G<G<[P1].A>>>>>>>>>>>>>] => [P1:B]}}
77
associatedtype A where A == G<B>
88
associatedtype B where B == G<A>
99
}

‎test/Generics/non_confluent.swift

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,3 +52,64 @@ protocol P3 : P3Base where T == S<U> {
5252
// expected-error@-1 {{cannot build rewrite system for protocol; rule length limit exceeded}}
5353
// expected-note@-2 {{failed rewrite rule is [P3:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[concrete: S<S<S<S<S<S<S<S<S<S<S<S<S<S<[P3:U]>>>>>>>>>>>>>>] => [P3:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T]}}
5454
}
55+
56+
protocol Exponential {
57+
// expected-error@-1 {{cannot build rewrite system for protocol; concrete type size limit exceeded}}
58+
// expected-note@-2 {{failed rewrite rule is }}
59+
associatedtype A where A == (A, A)
60+
}
61+
62+
class Base<T> {}
63+
64+
class Derived<T, U> : Base<(T, U)> {}
65+
66+
protocol TooManyDifferences {
67+
// expected-error@-1 {{cannot build rewrite system for protocol; concrete type difference limit exceeded}}
68+
// expected-note@-2 {{failed rewrite rule is }}
69+
associatedtype A1 where A1: Derived<B, C>, A2: Base<B>, A1 == A2
70+
associatedtype A2
71+
associatedtype B
72+
associatedtype C
73+
}
74+
75+
protocol M0 {
76+
// expected-error@-1 {{cannot build rewrite system for protocol; rule length limit exceeded}}
77+
// expected-note@-2 {{failed rewrite rule is }}
78+
associatedtype A: M0
79+
associatedtype B: M0
80+
associatedtype C: M0
81+
where A.B == Self, C.A == B.C // expected-error *{{is not a member type}}
82+
}
83+
84+
protocol M1 {
85+
// expected-error@-1 {{cannot build rewrite system for protocol; rule length limit exceeded}}
86+
// expected-note@-2 {{failed rewrite rule is }}
87+
associatedtype A: M1
88+
associatedtype B: M1
89+
associatedtype C: M1
90+
where C.A.C == A, A.B.A == A // expected-error *{{is not a member type}}
91+
}
92+
93+
protocol M2 {
94+
// expected-error@-1 {{cannot build rewrite system for protocol; rule length limit exceeded}}
95+
// expected-note@-2 {{failed rewrite rule is }}
96+
associatedtype A: M2
97+
associatedtype B: M2
98+
associatedtype C: M2
99+
where B.A == A.B, C.A == A, A.C == A // expected-error *{{is not a member type}}
100+
}
101+
102+
// FIXME: This should be rejected
103+
protocol M3 {
104+
associatedtype A: M3
105+
associatedtype B: M3
106+
where A.A.A == A, A.B.B.A == B.B
107+
}
108+
109+
protocol M4 {
110+
// expected-error@-1 {{cannot build rewrite system for protocol; rule length limit exceeded}}
111+
// expected-note@-2 {{failed rewrite rule is }}
112+
associatedtype A: M4
113+
associatedtype B: M4
114+
where B.A.A == A.A.B, A.B.A == A.A // expected-error *{{is not a member type}}
115+
}

0 commit comments

Comments
 (0)