From cf1732cce21b7e7f68c5c399ed9e1585d1919c2c Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Thu, 22 Aug 2019 09:38:21 -0700 Subject: [PATCH 1/2] [Constraint graph] Reinstate the adjacencies of constraint graph nodes. Reinstate the list of adjacencies in each constraint graph node, effectively reverting dfdd352d3d236851a0e5b7fb93d1286966032089. Exclude one-way constraints from this computation; we'll handle them separately. --- lib/Sema/ConstraintGraph.cpp | 231 +++++++++++++++++++++++++++++++---- lib/Sema/ConstraintGraph.h | 42 +++++++ 2 files changed, 247 insertions(+), 26 deletions(-) diff --git a/lib/Sema/ConstraintGraph.cpp b/lib/Sema/ConstraintGraph.cpp index 4a34af40fa85f..ff903083f71f7 100644 --- a/lib/Sema/ConstraintGraph.cpp +++ b/lib/Sema/ConstraintGraph.cpp @@ -128,6 +128,74 @@ void ConstraintGraphNode::removeConstraint(Constraint *constraint) { Constraints.pop_back(); } +ConstraintGraphNode::Adjacency & +ConstraintGraphNode::getAdjacency(TypeVariableType *typeVar) { + assert(typeVar != TypeVar && "Cannot be adjacent to oneself"); + + // Look for existing adjacency information. + auto pos = AdjacencyInfo.find(typeVar); + + if (pos != AdjacencyInfo.end()) + return pos->second; + + // If we weren't already adjacent to this type variable, add it to the + // list of adjacencies. + pos = AdjacencyInfo.insert( + { typeVar, { static_cast(Adjacencies.size()), 0 } }) + .first; + Adjacencies.push_back(typeVar); + return pos->second; +} + +void ConstraintGraphNode::modifyAdjacency( + TypeVariableType *typeVar, + llvm::function_ref modify) { + // Find the adjacency information. + auto pos = AdjacencyInfo.find(typeVar); + assert(pos != AdjacencyInfo.end() && "Type variables not adjacent"); + assert(Adjacencies[pos->second.Index] == typeVar && "Mismatched adjacency"); + + // Perform the modification . + modify(pos->second); + + // If the adjacency is not empty, leave the information in there. + if (!pos->second.empty()) + return; + + // Remove this adjacency from the mapping. + unsigned index = pos->second.Index; + AdjacencyInfo.erase(pos); + + // If this adjacency is last in the vector, just pop it off. + unsigned lastIndex = Adjacencies.size()-1; + if (index == lastIndex) { + Adjacencies.pop_back(); + return; + } + + // This adjacency is somewhere in the middle; swap it with the last + // adjacency so we can remove the adjacency from the vector in O(1) time + // rather than O(n) time. + auto lastTypeVar = Adjacencies[lastIndex]; + Adjacencies[index] = lastTypeVar; + AdjacencyInfo[lastTypeVar].Index = index; + Adjacencies.pop_back(); +} + +void ConstraintGraphNode::addAdjacency(TypeVariableType *typeVar) { + auto &adjacency = getAdjacency(typeVar); + + // Bump the degree of the adjacency. + ++adjacency.NumConstraints; +} + +void ConstraintGraphNode::removeAdjacency(TypeVariableType *typeVar) { + modifyAdjacency(typeVar, [](Adjacency &adj) { + assert(adj.NumConstraints > 0 && "No adjacency to remove?"); + --adj.NumConstraints; + }); +} + void ConstraintGraphNode::addToEquivalenceClass( ArrayRef typeVars) { assert(TypeVar == TypeVar->getImpl().getRepresentative(nullptr) && @@ -258,52 +326,80 @@ void ConstraintGraph::removeNode(TypeVariableType *typeVar) { TypeVariables.pop_back(); } -void ConstraintGraph::addConstraint(Constraint *constraint) { - // For the nodes corresponding to each type variable... +/// Enumerate the adjacency edges for the given constraint. +static void enumerateAdjacencies( + Constraint *constraint, + llvm::function_ref visitor) { + // Don't record adjacencies for one-way constraints. + if (constraint->isOneWayConstraint()) + return; + + // O(N^2) update for all of the adjacent type variables. auto referencedTypeVars = constraint->getTypeVariables(); for (auto typeVar : referencedTypeVars) { - // Find the node for this type variable. - auto &node = (*this)[typeVar]; + for (auto otherTypeVar : referencedTypeVars) { + if (typeVar == otherTypeVar) + continue; + + visitor(typeVar, otherTypeVar); + } + } +} - // Note the constraint within the node for that type variable. - node.addConstraint(constraint); +void ConstraintGraph::addConstraint(Constraint *constraint) { + // Record the change, if there are active scopes. + if (ActiveScope) { + Changes.push_back(Change::addedConstraint(constraint)); } - // If the constraint doesn't reference any type variables, it's orphaned; - // track it as such. - if (referencedTypeVars.empty()) { + if (constraint->getTypeVariables().empty()) { + // A constraint that doesn't reference any type variables is orphaned; + // track it as such. OrphanedConstraints.push_back(constraint); + return; } - // Record the change, if there are active scopes. - if (ActiveScope) - Changes.push_back(Change::addedConstraint(constraint)); + // Record this constraint in each type variable. + for (auto typeVar : constraint->getTypeVariables()) { + (*this)[typeVar].addConstraint(constraint); + } + + // Record adjacencies. + enumerateAdjacencies(constraint, + [&](TypeVariableType *lhs, TypeVariableType *rhs) { + assert(lhs != rhs); + (*this)[lhs].addAdjacency(rhs); + }); } void ConstraintGraph::removeConstraint(Constraint *constraint) { - // For the nodes corresponding to each type variable... - auto referencedTypeVars = constraint->getTypeVariables(); - for (auto typeVar : referencedTypeVars) { - // Find the node for this type variable. - auto &node = (*this)[typeVar]; - - // Remove the constraint. - node.removeConstraint(constraint); - } + // Record the change, if there are active scopes. + if (ActiveScope) + Changes.push_back(Change::removedConstraint(constraint)); - // If this is an orphaned constraint, remove it from the list. - if (referencedTypeVars.empty()) { + if (constraint->getTypeVariables().empty()) { + // A constraint that doesn't reference any type variables is orphaned; + // remove it from the list of orphaned constraints. auto known = std::find(OrphanedConstraints.begin(), OrphanedConstraints.end(), constraint); assert(known != OrphanedConstraints.end() && "missing orphaned constraint"); *known = OrphanedConstraints.back(); OrphanedConstraints.pop_back(); + return; } - // Record the change, if there are active scopes. - if (ActiveScope) - Changes.push_back(Change::removedConstraint(constraint)); + // Remove the constraint from each type variable. + for (auto typeVar : constraint->getTypeVariables()) { + (*this)[typeVar].removeConstraint(constraint); + } + + // Remove all adjacencies for all type variables. + enumerateAdjacencies(constraint, + [&](TypeVariableType *lhs, TypeVariableType *rhs) { + assert(lhs != rhs); + (*this)[lhs].removeAdjacency(rhs); + }); } void ConstraintGraph::mergeNodes(TypeVariableType *typeVar1, @@ -1228,6 +1324,28 @@ void ConstraintGraphNode::print(llvm::raw_ostream &out, unsigned indent) { } } + if (!Adjacencies.empty()) { + out.indent(indent + 2); + out << "Adjacencies:"; + SmallVector sortedAdjacencies(Adjacencies.begin(), + Adjacencies.end()); + std::sort(sortedAdjacencies.begin(), sortedAdjacencies.end(), + [](TypeVariableType *lhs, TypeVariableType *rhs) { + return lhs->getID() < rhs->getID(); + }); + for (auto adj : sortedAdjacencies) { + out << ' '; + adj->print(out); + + auto &info = AdjacencyInfo[adj]; + auto degree = info.NumConstraints; + if (degree > 1) { + out << " (" << degree << ")"; + } + } + out << "\n"; + } + // Print fixed bindings. if (!FixedBindings.empty()) { out.indent(indent + 2); @@ -1396,6 +1514,67 @@ void ConstraintGraphNode::verify(ConstraintGraph &cg) { requireSameValue(info.first, Constraints[info.second], "constraint map provides wrong index into vector"); } + + // Verify that the adjacency map/vector haven't gotten out of sync. + requireSameValue(Adjacencies.size(), AdjacencyInfo.size(), + "adjacency vector and map have different sizes"); + for (auto info : AdjacencyInfo) { + require(info.second.Index < Adjacencies.size(), + "adjacency index out-of-range"); + requireSameValue(info.first, Adjacencies[info.second.Index], + "adjacency map provides wrong index into vector"); + require(!info.second.empty(), + "adjacency information should have been removed"); + require(info.second.NumConstraints <= Constraints.size(), + "adjacency information has higher degree than # of constraints"); + } + + // Based on the constraints we have, build up a representation of what + // we expect the adjacencies to look like. + llvm::DenseMap expectedAdjacencies; + for (auto constraint : Constraints) { + if (constraint->isOneWayConstraint()) + continue; + + for (auto adjTypeVar : constraint->getTypeVariables()) { + if (adjTypeVar == TypeVar) + continue; + + ++expectedAdjacencies[adjTypeVar]; + } + } + + // Make sure that the adjacencies we expect are the adjacencies we have. + for (auto adj : expectedAdjacencies) { + auto knownAdj = AdjacencyInfo.find(adj.first); + requireWithContext(knownAdj != AdjacencyInfo.end(), + "missing adjacency information for type variable", + [&] { + llvm::dbgs() << " type variable=" << adj.first->getString() << 'n'; + }); + + requireWithContext(adj.second == knownAdj->second.NumConstraints, + "wrong number of adjacencies for type variable", + [&] { + llvm::dbgs() << " type variable=" << adj.first->getString() + << " (" << adj.second << " vs. " + << knownAdj->second.NumConstraints + << ")\n"; + }); + } + + if (AdjacencyInfo.size() != expectedAdjacencies.size()) { + // The adjacency information has something extra in it. Find the + // extraneous type variable. + for (auto adj : AdjacencyInfo) { + requireWithContext(AdjacencyInfo.count(adj.first) > 0, + "extraneous adjacency info for type variable", + [&] { + llvm::dbgs() << " type variable=" << adj.first->getString() << '\n'; + }); + } + } + #undef requireSameValue #undef requireWithContext #undef require diff --git a/lib/Sema/ConstraintGraph.h b/lib/Sema/ConstraintGraph.h index bfceefa0afe06..59bc1833a7aa6 100644 --- a/lib/Sema/ConstraintGraph.h +++ b/lib/Sema/ConstraintGraph.h @@ -45,6 +45,20 @@ class ConstraintSystem; /// A single node in the constraint graph, which represents a type variable. class ConstraintGraphNode { + /// Describes information about an adjacency between two type variables. + struct Adjacency { + /// Index into the vector of adjacent type variables, \c Adjacencies. + unsigned Index; + + /// The number of constraints that link this type variable to the + /// enclosing node. + unsigned NumConstraints; + + bool empty() const { + return NumConstraints == 0; + } + }; + public: explicit ConstraintGraphNode(TypeVariableType *typeVar) : TypeVar(typeVar) { } @@ -60,6 +74,11 @@ class ConstraintGraphNode { /// various other nodes. ArrayRef getConstraints() const { return Constraints; } + /// Retrieve the set of type variables to which this node is adjacent. + ArrayRef getAdjacencies() const { + return Adjacencies; + } + /// Retrieve the set of type variables that are adjacent due to fixed /// bindings. ArrayRef getFixedBindings() const { @@ -84,6 +103,21 @@ class ConstraintGraphNode { /// remove the corresponding adjacencies. void removeConstraint(Constraint *constraint); + /// Retrieve adjacency information for the given type variable. + Adjacency &getAdjacency(TypeVariableType *typeVar); + + /// Modify the adjacency information for the given type variable + /// directly. If the adjacency becomes empty afterward, it will be + /// removed. + void modifyAdjacency(TypeVariableType *typeVar, + llvm::function_ref modify); + + /// Add an adjacency to the list of adjacencies. + void addAdjacency(TypeVariableType *typeVar); + + /// Remove an adjacency from the list of adjacencies. + void removeAdjacency(TypeVariableType *typeVar); + /// Add the given type variables to this node's equivalence class. void addToEquivalenceClass(ArrayRef typeVars); @@ -105,6 +139,14 @@ class ConstraintGraphNode { /// to the index within the vector of constraints. llvm::SmallDenseMap ConstraintIndex; + /// The set of adjacent type variables, in a stable order. + SmallVector Adjacencies; + + /// A mapping from each of the type variables adjacent to this + /// type variable to the index of the adjacency information in + /// \c Adjacencies. + llvm::SmallDenseMap AdjacencyInfo; + /// The set of type variables that occur within the fixed binding of /// this type variable. SmallVector FixedBindings; From 34a7515aee1fec428dfc9d3be8ef632df1fb175c Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Thu, 22 Aug 2019 10:02:01 -0700 Subject: [PATCH 2/2] [Constraint graph] Use adjacency info for constraint gathering. This reinstates the use of direct adjacency information when gathering constraints, effectively reverting 54bdd7b840721523f363e343d7b25997a8a332fe. Fixes the regression that commit caused, which is tracked by rdar://problem/54274245. --- lib/Sema/ConstraintGraph.cpp | 60 ++++++++++++++++++++------------- test/Constraints/bridging.swift | 5 +++ 2 files changed, 42 insertions(+), 23 deletions(-) diff --git a/lib/Sema/ConstraintGraph.cpp b/lib/Sema/ConstraintGraph.cpp index ff903083f71f7..85b28be0343c4 100644 --- a/lib/Sema/ConstraintGraph.cpp +++ b/lib/Sema/ConstraintGraph.cpp @@ -473,7 +473,6 @@ llvm::TinyPtrVector ConstraintGraph::gatherConstraints( TypeVariableType *typeVar, GatheringKind kind, llvm::function_ref acceptConstraintFn) { llvm::TinyPtrVector constraints; - // Whether we should consider this constraint at all. auto rep = CS.getRepresentative(typeVar); auto shouldConsiderConstraint = [&](Constraint *constraint) { @@ -505,19 +504,29 @@ llvm::TinyPtrVector ConstraintGraph::gatherConstraints( // Add constraints for the given adjacent type variable. llvm::SmallPtrSet typeVars; + + // Local function to add constraints llvm::SmallPtrSet visitedConstraints; - auto addAdjacentConstraints = [&](TypeVariableType *adjTypeVar) { - auto adjTypeVarsToVisit = - (*this)[CS.getRepresentative(adjTypeVar)].getEquivalenceClass(); + auto addConstraintsOfAdjacency = [&](TypeVariableType *adjTypeVar) { + ArrayRef adjTypeVarsToVisit; + switch (kind) { + case GatheringKind::EquivalenceClass: + adjTypeVarsToVisit = adjTypeVar; + break; + + case GatheringKind::AllMentions: + adjTypeVarsToVisit + = (*this)[CS.getRepresentative(adjTypeVar)].getEquivalenceClass(); + break; + } + for (auto adjTypeVarEquiv : adjTypeVarsToVisit) { if (!typeVars.insert(adjTypeVarEquiv).second) continue; for (auto constraint : (*this)[adjTypeVarEquiv].getConstraints()) { - if (!visitedConstraints.insert(constraint).second) - continue; - - if (acceptConstraint(constraint)) + if (visitedConstraints.insert(constraint).second && + acceptConstraint(constraint)) constraints.push_back(constraint); } } @@ -526,29 +535,34 @@ llvm::TinyPtrVector ConstraintGraph::gatherConstraints( auto &reprNode = (*this)[CS.getRepresentative(typeVar)]; auto equivClass = reprNode.getEquivalenceClass(); for (auto typeVar : equivClass) { - auto &node = (*this)[typeVar]; - for (auto constraint : node.getConstraints()) { + if (!typeVars.insert(typeVar).second) + continue; + + for (auto constraint : (*this)[typeVar].getConstraints()) { 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; + auto &node = (*this)[typeVar]; - for (auto adjTypeVar : constraint->getTypeVariables()) { - addAdjacentConstraints(adjTypeVar); - } - } + for (auto adjTypeVar : node.getFixedBindings()) { + addConstraintsOfAdjacency(adjTypeVar); } - // For any type variable mentioned in a fixed binding, add adjacent - // constraints. - for (auto adjTypeVar : node.getFixedBindings()) { - addAdjacentConstraints(adjTypeVar); + switch (kind) { + case GatheringKind::EquivalenceClass: + break; + + case GatheringKind::AllMentions: + // Retrieve the constraints from adjacent bindings. + for (auto adjTypeVar : node.getAdjacencies()) { + addConstraintsOfAdjacency(adjTypeVar); + } + + break; } + } return constraints; diff --git a/test/Constraints/bridging.swift b/test/Constraints/bridging.swift index 5c65f80eb53e3..904b7f6b7817e 100644 --- a/test/Constraints/bridging.swift +++ b/test/Constraints/bridging.swift @@ -374,3 +374,8 @@ func bridgeTupleToAnyObject() { let y = x as AnyObject _ = y } + +// Array defaulting and bridging type checking error per rdar://problem/54274245 +func rdar54274245(_ arr: [Any]?) { + _ = (arr ?? []) as [NSObject] +}