Merge pull request #30844 from hborla/diagnose-for-ambiguity

[Diagnostics] In DefineMemberBasedOnUse::diagnoseForAmbiguity, use the base type from each solution

Author: hborla

lib/Sema/CSFix.cpp

@@ -489,10 +489,13 @@ bool DefineMemberBasedOnUse::diagnose(const Solution &solution,
 }
 
 bool
-DefineMemberBasedOnUse::diagnoseForAmbiguity(ArrayRef<Solution> solutions) const {
+DefineMemberBasedOnUse::diagnoseForAmbiguity(CommonFixesArray commonFixes) const {
   Type concreteBaseType;
-  for (const auto &solution: solutions) {
-    auto baseType = solution.simplifyType(BaseType);
+  for (const auto &solutionAndFix : commonFixes) {
+    const auto *solution = solutionAndFix.first;
+    const auto *fix = solutionAndFix.second->getAs<DefineMemberBasedOnUse>();
+
+    auto baseType = solution->simplifyType(fix->BaseType);
     if (!concreteBaseType)
       concreteBaseType = baseType;
 
@@ -503,7 +506,7 @@ DefineMemberBasedOnUse::diagnoseForAmbiguity(ArrayRef<Solution> solutions) const
     }
   }
 
-  return diagnose(solutions.front());
+  return diagnose(*commonFixes.front().first);
 }
 
 DefineMemberBasedOnUse *

lib/Sema/CSFix.h

@@ -291,7 +291,12 @@ class ConstraintFix {
   virtual bool diagnose(const Solution &solution,
                         bool asNote = false) const = 0;
 
-  virtual bool diagnoseForAmbiguity(ArrayRef<Solution> solutions) const { return false; }
+  using CommonFixesArray =
+      ArrayRef<std::pair<const Solution *, const ConstraintFix *>>;
+
+  virtual bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const {
+    return false;
+  }
 
   void print(llvm::raw_ostream &Out) const;
 
@@ -847,11 +852,15 @@ class DefineMemberBasedOnUse final : public ConstraintFix {
 
   bool diagnose(const Solution &solution, bool asNote = false) const override;
 
-  bool diagnoseForAmbiguity(ArrayRef<Solution> solutions) const override;
+  bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override;
 
   static DefineMemberBasedOnUse *create(ConstraintSystem &cs, Type baseType,
                                         DeclNameRef member, bool alreadyDiagnosed,
                                         ConstraintLocator *locator);
+
+  static bool classof(const ConstraintFix *fix) {
+    return fix->getKind() == FixKind::DefineMemberBasedOnUse;
+  }
 };
 
 class AllowInvalidMemberRef : public ConstraintFix {
@@ -1117,8 +1126,8 @@ class AddMissingArguments final
 
   bool diagnose(const Solution &solution, bool asNote = false) const override;
 
-  bool diagnoseForAmbiguity(ArrayRef<Solution> solutions) const override {
-    return diagnose(solutions.front());
+  bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override {
+    return diagnose(*commonFixes.front().first);
   }
 
   static AddMissingArguments *create(ConstraintSystem &cs,
@@ -1161,8 +1170,8 @@ class RemoveExtraneousArguments final
 
   bool diagnose(const Solution &solution, bool asNote = false) const override;
 
-  bool diagnoseForAmbiguity(ArrayRef<Solution> solutions) const override {
-    return diagnose(solutions.front());
+  bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override {
+    return diagnose(*commonFixes.front().first);
   }
 
   /// FIXME(diagnostics): Once `resolveDeclRefExpr` is gone this
@@ -1371,8 +1380,8 @@ class DefaultGenericArgument final : public ConstraintFix {
 
   bool diagnose(const Solution &solution, bool asNote = false) const override;
 
-  bool diagnoseForAmbiguity(ArrayRef<Solution> solutions) const override {
-    return diagnose(solutions.front());
+  bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override {
+    return diagnose(*commonFixes.front().first);
   }
 
   static DefaultGenericArgument *create(ConstraintSystem &cs,
@@ -1446,8 +1455,8 @@ class IgnoreContextualType : public ContextualMismatch {
 
   bool diagnose(const Solution &solution, bool asNote = false) const override;
 
-  bool diagnoseForAmbiguity(ArrayRef<Solution> solutions) const override {
-    return diagnose(solutions.front());
+  bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override {
+    return diagnose(*commonFixes.front().first);
   }
 
   static IgnoreContextualType *create(ConstraintSystem &cs, Type resultTy,

lib/Sema/ConstraintSystem.cpp

@@ -2947,12 +2947,15 @@ bool ConstraintSystem::diagnoseAmbiguityWithFixes(
     return true;
 
   // Collect aggregated fixes from all solutions
-  llvm::SmallMapVector<std::pair<ConstraintLocator *, FixKind>,
-                       llvm::TinyPtrVector<ConstraintFix *>, 4>
+  using LocatorAndKind = std::pair<ConstraintLocator *, FixKind>;
+  using SolutionAndFix = std::pair<const Solution *, const ConstraintFix *>;
+  llvm::SmallMapVector<LocatorAndKind, llvm::SmallVector<SolutionAndFix, 4>, 4>
       aggregatedFixes;
   for (const auto &solution : solutions) {
-    for (auto *fix : solution.Fixes)
-      aggregatedFixes[{fix->getLocator(), fix->getKind()}].push_back(fix);
+    for (const auto *fix : solution.Fixes) {
+      LocatorAndKind key(fix->getLocator(), fix->getKind());
+      aggregatedFixes[key].emplace_back(&solution, fix);
+    }
   }
 
   // If there is an overload difference, let's see if there's a common callee
@@ -2983,8 +2986,11 @@ bool ConstraintSystem::diagnoseAmbiguityWithFixes(
     bool diagnosed = false;
     for (auto fixes: aggregatedFixes) {
       // A common fix must appear in all solutions
-      if (fixes.second.size() < solutions.size()) continue;
-      diagnosed |= fixes.second.front()->diagnoseForAmbiguity(solutions);
+      auto &commonFixes = fixes.second;
+      if (commonFixes.size() != solutions.size()) continue;
+
+      auto *firstFix = commonFixes.front().second;
+      diagnosed |= firstFix->diagnoseForAmbiguity(commonFixes);
     }
     return diagnosed;
   }

test/Sema/diag_ambiguous_overloads.swift

@@ -40,6 +40,14 @@ struct S {
   }
 }
 
+struct School {
+  var name: String
+}
+func testDiagnoseForAmbiguityCrash(schools: [School]) {
+  schools.map({ $0.name }).sorted(by: { $0.nothing < $1.notAThing })
+  // expected-error@-1 {{value of type 'String' has no member 'nothing'}}
+  // expected-error@-2 {{value of type 'String' has no member 'notAThing'}}
+}
 
 class DefaultValue {
   static func foo(_ a: Int) {}