From 1d5fdc3e624189d9c852409bda0a5279bb2a21a6 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Mon, 13 Dec 2021 16:49:34 -0500 Subject: [PATCH 1/7] RequirementMachine: Implement linear order on concrete type symbols with the same concrete type --- lib/AST/RequirementMachine/Symbol.cpp | 31 ++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/lib/AST/RequirementMachine/Symbol.cpp b/lib/AST/RequirementMachine/Symbol.cpp index d57a0b215472b..665c11290339c 100644 --- a/lib/AST/RequirementMachine/Symbol.cpp +++ b/lib/AST/RequirementMachine/Symbol.cpp @@ -623,17 +623,42 @@ int Symbol::compare(Symbol other, RewriteContext &ctx) const { auto *proto = getProtocol(); auto *otherProto = other.getProtocol(); + // For concrete conformance symbols, order by protocol first. result = ctx.compareProtocols(proto, otherProto); - break; + if (result != 0) + return result; + + // Then, check if they have the same concrete type and order + // substitutions. + LLVM_FALLTHROUGH; } case Kind::Superclass: - case Kind::ConcreteType: + case Kind::ConcreteType: { + if (kind == Kind::Superclass + ? (getSuperclass() == other.getSuperclass()) + : (getConcreteType() == other.getConcreteType())) { + + // If the concrete types are identical, compare substitution terms. + assert(getSubstitutions().size() == other.getSubstitutions().size()); + for (unsigned i : indices(getSubstitutions())) { + auto term = getSubstitutions()[i]; + auto otherTerm = other.getSubstitutions()[i]; + + result = term.compare(otherTerm, ctx); + if (result != 0) + return result; + } + + break; + } + + // We don't support comparing arbitrary concrete types. llvm::errs() << "Cannot compare concrete types yet\n"; llvm::errs() << "LHS: " << *this << "\n"; llvm::errs() << "RHS: " << other << "\n"; abort(); - + } } if (result == 0) { From e86fe2706ae13de4e15d4704a23a16f56b801e84 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Mon, 13 Dec 2021 17:59:44 -0500 Subject: [PATCH 2/7] RequirementMachine: Allow simplified rules to appear in rewrite paths --- lib/AST/RequirementMachine/RewriteSystem.cpp | 26 ++++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/lib/AST/RequirementMachine/RewriteSystem.cpp b/lib/AST/RequirementMachine/RewriteSystem.cpp index 514aef9ec3fe7..1e896ced2e599 100644 --- a/lib/AST/RequirementMachine/RewriteSystem.cpp +++ b/lib/AST/RequirementMachine/RewriteSystem.cpp @@ -205,24 +205,23 @@ bool RewriteSystem::simplify(MutableTerm &term, RewritePath *path) const { auto ruleID = Trie.find(from, end); if (ruleID) { const auto &rule = getRule(*ruleID); - if (!rule.isSimplified()) { - auto to = from + rule.getLHS().size(); - assert(std::equal(from, to, rule.getLHS().begin())); - unsigned startOffset = (unsigned)(from - term.begin()); - unsigned endOffset = term.size() - rule.getLHS().size() - startOffset; + auto to = from + rule.getLHS().size(); + assert(std::equal(from, to, rule.getLHS().begin())); - term.rewriteSubTerm(from, to, rule.getRHS()); + unsigned startOffset = (unsigned)(from - term.begin()); + unsigned endOffset = term.size() - rule.getLHS().size() - startOffset; - if (path || debug) { - subpath.add(RewriteStep::forRewriteRule(startOffset, endOffset, *ruleID, - /*inverse=*/false)); - } + term.rewriteSubTerm(from, to, rule.getRHS()); - changed = true; - tryAgain = true; - break; + if (path || debug) { + subpath.add(RewriteStep::forRewriteRule(startOffset, endOffset, *ruleID, + /*inverse=*/false)); } + + changed = true; + tryAgain = true; + break; } ++from; @@ -433,6 +432,7 @@ bool RewriteSystem::addRule(MutableTerm lhs, MutableTerm rhs, MutableTerm term = lhs; simplify(lhs); + dump(llvm::errs()); abort(); } From 5ac43b14d2b7c358b25683ee096e4fb287df8f12 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Mon, 13 Dec 2021 18:39:19 -0500 Subject: [PATCH 3/7] RequirementMachine: Fully canonicalize substitutions in concrete type rules Previously we did this when adding new concrete type rules, but we don't have a complete rewrite system at that point yet, so there was no guarantee concrete substitution terms would be canonical. Now, perform simplification in a post-pass after completion, at the same time as simplifying rule right hand sides. Rewrite loops are recorded relating the original rule with the simplified substitutions. --- lib/AST/RequirementMachine/KnuthBendix.cpp | 4 +- lib/AST/RequirementMachine/RewriteSystem.cpp | 72 ++++++++++++++----- lib/AST/RequirementMachine/RewriteSystem.h | 6 +- ...l_concrete_substitutions_in_protocol.swift | 23 ++++++ test/Generics/unify_concrete_types_1.swift | 2 +- test/Generics/unify_superclass_types_2.swift | 4 +- test/Generics/unify_superclass_types_3.swift | 3 + test/Generics/unify_superclass_types_4.swift | 2 + 8 files changed, 95 insertions(+), 21 deletions(-) create mode 100644 test/Generics/canonical_concrete_substitutions_in_protocol.swift diff --git a/lib/AST/RequirementMachine/KnuthBendix.cpp b/lib/AST/RequirementMachine/KnuthBendix.cpp index 1438a78ff4716..7fe788c599c7a 100644 --- a/lib/AST/RequirementMachine/KnuthBendix.cpp +++ b/lib/AST/RequirementMachine/KnuthBendix.cpp @@ -596,7 +596,7 @@ RewriteSystem::computeConfluentCompletion(unsigned maxIterations, } } - simplifyRewriteSystem(); + simplifyLeftHandSides(); assert(resolvedCriticalPairs.size() == resolvedPaths.size()); @@ -629,6 +629,8 @@ RewriteSystem::computeConfluentCompletion(unsigned maxIterations, resolvedPaths.clear(); resolvedLoops.clear(); + simplifyRightHandSidesAndSubstitutions(); + // If the added rules merged any associated types, process the merges now // before we continue with the completion procedure. This is important // to perform incrementally since merging is required to repair confluence diff --git a/lib/AST/RequirementMachine/RewriteSystem.cpp b/lib/AST/RequirementMachine/RewriteSystem.cpp index 1e896ced2e599..6156cdc09e118 100644 --- a/lib/AST/RequirementMachine/RewriteSystem.cpp +++ b/lib/AST/RequirementMachine/RewriteSystem.cpp @@ -251,14 +251,13 @@ bool RewriteSystem::simplify(MutableTerm &term, RewritePath *path) const { /// Simplify terms appearing in the substitutions of the last symbol of \p term, /// which must be a superclass or concrete type symbol. -void RewriteSystem::simplifySubstitutions(MutableTerm &term, +bool RewriteSystem::simplifySubstitutions(Symbol &symbol, RewritePath &path) const { - auto symbol = term.back(); assert(symbol.hasSubstitutions()); auto substitutions = symbol.getSubstitutions(); if (substitutions.empty()) - return; + return false; // Save the original rewrite path length so that we can reset if if we don't // find anything to simplify. @@ -307,11 +306,12 @@ void RewriteSystem::simplifySubstitutions(MutableTerm &term, #endif path.resize(oldSize); - return; + return false; } // Build the new symbol with simplified substitutions. - term.back() = symbol.withConcreteSubstitutions(newSubstitutions, Context); + symbol = symbol.withConcreteSubstitutions(newSubstitutions, Context); + return true; } /// Adds a rewrite rule, returning true if the new rule was non-trivial. @@ -341,12 +341,6 @@ bool RewriteSystem::addRule(MutableTerm lhs, MutableTerm rhs, RewritePath lhsPath; RewritePath rhsPath; - if (lhs.back().hasSubstitutions()) - simplifySubstitutions(lhs, lhsPath); - - if (rhs.back().hasSubstitutions()) - simplifySubstitutions(rhs, rhsPath); - simplify(lhs, &lhsPath); simplify(rhs, &rhsPath); @@ -460,13 +454,11 @@ bool RewriteSystem::addExplicitRule(MutableTerm lhs, MutableTerm rhs) { return added; } -/// Delete any rules whose left hand sides can be reduced by other rules, -/// and reduce the right hand sides of all remaining rules as much as -/// possible. +/// Delete any rules whose left hand sides can be reduced by other rules. /// /// Must be run after the completion procedure, since the deletion of /// rules is only valid to perform if the rewrite system is confluent. -void RewriteSystem::simplifyRewriteSystem() { +void RewriteSystem::simplifyLeftHandSides() { assert(Complete); for (unsigned ruleID = 0, e = Rules.size(); ruleID < e; ++ruleID) { @@ -500,8 +492,19 @@ void RewriteSystem::simplifyRewriteSystem() { break; } } + } +} + +/// Reduce the right hand sides of all remaining rules as much as +/// possible. +/// +/// Must be run after the completion procedure, since the deletion of +/// rules is only valid to perform if the rewrite system is confluent. +void RewriteSystem::simplifyRightHandSidesAndSubstitutions() { + assert(Complete); - // If the rule was deleted above, skip the rest. + for (unsigned ruleID = 0, e = Rules.size(); ruleID < e; ++ruleID) { + auto &rule = getRule(ruleID); if (rule.isSimplified()) continue; @@ -511,6 +514,8 @@ void RewriteSystem::simplifyRewriteSystem() { if (!simplify(rhs, &rhsPath)) continue; + auto lhs = rule.getLHS(); + // We're adding a new rule, so the old rule won't apply anymore. rule.markSimplified(); @@ -544,6 +549,41 @@ void RewriteSystem::simplifyRewriteSystem() { recordRewriteLoop(MutableTerm(lhs), loop); } + + // Finally try to simplify substitutions in superclass, concrete type and + // concrete conformance symbols. + for (unsigned ruleID = 0, e = Rules.size(); ruleID < e; ++ruleID) { + auto &rule = getRule(ruleID); + if (rule.isSimplified()) + continue; + + auto lhs = rule.getLHS(); + auto symbol = lhs.back(); + if (!symbol.hasSubstitutions()) + continue; + + RewritePath path; + + // (1) First, apply the original rule to produce the original lhs. + path.add(RewriteStep::forRewriteRule(/*startOffset=*/0, /*endOffset=*/0, + ruleID, /*inverse=*/true)); + + // (2) Now, simplify the substitutions to get the new lhs. + if (!simplifySubstitutions(symbol, path)) + continue; + + // We're either going to add a new rule or record an identity, so + // mark the old rule as simplified. + rule.markSimplified(); + + MutableTerm newLHS(lhs.begin(), lhs.end() - 1); + newLHS.add(symbol); + + // Invert the path to get a path from the new lhs to the old rhs. + path.invert(); + + addRule(newLHS, MutableTerm(rule.getRHS()), &path); + } } void RewriteSystem::verifyRewriteRules(ValidityPolicy policy) const { diff --git a/lib/AST/RequirementMachine/RewriteSystem.h b/lib/AST/RequirementMachine/RewriteSystem.h index 6dd9e6e6962d1..5de4b6a5cc45b 100644 --- a/lib/AST/RequirementMachine/RewriteSystem.h +++ b/lib/AST/RequirementMachine/RewriteSystem.h @@ -278,7 +278,7 @@ class RewriteSystem final { bool simplify(MutableTerm &term, RewritePath *path=nullptr) const; - void simplifySubstitutions(MutableTerm &term, RewritePath &path) const; + bool simplifySubstitutions(Symbol &symbol, RewritePath &path) const; ////////////////////////////////////////////////////////////////////////////// /// @@ -290,7 +290,9 @@ class RewriteSystem final { computeConfluentCompletion(unsigned maxIterations, unsigned maxDepth); - void simplifyRewriteSystem(); + void simplifyLeftHandSides(); + + void simplifyRightHandSidesAndSubstitutions(); enum ValidityPolicy { AllowInvalidRequirements, diff --git a/test/Generics/canonical_concrete_substitutions_in_protocol.swift b/test/Generics/canonical_concrete_substitutions_in_protocol.swift new file mode 100644 index 0000000000000..784b2e5309866 --- /dev/null +++ b/test/Generics/canonical_concrete_substitutions_in_protocol.swift @@ -0,0 +1,23 @@ +// RUN: %target-swift-frontend -typecheck %s -debug-generic-signatures -requirement-machine-protocol-signatures=on 2>&1 | %FileCheck %s + +struct G {} + +// CHECK-LABEL: canonical_concrete_substitutions_in_protocol.(file).P@ +// CHECK-NEXT: Requirement signature: , Self.B == Self.T.X, Self.T : Q> + +protocol P { + associatedtype A where A == G + associatedtype B where B == T.X + associatedtype T : Q +} + +protocol Q { + associatedtype X +} + +protocol QQ : Q {} + +protocol R { + associatedtype A + associatedtype C: QQ where C.X == G +} \ No newline at end of file diff --git a/test/Generics/unify_concrete_types_1.swift b/test/Generics/unify_concrete_types_1.swift index b2d5f8ff1eda6..031c76fa33de7 100644 --- a/test/Generics/unify_concrete_types_1.swift +++ b/test/Generics/unify_concrete_types_1.swift @@ -28,5 +28,5 @@ struct MergeTest { // CHECK: [P1:X] => { concrete_type: [concrete: Foo<τ_0_0, τ_0_1> with <[P1:Y1], [P1:Z1]>] } // CHECK: [P2:X] => { concrete_type: [concrete: Foo<τ_0_0, τ_0_1> with <[P2:Y2], [P2:Z2]>] } // CHECK: τ_0_0 => { conforms_to: [P1 P2] } -// CHECK: τ_0_0.[P1&P2:X] => { concrete_type: [concrete: Foo<τ_0_0, τ_0_1> with <τ_0_0.[P2:Y2], τ_0_0.[P2:Z2]>] } +// CHECK: τ_0_0.[P1&P2:X] => { concrete_type: [concrete: Foo<τ_0_0, τ_0_1> with <τ_0_0.[P1:Y1], τ_0_0.[P1:Z1]>] } // CHECK: } diff --git a/test/Generics/unify_superclass_types_2.swift b/test/Generics/unify_superclass_types_2.swift index fda9e2ed1b97b..b69ef1790ee79 100644 --- a/test/Generics/unify_superclass_types_2.swift +++ b/test/Generics/unify_superclass_types_2.swift @@ -33,6 +33,8 @@ func unifySuperclassTest(_: T) { // CHECK: - τ_0_0.[P1&P2:X].[superclass: Generic<τ_0_0, String, τ_0_1> with <τ_0_0.[P2:A2], τ_0_0.[P2:B2]>] => τ_0_0.[P1&P2:X] // CHECK-NEXT: - τ_0_0.[P1&P2:X].[layout: _NativeClass] => τ_0_0.[P1&P2:X] // CHECK-NEXT: - τ_0_0.[P1&P2:X].[superclass: Generic with <τ_0_0.[P1:A1], τ_0_0.[P1:B1]>] => τ_0_0.[P1&P2:X] +// CHECK-NEXT: - τ_0_0.X => τ_0_0.[P1&P2:X] +// CHECK-NEXT: - τ_0_0.[P2:X] => τ_0_0.[P1&P2:X] // CHECK-NEXT: - τ_0_0.[P2:A2].[concrete: Int] => τ_0_0.[P2:A2] // CHECK-NEXT: - τ_0_0.[P1:A1].[concrete: String] => τ_0_0.[P1:A1] // CHECK-NEXT: - τ_0_0.[P2:B2] => τ_0_0.[P1:B1] @@ -43,7 +45,7 @@ func unifySuperclassTest(_: T) { // CHECK-NEXT: [P1:X] => { layout: _NativeClass superclass: [superclass: Generic with <[P1:A1], [P1:B1]>] } // CHECK-NEXT: [P2:X] => { layout: _NativeClass superclass: [superclass: Generic<τ_0_0, String, τ_0_1> with <[P2:A2], [P2:B2]>] } // CHECK-NEXT: τ_0_0 => { conforms_to: [P1 P2] } -// CHECK-NEXT: τ_0_0.[P1&P2:X] => { layout: _NativeClass superclass: [superclass: Generic with <τ_0_0.[P1:A1], τ_0_0.[P1:B1]>] } +// CHECK-NEXT: τ_0_0.[P1&P2:X] => { layout: _NativeClass superclass: [superclass: Generic<τ_0_0, String, τ_0_1> with <τ_0_0.[P2:A2], τ_0_0.[P1:B1]>] } // CHECK-NEXT: τ_0_0.[P2:A2] => { concrete_type: [concrete: Int] } // CHECK-NEXT: τ_0_0.[P1:A1] => { concrete_type: [concrete: String] } // CHECK-NEXT: } diff --git a/test/Generics/unify_superclass_types_3.swift b/test/Generics/unify_superclass_types_3.swift index 05db678e2f280..0324e5aa6f0f8 100644 --- a/test/Generics/unify_superclass_types_3.swift +++ b/test/Generics/unify_superclass_types_3.swift @@ -35,10 +35,13 @@ func unifySuperclassTest(_: T) { // CHECK: - τ_0_0.[P1&P2:X].[superclass: Generic<τ_0_0, String, τ_0_1> with <τ_0_0.[P2:A2], τ_0_0.[P2:B2]>] => τ_0_0.[P1&P2:X] // CHECK-NEXT: - τ_0_0.[P1&P2:X].[layout: _NativeClass] => τ_0_0.[P1&P2:X] // CHECK-NEXT: - τ_0_0.[P1&P2:X].[superclass: Derived<τ_0_0, τ_0_1> with <τ_0_0.[P1:A1], τ_0_0.[P1:B1]>] => τ_0_0.[P1&P2:X] +// CHECK-NEXT: - τ_0_0.X => τ_0_0.[P1&P2:X] +// CHECK-NEXT: - τ_0_0.[P2:X] => τ_0_0.[P1&P2:X] // CHECK-NEXT: - τ_0_0.[P2:A2].[concrete: Int] => τ_0_0.[P2:A2] // CHECK-NEXT: - τ_0_0.[P1:A1].[concrete: String] => τ_0_0.[P1:A1] // CHECK-NEXT: - τ_0_0.[P2:B2] => τ_0_0.[P1:B1] // CHECK-NEXT: - τ_0_0.B2 => τ_0_0.[P1:B1] +// CHECK-NEXT: - τ_0_0.[P1&P2:X].[superclass: Generic<τ_0_0, String, τ_0_1> with <τ_0_0.[P2:A2], τ_0_0.[P1:B1]>] => τ_0_0.[P1&P2:X] // CHECK-NEXT: } // CHECK-NEXT: Rewrite loops: { // CHECK: } diff --git a/test/Generics/unify_superclass_types_4.swift b/test/Generics/unify_superclass_types_4.swift index 65c02953ee6df..28de39246cf61 100644 --- a/test/Generics/unify_superclass_types_4.swift +++ b/test/Generics/unify_superclass_types_4.swift @@ -38,6 +38,8 @@ func unifySuperclassTest(_: T) { // CHECK: - τ_0_0.[P1&P2:X].[superclass: Derived<τ_0_0> with <τ_0_0.[P2:A2]>] => τ_0_0.[P1&P2:X] // CHECK-NEXT: - τ_0_0.[P1&P2:X].[layout: _NativeClass] => τ_0_0.[P1&P2:X] // CHECK-NEXT: - τ_0_0.[P1&P2:X].[superclass: Base<τ_0_0> with <τ_0_0.[P1:A1]>] => τ_0_0.[P1&P2:X] +// CHECK-NEXT: - τ_0_0.X => τ_0_0.[P1&P2:X] +// CHECK-NEXT: - τ_0_0.[P2:X] => τ_0_0.[P1&P2:X] // CHECK-NEXT: - τ_0_0.[P2:A2].[Q:T] => τ_0_0.[P1:A1] // CHECK-NEXT: } // CHECK-NEXT: Rewrite loops: { From 7a56ab8c182da7c27c8488dca7a901721ed81f81 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Thu, 9 Dec 2021 00:19:39 -0500 Subject: [PATCH 4/7] RequirementMachine: Mark conflicting rules This isn't quite right, because we really want the longer (more specific) of the two rules to be conflicting, not the most-recently added rule. --- .../RequirementMachine/HomotopyReduction.cpp | 15 ++++-- lib/AST/RequirementMachine/PropertyMap.cpp | 12 +++-- lib/AST/RequirementMachine/PropertyMap.h | 4 +- .../PropertyUnification.cpp | 48 +++++++++++-------- .../RequirementMachine/RequirementMachine.cpp | 2 +- lib/AST/RequirementMachine/RewriteSystem.cpp | 2 + lib/AST/RequirementMachine/RewriteSystem.h | 25 +++++++++- 7 files changed, 76 insertions(+), 32 deletions(-) diff --git a/lib/AST/RequirementMachine/HomotopyReduction.cpp b/lib/AST/RequirementMachine/HomotopyReduction.cpp index a063471f0b5e7..20c5029b0b51a 100644 --- a/lib/AST/RequirementMachine/HomotopyReduction.cpp +++ b/lib/AST/RequirementMachine/HomotopyReduction.cpp @@ -523,16 +523,19 @@ void RewriteSystem::minimizeRewriteSystem() { /// In a conformance-valid rewrite system, any rule with unresolved symbols on /// the left or right hand side should have been simplified by another rule. -bool RewriteSystem::hasNonRedundantUnresolvedRules() const { +bool RewriteSystem::hadError() const { assert(Complete); assert(Minimized); for (const auto &rule : Rules) { - if (!rule.isRedundant() && - !rule.isPermanent() && - rule.containsUnresolvedSymbols()) { + if (rule.isPermanent()) + continue; + + if (rule.isConflicting()) + return true; + + if (!rule.isRedundant() && rule.containsUnresolvedSymbols()) return true; - } } return false; @@ -555,6 +558,7 @@ RewriteSystem::getMinimizedProtocolRules( if (rule.isPermanent() || rule.isRedundant() || + rule.isConflicting() || rule.containsUnresolvedSymbols()) { continue; } @@ -584,6 +588,7 @@ RewriteSystem::getMinimizedGenericSignatureRules() const { if (rule.isPermanent() || rule.isRedundant() || + rule.isConflicting() || rule.containsUnresolvedSymbols()) { continue; } diff --git a/lib/AST/RequirementMachine/PropertyMap.cpp b/lib/AST/RequirementMachine/PropertyMap.cpp index fdf4ff4554308..a6e18e3fcd417 100644 --- a/lib/AST/RequirementMachine/PropertyMap.cpp +++ b/lib/AST/RequirementMachine/PropertyMap.cpp @@ -309,14 +309,15 @@ void PropertyMap::clear() { /// Record a protocol conformance, layout or superclass constraint on the given /// key. Must be called in monotonically non-decreasing key order. -void PropertyMap::addProperty( +bool PropertyMap::addProperty( Term key, Symbol property, unsigned ruleID, SmallVectorImpl &inducedRules) { assert(property.isProperty()); assert(*System.getRule(ruleID).isPropertyRule() == property); auto *props = getOrCreateProperties(key); - props->addProperty(property, ruleID, Context, - inducedRules, Debug.contains(DebugFlags::ConcreteUnification)); + bool debug = Debug.contains(DebugFlags::ConcreteUnification); + return props->addProperty(property, ruleID, Context, + inducedRules, debug); } /// Build the property map from all rules of the form T.[p] => T, where @@ -375,7 +376,10 @@ PropertyMap::buildPropertyMap(unsigned maxIterations, for (const auto &bucket : properties) { for (auto property : bucket) { - addProperty(property.key, property.symbol, property.ruleID, inducedRules); + bool conflict = addProperty(property.key, property.symbol, + property.ruleID, inducedRules); + if (conflict) + System.getRule(property.ruleID).markConflicting(); } } diff --git a/lib/AST/RequirementMachine/PropertyMap.h b/lib/AST/RequirementMachine/PropertyMap.h index 12bffa16f3ed0..98291c366f737 100644 --- a/lib/AST/RequirementMachine/PropertyMap.h +++ b/lib/AST/RequirementMachine/PropertyMap.h @@ -100,7 +100,7 @@ class PropertyBag { explicit PropertyBag(Term key) : Key(key) {} - void addProperty(Symbol property, + bool addProperty(Symbol property, unsigned ruleID, RewriteContext &ctx, SmallVectorImpl &inducedRules, @@ -202,7 +202,7 @@ class PropertyMap { private: void clear(); - void addProperty(Term key, Symbol property, unsigned ruleID, + bool addProperty(Term key, Symbol property, unsigned ruleID, SmallVectorImpl &inducedRules); void computeConcreteTypeInDomainMap(); diff --git a/lib/AST/RequirementMachine/PropertyUnification.cpp b/lib/AST/RequirementMachine/PropertyUnification.cpp index bfd4ad90bc3fb..fc856fc96c427 100644 --- a/lib/AST/RequirementMachine/PropertyUnification.cpp +++ b/lib/AST/RequirementMachine/PropertyUnification.cpp @@ -250,7 +250,7 @@ static bool unifyConcreteTypes( /// /// Returns the most derived superclass, which becomes the new superclass /// that gets recorded in the property map. -static Symbol unifySuperclasses( +static std::pair unifySuperclasses( Symbol lhs, Symbol rhs, RewriteContext &ctx, SmallVectorImpl &inducedRules, bool debug) { @@ -282,7 +282,7 @@ static Symbol unifySuperclasses( llvm::dbgs() << "%% Unrelated superclass types\n"; } - return lhs; + return std::make_pair(lhs, true); } if (lhsClass != rhsClass) { @@ -300,14 +300,15 @@ static Symbol unifySuperclasses( if (debug) { llvm::dbgs() << "%% Superclass conflict\n"; } - return rhs; + return std::make_pair(rhs, true); } // Record the more specific class. - return rhs; + return std::make_pair(rhs, false); } -void PropertyBag::addProperty( +/// Returns true if there was a conflict. +bool PropertyBag::addProperty( Symbol property, unsigned ruleID, RewriteContext &ctx, SmallVectorImpl &inducedRules, bool debug) { @@ -316,46 +317,55 @@ void PropertyBag::addProperty( case Symbol::Kind::Protocol: ConformsTo.push_back(property.getProtocol()); ConformsToRules.push_back(ruleID); - return; + return false; case Symbol::Kind::Layout: if (!Layout) { Layout = property.getLayoutConstraint(); LayoutRule = ruleID; - } else + } else { Layout = Layout.merge(property.getLayoutConstraint()); + if (!Layout->isKnownLayout()) + return true; + } - return; + return false; case Symbol::Kind::Superclass: { // FIXME: Also handle superclass vs concrete - if (Superclass) { - Superclass = unifySuperclasses(*Superclass, property, - ctx, inducedRules, debug); - } else { + if (!Superclass) { Superclass = property; SuperclassRule = ruleID; + } else { + auto pair = unifySuperclasses(*Superclass, property, + ctx, inducedRules, debug); + Superclass = pair.first; + bool conflict = pair.second; + if (conflict) + return true; } - return; + return false; } case Symbol::Kind::ConcreteType: { - if (ConcreteType) { - (void) unifyConcreteTypes(*ConcreteType, property, - ctx, inducedRules, debug); - } else { + if (!ConcreteType) { ConcreteType = property; ConcreteTypeRule = ruleID; + } else { + bool conflict = unifyConcreteTypes(*ConcreteType, property, + ctx, inducedRules, debug); + if (conflict) + return true; } - return; + return false; } case Symbol::Kind::ConcreteConformance: // FIXME - return; + return false; case Symbol::Kind::Name: case Symbol::Kind::GenericParam: diff --git a/lib/AST/RequirementMachine/RequirementMachine.cpp b/lib/AST/RequirementMachine/RequirementMachine.cpp index 85737ddfd1ae1..7018d3f2991e3 100644 --- a/lib/AST/RequirementMachine/RequirementMachine.cpp +++ b/lib/AST/RequirementMachine/RequirementMachine.cpp @@ -279,7 +279,7 @@ bool RequirementMachine::isComplete() const { bool RequirementMachine::hadError() const { // FIXME: Implement other checks here // FIXME: Assert if hadError() is true but we didn't emit any diagnostics? - return System.hasNonRedundantUnresolvedRules(); + return System.hadError(); } void RequirementMachine::dump(llvm::raw_ostream &out) const { diff --git a/lib/AST/RequirementMachine/RewriteSystem.cpp b/lib/AST/RequirementMachine/RewriteSystem.cpp index 6156cdc09e118..415474795be3e 100644 --- a/lib/AST/RequirementMachine/RewriteSystem.cpp +++ b/lib/AST/RequirementMachine/RewriteSystem.cpp @@ -150,6 +150,8 @@ void Rule::dump(llvm::raw_ostream &out) const { out << " [simplified]"; if (Redundant) out << " [redundant]"; + if (Conflicting) + out << "[conflicting]"; } RewriteSystem::RewriteSystem(RewriteContext &ctx) diff --git a/lib/AST/RequirementMachine/RewriteSystem.h b/lib/AST/RequirementMachine/RewriteSystem.h index 5de4b6a5cc45b..0af3c88b05c6d 100644 --- a/lib/AST/RequirementMachine/RewriteSystem.h +++ b/lib/AST/RequirementMachine/RewriteSystem.h @@ -66,6 +66,17 @@ class Rule final { /// set of requirements in a generic signature. unsigned Redundant : 1; + /// A 'conflicting' rule is a property rule which cannot be satisfied by any + /// concrete type because it is mutually exclusive with some other rule. + /// An example would be a pair of concrete type rules: + /// + /// T.[concrete: Int] => T + /// T.[concrete: String] => T + /// + /// Conflicting rules are detected in property map construction, and are + /// dropped from the minimal set of requirements. + unsigned Conflicting : 1; + public: Rule(Term lhs, Term rhs) : LHS(lhs), RHS(rhs) { @@ -73,6 +84,7 @@ class Rule final { Explicit = false; Simplified = false; Redundant = false; + Conflicting = false; } const Term &getLHS() const { return LHS; } @@ -105,6 +117,10 @@ class Rule final { return Redundant; } + bool isConflicting() const { + return Conflicting; + } + bool containsUnresolvedSymbols() const { return (LHS.containsUnresolvedSymbols() || RHS.containsUnresolvedSymbols()); @@ -132,6 +148,13 @@ class Rule final { Redundant = true; } + void markConflicting() { + // It's okay to mark a rule as conflicting multiple times, but it must not + // be a permanent rule. + assert(!Permanent && "Permanent rule should not conflict with anything"); + Conflicting = true; + } + unsigned getDepth() const; int compare(const Rule &other, RewriteContext &ctx) const; @@ -356,7 +379,7 @@ class RewriteSystem final { public: void minimizeRewriteSystem(); - bool hasNonRedundantUnresolvedRules() const; + bool hadError() const; llvm::DenseMap> getMinimizedProtocolRules(ArrayRef protos) const; From d07812e479600c25cbb3ae237bab404e32c3b1b8 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Thu, 9 Dec 2021 16:44:45 -0500 Subject: [PATCH 5/7] RequirementMachine: Factor out PropertyMap::concretizeTypeWitnessInConformance() --- lib/AST/RequirementMachine/PropertyMap.h | 7 + .../PropertyUnification.cpp | 143 +++++++++++------- 2 files changed, 97 insertions(+), 53 deletions(-) diff --git a/lib/AST/RequirementMachine/PropertyMap.h b/lib/AST/RequirementMachine/PropertyMap.h index 98291c366f737..0f9ef7f7bda10 100644 --- a/lib/AST/RequirementMachine/PropertyMap.h +++ b/lib/AST/RequirementMachine/PropertyMap.h @@ -216,6 +216,13 @@ class PropertyMap { llvm::TinyPtrVector &conformances, SmallVectorImpl &inducedRules) const; + void concretizeTypeWitnessInConformance( + Term key, RequirementKind requirementKind, + CanType concreteType, ArrayRef substitutions, + const ProtocolDecl *proto, ProtocolConformance *concrete, + AssociatedTypeDecl *assocType, + SmallVectorImpl &inducedRules) const; + MutableTerm computeConstraintTermForTypeWitness( Term key, CanType concreteType, CanType typeWitness, const MutableTerm &subjectType, ArrayRef substitutions) const; diff --git a/lib/AST/RequirementMachine/PropertyUnification.cpp b/lib/AST/RequirementMachine/PropertyUnification.cpp index fc856fc96c427..a8eb132f7ce75 100644 --- a/lib/AST/RequirementMachine/PropertyUnification.cpp +++ b/lib/AST/RequirementMachine/PropertyUnification.cpp @@ -428,7 +428,7 @@ void PropertyMap::concretizeNestedTypesFromConcreteParents( RequirementKind::SameType, props->ConcreteType->getConcreteType(), props->ConcreteType->getSubstitutions(), - props->getConformsTo(), + props->ConformsTo, props->ConcreteConformances, inducedRules); } @@ -523,70 +523,83 @@ void PropertyMap::concretizeNestedTypesFromConcreteParent( continue; for (auto *assocType : assocTypes) { - if (Debug.contains(DebugFlags::ConcretizeNestedTypes)) { - llvm::dbgs() << "^^ " << "Looking up type witness for " - << proto->getName() << ":" << assocType->getName() - << " on " << concreteType << "\n"; - } + concretizeTypeWitnessInConformance(key, requirementKind, + concreteType, substitutions, + proto, concrete, assocType, + inducedRules); + } + } +} - auto t = concrete->getTypeWitness(assocType); - if (!t) { - if (Debug.contains(DebugFlags::ConcretizeNestedTypes)) { - llvm::dbgs() << "^^ " << "Type witness for " << assocType->getName() - << " of " << concreteType << " could not be inferred\n"; - } +void PropertyMap::concretizeTypeWitnessInConformance( + Term key, RequirementKind requirementKind, + CanType concreteType, ArrayRef substitutions, + const ProtocolDecl *proto, ProtocolConformance *concrete, + AssociatedTypeDecl *assocType, + SmallVectorImpl &inducedRules) const { - t = ErrorType::get(concreteType); - } + if (Debug.contains(DebugFlags::ConcretizeNestedTypes)) { + llvm::dbgs() << "^^ " << "Looking up type witness for " + << proto->getName() << ":" << assocType->getName() + << " on " << concreteType << "\n"; + } - auto typeWitness = t->getCanonicalType(); + auto t = concrete->getTypeWitness(assocType); + if (!t) { + if (Debug.contains(DebugFlags::ConcretizeNestedTypes)) { + llvm::dbgs() << "^^ " << "Type witness for " << assocType->getName() + << " of " << concreteType << " could not be inferred\n"; + } - if (Debug.contains(DebugFlags::ConcretizeNestedTypes)) { - llvm::dbgs() << "^^ " << "Type witness for " << assocType->getName() - << " of " << concreteType << " is " << typeWitness << "\n"; - } + t = ErrorType::get(concreteType); + } - MutableTerm subjectType(key); - subjectType.add(Symbol::forAssociatedType(proto, assocType->getName(), - Context)); + auto typeWitness = t->getCanonicalType(); - MutableTerm constraintType; + if (Debug.contains(DebugFlags::ConcretizeNestedTypes)) { + llvm::dbgs() << "^^ " << "Type witness for " << assocType->getName() + << " of " << concreteType << " is " << typeWitness << "\n"; + } - auto simplify = [&](CanType t) -> CanType { - return CanType(t.transformRec([&](Type t) -> Optional { - if (!t->isTypeParameter()) - return None; + MutableTerm subjectType(key); + subjectType.add(Symbol::forAssociatedType(proto, assocType->getName(), + Context)); - auto term = Context.getRelativeTermForType(t->getCanonicalType(), - substitutions); - System.simplify(term); - return Context.getTypeForTerm(term, { }); - })); - }; + MutableTerm constraintType; - if (simplify(concreteType) == simplify(typeWitness) && - requirementKind == RequirementKind::SameType) { - // FIXME: ConcreteTypeInDomainMap should support substitutions so - // that we can remove this. + auto simplify = [&](CanType t) -> CanType { + return CanType(t.transformRec([&](Type t) -> Optional { + if (!t->isTypeParameter()) + return None; - if (Debug.contains(DebugFlags::ConcretizeNestedTypes)) { - llvm::dbgs() << "^^ Type witness is the same as the concrete type\n"; - } + auto term = Context.getRelativeTermForType(t->getCanonicalType(), + substitutions); + System.simplify(term); + return Context.getTypeForTerm(term, { }); + })); + }; - // Add a rule T.[P:A] => T. - constraintType = MutableTerm(key); - } else { - constraintType = computeConstraintTermForTypeWitness( - key, concreteType, typeWitness, subjectType, - substitutions); - } + if (simplify(concreteType) == simplify(typeWitness) && + requirementKind == RequirementKind::SameType) { + // FIXME: ConcreteTypeInDomainMap should support substitutions so + // that we can remove this. - inducedRules.emplace_back(subjectType, constraintType); - if (Debug.contains(DebugFlags::ConcretizeNestedTypes)) { - llvm::dbgs() << "^^ Induced rule " << constraintType - << " => " << subjectType << "\n"; - } + if (Debug.contains(DebugFlags::ConcretizeNestedTypes)) { + llvm::dbgs() << "^^ Type witness is the same as the concrete type\n"; } + + // Add a rule T.[P:A] => T. + constraintType = MutableTerm(key); + } else { + constraintType = computeConstraintTermForTypeWitness( + key, concreteType, typeWitness, subjectType, + substitutions); + } + + inducedRules.emplace_back(subjectType, constraintType); + if (Debug.contains(DebugFlags::ConcretizeNestedTypes)) { + llvm::dbgs() << "^^ Induced rule " << constraintType + << " => " << subjectType << "\n"; } } @@ -677,12 +690,36 @@ void PropertyMap::recordConcreteConformanceRules( // minimizes as // // . + // + // We model this by marking unsatisfied conformance rules as conflicts. + + // The conformances in ConcreteConformances should appear in the same + // order as the protocols in ConformsTo. + auto conformanceIter = props->ConcreteConformances.begin(); + for (unsigned i : indices(props->ConformsTo)) { - auto *proto = props->ConformsTo[i]; auto conformanceRuleID = props->ConformsToRules[i]; + if (conformanceIter == props->ConcreteConformances.end()) { + // FIXME: We should mark the more specific rule of the conformance and + // concrete type rules as conflicting. + System.getRule(conformanceRuleID).markConflicting(); + continue; + } + + auto *proto = props->ConformsTo[i]; + if (proto != (*conformanceIter)->getProtocol()) { + // FIXME: We should mark the more specific rule of the conformance and + // concrete type rules as conflicting. + System.getRule(conformanceRuleID).markConflicting(); + continue; + } + recordConcreteConformanceRule(concreteRuleID, conformanceRuleID, proto, inducedRules); + ++conformanceIter; } + + assert(conformanceIter == props->ConcreteConformances.end()); } if (props->Superclass) { From d25b0db75cc7d8535dc88136b5efcc92d990ee62 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Mon, 13 Dec 2021 13:37:07 -0500 Subject: [PATCH 6/7] RequirementMachine: Fold recordConcreteConformanceRules() into concretizeNestedTypesFromConcreteParent() --- lib/AST/RequirementMachine/PropertyMap.cpp | 4 - lib/AST/RequirementMachine/PropertyMap.h | 12 +- .../PropertyUnification.cpp | 128 +++++------------- 3 files changed, 42 insertions(+), 102 deletions(-) diff --git a/lib/AST/RequirementMachine/PropertyMap.cpp b/lib/AST/RequirementMachine/PropertyMap.cpp index a6e18e3fcd417..772abd920f3b3 100644 --- a/lib/AST/RequirementMachine/PropertyMap.cpp +++ b/lib/AST/RequirementMachine/PropertyMap.cpp @@ -392,10 +392,6 @@ PropertyMap::buildPropertyMap(unsigned maxIterations, // the concrete type witnesses in the concrete type's conformance. concretizeNestedTypesFromConcreteParents(inducedRules); - // Finally, introduce concrete conformance rules, relating conformance rules - // to concrete type and superclass rules. - recordConcreteConformanceRules(inducedRules); - // Some of the induced rules might be trivial; only count the induced rules // where the left hand side is not already equivalent to the right hand side. unsigned addedNewRules = 0; diff --git a/lib/AST/RequirementMachine/PropertyMap.h b/lib/AST/RequirementMachine/PropertyMap.h index 0f9ef7f7bda10..ee4da9fbdf254 100644 --- a/lib/AST/RequirementMachine/PropertyMap.h +++ b/lib/AST/RequirementMachine/PropertyMap.h @@ -207,14 +207,17 @@ class PropertyMap { void computeConcreteTypeInDomainMap(); void concretizeNestedTypesFromConcreteParents( - SmallVectorImpl &inducedRules) const; + SmallVectorImpl &inducedRules); void concretizeNestedTypesFromConcreteParent( Term key, RequirementKind requirementKind, - CanType concreteType, ArrayRef substitutions, + unsigned concreteRuleID, + CanType concreteType, + ArrayRef substitutions, + ArrayRef conformsToRules, ArrayRef conformsTo, llvm::TinyPtrVector &conformances, - SmallVectorImpl &inducedRules) const; + SmallVectorImpl &inducedRules); void concretizeTypeWitnessInConformance( Term key, RequirementKind requirementKind, @@ -227,9 +230,6 @@ class PropertyMap { Term key, CanType concreteType, CanType typeWitness, const MutableTerm &subjectType, ArrayRef substitutions) const; - void recordConcreteConformanceRules( - SmallVectorImpl &inducedRules); - void recordConcreteConformanceRule( unsigned concreteRuleID, unsigned conformanceRuleID, diff --git a/lib/AST/RequirementMachine/PropertyUnification.cpp b/lib/AST/RequirementMachine/PropertyUnification.cpp index a8eb132f7ce75..2b6cbb97550d5 100644 --- a/lib/AST/RequirementMachine/PropertyUnification.cpp +++ b/lib/AST/RequirementMachine/PropertyUnification.cpp @@ -404,7 +404,7 @@ void PropertyMap::computeConcreteTypeInDomainMap() { } void PropertyMap::concretizeNestedTypesFromConcreteParents( - SmallVectorImpl &inducedRules) const { + SmallVectorImpl &inducedRules) { for (const auto &props : Entries) { if (props->getConformsTo().empty()) continue; @@ -426,8 +426,10 @@ void PropertyMap::concretizeNestedTypesFromConcreteParents( concretizeNestedTypesFromConcreteParent( props->getKey(), RequirementKind::SameType, + *props->ConcreteTypeRule, props->ConcreteType->getConcreteType(), props->ConcreteType->getSubstitutions(), + props->ConformsToRules, props->ConformsTo, props->ConcreteConformances, inducedRules); @@ -441,9 +443,11 @@ void PropertyMap::concretizeNestedTypesFromConcreteParents( concretizeNestedTypesFromConcreteParent( props->getKey(), RequirementKind::Superclass, + *props->SuperclassRule, props->Superclass->getSuperclass(), props->Superclass->getSubstitutions(), - props->getConformsTo(), + props->ConformsToRules, + props->ConformsTo, props->SuperclassConformances, inducedRules); } @@ -484,14 +488,21 @@ void PropertyMap::concretizeNestedTypesFromConcreteParents( /// void PropertyMap::concretizeNestedTypesFromConcreteParent( Term key, RequirementKind requirementKind, - CanType concreteType, ArrayRef substitutions, + unsigned concreteRuleID, + CanType concreteType, + ArrayRef substitutions, + ArrayRef conformsToRules, ArrayRef conformsTo, llvm::TinyPtrVector &conformances, - SmallVectorImpl &inducedRules) const { + SmallVectorImpl &inducedRules) { assert(requirementKind == RequirementKind::SameType || requirementKind == RequirementKind::Superclass); + assert(conformsTo.size() == conformsToRules.size()); + + for (unsigned i : indices(conformsTo)) { + auto *proto = conformsTo[i]; + unsigned conformanceRuleID = conformsToRules[i]; - for (auto *proto : conformsTo) { // FIXME: Either remove the ModuleDecl entirely from conformance lookup, // or pass the correct one down in here. auto *module = proto->getParentModule(); @@ -499,7 +510,23 @@ void PropertyMap::concretizeNestedTypesFromConcreteParent( auto conformance = module->lookupConformance(concreteType, const_cast(proto)); if (conformance.isInvalid()) { - // FIXME: Diagnose conflict + // For superclass rules, it is totally fine to have a signature like: + // + // protocol P {} + // class C {} + // + // + // There is no relation between P and C here. + // + // With concrete types, a missing conformance is a conflict. + if (requirementKind == RequirementKind::SameType) { + // FIXME: Diagnose conflict + // + // FIXME: We should mark the more specific rule of the two + // as conflicting. + System.getRule(conformanceRuleID).markConflicting(); + } + if (Debug.contains(DebugFlags::ConcretizeNestedTypes)) { llvm::dbgs() << "^^ " << concreteType << " does not conform to " << proto->getName() << "\n"; @@ -514,6 +541,9 @@ void PropertyMap::concretizeNestedTypesFromConcreteParent( auto *concrete = conformance.getConcrete(); + recordConcreteConformanceRule(concreteRuleID, conformanceRuleID, proto, + inducedRules); + // Record the conformance for use by // PropertyBag::getConformsToExcludingSuperclassConformances(). conformances.push_back(concrete); @@ -672,92 +702,6 @@ MutableTerm PropertyMap::computeConstraintTermForTypeWitness( return constraintType; } -void PropertyMap::recordConcreteConformanceRules( - SmallVectorImpl &inducedRules) { - for (const auto &props : Entries) { - if (props->getConformsTo().empty()) - continue; - - if (props->ConcreteType) { - unsigned concreteRuleID = *props->ConcreteTypeRule; - - // The GSB drops all conformance requirements on a type parameter equated - // to a concrete type, even if the concrete type doesn't conform. That is, - // - // protocol P {} - // - // - // minimizes as - // - // . - // - // We model this by marking unsatisfied conformance rules as conflicts. - - // The conformances in ConcreteConformances should appear in the same - // order as the protocols in ConformsTo. - auto conformanceIter = props->ConcreteConformances.begin(); - - for (unsigned i : indices(props->ConformsTo)) { - auto conformanceRuleID = props->ConformsToRules[i]; - if (conformanceIter == props->ConcreteConformances.end()) { - // FIXME: We should mark the more specific rule of the conformance and - // concrete type rules as conflicting. - System.getRule(conformanceRuleID).markConflicting(); - continue; - } - - auto *proto = props->ConformsTo[i]; - if (proto != (*conformanceIter)->getProtocol()) { - // FIXME: We should mark the more specific rule of the conformance and - // concrete type rules as conflicting. - System.getRule(conformanceRuleID).markConflicting(); - continue; - } - - recordConcreteConformanceRule(concreteRuleID, conformanceRuleID, proto, - inducedRules); - ++conformanceIter; - } - - assert(conformanceIter == props->ConcreteConformances.end()); - } - - if (props->Superclass) { - unsigned superclassRuleID = *props->SuperclassRule; - - // For superclass rules, we only introduce a concrete conformance if the - // superclass actually conforms. Otherwise, it is totally fine to have a - // signature like - // - // protocol P {} - // class C {} - // - // - // There is no relation between P and C here. - - // The conformances in SuperclassConformances should appear in the same - // order as the protocols in ConformsTo. - auto conformanceIter = props->SuperclassConformances.begin(); - - for (unsigned i : indices(props->ConformsTo)) { - if (conformanceIter == props->SuperclassConformances.end()) - break; - - auto *proto = props->ConformsTo[i]; - if (proto != (*conformanceIter)->getProtocol()) - continue; - - unsigned conformanceRuleID = props->ConformsToRules[i]; - recordConcreteConformanceRule(superclassRuleID, conformanceRuleID, proto, - inducedRules); - ++conformanceIter; - } - - assert(conformanceIter == props->SuperclassConformances.end()); - } - } -} - void PropertyMap::recordConcreteConformanceRule( unsigned concreteRuleID, unsigned conformanceRuleID, From 56fb3cc937f022c1d0ebf18725cf2472fdd4e252 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Mon, 13 Dec 2021 15:26:54 -0500 Subject: [PATCH 7/7] RequirementMachine: Refactor nested type concretization a bit --- lib/AST/RequirementMachine/PropertyMap.h | 9 +- .../PropertyUnification.cpp | 104 ++++++++++-------- 2 files changed, 64 insertions(+), 49 deletions(-) diff --git a/lib/AST/RequirementMachine/PropertyMap.h b/lib/AST/RequirementMachine/PropertyMap.h index ee4da9fbdf254..13d6632694f1d 100644 --- a/lib/AST/RequirementMachine/PropertyMap.h +++ b/lib/AST/RequirementMachine/PropertyMap.h @@ -221,8 +221,8 @@ class PropertyMap { void concretizeTypeWitnessInConformance( Term key, RequirementKind requirementKind, - CanType concreteType, ArrayRef substitutions, - const ProtocolDecl *proto, ProtocolConformance *concrete, + Symbol concreteConformanceSymbol, + ProtocolConformance *concrete, AssociatedTypeDecl *assocType, SmallVectorImpl &inducedRules) const; @@ -233,8 +233,9 @@ class PropertyMap { void recordConcreteConformanceRule( unsigned concreteRuleID, unsigned conformanceRuleID, - const ProtocolDecl *proto, - SmallVectorImpl &inducedRules); + RequirementKind requirementKind, + Symbol concreteConformanceSymbol, + SmallVectorImpl &inducedRules) const; void verify() const; }; diff --git a/lib/AST/RequirementMachine/PropertyUnification.cpp b/lib/AST/RequirementMachine/PropertyUnification.cpp index 2b6cbb97550d5..0f5850c127e95 100644 --- a/lib/AST/RequirementMachine/PropertyUnification.cpp +++ b/lib/AST/RequirementMachine/PropertyUnification.cpp @@ -541,21 +541,33 @@ void PropertyMap::concretizeNestedTypesFromConcreteParent( auto *concrete = conformance.getConcrete(); - recordConcreteConformanceRule(concreteRuleID, conformanceRuleID, proto, - inducedRules); - // Record the conformance for use by // PropertyBag::getConformsToExcludingSuperclassConformances(). conformances.push_back(concrete); + // All subsequent logic just records new rewrite rules, and can be + // skipped if we've already processed this pair of rules. + if (!ConcreteConformanceRules.insert( + std::make_pair(concreteRuleID, conformanceRuleID)).second) { + // We've already processed this pair of rules. + continue; + } + + auto concreteConformanceSymbol = Symbol::forConcreteConformance( + concreteType, substitutions, proto, Context); + + recordConcreteConformanceRule(concreteRuleID, conformanceRuleID, + requirementKind, concreteConformanceSymbol, + inducedRules); + auto assocTypes = proto->getAssociatedTypeMembers(); if (assocTypes.empty()) continue; for (auto *assocType : assocTypes) { concretizeTypeWitnessInConformance(key, requirementKind, - concreteType, substitutions, - proto, concrete, assocType, + concreteConformanceSymbol, + concrete, assocType, inducedRules); } } @@ -563,10 +575,13 @@ void PropertyMap::concretizeNestedTypesFromConcreteParent( void PropertyMap::concretizeTypeWitnessInConformance( Term key, RequirementKind requirementKind, - CanType concreteType, ArrayRef substitutions, - const ProtocolDecl *proto, ProtocolConformance *concrete, + Symbol concreteConformanceSymbol, + ProtocolConformance *concrete, AssociatedTypeDecl *assocType, SmallVectorImpl &inducedRules) const { + auto concreteType = concreteConformanceSymbol.getConcreteType(); + auto substitutions = concreteConformanceSymbol.getSubstitutions(); + auto *proto = concreteConformanceSymbol.getProtocol(); if (Debug.contains(DebugFlags::ConcretizeNestedTypes)) { llvm::dbgs() << "^^ " << "Looking up type witness for " @@ -591,7 +606,9 @@ void PropertyMap::concretizeTypeWitnessInConformance( << " of " << concreteType << " is " << typeWitness << "\n"; } + // Build the term T.[concrete: C : P].[P:X]. MutableTerm subjectType(key); + subjectType.add(concreteConformanceSymbol); subjectType.add(Symbol::forAssociatedType(proto, assocType->getName(), Context)); @@ -682,11 +699,19 @@ MutableTerm PropertyMap::computeConstraintTermForTypeWitness( // The type witness is a type parameter of the form τ_0_n.X.Y...Z, // where 'n' is an index into the substitution array. // - // Add a rule T => S.X.Y...Z, where S is the nth substitution term. + // Add a rule: + // + // T.[concrete: C : P].[P:X] => S[n].X.Y...Z + // + // Where S[n] is the nth substitution term. return Context.getRelativeTermForType(typeWitness, substitutions); } // The type witness is a concrete type. + // + // Add a rule: + // + // T.[concrete: C : P].[P:X].[concrete: Foo.A] => T.[concrete: C : P].[P:A]. MutableTerm constraintType = subjectType; SmallVector result; @@ -694,7 +719,6 @@ MutableTerm PropertyMap::computeConstraintTermForTypeWitness( remapConcreteSubstitutionSchema(typeWitness, substitutions, Context, result); - // Add a rule T.[P:A].[concrete: Foo.A] => T.[P:A]. constraintType.add( Symbol::forConcreteType( typeWitnessSchema, result, Context)); @@ -705,24 +729,28 @@ MutableTerm PropertyMap::computeConstraintTermForTypeWitness( void PropertyMap::recordConcreteConformanceRule( unsigned concreteRuleID, unsigned conformanceRuleID, - const ProtocolDecl *proto, - SmallVectorImpl &inducedRules) { - if (!ConcreteConformanceRules.insert( - std::make_pair(concreteRuleID, conformanceRuleID)).second) { - // We've already emitted this rule. - return; - } - + RequirementKind requirementKind, + Symbol concreteConformanceSymbol, + SmallVectorImpl &inducedRules) const { const auto &concreteRule = System.getRule(concreteRuleID); const auto &conformanceRule = System.getRule(conformanceRuleID); - auto conformanceSymbol = *conformanceRule.isPropertyRule(); - assert(conformanceSymbol.getKind() == Symbol::Kind::Protocol); - assert(conformanceSymbol.getProtocol() == proto); - - auto concreteSymbol = *concreteRule.isPropertyRule(); - assert(concreteSymbol.getKind() == Symbol::Kind::ConcreteType || - concreteSymbol.getKind() == Symbol::Kind::Superclass); +#ifndef NDEBUG + { + auto conformanceSymbol = *conformanceRule.isPropertyRule(); + assert(conformanceSymbol.getKind() == Symbol::Kind::Protocol); + assert(conformanceSymbol.getProtocol() == + concreteConformanceSymbol.getProtocol()); + + auto concreteSymbol = *concreteRule.isPropertyRule(); + if (concreteSymbol.getKind() == Symbol::Kind::Superclass) + assert(requirementKind == RequirementKind::Superclass); + else { + assert(concreteSymbol.getKind() == Symbol::Kind::ConcreteType); + assert(requirementKind == RequirementKind::SameType); + } + } +#endif RewritePath path; @@ -752,32 +780,18 @@ void PropertyMap::recordConcreteConformanceRule( // than T. unsigned adjustment = rhs.size() - concreteRule.getRHS().size(); if (adjustment > 0 && - !concreteSymbol.getSubstitutions().empty()) { + !concreteConformanceSymbol.getSubstitutions().empty()) { path.add(RewriteStep::forAdjustment(adjustment, /*endOffset=*/1, /*inverse=*/false)); - - concreteSymbol = concreteSymbol.prependPrefixToConcreteSubstitutions( - MutableTerm(rhs.begin(), rhs.begin() + adjustment), - Context); } // Now, transform T''.[concrete: C].[P] into T''.[concrete: C : P]. - Symbol concreteConformanceSymbol = [&]() { - if (concreteSymbol.getKind() == Symbol::Kind::ConcreteType) { - path.add(RewriteStep::forConcreteConformance(/*inverse=*/false)); - return Symbol::forConcreteConformance( - concreteSymbol.getConcreteType(), - concreteSymbol.getSubstitutions(), - proto, Context); - } else { - assert(concreteSymbol.getKind() == Symbol::Kind::Superclass); - path.add(RewriteStep::forSuperclassConformance(/*inverse=*/false)); - return Symbol::forConcreteConformance( - concreteSymbol.getSuperclass(), - concreteSymbol.getSubstitutions(), - proto, Context); - } - }(); + if (requirementKind == RequirementKind::Superclass) { + path.add(RewriteStep::forSuperclassConformance(/*inverse=*/false)); + } else { + assert(requirementKind == RequirementKind::SameType); + path.add(RewriteStep::forConcreteConformance(/*inverse=*/false)); + } MutableTerm lhs(rhs); lhs.add(concreteConformanceSymbol);