Merge pull request #27443 from hborla/ambiguous-generic-parameters

[ConstraintSystem] Extend the ExplicitlySpecifyGenericArguments fix to cover all cases of missing generic parameters.

Author: hborla

include/swift/AST/Types.h

@@ -530,6 +530,8 @@ class alignas(1 << TypeAlignInBits) TypeBase {
   /// Is this the 'Any' type?
   bool isAny();
 
+  bool isHole();
+
   /// Does the type have outer parenthesis?
   bool hasParenSugar() const { return getKind() == TypeKind::Paren; }
 

lib/AST/Type.cpp

@@ -157,6 +157,10 @@ bool TypeBase::isAny() {
   return isEqual(getASTContext().TheAnyType);
 }
 
+bool TypeBase::isHole() {
+  return isEqual(getASTContext().TheUnresolvedType);
+}
+
 bool TypeBase::isAnyClassReferenceType() {
   return getCanonicalType().isAnyClassReferenceType();
 }

lib/Sema/CSApply.cpp

@@ -7455,9 +7455,20 @@ bool ConstraintSystem::applySolutionFixes(Expr *E, const Solution &solution) {
       if (fixes == fixesPerExpr.end())
         return;
 
-      for (const auto *fix : fixes->second) {
-        auto diagnosed = fix->diagnose(root);
-        if (fix->isWarning()) {
+      // Coalesce fixes with the same locator to avoid duplicating notes.
+      llvm::SmallMapVector<ConstraintLocator *,
+          llvm::SmallVector<ConstraintFix *, 4>, 4> aggregatedFixes;
+      for (auto *fix : fixes->second)
+        aggregatedFixes[fix->getLocator()].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(root);
+        if (coalescedFix->isWarning()) {
           assert(diagnosed && "warnings should always be diagnosed");
           (void)diagnosed;
         } else {

lib/Sema/CSBindings.cpp

@@ -735,6 +735,16 @@ 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 (shouldAttemptFixes() && result.Bindings.empty() &&
+      (isPotentialHole(typeVar) || result.isGenericParameter())) {
+    result.IsHole = true;
+    result.addPotentialBinding({getASTContext().TheUnresolvedType,
+        AllowedBindingKind::Exact, ConstraintKind::Defaultable, nullptr,
+        typeVar->getImpl().getLocator()});
+  }
+
   // Determine if the bindings only constrain the type variable from above with
   // an existential type; such a binding is not very helpful because it's
   // impossible to enumerate the existential type's subtypes.
@@ -979,31 +989,18 @@ bool TypeVariableBinding::attempt(ConstraintSystem &cs) const {
 
   // If this was from a defaultable binding note that.
   if (Binding.isDefaultableBinding()) {
-    auto *locator = Binding.DefaultableBinding;
-    // If this default binding comes from a "hole"
-    // in the constraint system, we have to propagate
-    // this information and mark this type variable
-    // as well as mark everything adjacent to it as
-    // a potential "hole".
-    //
-    // Consider this example:
-    //
-    // func foo<T: BinaryInteger>(_: T) {}
-    // foo(.bar) <- Since `.bar` can't be inferred due to
-    //              luck of information about its base type,
-    //              it's member type is going to get defaulted
-    //              to `Any` which has to be propaged to type
-    //              variable associated with `T` and vice versa.
-    if (cs.shouldAttemptFixes() && cs.isHoleAt(locator)) {
-      auto &CG = cs.getConstraintGraph();
-      for (auto *constraint : CG.gatherConstraints(
-               TypeVar, ConstraintGraph::GatheringKind::EquivalenceClass)) {
-        for (auto *typeVar : constraint->getTypeVariables())
-          cs.recordHole(typeVar);
-      }
+    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.
+      auto path = locator->getPath();
+      auto genericParam = locator->getGenericParameter();
+      auto *fix = ExplicitlySpecifyGenericArguments::create(cs, {genericParam},
+          cs.getConstraintLocator(locator->getAnchor(), path.drop_back()));
+      if (cs.recordFix(fix))
+        return true;
     }
-
-    cs.DefaultedConstraints.push_back(locator);
   }
 
   return !cs.failedConstraint && !cs.simplify();

lib/Sema/CSDiag.cpp

@@ -236,17 +236,6 @@ class FailureDiagnosis :public ASTVisitor<FailureDiagnosis, /*exprresult*/bool>{
   /// true.
   bool diagnoseAmbiguousMultiStatementClosure(ClosureExpr *closure);
 
-  /// Check the associated constraint system to see if it has any opened generic
-  /// parameters that were not bound to a fixed type. If so, diagnose the
-  /// problem with an error and return true.
-  bool diagnoseAmbiguousGenericParameters();
-
-  /// Emit an error message about an unbound generic parameter, and emit notes
-  /// referring to the target of a diagnostic, e.g., the function or parameter
-  /// being used.
-  void diagnoseAmbiguousGenericParameter(GenericTypeParamType *paramTy,
-                                         Expr *anchor);
-
   /// Produce a diagnostic for a general member-lookup failure (irrespective of
   /// the exact expression kind).
   bool diagnoseGeneralMemberFailure(Constraint *constraint);
@@ -4529,122 +4518,8 @@ diagnoseAmbiguousMultiStatementClosure(ClosureExpr *closure) {
   return true;
 }
 
-/// Check the associated constraint system to see if it has any archetypes
-/// not properly resolved or missing. If so, diagnose the problem with
-/// an error and return true.
-bool FailureDiagnosis::diagnoseAmbiguousGenericParameters() {
-  using GenericParameter = std::tuple<GenericTypeParamType *,
-                                      ConstraintLocator *,
-                                      unsigned>;
-
-  llvm::SmallVector<GenericParameter, 2> unboundParams;
-  // Check out all of the type variables lurking in the system.  If any free
-  // type variables were created when opening generic parameters, diagnose
-  // that the generic parameter could not be inferred.
-  for (auto tv : CS.getTypeVariables()) {
-    auto &impl = tv->getImpl();
-
-    if (impl.hasRepresentativeOrFixed())
-      continue;
-
-    auto *paramTy = impl.getGenericParameter();
-    if (!paramTy)
-      continue;
-
-    // Number of constraints related to particular unbound parameter
-    // is significant indicator of the problem, because if there are
-    // no constraints associated with it, that means it can't ever be resolved,
-    // such helps to diagnose situations like: struct S<A, B> { init(_ a: A) {}}
-    // because type B would have no constraints associated with it.
-    unsigned numConstraints = 0;
-    {
-      auto constraints = CS.getConstraintGraph().gatherConstraints(
-          tv, ConstraintGraph::GatheringKind::EquivalenceClass,
-          [&](Constraint *constraint) -> bool {
-            // We are not interested in ConformsTo constraints because
-            // we can't derive any concrete type information from them.
-            if (constraint->getKind() == ConstraintKind::ConformsTo)
-              return false;
-
-            if (constraint->getKind() == ConstraintKind::Bind) {
-              if (auto locator = constraint->getLocator()) {
-                auto anchor = locator->getAnchor();
-                if (anchor && isa<UnresolvedDotExpr>(anchor))
-                  return false;
-              }
-            }
-
-            return true;
-          });
-
-      numConstraints = constraints.size();
-    }
-
-    auto locator = impl.getLocator();
-    unboundParams.emplace_back(paramTy, locator, numConstraints);
-  }
-
-  // We've found unbound generic parameters, let's diagnose
-  // based on the number of constraints each one is related to.
-  if (!unboundParams.empty()) {
-    // Let's prioritize generic parameters that don't have any constraints
-    // associated.
-    std::stable_sort(unboundParams.begin(), unboundParams.end(),
-                     [](GenericParameter a, GenericParameter b) {
-                       return std::get<2>(a) < std::get<2>(b);
-                     });
-
-    auto param = unboundParams.front();
-    diagnoseAmbiguousGenericParameter(std::get<0>(param),
-                                      std::get<1>(param)->getAnchor());
-    return true;
-  }
-
-  return false;
-}
-
-/// Emit an error message about an unbound generic parameter existing, and
-/// emit notes referring to the target of a diagnostic, e.g., the function
-/// or parameter being used.
-void FailureDiagnosis::
-diagnoseAmbiguousGenericParameter(GenericTypeParamType *paramTy,
-                                  Expr *anchor) {
-  // A very common cause of this diagnostic is a situation where a closure expr
-  // has no inferred type, due to being a multiline closure.  Check to see if
-  // this is the case and (if so), speculatively diagnose that as the problem.
-  bool didDiagnose = false;
-  expr->forEachChildExpr([&](Expr *subExpr) -> Expr*{
-    auto closure = dyn_cast<ClosureExpr>(subExpr);
-    if (!didDiagnose && closure)
-      didDiagnose = diagnoseAmbiguousMultiStatementClosure(closure);
-    
-    return subExpr;
-  });
-
-  if (didDiagnose) return;
-
-  
-  // Otherwise, emit an error message on the expr we have, and emit a note
-  // about where the generic parameter came from.
-  if (!anchor) {
-    auto &tc = CS.getTypeChecker();
-    tc.diagnose(expr->getLoc(), diag::unbound_generic_parameter, paramTy);
-    return;
-  }
-
-  MissingGenericArgumentsFailure failure(expr, CS, {paramTy},
-                                         CS.getConstraintLocator(anchor));
-  failure.diagnoseAsError();
-}
-
-
 /// Emit an ambiguity diagnostic about the specified expression.
 void FailureDiagnosis::diagnoseAmbiguity(Expr *E) {
-  // First, let's try to diagnose any problems related to ambiguous
-  // generic parameters present in the constraint system.
-  if (diagnoseAmbiguousGenericParameters())
-    return;
-
   // Unresolved/Anonymous ClosureExprs are common enough that we should give
   // them tailored diagnostics.
   if (auto CE = dyn_cast<ClosureExpr>(E->getValueProvidingExpr())) {

lib/Sema/CSDiagnostics.h

@@ -98,12 +98,23 @@ class FailureDiagnostic {
   /// Resolve type variables present in the raw type, if any.
   Type resolveType(Type rawType, bool reconstituteSugar = false,
                    bool wantRValue = true) const {
-    auto resolvedType = CS.simplifyType(rawType);
-
-    if (reconstituteSugar)
-      resolvedType = resolvedType->reconstituteSugar(/*recursive*/ true);
+    if (!rawType->hasTypeVariable()) {
+      if (reconstituteSugar)
+        rawType = rawType->reconstituteSugar(/*recursive*/ true);
+      return wantRValue ? rawType->getRValueType() : rawType;
+    }
 
-    return wantRValue ? resolvedType->getRValueType() : resolvedType;
+    auto &cs = getConstraintSystem();
+    return cs.simplifyTypeImpl(rawType,
+        [&](TypeVariableType *typeVar) -> Type {
+          if (auto fixed = cs.getFixedType(typeVar)) {
+            auto *genericParam = typeVar->getImpl().getGenericParameter();
+            if (fixed->isHole() && genericParam)
+                return genericParam;
+            return resolveType(fixed, reconstituteSugar, wantRValue);
+          }
+          return cs.getRepresentative(typeVar);
+        });
   }
 
   /// Resolve type variables present in the raw type, using generic parameter
@@ -782,12 +793,7 @@ class ContextualFailure : public FailureDiagnostic {
 
 private:
   Type resolve(Type rawType) {
-    auto type = resolveType(rawType)->getWithoutSpecifierType();
-    if (auto *BGT = type->getAs<BoundGenericType>()) {
-      if (BGT->hasUnresolvedType())
-        return BGT->getDecl()->getDeclaredInterfaceType();
-    }
-    return type;
+    return resolveType(rawType)->getWithoutSpecifierType();
   }
 
   /// Try to add a fix-it to convert a stored property into a computed

lib/Sema/CSFix.cpp

@@ -714,6 +714,25 @@ bool ExplicitlySpecifyGenericArguments::diagnose(Expr *root,
   return failure.diagnose(asNote);
 }
 
+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());
+    }
+  }
+
+  return ExplicitlySpecifyGenericArguments::create(getConstraintSystem(),
+                                                   missingParams, getLocator());
+}
+
 ExplicitlySpecifyGenericArguments *ExplicitlySpecifyGenericArguments::create(
     ConstraintSystem &cs, ArrayRef<GenericTypeParamType *> params,
     ConstraintLocator *locator) {

lib/Sema/CSFix.h

@@ -241,6 +241,11 @@ class ConstraintFix {
 
   virtual ~ConstraintFix();
 
+  template <typename Fix>
+  const Fix *getAs() const {
+    return Fix::classof(this) ? static_cast<const Fix *>(this) : nullptr;
+  }
+
   FixKind getKind() const { return Kind; }
 
   bool isWarning() const { return IsWarning; }
@@ -251,6 +256,10 @@ class ConstraintFix {
   /// root expression and information from constraint system.
   virtual bool diagnose(Expr *root, bool asNote = false) const = 0;
 
+  virtual ConstraintFix *coalescedWith(ArrayRef<ConstraintFix *> fixes) {
+    return this;
+  }
+
   void print(llvm::raw_ostream &Out) const;
 
   SWIFT_DEBUG_DUMP;
@@ -1259,6 +1268,10 @@ class ExplicitlySpecifyGenericArguments final
   }
 
 public:
+  static bool classof(const ConstraintFix *fix) {
+    return fix->getKind() == FixKind::ExplicitlySpecifyGenericArguments;
+  }
+
   std::string getName() const override {
     return "default missing generic arguments to `Any`";
   }
@@ -1269,6 +1282,8 @@ class ExplicitlySpecifyGenericArguments final
 
   bool diagnose(Expr *root, bool asNote = false) const override;
 
+  ConstraintFix *coalescedWith(ArrayRef<ConstraintFix *> fixes) override;
+
   static ExplicitlySpecifyGenericArguments *
   create(ConstraintSystem &cs, ArrayRef<GenericTypeParamType *> params,
          ConstraintLocator *locator);