Merge pull request swiftlang#28243 from hborla/missing-generic-args-refactoring

[ConstraintSystem] Missing generic args/hole refactoring

Author: hborla

include/swift/AST/Types.h

@@ -177,7 +177,7 @@ class RecursiveTypeProperties {
 
   /// Does a type with these properties have an unresolved type somewhere in it?
   bool hasUnresolvedType() const { return Bits & HasUnresolvedType; }
-  
+
   /// Is a type with these properties an lvalue?
   bool isLValue() const { return Bits & IsLValue; }
 
@@ -349,12 +349,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
   );
@@ -565,7 +563,12 @@ class alignas(1 << TypeAlignInBits) TypeBase {
   bool hasUnresolvedType() const {
     return getRecursiveProperties().hasUnresolvedType();
   }
-  
+
+  /// Determine whether this type involves a hole.
+  bool hasHole() const {
+    return getRecursiveProperties().hasUnresolvedType();
+  }
+
   /// Determine whether the type involves a context-dependent archetype.
   bool hasArchetype() const {
     return getRecursiveProperties().hasArchetype();

lib/Sema/CSApply.cpp

@@ -7367,23 +7367,25 @@ bool ConstraintSystem::applySolutionFixes(Expr *E, const Solution &solution) {
         return;
 
       // Coalesce fixes with the same locator to avoid duplicating notes.
+      using ConstraintFixVector = llvm::SmallVector<ConstraintFix *, 4>;
       llvm::SmallMapVector<ConstraintLocator *,
-          llvm::SmallVector<ConstraintFix *, 4>, 4> aggregatedFixes;
+          llvm::SmallMapVector<FixKind, ConstraintFixVector, 4>, 4> aggregatedFixes;
       for (auto *fix : fixes->second)
-        aggregatedFixes[fix->getLocator()].push_back(fix);
+        aggregatedFixes[fix->getLocator()][fix->getKind()].push_back(fix);
 
       for (auto fixesPerLocator : aggregatedFixes) {
-        auto fixes = fixesPerLocator.second;
-        auto *primaryFix = fixes[0];
-        ArrayRef<ConstraintFix *> secondaryFixes{fixes.begin() + 1, fixes.end()};
-
-        auto *coalescedFix = primaryFix->coalescedWith(secondaryFixes);
-        auto diagnosed = coalescedFix->diagnose();
-        if (coalescedFix->isWarning()) {
-          assert(diagnosed && "warnings should always be diagnosed");
-          (void)diagnosed;
-        } else {
-          DiagnosedAnyErrors |= diagnosed;
+        for (auto fixesPerKind : fixesPerLocator.second) {
+          auto fixes = fixesPerKind.second;
+          auto *primaryFix = fixes[0];
+          ArrayRef<ConstraintFix *> secondaryFixes{fixes.begin() + 1, fixes.end()};
+
+          auto diagnosed = primaryFix->coalesceAndDiagnose(secondaryFixes);
+          if (primaryFix->isWarning()) {
+            assert(diagnosed && "warnings should always be diagnosed");
+            (void)diagnosed;
+          } else {
+            DiagnosedAnyErrors |= diagnosed;
+          }
         }
       }
     }

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,
@@ -991,11 +990,11 @@ bool TypeVariableBinding::attempt(ConstraintSystem &cs) const {
     cs.DefaultedConstraints.push_back(Binding.DefaultableBinding);
 
     if (locator->isForGenericParameter() && type->isHole()) {
-      // Drop `generic parameter '...'` part of the locator to group all of the
-      // missing generic parameters related to the same path together.
+      // Drop `generic parameter` locator element so that all missing
+      // generic parameters related to the same path can be coalesced later.
       auto path = locator->getPath();
       auto genericParam = locator->getGenericParameter();
-      auto *fix = ExplicitlySpecifyGenericArguments::create(cs, {genericParam},
+      auto *fix = DefaultGenericArgument::create(cs, genericParam,
           cs.getConstraintLocator(locator->getAnchor(), path.drop_back()));
       if (cs.recordFix(fix))
         return true;

lib/Sema/CSDiagnostics.cpp

@@ -3172,6 +3172,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/CSFix.cpp

@@ -702,39 +702,28 @@ CollectionElementContextualMismatch::create(ConstraintSystem &cs, Type srcType,
       CollectionElementContextualMismatch(cs, srcType, dstType, locator);
 }
 
-bool ExplicitlySpecifyGenericArguments::diagnose(bool asNote) const {
-  auto &cs = getConstraintSystem();
-  MissingGenericArgumentsFailure failure(cs, getParameters(),
-                                         getLocator());
-  return failure.diagnose(asNote);
-}
+bool DefaultGenericArgument::coalesceAndDiagnose(
+    ArrayRef<ConstraintFix *> fixes, bool asNote) const {
+  llvm::SmallVector<GenericTypeParamType *, 4> missingParams{Param};
 
-ConstraintFix *
-ExplicitlySpecifyGenericArguments::coalescedWith(ArrayRef<ConstraintFix *> fixes) {
-  if (fixes.empty())
-    return this;
-
-  auto params = getParameters();
-  llvm::SmallVector<GenericTypeParamType *, 4> missingParams{params.begin(),
-                                                             params.end()};
   for (auto *otherFix : fixes) {
-    if (auto *fix = otherFix->getAs<ExplicitlySpecifyGenericArguments>()) {
-      auto additionalParams = fix->getParameters();
-      missingParams.append(additionalParams.begin(), additionalParams.end());
-    }
+    if (auto *fix = otherFix->getAs<DefaultGenericArgument>())
+      missingParams.push_back(fix->Param);
   }
 
-  return ExplicitlySpecifyGenericArguments::create(getConstraintSystem(),
-                                                   missingParams, getLocator());
+  auto &cs = getConstraintSystem();
+  MissingGenericArgumentsFailure failure(cs, missingParams, getLocator());
+  return failure.diagnose(asNote);
 }
 
-ExplicitlySpecifyGenericArguments *ExplicitlySpecifyGenericArguments::create(
-    ConstraintSystem &cs, ArrayRef<GenericTypeParamType *> params,
-    ConstraintLocator *locator) {
-  unsigned size = totalSizeToAlloc<GenericTypeParamType *>(params.size());
-  void *mem = cs.getAllocator().Allocate(
-      size, alignof(ExplicitlySpecifyGenericArguments));
-  return new (mem) ExplicitlySpecifyGenericArguments(cs, params, locator);
+bool DefaultGenericArgument::diagnose(bool asNote) const {
+  return coalesceAndDiagnose({}, asNote);
+}
+
+DefaultGenericArgument *
+DefaultGenericArgument::create(ConstraintSystem &cs, GenericTypeParamType *param,
+                               ConstraintLocator *locator) {
+  return new (cs.getAllocator()) DefaultGenericArgument(cs, param, locator);
 }
 
 SkipUnhandledConstructInFunctionBuilder *

lib/Sema/CSFix.h

@@ -47,9 +47,11 @@ class Solution;
 
 /// Describes the kind of fix to apply to the given constraint before
 /// visiting it.
+///
+/// Note: values 0 and 1 are reserved for empty and tombstone kinds.
 enum class FixKind : uint8_t {
   /// Introduce a '!' to force an optional unwrap.
-  ForceOptional,
+  ForceOptional = 2,
 
   /// Unwrap an optional base when we have a member access.
   UnwrapOptionalBase,
@@ -181,10 +183,8 @@ enum class FixKind : uint8_t {
   /// function to `Void` to conform to expected result type.
   RemoveReturn,
 
-  /// Generic parameters could not be inferred and have to be explicitly
-  /// specified in the source. This fix groups all of the missing arguments
-  /// associated with single declaration.
-  ExplicitlySpecifyGenericArguments,
+  /// Default ambiguous generic arguments to \c Any
+  DefaultGenericArgument,
 
   /// Skip any unhandled constructs that occur within a closure argument that
   /// matches up with a
@@ -252,14 +252,18 @@ class ConstraintFix {
 
   virtual std::string getName() const = 0;
 
-  /// Diagnose a failure associated with this fix given
-  /// root expression and information from constraint system.
-  virtual bool diagnose(bool asNote = false) const = 0;
-
-  virtual ConstraintFix *coalescedWith(ArrayRef<ConstraintFix *> fixes) {
-    return this;
+  /// Coalesce this fix with the given secondary fixes and diagnose the failure.
+  ///
+  /// The default implementation ignores \c secondaryFixes and calls
+  /// \c diagnose.
+  virtual bool coalesceAndDiagnose(ArrayRef<ConstraintFix *> secondaryFixes,
+                                   bool asNote = false) const {
+    return diagnose(asNote);
   }
 
+  /// Diagnose a failure associated with this fix.
+  virtual bool diagnose(bool asNote = false) const = 0;
+
   void print(llvm::raw_ostream &Out) const;
 
   SWIFT_DEBUG_DUMP;
@@ -1249,49 +1253,32 @@ class CollectionElementContextualMismatch final : public ContextualMismatch {
          ConstraintLocator *locator);
 };
 
-class ExplicitlySpecifyGenericArguments final
-    : public ConstraintFix,
-      private llvm::TrailingObjects<ExplicitlySpecifyGenericArguments,
-                                    GenericTypeParamType *> {
-  friend TrailingObjects;
-
-  unsigned NumMissingParams;
+class DefaultGenericArgument final : public ConstraintFix {
+  GenericTypeParamType *Param;
 
-  ExplicitlySpecifyGenericArguments(ConstraintSystem &cs,
-                                    ArrayRef<GenericTypeParamType *> params,
-                                    ConstraintLocator *locator)
-      : ConstraintFix(cs, FixKind::ExplicitlySpecifyGenericArguments, locator),
-        NumMissingParams(params.size()) {
-    assert(!params.empty());
-    std::uninitialized_copy(params.begin(), params.end(),
-                            getParametersBuf().begin());
-  }
+  DefaultGenericArgument(ConstraintSystem &cs, GenericTypeParamType *param,
+                         ConstraintLocator *locator)
+      : ConstraintFix(cs, FixKind::DefaultGenericArgument, locator),
+        Param(param) {}
 
 public:
   static bool classof(const ConstraintFix *fix) {
-    return fix->getKind() == FixKind::ExplicitlySpecifyGenericArguments;
+    return fix->getKind() == FixKind::DefaultGenericArgument;
   }
 
   std::string getName() const override {
-    return "default missing generic arguments to `Any`";
+    auto paramName = Param->getString();
+    return "default generic argument '" + paramName + "' to 'Any'";
   }
 
-  ArrayRef<GenericTypeParamType *> getParameters() const {
-    return {getTrailingObjects<GenericTypeParamType *>(), NumMissingParams};
-  }
+  bool coalesceAndDiagnose(ArrayRef<ConstraintFix *> secondaryFixes,
+                           bool asNote = false) const override;
 
   bool diagnose(bool asNote = false) const override;
 
-  ConstraintFix *coalescedWith(ArrayRef<ConstraintFix *> fixes) override;
-
-  static ExplicitlySpecifyGenericArguments *
-  create(ConstraintSystem &cs, ArrayRef<GenericTypeParamType *> params,
-         ConstraintLocator *locator);
-
-private:
-  MutableArrayRef<GenericTypeParamType *> getParametersBuf() {
-    return {getTrailingObjects<GenericTypeParamType *>(), NumMissingParams};
-  }
+  static DefaultGenericArgument *create(ConstraintSystem &cs,
+                                        GenericTypeParamType *param,
+                                        ConstraintLocator *locator);
 };
 
 class SkipUnhandledConstructInFunctionBuilder final : public ConstraintFix {
@@ -1553,4 +1540,23 @@ class TreatEphemeralAsNonEphemeral final : public AllowArgumentMismatch {
 } // end namespace constraints
 } // end namespace swift
 
+namespace llvm {
+  template <>
+  struct DenseMapInfo<swift::constraints::FixKind> {
+    using FixKind = swift::constraints::FixKind;
+    static inline FixKind getEmptyKey() {
+      return static_cast<FixKind>(0);
+    }
+    static inline FixKind getTombstoneKey() {
+      return static_cast<FixKind>(1);
+    }
+    static unsigned getHashValue(FixKind kind) {
+      return static_cast<unsigned>(kind);
+    }
+    static bool isEqual(FixKind lhs, FixKind rhs) {
+      return lhs == rhs;
+    }
+  };
+}
+
 #endif // SWIFT_SEMA_CSFIX_H