Skip to content

Commit 7f8175b

Browse files
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

0 commit comments

Comments
 (0)