[WIP] [TypeChecker] Score disjunctions and constraint components to prune search space #9220

xedin · 2017-05-03T07:58:15Z

Before trying to solve constraint system components:

Score constraint components/buckets based on how many disjunctions
they have, that helps to prune some of the branches with incorrect
solutions early which limits overall depth of the search.
In each of the constraint component score disjunctions
based on how much information do we have about them, the
more information we have the easier it is to prune search
space of incorrect branches earlier.
When dealing with explicit coercions apply their conversions
early to avoid searching for solutions with incorrect types.

xedin · 2017-05-03T07:59:45Z

@swift-ci please test

xedin · 2017-05-03T08:20:52Z

@swift-ci Please test source compatibility

xedin · 2017-05-03T16:09:55Z

@swift-ci Please test source compatibility

DougGregor

Interesting approach!

DougGregor · 2017-05-11T05:24:11Z

lib/Sema/CSSolver.cpp

+      typeVar, constraints, ConstraintGraph::GatheringKind::EquivalenceClass);
+
+  for (auto constraint : constraints) {
+    if (constraint->getKind() != ConstraintKind::Conversion)


There are other constraints like SubType constraints that have similar properties. I suggest switching on constraint->getKind() so it's clear that we've thought about all of these.

Thanks, will do!

DougGregor · 2017-05-11T05:34:32Z

lib/Sema/CSSolver.cpp

+    // that helps to avoid invalid disjunction choices.
+    if (LHS->isEqual(typeVar) && !RHS->hasUnresolvedType()) {
+      // No type variables, means pretty good chance that
+      // this is conversion to some concerete type, which


Typo "concerete".

xedin · 2017-06-18T00:22:22Z

@DougGregor @rudkx I've rebased everything with the latest master and made conversion propagation a part of DisjunctionChoice solving, and addressed both of the comments.

xedin · 2017-06-18T00:22:34Z

@swift-ci please test

xedin · 2017-06-18T00:50:35Z

Looks like one of the expressions is no longer too complex :)

xedin · 2017-06-18T00:50:41Z

@swift-ci please test

xedin · 2017-06-18T01:52:07Z

@swift-ci please test source compatibility

DougGregor · 2017-06-19T16:53:35Z

lib/Sema/CSSolver.cpp

+  for (unsigned component = 0; component != numComponents; ++component)
+    componentOrdering.push_back(component);
+  // Now sort the components in the ordering based on the scores of the buckets.
+  std::sort(


You could sort the buckets directly, so you don't have to separately maintain the componentOrdering. std::sort will std::move the ConstraintBucket instances, so it shouldn't be any more expensive.

I've separated ordered set from the original one because components is a vector and some of the code down the line relies on the original indexes the way they are produced by constraint graph e.g. associating constraints into their buckets and orphan handling logic. I think that should be fixed down the road but I just didn't want to tackle it right away...

DougGregor · 2017-06-19T17:00:24Z

lib/Sema/CSSolver.cpp

+  };
+
+  auto evaluateDisjunction = [&](Constraint *contender, float score) {
+    // Score has to be strictly greather than best because that would


typo "greather"

DougGregor · 2017-06-19T17:00:31Z

lib/Sema/CSSolver.cpp

+  auto evaluateDisjunction = [&](Constraint *contender, float score) {
+    // Score has to be strictly greather than best because that would
+    // allow us to apply disjunctions in their semantic order, which gives
+    // better changes of avoiding of the the obviously wrong choices with


DougGregor · 2017-06-19T17:04:32Z

lib/Sema/CSSolver.cpp

+    // everything else being equal.
+    if (score > bestScore ||
+        (score == bestScore &&
+         getDisjunctionId(bestDisjunction) > getDisjunctionId(contender))) {


Rather than doing a bit of work to dig out type variable IDs to compare (which isn't always going to give an ordering), why not keep track of the index of the best option within the disjunction? It'll be more predictable and eliminate the need for getDisjunctionId.

DougGregor · 2017-06-19T17:06:56Z

lib/Sema/ConstraintSystem.h

@@ -1900,6 +1901,10 @@ class ConstraintSystem {
  /// reads the type from the ConstraintSystem expression type map.
  Type getResultType(const AbstractClosureExpr *E);

+  /// Determine if the given constraint represents explicit conversion,
+  /// e.g. coercion constraint "as X" which forms a disjunction.
+  bool isExplicitConversionConstraint(Constraint *constraint);


I'd love to have "disjunction" in the name, too.

Sounds good, thanks!

DougGregor · 2017-06-19T17:07:53Z

lib/Sema/ConstraintSystem.h

@@ -2728,6 +2748,48 @@ class DisjunctionChoice {
    return decl->isOperator() ? decl : nullptr;
  }
 };
+
+class ConstraintBucket {


I'm very glad to have this functionality split out into a separate class, but please document what this class does via comments.

xedin · 2017-06-24T01:00:23Z

@swift-ci please test source compatibility

xedin · 2017-06-30T08:20:16Z

@swift-ci please test

xedin · 2017-06-30T08:21:14Z

@swift-ci please test source compatibility

xedin · 2017-07-14T22:24:43Z

@swift-ci please smoke test

xedin · 2017-07-17T22:53:43Z