From 2bcc2f623cfd2a9ae91471d73726ff71b1f26e20 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Thu, 15 Aug 2019 22:10:49 -0700 Subject: [PATCH 1/4] [Constraint graph] Gathering "all mentions" constraints means all mentions. When requesting all constraints that mention the given type variable, use a depth-first search to ensure that we find all mentions, rather than some subset of them. Fixes rdar://problem/54322807. --- lib/Sema/ConstraintGraph.cpp | 155 ++++++++++-------- test/Constraints/keypath.swift | 22 +++ .../fast/rdar25866240.swift.gyb | 2 +- 3 files changed, 106 insertions(+), 73 deletions(-) diff --git a/lib/Sema/ConstraintGraph.cpp b/lib/Sema/ConstraintGraph.cpp index 4a34af40fa85f..a40892051fa4f 100644 --- a/lib/Sema/ConstraintGraph.cpp +++ b/lib/Sema/ConstraintGraph.cpp @@ -373,6 +373,67 @@ void ConstraintGraph::unbindTypeVariable(TypeVariableType *typeVar, Type fixed){ } } +#pragma mark Algorithms + +/// Perform a depth-first search. +/// +/// \param cg The constraint graph. +/// \param typeVar The type variable we're searching from. +/// \param preVisitNode Called before traversing a node. Must return \c +/// false when the node has already been visited. +/// \param visitConstraint Called before considering a constraint. If it +/// returns \c false, that constraint will be skipped. +/// \param visitedConstraints Set of already-visited constraints, used +/// internally to avoid duplicated work. +static void depthFirstSearch( + ConstraintGraph &cg, + TypeVariableType *typeVar, + llvm::function_ref preVisitNode, + llvm::function_ref visitConstraint, + llvm::SmallPtrSet &visitedConstraints) { + // Visit this node. If we've already seen it, bail out. + if (!preVisitNode(typeVar)) + return; + + // Local function to visit adjacent type variables. + auto visitAdjacencies = [&](ArrayRef adjTypeVars) { + for (auto adj : adjTypeVars) { + if (adj == typeVar) + continue; + + // Recurse into this node. + depthFirstSearch(cg, adj, preVisitNode, visitConstraint, + visitedConstraints); + } + }; + + // Walk all of the constraints associated with this node to find related + // nodes. + auto &node = cg[typeVar]; + for (auto constraint : node.getConstraints()) { + // If we've already seen this constraint, skip it. + if (!visitedConstraints.insert(constraint).second) + continue; + + if (visitConstraint(constraint)) + visitAdjacencies(constraint->getTypeVariables()); + } + + // Visit all of the other nodes in the equivalence class. + auto repTypeVar = cg.getConstraintSystem().getRepresentative(typeVar); + if (typeVar == repTypeVar) { + // We are the representative, so visit all of the other type variables + // in this equivalence class. + visitAdjacencies(node.getEquivalenceClass()); + } else { + // We are not the representative; visit the representative. + visitAdjacencies(repTypeVar); + } + + // Walk any type variables related via fixed bindings. + visitAdjacencies(node.getFixedBindings()); +} + llvm::TinyPtrVector ConstraintGraph::gatherConstraints( TypeVariableType *typeVar, GatheringKind kind, llvm::function_ref acceptConstraintFn) { @@ -407,6 +468,28 @@ llvm::TinyPtrVector ConstraintGraph::gatherConstraints( acceptConstraintFn(constraint); }; + if (kind == GatheringKind::AllMentions) { + // Perform a depth-first search in the constraint graph to find all of the + // constraints that could be affected. + SmallPtrSet typeVariables; + llvm::SmallPtrSet visitedConstraints; + depthFirstSearch(*this, typeVar, + [&](TypeVariableType *typeVar) { + return typeVariables.insert(typeVar).second; + }, + [&](Constraint *constraint) { + if (!shouldConsiderConstraint(constraint)) + return false; + + if (acceptConstraintFn(constraint)) + constraints.push_back(constraint); + + return true; + }, + visitedConstraints); + return constraints; + } + // Add constraints for the given adjacent type variable. llvm::SmallPtrSet typeVars; llvm::SmallPtrSet visitedConstraints; @@ -435,17 +518,6 @@ llvm::TinyPtrVector ConstraintGraph::gatherConstraints( if (visitedConstraints.insert(constraint).second && acceptConstraint(constraint)) constraints.push_back(constraint); - - // If we want all mentions, visit type variables within each of our - // constraints. - if (kind == GatheringKind::AllMentions) { - if (!shouldConsiderConstraint(constraint)) - continue; - - for (auto adjTypeVar : constraint->getTypeVariables()) { - addAdjacentConstraints(adjTypeVar); - } - } } // For any type variable mentioned in a fixed binding, add adjacent @@ -458,67 +530,6 @@ llvm::TinyPtrVector ConstraintGraph::gatherConstraints( return constraints; } -#pragma mark Algorithms - -/// Perform a depth-first search. -/// -/// \param cg The constraint graph. -/// \param typeVar The type variable we're searching from. -/// \param preVisitNode Called before traversing a node. Must return \c -/// false when the node has already been visited. -/// \param visitConstraint Called before considering a constraint. If it -/// returns \c false, that constraint will be skipped. -/// \param visitedConstraints Set of already-visited constraints, used -/// internally to avoid duplicated work. -static void depthFirstSearch( - ConstraintGraph &cg, - TypeVariableType *typeVar, - llvm::function_ref preVisitNode, - llvm::function_ref visitConstraint, - llvm::SmallPtrSet &visitedConstraints) { - // Visit this node. If we've already seen it, bail out. - if (!preVisitNode(typeVar)) - return; - - // Local function to visit adjacent type variables. - auto visitAdjacencies = [&](ArrayRef adjTypeVars) { - for (auto adj : adjTypeVars) { - if (adj == typeVar) - continue; - - // Recurse into this node. - depthFirstSearch(cg, adj, preVisitNode, visitConstraint, - visitedConstraints); - } - }; - - // Walk all of the constraints associated with this node to find related - // nodes. - auto &node = cg[typeVar]; - for (auto constraint : node.getConstraints()) { - // If we've already seen this constraint, skip it. - if (!visitedConstraints.insert(constraint).second) - continue; - - if (visitConstraint(constraint)) - visitAdjacencies(constraint->getTypeVariables()); - } - - // Visit all of the other nodes in the equivalence class. - auto repTypeVar = cg.getConstraintSystem().getRepresentative(typeVar); - if (typeVar == repTypeVar) { - // We are the representative, so visit all of the other type variables - // in this equivalence class. - visitAdjacencies(node.getEquivalenceClass()); - } else { - // We are not the representative; visit the representative. - visitAdjacencies(repTypeVar); - } - - // Walk any type variables related via fixed bindings. - visitAdjacencies(node.getFixedBindings()); -} - namespace { /// A union-find connected components algorithm used to find the connected /// components within a constraint graph. diff --git a/test/Constraints/keypath.swift b/test/Constraints/keypath.swift index d5bbbfed8b17b..f158345217129 100644 --- a/test/Constraints/keypath.swift +++ b/test/Constraints/keypath.swift @@ -51,3 +51,25 @@ func testFunc() { let f = \S.i let _: (S) -> Int = f // expected-error {{cannot convert value of type 'KeyPath' to specified type '(S) -> Int'}} } + +// rdar://problem/54322807 +struct X { + init(foo: KeyPath) { } + init(foo: KeyPath) { } +} + +struct Wibble { + var boolProperty = false +} + +struct Bar { + var optWibble: Wibble? = nil +} + +class Foo { + var optBar: Bar? = nil +} + +func testFoo(_: T) { + let _: X = .init(foo: \.optBar!.optWibble?.boolProperty) +} diff --git a/validation-test/Sema/type_checker_perf/fast/rdar25866240.swift.gyb b/validation-test/Sema/type_checker_perf/fast/rdar25866240.swift.gyb index 116a4e5c14c3e..a218b2f1ecbea 100644 --- a/validation-test/Sema/type_checker_perf/fast/rdar25866240.swift.gyb +++ b/validation-test/Sema/type_checker_perf/fast/rdar25866240.swift.gyb @@ -1,4 +1,4 @@ -// RUN: %scale-test --begin 2 --end 10 --step 1 --select NumConstraintScopes %s -Xfrontend=-solver-disable-shrink -Xfrontend=-disable-constraint-solver-performance-hacks -Xfrontend=-solver-enable-operator-designated-types +// RUN: %scale-test --begin 2 --end 15 --step 1 --select NumConstraintScopes %s -Xfrontend=-solver-disable-shrink -Xfrontend=-disable-constraint-solver-performance-hacks -Xfrontend=-solver-enable-operator-designated-types // REQUIRES: OS=macosx // REQUIRES: asserts From cf8184785934230e150f751182de0a11178206f3 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Fri, 16 Aug 2019 14:57:05 -0700 Subject: [PATCH 2/4] [Constraint graph] Further subdivide "gathering kind" for gatherConstraints Most clients of gatherConstraints() only care about the constraints directly on the given type variable or other type variables equivalent to it, while some also want to see constraints on fixed bindings as well. Split "gathering kind" further so we do less work to retrieve the constraints within an equivalence class for clients that only need that information. --- lib/Sema/CSBindings.cpp | 2 +- lib/Sema/CSSolver.cpp | 2 +- lib/Sema/ConstraintGraph.cpp | 10 ++++++---- lib/Sema/ConstraintGraph.h | 4 ++++ 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp index 423e27eea66c0..c2adcebe124b4 100644 --- a/lib/Sema/CSBindings.cpp +++ b/lib/Sema/CSBindings.cpp @@ -394,7 +394,7 @@ ConstraintSystem::getPotentialBindings(TypeVariableType *typeVar) { // Gather the constraints associated with this type variable. auto constraints = getConstraintGraph().gatherConstraints( - typeVar, ConstraintGraph::GatheringKind::EquivalenceClass); + typeVar, ConstraintGraph::GatheringKind::PotentialBindings); PotentialBindings result(typeVar); diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 34833abc2dc27..0cabf98b817ad 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -1581,7 +1581,7 @@ void ConstraintSystem::ArgumentInfoCollector::walk(Type argType) { visited.insert(rep); auto constraints = CS.getConstraintGraph().gatherConstraints( - rep, ConstraintGraph::GatheringKind::EquivalenceClass); + rep, ConstraintGraph::GatheringKind::PotentialBindings); for (auto *constraint : constraints) { switch (constraint->getKind()) { diff --git a/lib/Sema/ConstraintGraph.cpp b/lib/Sema/ConstraintGraph.cpp index a40892051fa4f..0b651f2fac36c 100644 --- a/lib/Sema/ConstraintGraph.cpp +++ b/lib/Sema/ConstraintGraph.cpp @@ -520,10 +520,12 @@ llvm::TinyPtrVector ConstraintGraph::gatherConstraints( constraints.push_back(constraint); } - // For any type variable mentioned in a fixed binding, add adjacent - // constraints. - for (auto adjTypeVar : node.getFixedBindings()) { - addAdjacentConstraints(adjTypeVar); + if (kind == GatheringKind::PotentialBindings) { + // For any type variable mentioned in a fixed binding, add adjacent + // constraints. + for (auto adjTypeVar : node.getFixedBindings()) { + addAdjacentConstraints(adjTypeVar); + } } } diff --git a/lib/Sema/ConstraintGraph.h b/lib/Sema/ConstraintGraph.h index bfceefa0afe06..6e53b3f6702a2 100644 --- a/lib/Sema/ConstraintGraph.h +++ b/lib/Sema/ConstraintGraph.h @@ -181,6 +181,10 @@ class ConstraintGraph { /// Gather constraints associated with all of the variables within the /// same equivalence class as the given type variable. EquivalenceClass, + /// Gather constraints that might be useful for potential bindings, + /// which also involves gathering constraints for type variables found + /// via fixed bindings. + PotentialBindings, /// Gather all constraints that mention this type variable or type variables /// that it is equivalent to. AllMentions, From 799f6d704c03a003b020dd129a246b4ae2a8bf43 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Sat, 17 Aug 2019 10:33:26 -0700 Subject: [PATCH 3/4] [Constraint graph] Use DFS for gathering EquivalenceClass constraints. Most of the logic is identical to that of AllMentions, except that we don't look through fixed bindings or traverse constraints. --- lib/Sema/ConstraintGraph.cpp | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/lib/Sema/ConstraintGraph.cpp b/lib/Sema/ConstraintGraph.cpp index 0b651f2fac36c..7db361842ef7c 100644 --- a/lib/Sema/ConstraintGraph.cpp +++ b/lib/Sema/ConstraintGraph.cpp @@ -390,6 +390,7 @@ static void depthFirstSearch( TypeVariableType *typeVar, llvm::function_ref preVisitNode, llvm::function_ref visitConstraint, + bool visitFixedBindings, llvm::SmallPtrSet &visitedConstraints) { // Visit this node. If we've already seen it, bail out. if (!preVisitNode(typeVar)) @@ -403,7 +404,7 @@ static void depthFirstSearch( // Recurse into this node. depthFirstSearch(cg, adj, preVisitNode, visitConstraint, - visitedConstraints); + visitFixedBindings, visitedConstraints); } }; @@ -430,8 +431,10 @@ static void depthFirstSearch( visitAdjacencies(repTypeVar); } - // Walk any type variables related via fixed bindings. - visitAdjacencies(node.getFixedBindings()); + if (visitFixedBindings) { + // Walk any type variables related via fixed bindings. + visitAdjacencies(node.getFixedBindings()); + } } llvm::TinyPtrVector ConstraintGraph::gatherConstraints( @@ -468,7 +471,8 @@ llvm::TinyPtrVector ConstraintGraph::gatherConstraints( acceptConstraintFn(constraint); }; - if (kind == GatheringKind::AllMentions) { + if (kind == GatheringKind::AllMentions || + kind == GatheringKind::EquivalenceClass) { // Perform a depth-first search in the constraint graph to find all of the // constraints that could be affected. SmallPtrSet typeVariables; @@ -484,8 +488,9 @@ llvm::TinyPtrVector ConstraintGraph::gatherConstraints( if (acceptConstraintFn(constraint)) constraints.push_back(constraint); - return true; + return kind == GatheringKind::AllMentions; }, + /*visitFixedBindings=*/kind == GatheringKind::AllMentions, visitedConstraints); return constraints; } @@ -800,6 +805,7 @@ namespace { return true; }, + /*visitFixedBindings=*/true, visitedConstraints); } From 8346714675aa6627e40db13d12b9e4789265e560 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Mon, 19 Aug 2019 10:25:08 -0700 Subject: [PATCH 4/4] [Constraint graph] Visit one level of fixed bindings when gathering constraints. Mimic the Swift <= 5.1 behavior of visiting only a single level of fixed bindings when gathering constraints, which should limit the amount of reprocessing of constraints. --- lib/Sema/ConstraintGraph.cpp | 52 +++++++++++++++++++++++++++++------- 1 file changed, 42 insertions(+), 10 deletions(-) diff --git a/lib/Sema/ConstraintGraph.cpp b/lib/Sema/ConstraintGraph.cpp index 7db361842ef7c..702df3d0435d9 100644 --- a/lib/Sema/ConstraintGraph.cpp +++ b/lib/Sema/ConstraintGraph.cpp @@ -375,6 +375,18 @@ void ConstraintGraph::unbindTypeVariable(TypeVariableType *typeVar, Type fixed){ #pragma mark Algorithms +namespace { + /// When to visit the fixed bindings of a type variable. + enum class VisitFixedBindings { + /// Always visit the fixed bindings + Always, + /// Never visit the fixed bindings + Never, + /// Visit fixed bindings one level down, but no further. + Once, + }; +} + /// Perform a depth-first search. /// /// \param cg The constraint graph. @@ -390,14 +402,15 @@ static void depthFirstSearch( TypeVariableType *typeVar, llvm::function_ref preVisitNode, llvm::function_ref visitConstraint, - bool visitFixedBindings, + VisitFixedBindings visitFixedBindings, llvm::SmallPtrSet &visitedConstraints) { // Visit this node. If we've already seen it, bail out. if (!preVisitNode(typeVar)) return; // Local function to visit adjacent type variables. - auto visitAdjacencies = [&](ArrayRef adjTypeVars) { + auto visitAdjacencies = [&](ArrayRef adjTypeVars, + VisitFixedBindings visitFixedBindings) { for (auto adj : adjTypeVars) { if (adj == typeVar) continue; @@ -417,7 +430,7 @@ static void depthFirstSearch( continue; if (visitConstraint(constraint)) - visitAdjacencies(constraint->getTypeVariables()); + visitAdjacencies(constraint->getTypeVariables(), visitFixedBindings); } // Visit all of the other nodes in the equivalence class. @@ -425,15 +438,22 @@ static void depthFirstSearch( if (typeVar == repTypeVar) { // We are the representative, so visit all of the other type variables // in this equivalence class. - visitAdjacencies(node.getEquivalenceClass()); + visitAdjacencies(node.getEquivalenceClass(), visitFixedBindings); } else { // We are not the representative; visit the representative. - visitAdjacencies(repTypeVar); + visitAdjacencies(repTypeVar, visitFixedBindings); } - if (visitFixedBindings) { - // Walk any type variables related via fixed bindings. - visitAdjacencies(node.getFixedBindings()); + // Walk any type variables related via fixed bindings. + switch (visitFixedBindings) { + case VisitFixedBindings::Always: + visitAdjacencies(node.getFixedBindings(), VisitFixedBindings::Always); + break; + case VisitFixedBindings::Never: + break; + case VisitFixedBindings::Once: + visitAdjacencies(node.getFixedBindings(), VisitFixedBindings::Never); + break; } } @@ -475,6 +495,18 @@ llvm::TinyPtrVector ConstraintGraph::gatherConstraints( kind == GatheringKind::EquivalenceClass) { // Perform a depth-first search in the constraint graph to find all of the // constraints that could be affected. + VisitFixedBindings visitFixedBindings; + switch (kind) { + case GatheringKind::AllMentions: + case GatheringKind::PotentialBindings: + visitFixedBindings = VisitFixedBindings::Once; + break; + + case GatheringKind::EquivalenceClass: + visitFixedBindings = VisitFixedBindings::Never; + break; + } + SmallPtrSet typeVariables; llvm::SmallPtrSet visitedConstraints; depthFirstSearch(*this, typeVar, @@ -490,7 +522,7 @@ llvm::TinyPtrVector ConstraintGraph::gatherConstraints( return kind == GatheringKind::AllMentions; }, - /*visitFixedBindings=*/kind == GatheringKind::AllMentions, + visitFixedBindings, visitedConstraints); return constraints; } @@ -805,7 +837,7 @@ namespace { return true; }, - /*visitFixedBindings=*/true, + VisitFixedBindings::Always, visitedConstraints); }