Merge pull request #18305 from DougGregor/constraint-solver-timer-perf

[Constraint solver] Reduce timer overhead associated with "expression too complex"

Author: DougGregor

lib/Sema/CSSolver.cpp

@@ -484,9 +484,6 @@ ConstraintSystem::SolverScope::SolverScope(ConstraintSystem &cs)
   numDefaultedConstraints = cs.DefaultedConstraints.size();
   numCheckedConformances = cs.CheckedConformances.size();
   PreviousScore = cs.CurrentScore;
-  if (cs.Timer) {
-    startTime = cs.Timer->getElapsedProcessTimeInFractionalSeconds();
-  }
 
   cs.solverState->registerScope(this);
   assert(!cs.failedConstraint && "Unexpected failed constraint!");
@@ -565,10 +562,6 @@ bool ConstraintSystem::tryTypeVariableBindings(
   auto &tc = getTypeChecker();
   ++solverState->NumTypeVariablesBound;
 
-  // If we've already explored a lot of potential solutions, bail.
-  if (getExpressionTooComplex(solutions))
-    return true;
-
   for (unsigned tryCount = 0; !anySolved && !bindings.empty(); ++tryCount) {
     // Try each of the bindings in turn.
     ++solverState->NumTypeVariableBindings;
@@ -1462,15 +1455,7 @@ ConstraintSystem::solveImpl(Expr *&expr,
 
   // If there are no solutions let's mark system as unsolved,
   // and solved otherwise even if there are multiple solutions still present.
-
-  // There was a Swift 3 bug that allowed us to return Solved if we
-  // had found at least one solution before deciding an expression was
-  // "too complex". Maintain that behavior, but for Swift > 3 return
-  // Unsolved in these cases.
-  auto tooComplex = getExpressionTooComplex(solutions);
-  auto unsolved = tooComplex || solutions.empty();
-
-  return unsolved ? SolutionKind::Unsolved : SolutionKind::Solved;
+  return solutions.empty() ? SolutionKind::Unsolved : SolutionKind::Solved;
 }
 
 bool ConstraintSystem::solve(Expr *const expr,
@@ -1501,8 +1486,8 @@ bool ConstraintSystem::solve(Expr *const expr,
   if (!retainAllSolutions())
     filterSolutions(solutions, state.ExprWeights);
 
-  // We fail if there is no solution.
-  return solutions.empty();
+  // We fail if there is no solution or the expression was too complex.
+  return solutions.empty() || getExpressionTooComplex(solutions);
 }
 
 bool ConstraintSystem::solveRec(SmallVectorImpl<Solution> &solutions,
@@ -2045,10 +2030,6 @@ bool ConstraintSystem::solveForDisjunctionChoices(
         break;
     }
 
-    // If the expression was deemed "too complex", stop now and salvage.
-    if (getExpressionTooComplex(solutions))
-      break;
-
     // Try to solve the system with this option in the disjunction.
     SolverScope scope(*this);
     ++solverState->NumDisjunctionTerms;
@@ -2119,6 +2100,10 @@ bool ConstraintSystem::solveSimplified(
 
   auto bestBindings = determineBestBindings();
 
+  // If we've already explored a lot of potential solutions, bail.
+  if (getExpressionTooComplex(solutions))
+    return true;
+
   // If we have a binding that does not involve type variables, and is
   // not fully bound, or we have no disjunction to attempt instead,
   // go ahead and try the bindings for this type variable.
@@ -2130,13 +2115,8 @@ bool ConstraintSystem::solveSimplified(
   }
 
   if (disjunction) {
-    bool foundSolution = solveForDisjunctionChoices(disjunction, solutions,
-                                                    allowFreeTypeVariables);
-
-    // If we are exiting due to an expression that is too complex, do
-    // not allow our caller to continue as if we have been successful.
-    auto tooComplex = getExpressionTooComplex(solutions);
-    return tooComplex || !foundSolution;
+    return !solveForDisjunctionChoices(disjunction, solutions,
+                                       allowFreeTypeVariables);
   }
 
   // If there are no disjunctions we can't solve this system unless we have

lib/Sema/ConstraintSystem.h

@@ -1453,9 +1453,6 @@ class ConstraintSystem {
     /// The scope number of this scope. Set when the scope is registered.
     unsigned scopeNumber = 0;
 
-    /// Time in fractional seconds at which we entered this scope.
-    double startTime;
-
     /// Constraint graph scope associated with this solver scope.
     ConstraintGraphScope CGScope;
 
@@ -1467,13 +1464,6 @@ class ConstraintSystem {
   public:
     explicit SolverScope(ConstraintSystem &cs);
     ~SolverScope();
-
-    Optional<double> getElapsedTimeInFractionalSeconds() {
-      if (!cs.Timer)
-        return None;
-
-      return cs.Timer->getElapsedProcessTimeInFractionalSeconds() - startTime;
-    }
   };
 
   ConstraintSystem(TypeChecker &tc, DeclContext *dc,

lib/Sema/DerivedConformanceEquatableHashable.cpp

@@ -665,7 +665,9 @@ deriveEquatable_eq(DerivedConformance &derived, Identifier generatedIdentifier,
 
 bool DerivedConformance::canDeriveEquatable(TypeChecker &tc, DeclContext *DC,
                                             NominalTypeDecl *type) {
-  auto equatableProto = tc.Context.getProtocol(KnownProtocolKind::Equatable);
+  ASTContext &ctx = DC->getASTContext();
+  auto equatableProto = ctx.getProtocol(KnownProtocolKind::Equatable);
+  if (!equatableProto) return false;
   return canDeriveConformance(tc, DC, type, equatableProto);
 }
 
@@ -1135,8 +1137,7 @@ getHashableConformance(Decl *parentDecl) {
   return nullptr;
 }
 
-bool DerivedConformance::canDeriveHashable(TypeChecker &tc,
-                                           NominalTypeDecl *type) {
+bool DerivedConformance::canDeriveHashable(NominalTypeDecl *type) {
   if (!isa<EnumDecl>(type) && !isa<StructDecl>(type) && !isa<ClassDecl>(type))
     return false;
   // FIXME: This is not actually correct. We cannot promise to always

lib/Sema/DerivedConformances.cpp

@@ -61,7 +61,7 @@ bool DerivedConformance::derivesProtocolConformance(TypeChecker &TC,
     // We can always complete a partial Hashable implementation, and we can
     // synthesize a full Hashable implementation for structs and enums with
     // Hashable components.
-    return canDeriveHashable(TC, Nominal);
+    return canDeriveHashable(Nominal);
   }
 
   if (auto *enumDecl = dyn_cast<EnumDecl>(Nominal)) {

lib/Sema/DerivedConformances.h

@@ -137,7 +137,7 @@ class DerivedConformance {
   /// associated values, and for structs with all-Hashable stored properties.
   ///
   /// \returns True if the requirement can be derived.
-  static bool canDeriveHashable(TypeChecker &tc, NominalTypeDecl *type);
+  static bool canDeriveHashable(NominalTypeDecl *type);
 
   /// Derive a Hashable requirement for a type.
   ///

lib/Sema/TypeCheckGeneric.cpp

@@ -1210,7 +1210,7 @@ RequirementCheckResult TypeChecker::checkGenericArguments(
   pendingReqs.push_back({requirements, {}});
 
   auto *env = dc->getGenericEnvironmentOfContext();
-
+  ASTContext &ctx = dc->getASTContext();
   while (!pendingReqs.empty()) {
     auto current = pendingReqs.pop_back_val();
 
@@ -1342,19 +1342,19 @@ RequirementCheckResult TypeChecker::checkGenericArguments(
 
       if (loc.isValid()) {
         // FIXME: Poor source-location information.
-        diagnose(loc, diagnostic, owner, firstType, secondType);
+        ctx.Diags.diagnose(loc, diagnostic, owner, firstType, secondType);
 
         std::string genericParamBindingsText;
         if (!genericParams.empty()) {
           genericParamBindingsText =
             gatherGenericParamBindingsText(
               {rawFirstType, rawSecondType}, genericParams, substitutions);
         }
-        diagnose(noteLoc, diagnosticNote, rawFirstType, rawSecondType,
-                 genericParamBindingsText);
+        ctx.Diags.diagnose(noteLoc, diagnosticNote, rawFirstType, rawSecondType,
+                           genericParamBindingsText);
 
-        ParentConditionalConformance::diagnoseConformanceStack(Diags, noteLoc,
-                                                               current.Parents);
+        ParentConditionalConformance::diagnoseConformanceStack(
+            ctx.Diags, noteLoc, current.Parents);
       }
 
       return RequirementCheckResult::Failure;

lib/Sema/TypeCheckProtocol.cpp

@@ -3663,26 +3663,29 @@ static void diagnoseConformanceFailure(TypeChecker &TC, Type T,
   if (T->hasError())
     return;
 
+  ASTContext &ctx = DC->getASTContext();
+  auto &diags = ctx.Diags;
+
   // If we're checking conformance of an existential type to a protocol,
   // do a little bit of extra work to produce a better diagnostic.
   if (T->isExistentialType() &&
       TC.containsProtocol(T, Proto, DC, None)) {
 
     if (!T->isObjCExistentialType()) {
-      TC.diagnose(ComplainLoc, diag::protocol_does_not_conform_objc,
-                  T, Proto->getDeclaredType());
+      diags.diagnose(ComplainLoc, diag::protocol_does_not_conform_objc,
+                     T, Proto->getDeclaredType());
       return;
     }
 
-    TC.diagnose(ComplainLoc, diag::protocol_does_not_conform_static,
-                T, Proto->getDeclaredType());
+    diags.diagnose(ComplainLoc, diag::protocol_does_not_conform_static,
+                   T, Proto->getDeclaredType());
     return;
   }
 
   // Special case: diagnose conversion to ExpressibleByNilLiteral, since we
   // know this is something involving 'nil'.
   if (Proto->isSpecificProtocol(KnownProtocolKind::ExpressibleByNilLiteral)) {
-    TC.diagnose(ComplainLoc, diag::cannot_use_nil_with_this_type, T);
+    diags.diagnose(ComplainLoc, diag::cannot_use_nil_with_this_type, T);
     return;
   }
 
@@ -3697,29 +3700,28 @@ static void diagnoseConformanceFailure(TypeChecker &TC, Type T,
 
       auto rawType = enumDecl->getRawType();
 
-      TC.diagnose(enumDecl->getInherited()[0].getSourceRange().Start,
-                  diag::enum_raw_type_nonconforming_and_nonsynthable,
-                  T, rawType);
+      diags.diagnose(enumDecl->getInherited()[0].getSourceRange().Start,
+                     diag::enum_raw_type_nonconforming_and_nonsynthable,
+                     T, rawType);
 
       // If the reason is that the raw type does not conform to
       // Equatable, say so.
-      auto equatableProto = TC.getProtocol(enumDecl->getLoc(),
-                                           KnownProtocolKind::Equatable);
+      auto equatableProto = ctx.getProtocol(KnownProtocolKind::Equatable);
       if (!equatableProto)
         return;
 
       if (!TC.conformsToProtocol(rawType, equatableProto, enumDecl, None)) {
         SourceLoc loc = enumDecl->getInherited()[0].getSourceRange().Start;
-        TC.diagnose(loc, diag::enum_raw_type_not_equatable, rawType);
+        diags.diagnose(loc, diag::enum_raw_type_not_equatable, rawType);
         return;
       }
 
       return;
     }
   }
 
-  TC.diagnose(ComplainLoc, diag::type_does_not_conform,
-              T, Proto->getDeclaredType());
+  diags.diagnose(ComplainLoc, diag::type_does_not_conform,
+                 T, Proto->getDeclaredType());
 }
 
 void ConformanceChecker::diagnoseOrDefer(

stdlib/public/core/Hasher.swift

@@ -90,7 +90,10 @@ internal struct _HasherTailBuffer {
   internal init(tail: UInt64, byteCount: UInt64) {
     // byteCount can be any value, but we only keep the lower 8 bits.  (The
     // lower three bits specify the count of bytes stored in this buffer.)
-    _sanityCheck(tail & ~(1 << ((byteCount & 7) << 3) - 1) == 0)
+    // FIXME: The "as UInt64" annotations here eliminate some exponential
+    // behavior in the expression type checker <rdar://problem/42672946>.
+    _sanityCheck(tail & ~((1 as UInt64) << ((byteCount & (7 as UInt64))
+          << (3 as UInt64)) - (1 as UInt64)) == (0 as UInt64))
     self.value = (byteCount &<< 56 | tail)
   }
 

validation-test/Sema/type_checker_perf/slow/rdar42672946.swift

@@ -0,0 +1,5 @@
+// RUN: %target-typecheck-verify-swift -solver-expression-time-threshold=1
+func f(tail: UInt64, byteCount: UInt64) {
+  if tail & ~(1 << ((byteCount & 7) << 3) - 1) == 0 { }
+  // expected-error@-1 {{reasonable time}}
+}