[ConstraintSystem] Record holes in the constraint system using a new flag in

`TypeVariableOptions` rather than using a separate data structure in the
constraint system.

Author: hborla

include/swift/AST/Types.h

@@ -347,12 +347,10 @@ class alignas(1 << TypeAlignInBits) TypeBase {
     NumProtocols : 16
   );
 
-  SWIFT_INLINE_BITFIELD_FULL(TypeVariableType, TypeBase, 4+32,
-    : NumPadBits,
-
+  SWIFT_INLINE_BITFIELD_FULL(TypeVariableType, TypeBase, 5+32,
     /// Type variable options.
-    Options : 4,
-
+    Options : 5,
+    : NumPadBits,
     /// The unique number assigned to this type variable.
     ID : 32
   );

lib/Sema/CSBindings.cpp

@@ -734,10 +734,9 @@ ConstraintSystem::getPotentialBindings(TypeVariableType *typeVar) const {
                                 constraint->getLocator()});
   }
 
-  // If we don't have any potential bindings, allow generic
-  // parameters and potential holes to default to `Unresolved`.
+  // If there are no bindings, typeVar may be a hole.
   if (shouldAttemptFixes() && result.Bindings.empty() &&
-      (isPotentialHole(typeVar) || result.isGenericParameter())) {
+      typeVar->getImpl().canBindToHole()) {
     result.IsHole = true;
     result.addPotentialBinding({getASTContext().TheUnresolvedType,
         AllowedBindingKind::Exact, ConstraintKind::Defaultable, nullptr,

lib/Sema/CSDiagnostics.cpp

@@ -3161,6 +3161,17 @@ bool MissingMemberFailure::diagnoseAsError() {
     if (baseType->is<TupleType>())
       diagnostic = diag::could_not_find_tuple_member;
 
+    bool hasUnresolvedPattern = false;
+    anchor->forEachChildExpr([&](Expr *expr) {
+      hasUnresolvedPattern |= isa<UnresolvedPatternExpr>(expr);
+      return hasUnresolvedPattern ? nullptr : expr;
+    });
+    if (hasUnresolvedPattern && !baseType->getAs<EnumType>()) {
+      emitDiagnostic(anchor->getLoc(),
+          diag::cannot_match_unresolved_expr_pattern_with_value, baseType);
+      return;
+    }
+
     emitDiagnostic(anchor->getLoc(), diagnostic, baseType, getName())
         .highlight(baseExpr->getSourceRange())
         .highlight(nameLoc.getSourceRange());

lib/Sema/CSSimplify.cpp

@@ -867,9 +867,7 @@ class ArgumentFailureTracker : public MatchCallArgumentListener {
 
     auto *argType =
         CS.createTypeVariable(argLoc, TVO_CanBindToInOut | TVO_CanBindToLValue |
-                                          TVO_CanBindToNoEscape);
-
-    CS.recordPotentialHole(argType);
+                                      TVO_CanBindToNoEscape | TVO_CanBindToHole);
 
     Arguments.push_back(param.withType(argType));
     ++NumSynthesizedArgs;
@@ -4954,6 +4952,12 @@ ConstraintSystem::simplifyFunctionComponentConstraint(
       // Track how many times we do this so that we can record a fix for each.
       ++unwrapCount;
     }
+
+    if (simplified->isHole()) {
+      if (auto *typeVar = second->getAs<TypeVariableType>())
+        recordPotentialHole(typeVar);
+      return SolutionKind::Solved;
+    }
   }
 
   if (simplified->isTypeVariableOrMember()) {
@@ -6743,17 +6747,21 @@ ConstraintSystem::simplifyKeyPathConstraint(
         }
 
         if (shouldAttemptFixes()) {
-          auto *componentLoc = getConstraintLocator(
-              keyPath, {LocatorPathElt::KeyPathComponent(i),
-                        LocatorPathElt::KeyPathComponentResult()});
+          auto typeVar =
+              llvm::find_if(componentTypeVars, [&](TypeVariableType *typeVar) {
+                auto *locator = typeVar->getImpl().getLocator();
+                auto elt = locator->findLast<LocatorPathElt::KeyPathComponent>();
+                return elt && elt->getIndex() == i;
+              });
 
           // If one of the components haven't been resolved, let's check
           // whether it has been determined to be a "hole" and if so,
           // let's allow component validation to contiunue.
           //
           // This helps to, for example, diagnose problems with missing
           // members used as part of a key path.
-          if (isPotentialHoleAt(componentLoc)) {
+          if (typeVar != componentTypeVars.end() &&
+              (*typeVar)->getImpl().canBindToHole()) {
             anyComponentsUnresolved = true;
             capability = ReadOnly;
             continue;
@@ -7152,25 +7160,6 @@ ConstraintSystem::simplifyApplicableFnConstraint(
   // following: $T1 -> $T2.
   auto func1 = type1->castTo<FunctionType>();
 
-  // Let's check if this member couldn't be found and is fixed
-  // to exist based on its usage.
-  if (auto *memberTy = type2->getAs<TypeVariableType>()) {
-    if (isPotentialHole(memberTy)) {
-      auto *funcTy = type1->castTo<FunctionType>();
-      auto *locator = memberTy->getImpl().getLocator();
-      // Bind type variable associated with member to a type of argument
-      // application, which makes it seem like member exists with the
-      // types of the parameters matching argument types exactly.
-      addConstraint(ConstraintKind::Bind, memberTy, funcTy, locator);
-      // There might be no contextual type for result of the application,
-      // in cases like `let _ = x.foo()`, so let's record a potential hole.
-      auto resultTy = funcTy->getResult();
-      if (auto *typeVar = resultTy->getAs<TypeVariableType>())
-        recordPotentialHole(typeVar);
-      return SolutionKind::Solved;
-    }
-  }
-
   // Before stripping lvalue-ness and optional types, save the original second
   // type for handling `func callAsFunction` and `@dynamicCallable`
   // applications. This supports the following cases:
@@ -8184,7 +8173,7 @@ bool ConstraintSystem::recordFix(ConstraintFix *fix, unsigned impact) {
 
 void ConstraintSystem::recordPotentialHole(TypeVariableType *typeVar) {
   assert(typeVar);
-  Holes.insert(typeVar->getImpl().getLocator());
+  typeVar->getImpl().enableCanBindToHole(getSavedBindings());
 }
 
 ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(

lib/Sema/CSSolver.cpp

@@ -436,7 +436,6 @@ ConstraintSystem::SolverScope::SolverScope(ConstraintSystem &cs)
   numSavedBindings = cs.solverState->savedBindings.size();
   numConstraintRestrictions = cs.ConstraintRestrictions.size();
   numFixes = cs.Fixes.size();
-  numHoles = cs.Holes.size();
   numFixedRequirements = cs.FixedRequirements.size();
   numDisjunctionChoices = cs.DisjunctionChoices.size();
   numOpenedTypes = cs.OpenedTypes.size();
@@ -484,9 +483,6 @@ ConstraintSystem::SolverScope::~SolverScope() {
   // Remove any fixes.
   truncate(cs.Fixes, numFixes);
 
-  // Remove any holes encountered along the current path.
-  truncate(cs.Holes, numHoles);
-
   // Remove any disjunction choices.
   truncate(cs.DisjunctionChoices, numDisjunctionChoices);
 

lib/Sema/ConstraintSystem.cpp

@@ -1120,7 +1120,8 @@ void ConstraintSystem::openGenericParameters(DeclContext *outerDC,
     auto *paramLocator = getConstraintLocator(
         locator.withPathElement(LocatorPathElt::GenericParameter(gp)));
 
-    auto typeVar = createTypeVariable(paramLocator, TVO_PrefersSubtypeBinding);
+    auto typeVar = createTypeVariable(paramLocator, TVO_PrefersSubtypeBinding |
+                                                    TVO_CanBindToHole);
     auto result = replacements.insert(std::make_pair(
         cast<GenericTypeParamType>(gp->getCanonicalType()), typeVar));
 

lib/Sema/ConstraintSystem.h

@@ -71,9 +71,12 @@ namespace constraints {
 /// A handle that holds the saved state of a type variable, which
 /// can be restored.
 class SavedTypeVariableBinding {
-  /// The type variable and type variable options.
-  llvm::PointerIntPair<TypeVariableType *, 4> TypeVarAndOptions;
-  
+  /// The type variable that we saved the state of.
+  TypeVariableType *TypeVar;
+
+  /// The saved type variable options.
+  unsigned Options;
+
   /// The parent or fixed type.
   llvm::PointerUnion<TypeVariableType *, TypeBase *> ParentOrFixed;
 
@@ -82,9 +85,6 @@ class SavedTypeVariableBinding {
 
   /// Restore the state of the type variable to the saved state.
   void restore();
-
-  TypeVariableType *getTypeVariable() { return TypeVarAndOptions.getPointer(); }
-  unsigned getOptions() { return TypeVarAndOptions.getInt(); }
 };
 
 /// A set of saved type variable bindings.
@@ -169,9 +169,12 @@ enum TypeVariableOptions {
   /// Whether the type variable can be bound to a non-escaping type or not.
   TVO_CanBindToNoEscape = 0x04,
 
+  /// Whether the type variable can be bound to a hole type or not.
+  TVO_CanBindToHole = 0x08,
+
   /// Whether a more specific deduction for this type variable implies a
   /// better solution to the constraint system.
-  TVO_PrefersSubtypeBinding = 0x08,
+  TVO_PrefersSubtypeBinding = 0x10,
 };
 
 /// The implementation object for a type variable used within the
@@ -238,6 +241,9 @@ class TypeVariableType::Implementation {
   /// Whether this type variable can bind to an inout type.
   bool canBindToNoEscape() const { return getRawOptions() & TVO_CanBindToNoEscape; }
 
+  /// Whether this type variable can bind to a hole type.
+  bool canBindToHole() const { return getRawOptions() & TVO_CanBindToHole; }
+
   /// Whether this type variable prefers a subtype binding over a supertype
   /// binding.
   bool prefersSubtypeBinding() const {
@@ -427,6 +433,14 @@ class TypeVariableType::Implementation {
           ~TVO_CanBindToNoEscape;
   }
 
+  void enableCanBindToHole(constraints::SavedTypeVariableBindings *record) {
+    auto &impl = getRepresentative(record)->getImpl();
+    if (record)
+      impl.recordBinding(*record);
+
+    impl.getTypeVariable()->Bits.TypeVariableType.Options |= TVO_CanBindToHole;
+  }
+
   /// Print the type variable to the given output stream.
   void print(llvm::raw_ostream &OS);
 };
@@ -1107,13 +1121,6 @@ class ConstraintSystem {
   /// The set of fixes applied to make the solution work.
   llvm::SmallVector<ConstraintFix *, 4> Fixes;
 
-  /// The set of "holes" in the constraint system encountered
-  /// along the current path identified by locator. A "hole" is
-  /// a type variable which type couldn't be determined due to
-  /// an inference failure e.g. missing member, ambiguous generic
-  /// parameter which hasn't been explicitly specified.
-  llvm::SmallSetVector<ConstraintLocator *, 4> Holes;
-
   /// The set of remembered disjunction choices used to reach
   /// the current constraint system.
   std::vector<std::pair<ConstraintLocator*, unsigned>>
@@ -1608,9 +1615,6 @@ class ConstraintSystem {
     /// The length of \c Fixes.
     unsigned numFixes;
 
-    /// The length of \c Holes.
-    unsigned numHoles;
-
     /// The length of \c FixedRequirements.
     unsigned numFixedRequirements;
 
@@ -2044,14 +2048,6 @@ class ConstraintSystem {
 
   void recordPotentialHole(TypeVariableType *typeVar);
 
-  bool isPotentialHole(TypeVariableType *typeVar) const {
-    return isPotentialHoleAt(typeVar->getImpl().getLocator());
-  }
-
-  bool isPotentialHoleAt(ConstraintLocator *locator) const {
-    return bool(Holes.count(locator));
-  }
-
   /// Determine whether constraint system already has a fix recorded
   /// for a particular location.
   bool hasFixFor(ConstraintLocator *locator,

lib/Sema/TypeCheckConstraints.cpp

@@ -68,13 +68,12 @@ void TypeVariableType::Implementation::print(llvm::raw_ostream &OS) {
 }
 
 SavedTypeVariableBinding::SavedTypeVariableBinding(TypeVariableType *typeVar)
-  : TypeVarAndOptions(typeVar, typeVar->getImpl().getRawOptions()),
+  : TypeVar(typeVar), Options(typeVar->getImpl().getRawOptions()),
     ParentOrFixed(typeVar->getImpl().ParentOrFixed) { }
 
 void SavedTypeVariableBinding::restore() {
-  auto *typeVar = getTypeVariable();
-  typeVar->getImpl().setRawOptions(getOptions());
-  typeVar->getImpl().ParentOrFixed = ParentOrFixed;
+  TypeVar->getImpl().setRawOptions(Options);
+  TypeVar->getImpl().ParentOrFixed = ParentOrFixed;
 }
 
 GenericTypeParamType *

test/decl/init/nonnominal_init.swift

@@ -8,9 +8,12 @@ indirect enum Or<A, B> {
 }
 
 func deMorgan<A, B>(_ ne: Not<Or<A, B>>) -> And<Not<A>, Not<B>> {
+  // FIXME(diagnostics): The error message about initialization here is confusing
   return And<Not<A>, Not<B>>(
-    Not<A> { a in ne(.left(a)) }, // expected-error {{non-nominal type 'Not<A>' (aka '(A) -> Never') does not support explicit initialization}}
-    Not<B> { a in ne(.right(a)) }
+    Not<A> { a in ne(.left(a)) }, // expected-error {{type 'Not<A>' (aka '(A) -> Never') has no member 'init'}}
+    // expected-error@-1 {{type 'Or<A, B>' has no member 'left'}}
+    Not<B> { a in ne(.right(a)) } // expected-error {{type 'Not<B>' (aka '(B) -> Never') has no member 'init'}}
+    // expected-error@-1 {{type 'Or<A, B>' has no member 'right'}}
   )
 }
 

test/expr/delayed-ident/static_var.swift

@@ -50,4 +50,5 @@ var _: HasClosure = .factoryOpt(3)
 // expected-error@-1 {{value of optional type '((Int) -> HasClosure)?' must be unwrapped to a value of type '(Int) -> HasClosure'}}
 // expected-note@-2 {{coalesce}}
 // expected-note@-3 {{force-unwrap}}
-var _: HasClosure = .factoryOpt!(4) // expected-error {{type '((Int) -> HasClosure)?' has no member 'factoryOpt'}}
+// FIXME: we should accept this
+var _: HasClosure = .factoryOpt!(4) // expected-error {{type 'Optional<_>' has no member 'factoryOpt'}}