From 7f8175b3daa3746236c09eb4dafe48d7fe6fc11d Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Tue, 17 Jun 2025 15:51:45 -0400 Subject: [PATCH 1/3] RequirementMachine: Add two more completion termination checks for concrete type requirements The concrete nesting limit, which defaults to 30, catches things like A == G. 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. --- include/swift/AST/DiagnosticsSema.def | 2 +- include/swift/Basic/LangOptions.h | 12 +++- include/swift/Option/FrontendOptions.td | 8 +++ .../RequirementMachine/HomotopyReduction.cpp | 4 +- .../RequirementMachine/RequirementMachine.cpp | 26 +++++++- .../RequirementMachine/RequirementMachine.h | 2 + lib/AST/RequirementMachine/RewriteSystem.cpp | 10 ++- lib/AST/RequirementMachine/RewriteSystem.h | 33 +++++++--- lib/AST/RequirementMachine/Rule.cpp | 27 +++++--- lib/AST/RequirementMachine/Rule.h | 2 +- lib/Frontend/CompilerInvocation.cpp | 22 +++++++ test/Constraints/same_types.swift | 2 +- test/Generics/infinite_concrete_type.swift | 4 +- test/Generics/non_confluent.swift | 61 +++++++++++++++++++ test/Generics/rdar90402519.swift | 4 +- test/attr/attr_specialize.swift | 2 +- test/decl/protocol/req/recursion.swift | 4 +- 17 files changed, 191 insertions(+), 34 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index f12bd7a6c03cf..49978b7d57dc6 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -3441,7 +3441,7 @@ WARNING(associated_type_override_typealias,none, ERROR(requirement_machine_completion_failed,none, "cannot build rewrite system for %select{generic signature|protocol}0; " - "%select{%error|rule count|rule length|concrete nesting}1 limit exceeded", + "%select{%error|rule count|rule length|concrete type nesting|concrete type size|concrete type difference}1 limit exceeded", (unsigned, unsigned)) NOTE(requirement_machine_completion_rule,none, "failed rewrite rule is %0", (StringRef)) diff --git a/include/swift/Basic/LangOptions.h b/include/swift/Basic/LangOptions.h index cb898f5b4fb37..e829c02d0045e 100644 --- a/include/swift/Basic/LangOptions.h +++ b/include/swift/Basic/LangOptions.h @@ -580,10 +580,18 @@ namespace swift { /// algorithm. unsigned RequirementMachineMaxRuleLength = 12; - /// Maximum concrete type nesting depth for requirement machine property map - /// algorithm. + /// Maximum concrete type nesting depth (when type is viewed as a tree) for + /// requirement machine property map algorithm. unsigned RequirementMachineMaxConcreteNesting = 30; + /// Maximum concrete type size (total number of nodes in the type tree) for + /// requirement machine property map algorithm. + unsigned RequirementMachineMaxConcreteSize = 4000; + + /// Maximum number of "type difference" structures for the requirement machine + /// property map algorithm. + unsigned RequirementMachineMaxTypeDifferences = 4000; + /// Maximum number of attempts to make when splitting concrete equivalence /// classes. unsigned RequirementMachineMaxSplitConcreteEquivClassAttempts = 2; diff --git a/include/swift/Option/FrontendOptions.td b/include/swift/Option/FrontendOptions.td index b072bd9345197..7fd8bfbff7642 100644 --- a/include/swift/Option/FrontendOptions.td +++ b/include/swift/Option/FrontendOptions.td @@ -451,6 +451,14 @@ def requirement_machine_max_concrete_nesting : Joined<["-"], "requirement-machin Flags<[FrontendOption, HelpHidden, DoesNotAffectIncrementalBuild]>, HelpText<"Set the maximum concrete type nesting depth before giving up">; +def requirement_machine_max_concrete_size : Joined<["-"], "requirement-machine-max-concrete-size=">, + Flags<[FrontendOption, HelpHidden, DoesNotAffectIncrementalBuild]>, + HelpText<"Set the maximum concrete type total size before giving up">; + +def requirement_machine_max_type_differences : Joined<["-"], "requirement-machine-max-type-differences=">, + Flags<[FrontendOption, HelpHidden, DoesNotAffectIncrementalBuild]>, + HelpText<"Set the maximum number of type difference structures allocated before giving up">; + def requirement_machine_max_split_concrete_equiv_class_attempts : Joined<["-"], "requirement-machine-max-split-concrete-equiv-class-attempts=">, Flags<[FrontendOption, HelpHidden, DoesNotAffectIncrementalBuild]>, HelpText<"Set the maximum concrete number of attempts at splitting " diff --git a/lib/AST/RequirementMachine/HomotopyReduction.cpp b/lib/AST/RequirementMachine/HomotopyReduction.cpp index 30bd13d795efd..c6ef27428e59f 100644 --- a/lib/AST/RequirementMachine/HomotopyReduction.cpp +++ b/lib/AST/RequirementMachine/HomotopyReduction.cpp @@ -291,8 +291,8 @@ RewriteSystem::findRuleToDelete(EliminationPredicate isRedundantRuleFn) { { // If both are concrete type requirements, prefer to eliminate the // one with the more deeply nested type. - unsigned ruleNesting = rule.getNesting(); - unsigned otherRuleNesting = otherRule.getNesting(); + unsigned ruleNesting = rule.getNestingAndSize().first; + unsigned otherRuleNesting = otherRule.getNestingAndSize().first; if (ruleNesting != otherRuleNesting) { if (ruleNesting > otherRuleNesting) diff --git a/lib/AST/RequirementMachine/RequirementMachine.cpp b/lib/AST/RequirementMachine/RequirementMachine.cpp index b1432766ff43a..c4dd539b3bf6a 100644 --- a/lib/AST/RequirementMachine/RequirementMachine.cpp +++ b/lib/AST/RequirementMachine/RequirementMachine.cpp @@ -212,6 +212,8 @@ RequirementMachine::RequirementMachine(RewriteContext &ctx) MaxRuleCount = langOpts.RequirementMachineMaxRuleCount; MaxRuleLength = langOpts.RequirementMachineMaxRuleLength; MaxConcreteNesting = langOpts.RequirementMachineMaxConcreteNesting; + MaxConcreteSize = langOpts.RequirementMachineMaxConcreteSize; + MaxTypeDifferences = langOpts.RequirementMachineMaxTypeDifferences; Stats = ctx.getASTContext().Stats; if (Stats) @@ -247,6 +249,18 @@ void RequirementMachine::checkCompletionResult(CompletionResult result) const { out << "Rewrite system exceeded concrete type nesting depth limit\n"; dump(out); }); + + case CompletionResult::MaxConcreteSize: + ABORT([&](auto &out) { + out << "Rewrite system exceeded concrete type size limit\n"; + dump(out); + }); + + case CompletionResult::MaxTypeDifferences: + ABORT([&](auto &out) { + out << "Rewrite system exceeded concrete type difference limit\n"; + dump(out); + }); } } @@ -496,16 +510,26 @@ RequirementMachine::computeCompletion(RewriteSystem::ValidityPolicy policy) { return std::make_pair(CompletionResult::MaxRuleLength, ruleCount + i); } - if (newRule.getNesting() > MaxConcreteNesting + System.getDeepestInitialRule()) { + auto nestingAndSize = newRule.getNestingAndSize(); + if (nestingAndSize.first > MaxConcreteNesting + System.getMaxNestingOfInitialRule()) { return std::make_pair(CompletionResult::MaxConcreteNesting, ruleCount + i); } + if (nestingAndSize.second > MaxConcreteSize + System.getMaxSizeOfInitialRule()) { + return std::make_pair(CompletionResult::MaxConcreteSize, + ruleCount + i); + } } if (System.getLocalRules().size() > MaxRuleCount) { return std::make_pair(CompletionResult::MaxRuleCount, System.getRules().size() - 1); } + + if (System.getTypeDifferenceCount() > MaxTypeDifferences) { + return std::make_pair(CompletionResult::MaxTypeDifferences, + System.getRules().size() - 1); + } } } diff --git a/lib/AST/RequirementMachine/RequirementMachine.h b/lib/AST/RequirementMachine/RequirementMachine.h index c64248e978f6e..91f3b862ce23d 100644 --- a/lib/AST/RequirementMachine/RequirementMachine.h +++ b/lib/AST/RequirementMachine/RequirementMachine.h @@ -68,6 +68,8 @@ class RequirementMachine final { unsigned MaxRuleCount; unsigned MaxRuleLength; unsigned MaxConcreteNesting; + unsigned MaxConcreteSize; + unsigned MaxTypeDifferences; UnifiedStatsReporter *Stats; diff --git a/lib/AST/RequirementMachine/RewriteSystem.cpp b/lib/AST/RequirementMachine/RewriteSystem.cpp index 2cf620b660e82..ac4dda392d95a 100644 --- a/lib/AST/RequirementMachine/RewriteSystem.cpp +++ b/lib/AST/RequirementMachine/RewriteSystem.cpp @@ -34,7 +34,8 @@ RewriteSystem::RewriteSystem(RewriteContext &ctx) Frozen = 0; RecordLoops = 0; LongestInitialRule = 0; - DeepestInitialRule = 0; + MaxNestingOfInitialRule = 0; + MaxSizeOfInitialRule = 0; } RewriteSystem::~RewriteSystem() { @@ -90,7 +91,12 @@ void RewriteSystem::initialize( for (const auto &rule : getLocalRules()) { LongestInitialRule = std::max(LongestInitialRule, rule.getDepth()); - DeepestInitialRule = std::max(DeepestInitialRule, rule.getNesting()); + + auto nestingAndSize = rule.getNestingAndSize(); + MaxNestingOfInitialRule = std::max(MaxNestingOfInitialRule, + nestingAndSize.first); + MaxSizeOfInitialRule = std::max(MaxSizeOfInitialRule, + nestingAndSize.second); } } diff --git a/lib/AST/RequirementMachine/RewriteSystem.h b/lib/AST/RequirementMachine/RewriteSystem.h index f1d939ee370e0..694f90cb08ab0 100644 --- a/lib/AST/RequirementMachine/RewriteSystem.h +++ b/lib/AST/RequirementMachine/RewriteSystem.h @@ -50,7 +50,13 @@ enum class CompletionResult { MaxRuleLength, /// Maximum concrete type nesting depth exceeded. - MaxConcreteNesting + MaxConcreteNesting, + + /// Maximum concrete type size exceeded. + MaxConcreteSize, + + /// Maximum type difference count exceeded. + MaxTypeDifferences, }; /// A term rewrite system for working with types in a generic signature. @@ -107,13 +113,16 @@ class RewriteSystem final { /// identities among rewrite rules discovered while resolving critical pairs. unsigned RecordLoops : 1; - /// The length of the longest initial rule, used for the MaxRuleLength - /// completion non-termination heuristic. + /// The length of the longest initial rule, for the MaxRuleLength limit. unsigned LongestInitialRule : 16; - /// The most deeply nested concrete type appearing in an initial rule, used - /// for the MaxConcreteNesting completion non-termination heuristic. - unsigned DeepestInitialRule : 16; + /// The most deeply nested concrete type appearing in an initial rule, + /// for the MaxConcreteNesting limit. + unsigned MaxNestingOfInitialRule : 16; + + /// The largest concrete type by total tree node count that appears in an + /// initial rule, for the MaxConcreteSize limit. + unsigned MaxSizeOfInitialRule : 16; public: explicit RewriteSystem(RewriteContext &ctx); @@ -143,8 +152,12 @@ class RewriteSystem final { return LongestInitialRule; } - unsigned getDeepestInitialRule() const { - return DeepestInitialRule; + unsigned getMaxNestingOfInitialRule() const { + return MaxNestingOfInitialRule; + } + + unsigned getMaxSizeOfInitialRule() const { + return MaxSizeOfInitialRule; } ArrayRef getProtocols() const { @@ -311,6 +324,10 @@ class RewriteSystem final { std::optional &lhsDifferenceID, std::optional &rhsDifferenceID); + unsigned getTypeDifferenceCount() const { + return Differences.size(); + } + const TypeDifference &getTypeDifference(unsigned index) const; void processTypeDifference(const TypeDifference &difference, diff --git a/lib/AST/RequirementMachine/Rule.cpp b/lib/AST/RequirementMachine/Rule.cpp index 4c79b60040d34..b4552ca45c0cf 100644 --- a/lib/AST/RequirementMachine/Rule.cpp +++ b/lib/AST/RequirementMachine/Rule.cpp @@ -221,7 +221,12 @@ bool Rule::isDerivedFromConcreteProtocolTypeAliasRule() const { return true; } -/// Returns the length of the left hand side. +/// Returns the maximum among the length of the left-hand side, +/// and the length of any substitution terms that appear in a +/// property symbol at the end of the left-hand side. +/// +/// This is a measure of the complexity of the rule, which stops +/// completion from running forever. unsigned Rule::getDepth() const { auto result = LHS.size(); @@ -234,26 +239,30 @@ unsigned Rule::getDepth() const { return result; } -/// Returns the nesting depth of the concrete symbol at the end of the -/// left hand side, or 0 if there isn't one. -unsigned Rule::getNesting() const { +/// Returns the complexity of the concrete type in the property symbol +/// at the end of the left-hand side, if there is one. +/// +/// This is a measure of the complexity of the rule, which stops +/// completion from running forever. +std::pair +Rule::getNestingAndSize() const { if (LHS.back().hasSubstitutions()) { auto type = LHS.back().getConcreteType(); struct Walker : TypeWalker { unsigned Nesting = 0; unsigned MaxNesting = 0; + unsigned Size = 0; Action walkToTypePre(Type ty) override { + ++Size; ++Nesting; - MaxNesting = std::max(Nesting, MaxNesting); - + MaxNesting = std::max(MaxNesting, Nesting); return Action::Continue; } Action walkToTypePost(Type ty) override { --Nesting; - return Action::Continue; } }; @@ -261,10 +270,10 @@ unsigned Rule::getNesting() const { Walker walker; type.walk(walker); - return walker.MaxNesting; + return std::make_pair(walker.MaxNesting, walker.Size); } - return 0; + return std::make_pair(0, 0); } /// Linear order on rules; compares LHS followed by RHS. diff --git a/lib/AST/RequirementMachine/Rule.h b/lib/AST/RequirementMachine/Rule.h index 8519bf1a13ea6..9077741e68fa0 100644 --- a/lib/AST/RequirementMachine/Rule.h +++ b/lib/AST/RequirementMachine/Rule.h @@ -224,7 +224,7 @@ class Rule final { unsigned getDepth() const; - unsigned getNesting() const; + std::pair getNestingAndSize() const; std::optional compare(const Rule &other, RewriteContext &ctx) const; diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp index ad9e5587e2584..85e5f392b35ed 100644 --- a/lib/Frontend/CompilerInvocation.cpp +++ b/lib/Frontend/CompilerInvocation.cpp @@ -1769,6 +1769,28 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args, } } + if (const Arg *A = Args.getLastArg(OPT_requirement_machine_max_concrete_size)) { + unsigned limit; + if (StringRef(A->getValue()).getAsInteger(10, limit)) { + Diags.diagnose(SourceLoc(), diag::error_invalid_arg_value, + A->getAsString(Args), A->getValue()); + HadError = true; + } else { + Opts.RequirementMachineMaxConcreteSize = limit; + } + } + + if (const Arg *A = Args.getLastArg(OPT_requirement_machine_max_type_differences)) { + unsigned limit; + if (StringRef(A->getValue()).getAsInteger(10, limit)) { + Diags.diagnose(SourceLoc(), diag::error_invalid_arg_value, + A->getAsString(Args), A->getValue()); + HadError = true; + } else { + Opts.RequirementMachineMaxTypeDifferences = limit; + } + } + if (const Arg *A = Args.getLastArg(OPT_requirement_machine_max_split_concrete_equiv_class_attempts)) { unsigned limit; if (StringRef(A->getValue()).getAsInteger(10, limit)) { diff --git a/test/Constraints/same_types.swift b/test/Constraints/same_types.swift index c97d1585660dd..44e52f6e9b647 100644 --- a/test/Constraints/same_types.swift +++ b/test/Constraints/same_types.swift @@ -284,7 +284,7 @@ protocol P2 { // CHECK-LABEL: same_types.(file).structuralSameTypeRecursive1@ // CHECK-NEXT: Generic signature: -// expected-error@+2 {{cannot build rewrite system for generic signature; concrete nesting limit exceeded}} +// expected-error@+2 {{cannot build rewrite system for generic signature; concrete type nesting limit exceeded}} // 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]}} func structuralSameTypeRecursive1(_: T, _: U) where T.Assoc1 == Tuple2 diff --git a/test/Generics/infinite_concrete_type.swift b/test/Generics/infinite_concrete_type.swift index 399efefc91d42..02c22b2cf20d6 100644 --- a/test/Generics/infinite_concrete_type.swift +++ b/test/Generics/infinite_concrete_type.swift @@ -2,8 +2,8 @@ class G {} -protocol P1 { // expected-error {{cannot build rewrite system for protocol; concrete nesting limit exceeded}} -// expected-note@-1 {{failed rewrite rule is [P1:A].[concrete: G>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>] => [P1:A]}} +protocol P1 { // expected-error {{cannot build rewrite system for protocol; concrete type difference limit exceeded}} +// expected-note@-1 {{failed rewrite rule is [P1:B].[superclass: G>>>>>>>>>>>>] => [P1:B]}} associatedtype A where A == G associatedtype B where B == G } diff --git a/test/Generics/non_confluent.swift b/test/Generics/non_confluent.swift index d0f9fbcc664f9..2d27003159f76 100644 --- a/test/Generics/non_confluent.swift +++ b/test/Generics/non_confluent.swift @@ -52,3 +52,64 @@ protocol P3 : P3Base where T == S { // expected-error@-1 {{cannot build rewrite system for protocol; rule length limit exceeded}} // 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>>>>>>>>>>>>>] => [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]}} } + +protocol Exponential { +// expected-error@-1 {{cannot build rewrite system for protocol; concrete type size limit exceeded}} +// expected-note@-2 {{failed rewrite rule is }} + associatedtype A where A == (A, A) +} + +class Base {} + +class Derived : Base<(T, U)> {} + +protocol TooManyDifferences { +// expected-error@-1 {{cannot build rewrite system for protocol; concrete type difference limit exceeded}} +// expected-note@-2 {{failed rewrite rule is }} + associatedtype A1 where A1: Derived, A2: Base, A1 == A2 + associatedtype A2 + associatedtype B + associatedtype C +} + +protocol M0 { +// expected-error@-1 {{cannot build rewrite system for protocol; rule length limit exceeded}} +// expected-note@-2 {{failed rewrite rule is }} + associatedtype A: M0 + associatedtype B: M0 + associatedtype C: M0 + where A.B == Self, C.A == B.C // expected-error *{{is not a member type}} +} + +protocol M1 { +// expected-error@-1 {{cannot build rewrite system for protocol; rule length limit exceeded}} +// expected-note@-2 {{failed rewrite rule is }} + associatedtype A: M1 + associatedtype B: M1 + associatedtype C: M1 + where C.A.C == A, A.B.A == A // expected-error *{{is not a member type}} +} + +protocol M2 { +// expected-error@-1 {{cannot build rewrite system for protocol; rule length limit exceeded}} +// expected-note@-2 {{failed rewrite rule is }} + associatedtype A: M2 + associatedtype B: M2 + associatedtype C: M2 + where B.A == A.B, C.A == A, A.C == A // expected-error *{{is not a member type}} +} + +// FIXME: This should be rejected +protocol M3 { + associatedtype A: M3 + associatedtype B: M3 + where A.A.A == A, A.B.B.A == B.B +} + +protocol M4 { +// expected-error@-1 {{cannot build rewrite system for protocol; rule length limit exceeded}} +// expected-note@-2 {{failed rewrite rule is }} + associatedtype A: M4 + associatedtype B: M4 + where B.A.A == A.A.B, A.B.A == A.A // expected-error *{{is not a member type}} +} diff --git a/test/Generics/rdar90402519.swift b/test/Generics/rdar90402519.swift index 7e6c848502bc5..7be346ec4d044 100644 --- a/test/Generics/rdar90402519.swift +++ b/test/Generics/rdar90402519.swift @@ -23,5 +23,5 @@ protocol P1 : P0 where C == G1 { } protocol P2 : P1 where C == G2 {} -// expected-error@-1 {{cannot build rewrite system for protocol; concrete nesting limit exceeded}} -// expected-note@-2 {{failed rewrite rule is [P2:I].[concrete: G>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>] => [P2:I]}} +// expected-error@-1 {{cannot build rewrite system for protocol; concrete type difference limit exceeded}} +// expected-note@-2 {{failed rewrite rule is [P2:I].[concrete: G>>>>>>>>>>>>>>>>>>>>>>>>> : Escapable] => [P2:I]}} diff --git a/test/attr/attr_specialize.swift b/test/attr/attr_specialize.swift index 679b8d6f275db..745cfd5ed19a3 100644 --- a/test/attr/attr_specialize.swift +++ b/test/attr/attr_specialize.swift @@ -53,7 +53,7 @@ class G { @_specialize(where T == T) // expected-error{{too few generic parameters are specified in '_specialize' attribute (got 0, but expected 1)}} // expected-note@-1 {{missing constraint for 'T' in '_specialize' attribute}} @_specialize(where T == S) - // expected-error@-1 {{cannot build rewrite system for generic signature; concrete nesting limit exceeded}} + // expected-error@-1 {{cannot build rewrite system for generic signature; concrete type nesting limit exceeded}} // expected-note@-2 {{failed rewrite rule is τ_0_0.[concrete: S>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>] => τ_0_0}} // expected-error@-3 {{too few generic parameters are specified in '_specialize' attribute (got 0, but expected 1)}} // expected-note@-4 {{missing constraint for 'T' in '_specialize' attribute}} diff --git a/test/decl/protocol/req/recursion.swift b/test/decl/protocol/req/recursion.swift index 013f6f133add3..8a42e5de71128 100644 --- a/test/decl/protocol/req/recursion.swift +++ b/test/decl/protocol/req/recursion.swift @@ -5,13 +5,13 @@ protocol SomeProtocol { } extension SomeProtocol where T == Optional { } -// expected-error@-1 {{cannot build rewrite system for generic signature; concrete nesting limit exceeded}} +// expected-error@-1 {{cannot build rewrite system for generic signature; concrete type nesting limit exceeded}} // expected-note@-2 {{failed rewrite rule is τ_0_0.[SomeProtocol:T].[concrete: Optional>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>] => τ_0_0.[SomeProtocol:T]}} // rdar://problem/19840527 class X where T == X { -// expected-error@-1 {{cannot build rewrite system for generic signature; concrete nesting limit exceeded}} +// expected-error@-1 {{cannot build rewrite system for generic signature; concrete type nesting limit exceeded}} // expected-note@-2 {{failed rewrite rule is τ_0_0.[concrete: X>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>] => τ_0_0}} // expected-error@-3 {{generic class 'X' has self-referential generic requirements}} var type: T { return Swift.type(of: self) } // expected-error{{cannot convert return expression of type 'X.Type' to return type 'T'}} From bf3f4a6d79ea815ddbb63d1ed1dd1add9bb781fd Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Tue, 17 Jun 2025 16:50:21 -0400 Subject: [PATCH 2/3] AST: Fix lost GenericSignatureErrors in getPlaceholderRequirementSignature() --- lib/AST/GenericSignature.cpp | 3 ++- .../b93e9e54c72f13c9.swift | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) rename validation-test/{compiler_crashers_2 => compiler_crashers_2_fixed}/b93e9e54c72f13c9.swift (86%) diff --git a/lib/AST/GenericSignature.cpp b/lib/AST/GenericSignature.cpp index f3d22a9b66f6a..afcef5ebb94da 100644 --- a/lib/AST/GenericSignature.cpp +++ b/lib/AST/GenericSignature.cpp @@ -1436,7 +1436,8 @@ RequirementSignature RequirementSignature::getPlaceholderRequirementSignature( }); return RequirementSignature(ctx.AllocateCopy(requirements), - ArrayRef()); + ArrayRef(), + errors); } void RequirementSignature::getRequirementsWithInverses( diff --git a/validation-test/compiler_crashers_2/b93e9e54c72f13c9.swift b/validation-test/compiler_crashers_2_fixed/b93e9e54c72f13c9.swift similarity index 86% rename from validation-test/compiler_crashers_2/b93e9e54c72f13c9.swift rename to validation-test/compiler_crashers_2_fixed/b93e9e54c72f13c9.swift index 7b8414457b88e..79a791d6d04f8 100644 --- a/validation-test/compiler_crashers_2/b93e9e54c72f13c9.swift +++ b/validation-test/compiler_crashers_2_fixed/b93e9e54c72f13c9.swift @@ -1,5 +1,5 @@ // {"signature":"swift::rewriting::RewriteSystem::processTypeDifference(swift::rewriting::TypeDifference const&, unsigned int, unsigned int, swift::rewriting::RewritePath const&)"} -// RUN: not --crash %target-swift-frontend -typecheck %s +// RUN: not %target-swift-frontend -typecheck %s class a < b class c : a<(b, h)> protocol d { associatedtype b : c associatedtype e associatedtype f associatedtype b : a From fee97ea96adf48c3c3eebd0084392a80bbc8b31e Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Tue, 17 Jun 2025 16:58:14 -0400 Subject: [PATCH 3/3] RequirementMachine: Fix the most embarassing bug of all time The implementation of Knuth-Bendix completion has had a subtle bookkeeping bug since I first wrote the code in 2021. It is possible for two rules to overlap in more than one position, but the ResolvedOverlaps set was a set of pairs (i, j), where i and j are the index of the two rules. So overlaps other than the first were not considered. Fix this by changing ResolvedOverlaps to a set of triples (i, j, k), where k is the position in the left-hand side of the first rule. The end result is that we would incorrectly accept the protocol M3 shown in the test case. I'm pretty sure the monoid that M3 encodes does not have a complete presentation over any alphabet, so of course it should not be accepted here. --- lib/AST/RequirementMachine/KnuthBendix.cpp | 3 ++- lib/AST/RequirementMachine/RewriteSystem.h | 2 +- test/Generics/non_confluent.swift | 5 +++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/AST/RequirementMachine/KnuthBendix.cpp b/lib/AST/RequirementMachine/KnuthBendix.cpp index a0fc682574b63..3b65af8e70847 100644 --- a/lib/AST/RequirementMachine/KnuthBendix.cpp +++ b/lib/AST/RequirementMachine/KnuthBendix.cpp @@ -399,7 +399,8 @@ RewriteSystem::performKnuthBendix(unsigned maxRuleCount, // We don't have to consider the same pair of rules more than once, // since those critical pairs were already resolved. - if (!CheckedOverlaps.insert(std::make_pair(i, j)).second) + unsigned k = from - lhs.getLHS().begin(); + if (!CheckedOverlaps.insert(std::make_tuple(i, j, k)).second) return; // Try to repair the confluence violation by adding a new rule. diff --git a/lib/AST/RequirementMachine/RewriteSystem.h b/lib/AST/RequirementMachine/RewriteSystem.h index 694f90cb08ab0..7f768270bc2e9 100644 --- a/lib/AST/RequirementMachine/RewriteSystem.h +++ b/lib/AST/RequirementMachine/RewriteSystem.h @@ -219,7 +219,7 @@ class RewriteSystem final { ////////////////////////////////////////////////////////////////////////////// /// Pairs of rules which have already been checked for overlap. - llvm::DenseSet> CheckedOverlaps; + llvm::DenseSet> CheckedOverlaps; std::pair performKnuthBendix(unsigned maxRuleCount, unsigned maxRuleLength); diff --git a/test/Generics/non_confluent.swift b/test/Generics/non_confluent.swift index 2d27003159f76..547c40fccca10 100644 --- a/test/Generics/non_confluent.swift +++ b/test/Generics/non_confluent.swift @@ -99,11 +99,12 @@ protocol M2 { where B.A == A.B, C.A == A, A.C == A // expected-error *{{is not a member type}} } -// FIXME: This should be rejected protocol M3 { +// expected-error@-1 {{cannot build rewrite system for protocol; rule length limit exceeded}} +// expected-note@-2 {{failed rewrite rule is }} associatedtype A: M3 associatedtype B: M3 - where A.A.A == A, A.B.B.A == B.B + where A.A.A == A, A.B.B.A == B.B // expected-error *{{is not a member type}} } protocol M4 {