Merge pull request #22480 from xedin/rdar-47871590

[ConstraintSystem] Diagnose conditional requirement failures via fixes

Author: xedin

lib/Sema/CSDiagnostics.cpp

@@ -24,6 +24,8 @@
 #include "swift/AST/GenericSignature.h"
 #include "swift/AST/ParameterList.h"
 #include "swift/AST/Pattern.h"
+#include "swift/AST/ProtocolConformance.h"
+#include "swift/AST/ProtocolConformanceRef.h"
 #include "swift/AST/Types.h"
 #include "swift/Basic/SourceLoc.h"
 #include "swift/Parse/Lexer.h"
@@ -94,13 +96,39 @@ const GenericContext *RequirementFailure::getGenericContext() const {
 }
 
 const Requirement &RequirementFailure::getRequirement() const {
+  // If this is a conditional requirement failure we need to
+  // fetch conformance from constraint system associated with
+  // type requirement this conditional conformance belongs to.
+  if (isConditional()) {
+    auto conformanceRef = getConformanceRef();
+    return conformanceRef.getConditionalRequirements()[getRequirementIndex()];
+  }
+
   return getGenericContext()->getGenericRequirements()[getRequirementIndex()];
 }
 
+ProtocolConformanceRef RequirementFailure::getConformanceRef() const {
+  assert(isConditional());
+
+  auto &cs = getConstraintSystem();
+  auto *locator = getLocator();
+
+  auto *typeReqLoc =
+      cs.getConstraintLocator(getRawAnchor(), locator->getPath().drop_back(),
+                              /*summaryFlags=*/0);
+
+  auto conformance = llvm::find_if(
+      cs.CheckedConformances,
+      [&](const std::pair<ConstraintLocator *, ProtocolConformanceRef>
+              &conformance) { return conformance.first == typeReqLoc; });
+  assert(conformance != cs.CheckedConformances.end());
+  return conformance->second;
+}
+
 ValueDecl *RequirementFailure::getDeclRef() const {
   auto &cs = getConstraintSystem();
 
-  auto *anchor = getAnchor();
+  auto *anchor = getRawAnchor();
   auto *locator = cs.getConstraintLocator(anchor);
   if (auto *AE = dyn_cast<CallExpr>(anchor)) {
     assert(isa<TypeExpr>(AE->getFn()));
@@ -142,6 +170,13 @@ ValueDecl *RequirementFailure::getDeclRef() const {
 }
 
 const DeclContext *RequirementFailure::getRequirementDC() const {
+  // In case of conditional requirement failure, we don't
+  // have to guess where the it comes from.
+  if (isConditional()) {
+    auto *conformance = getConformanceRef().getConcrete();
+    return conformance->getDeclContext();
+  }
+
   const auto &req = getRequirement();
   auto *DC = AffectedDecl->getDeclContext();
 
@@ -155,27 +190,41 @@ const DeclContext *RequirementFailure::getRequirementDC() const {
   return AffectedDecl->getAsGenericContext();
 }
 
+bool RequirementFailure::isStaticOrInstanceMember(const ValueDecl *decl) {
+  if (decl->isInstanceMember())
+    return true;
+
+  if (auto *AFD = dyn_cast<AbstractFunctionDecl>(decl))
+    return AFD->isStatic() && !AFD->isOperator();
+
+  return decl->isStatic();
+}
+
 bool RequirementFailure::diagnoseAsError() {
   if (!canDiagnoseFailure())
     return false;
 
-  auto *anchor = getAnchor();
+  auto *anchor = getRawAnchor();
   const auto *reqDC = getRequirementDC();
   auto *genericCtx = getGenericContext();
 
-  if (reqDC != genericCtx) {
+  auto lhs = resolveType(getLHS());
+  auto rhs = resolveType(getRHS());
+
+  if (genericCtx != reqDC && (genericCtx->isChildContextOf(reqDC) ||
+                              isStaticOrInstanceMember(AffectedDecl))) {
     auto *NTD = reqDC->getSelfNominalTypeDecl();
     emitDiagnostic(anchor->getLoc(), getDiagnosticInRereference(),
                    AffectedDecl->getDescriptiveKind(),
-                   AffectedDecl->getFullName(), NTD->getDeclaredType(),
-                   getLHS(), getRHS());
+                   AffectedDecl->getFullName(), NTD->getDeclaredType(), lhs,
+                   rhs);
   } else {
     emitDiagnostic(anchor->getLoc(), getDiagnosticOnDecl(),
                    AffectedDecl->getDescriptiveKind(),
-                   AffectedDecl->getFullName(), getLHS(), getRHS());
+                   AffectedDecl->getFullName(), lhs, rhs);
   }
 
-  emitRequirementNote(reqDC->getAsDecl());
+  emitRequirementNote(reqDC->getAsDecl(), lhs, rhs);
   return true;
 }
 
@@ -188,23 +237,32 @@ bool RequirementFailure::diagnoseAsNote() {
   return true;
 }
 
-void RequirementFailure::emitRequirementNote(const Decl *anchor) const {
+void RequirementFailure::emitRequirementNote(const Decl *anchor, Type lhs,
+                                             Type rhs) const {
   auto &req = getRequirement();
 
-  if (getRHS()->isEqual(req.getSecondType())) {
+  if (isConditional()) {
+    auto *conformance = getConformanceRef().getConcrete();
+    emitDiagnostic(anchor, diag::requirement_implied_by_conditional_conformance,
+                   resolveType(conformance->getType()),
+                   conformance->getProtocol()->getDeclaredInterfaceType());
+    return;
+  }
+
+  if (rhs->isEqual(req.getSecondType())) {
     emitDiagnostic(anchor, diag::where_requirement_failure_one_subst,
-                   req.getFirstType(), getLHS());
+                   req.getFirstType(), lhs);
     return;
   }
 
-  if (getLHS()->isEqual(req.getFirstType())) {
+  if (lhs->isEqual(req.getFirstType())) {
     emitDiagnostic(anchor, diag::where_requirement_failure_one_subst,
-                   req.getSecondType(), getRHS());
+                   req.getSecondType(), rhs);
     return;
   }
 
   emitDiagnostic(anchor, diag::where_requirement_failure_both_subst,
-                 req.getFirstType(), getLHS(), req.getSecondType(), getRHS());
+                 req.getFirstType(), lhs, req.getSecondType(), rhs);
 }
 
 bool MissingConformanceFailure::diagnoseAsError() {

lib/Sema/CSDiagnostics.h

@@ -172,6 +172,9 @@ class RequirementFailure : public FailureDiagnostic {
   /// to diagnose failures related to arguments.
   const ApplyExpr *Apply = nullptr;
 
+  /// If this failure associated with one of the conditional requirements.
+  bool IsConditional = false;
+
 public:
   RequirementFailure(ConstraintSystem &cs, Expr *expr, RequirementKind kind,
                      ConstraintLocator *locator)
@@ -183,9 +186,12 @@ class RequirementFailure : public FailureDiagnostic {
     assert(!path.empty());
 
     auto &last = path.back();
-    assert(last.getKind() == ConstraintLocator::TypeParameterRequirement);
+    assert(last.isTypeParameterRequirement() ||
+           last.isConditionalRequirement());
     assert(static_cast<RequirementKind>(last.getValue2()) == kind);
 
+    IsConditional = last.isConditionalRequirement();
+
     // It's possible sometimes not to have no base expression.
     if (!expr)
       return;
@@ -199,7 +205,8 @@ class RequirementFailure : public FailureDiagnostic {
     assert(!path.empty());
 
     auto &requirementLoc = path.back();
-    assert(requirementLoc.getKind() == PathEltKind::TypeParameterRequirement);
+    assert(requirementLoc.isTypeParameterRequirement() ||
+           requirementLoc.isConditionalRequirement());
     return requirementLoc.getValue();
   }
 
@@ -219,6 +226,13 @@ class RequirementFailure : public FailureDiagnostic {
   bool diagnoseAsNote() override;
 
 protected:
+  /// Determine whether this is a conditional requirement failure.
+  bool isConditional() const { return IsConditional; }
+
+  /// If this is a failure in condition requirement, retrieve
+  /// conformance information.
+  ProtocolConformanceRef getConformanceRef() const;
+
   /// Retrieve declaration contextual where current
   /// requirement has been introduced.
   const DeclContext *getRequirementDC() const;
@@ -230,6 +244,13 @@ class RequirementFailure : public FailureDiagnostic {
   /// Determine whether it would be possible to diagnose
   /// current requirement failure.
   bool canDiagnoseFailure() const {
+    // If this is a conditional requirement failure,
+    // we have a lot more information compared to
+    // type requirement case, because we know that
+    // underlying conformance requirement matched.
+    if (isConditional())
+      return true;
+
     // For static/initializer calls there is going to be
     // a separate fix, attached to the argument, which is
     // much easier to diagnose.
@@ -247,7 +268,11 @@ class RequirementFailure : public FailureDiagnostic {
   /// Retrieve declaration associated with failing generic requirement.
   ValueDecl *getDeclRef() const;
 
-  void emitRequirementNote(const Decl *anchor) const;
+  void emitRequirementNote(const Decl *anchor, Type lhs, Type rhs) const;
+
+  /// Determine whether given declaration represents a static
+  /// or instance property/method, excluding operators.
+  static bool isStaticOrInstanceMember(const ValueDecl *decl);
 };
 
 /// Diagnostics for failed conformance checks originating from

lib/Sema/CSSimplify.cpp

@@ -1602,7 +1602,7 @@ ConstraintSystem::matchTypesBindTypeVar(
 
 static ConstraintFix *fixRequirementFailure(ConstraintSystem &cs, Type type1,
                                             Type type2, Expr *anchor,
-                                            LocatorPathElt &req) {
+                                            ArrayRef<LocatorPathElt> path) {
   // Can't fix not yet properly resolved types.
   if (type1->hasTypeVariable() || type2->hasTypeVariable())
     return nullptr;
@@ -1612,9 +1612,21 @@ static ConstraintFix *fixRequirementFailure(ConstraintSystem &cs, Type type1,
   if (type1->hasDependentMember() || type2->hasDependentMember())
     return nullptr;
 
-  // Build simplified locator which only contains anchor and requirement info.
-  ConstraintLocatorBuilder requirement(cs.getConstraintLocator(anchor));
-  auto *reqLoc = cs.getConstraintLocator(requirement.withPathElement(req));
+  auto req = path.back();
+
+  ConstraintLocator *reqLoc = nullptr;
+  if (req.isConditionalRequirement()) {
+    // If underlaying conformance requirement has been fixed as
+    // we there is no reason to fix up conditional requirements.
+    if (cs.hasFixFor(cs.getConstraintLocator(anchor, req)))
+      return nullptr;
+
+    // For conditional requirements we need a full path.
+    reqLoc = cs.getConstraintLocator(anchor, path, /*summaryFlags=*/0);
+  } else {
+    // Build simplified locator which only contains anchor and requirement info.
+    reqLoc = cs.getConstraintLocator(anchor, req);
+  }
 
   auto reqKind = static_cast<RequirementKind>(req.getValue2());
   switch (reqKind) {
@@ -1644,8 +1656,9 @@ repairFailures(ConstraintSystem &cs, Type lhs, Type rhs,
 
   auto &elt = path.back();
   switch (elt.getKind()) {
-  case ConstraintLocator::TypeParameterRequirement: {
-    if (auto *fix = fixRequirementFailure(cs, lhs, rhs, anchor, elt))
+  case ConstraintLocator::TypeParameterRequirement:
+  case ConstraintLocator::ConditionalRequirement: {
+    if (auto *fix = fixRequirementFailure(cs, lhs, rhs, anchor, path))
       conversionsOrFixes.push_back(fix);
     break;
   }
@@ -2691,10 +2704,10 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyConformsToConstraint(
     if (conformance.isConcrete()) {
       unsigned index = 0;
       for (const auto &req : conformance.getConditionalRequirements()) {
-        addConstraint(
-          req,
-          locator.withPathElement(
-            LocatorPathElt::getConditionalRequirementComponent(index++)));
+        addConstraint(req,
+                      locator.withPathElement(
+                          LocatorPathElt::getConditionalRequirementComponent(
+                              index++, req.getKind())));
       }
     }
 
@@ -2764,15 +2777,27 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyConformsToConstraint(
     SmallVector<LocatorPathElt, 4> path;
     auto *anchor = locator.getLocatorParts(path);
 
-    if (!path.empty() && path.back().getKind() ==
-        ConstraintLocator::PathElementKind::TypeParameterRequirement) {
-      auto typeRequirement = path.back();
-      // Let's strip all of the unnecessary information from locator,
-      // diagnostics only care about anchor - to lookup type,
-      // and what was the requirement# which is not satisfied.
-      ConstraintLocatorBuilder requirement(getConstraintLocator(anchor));
-      auto *reqLoc =
-          getConstraintLocator(requirement.withPathElement(typeRequirement));
+    if (path.empty())
+      return SolutionKind::Error;
+
+    if (path.back().isTypeParameterRequirement() ||
+        path.back().isConditionalRequirement()) {
+      ConstraintLocator *reqLoc = nullptr;
+      if (path.back().isConditionalRequirement()) {
+        // Underlying conformance requirement is itself fixed,
+        // this wouldn't lead to right solution.
+        if (hasFixFor(getConstraintLocator(anchor, path.back())))
+          return SolutionKind::Error;
+
+        // For conditional requirements we need complete path, which includes
+        // type requirement position, to be able to fetch conformance later.
+        reqLoc = getConstraintLocator(locator);
+      } else {
+        // Let's strip all of the unnecessary information from locator,
+        // diagnostics only care about anchor - to lookup type,
+        // and what was the requirement# which is not satisfied.
+        reqLoc = getConstraintLocator(anchor, path.back());
+      }
 
       auto *fix = MissingConformance::create(*this, type, protocol, reqLoc);
       if (!recordFix(fix))
@@ -5417,12 +5442,7 @@ bool ConstraintSystem::recordFix(ConstraintFix *fix) {
     // Always useful, unless duplicate of exactly the same fix and location.
     // This situation might happen when the same fix kind is applicable to
     // different overload choices.
-    auto *loc = fix->getLocator();
-    auto existingFix = llvm::find_if(Fixes, [&](const ConstraintFix *e) {
-      // If we already have a fix like this recorded, let's not do it again,
-      return e->getKind() == fix->getKind() && e->getLocator() == loc;
-    });
-    if (existingFix == Fixes.end())
+    if (!hasFixFor(fix->getLocator()))
       Fixes.push_back(fix);
   } else {
     // Only useful to record if no pre-existing fix in the subexpr tree.

lib/Sema/ConstraintLocator.cpp

@@ -106,6 +106,25 @@ void ConstraintLocator::dump(SourceManager *sm, raw_ostream &out) {
     }
   }
 
+  auto dumpReqKind = [&out](RequirementKind kind) {
+    out << " (";
+    switch (kind) {
+    case RequirementKind::Conformance:
+      out << "conformance";
+      break;
+    case RequirementKind::Superclass:
+      out << "superclass";
+      break;
+    case RequirementKind::SameType:
+      out << "same-type";
+      break;
+    case RequirementKind::Layout:
+      out << "layout";
+      break;
+    }
+    out << ")";
+  };
+
   for (auto elt : getPath()) {
     out << " -> ";
     switch (elt.getKind()) {
@@ -222,26 +241,12 @@ void ConstraintLocator::dump(SourceManager *sm, raw_ostream &out) {
 
     case ConditionalRequirement:
       out << "conditional requirement #" << llvm::utostr(elt.getValue());
+      dumpReqKind(static_cast<RequirementKind>(elt.getValue2()));
       break;
 
     case TypeParameterRequirement: {
-      out << "type parameter requirement #" << llvm::utostr(elt.getValue())
-          << " (";
-      switch (static_cast<RequirementKind>(elt.getValue2())) {
-      case RequirementKind::Conformance:
-        out << "conformance";
-        break;
-      case RequirementKind::Superclass:
-        out << "superclass";
-        break;
-      case RequirementKind::SameType:
-        out << "same-type";
-        break;
-      case RequirementKind::Layout:
-        out << "layout";
-        break;
-      }
-      out << ")";
+      out << "type parameter requirement #" << llvm::utostr(elt.getValue());
+      dumpReqKind(static_cast<RequirementKind>(elt.getValue2()));
       break;
     }