Merge pull request #18484 from xedin/diag-req-failures-via-fixes

[ConstraintSystem] Diagnose missing conformance requirements via "fixes"

Author: xedin

include/swift/AST/DiagnosticsSema.def

@@ -1512,8 +1512,8 @@ ERROR(use_of_equal_instead_of_equality,none,
 
 
 ERROR(protocol_does_not_conform_objc,none,
-      "using %0 as a concrete type conforming to protocol %1 is not supported",
-      (Type, Type))
+      "protocol type %0 cannot conform to %1 because only concrete "
+      "types can conform to protocols", (Type, Type))
 ERROR(protocol_does_not_conform_static,none,
       "%0 cannot be used as a type conforming to protocol %1 because %1 "
       "has static requirements",

lib/Sema/CSApply.cpp

@@ -7906,7 +7906,7 @@ bool ConstraintSystem::applySolutionFixes(Expr *E, const Solution &solution) {
 
     bool diagnosed = false;
     for (auto &fix : fixes->second)
-      diagnosed |= applySolutionFix(expr, solution, fix);
+      diagnosed |= applySolutionFix(E, solution, fix);
     return diagnosed;
   };
 
@@ -7950,7 +7950,8 @@ bool ConstraintSystem::applySolutionFix(
   if (!resolved || !resolved->getAnchor() ||
       (!resolved->getPath().empty() &&
        fix.first.getKind() != FixKind::ExplicitlyEscaping &&
-       fix.first.getKind() != FixKind::ExplicitlyEscapingToAny))
+       fix.first.getKind() != FixKind::ExplicitlyEscapingToAny &&
+       fix.first.getKind() != FixKind::AddConformance))
     return false;
 
   Expr *affected = resolved->getAnchor();
@@ -8128,6 +8129,95 @@ bool ConstraintSystem::applySolutionFix(
                                       fix.first.getArgumentLabels(*this),
                                       isa<SubscriptExpr>(call->getFn()));
   }
+
+  case FixKind::AddConformance: {
+    auto getMissingConformance = [&](ConstraintLocator *locator) {
+      auto *anchor = locator->getAnchor();
+      auto &requirement = locator->getPath().back();
+      auto result = MissingConformances.find({anchor, requirement.getValue()});
+      assert(result != MissingConformances.end());
+      return result->getSecond();
+    };
+
+    auto conformance = getMissingConformance(locator);
+
+    auto *anchor = locator->getAnchor();
+    auto owner = solution.simplifyType(getType(anchor))->getRValueInstanceType();
+
+    auto type = conformance.first;
+    auto protocolType = conformance.second->getDeclaredType();
+
+    //  Find `ApplyExpr` based on a function expression attached to it.
+    auto findApplyExpr = [](Expr *parent, Expr *fnExpr) -> ApplyExpr * {
+      ApplyExpr *applyExpr = nullptr;
+      parent->forEachChildExpr([&applyExpr, &fnExpr](Expr *subExpr) -> Expr * {
+        auto *AE = dyn_cast<ApplyExpr>(subExpr);
+        if (!AE || AE->getFn() != fnExpr)
+          return subExpr;
+
+        applyExpr = AE;
+        return nullptr;
+      });
+      return applyExpr;
+    };
+
+    auto getArgumentAt = [](ApplyExpr *AE, unsigned index) -> Expr * {
+      assert(AE);
+
+      auto *arg = AE->getArg();
+      if (auto *TE = dyn_cast<TupleExpr>(arg))
+        return TE->getElement(index);
+
+      assert(index == 0);
+      if (auto *PE = dyn_cast<ParenExpr>(arg))
+        return PE->getSubExpr();
+
+      return arg;
+    };
+
+    auto *applyExpr = findApplyExpr(expr, anchor);
+
+    Optional<unsigned> atParameterPos;
+    // Sometimes fix is recorded by type-checking sub-expression
+    // during normal diagnostics, in such case call expression
+    // is unavailable.
+    if (applyExpr) {
+      // If this is a static, initializer or operator call,
+      // let's not try to diagnose it here, but refer to expression
+      // diagnostics.
+      if (isa<BinaryExpr>(applyExpr) || isa<TypeExpr>(anchor))
+        return false;
+
+      if (auto *fnType = owner->getAs<AnyFunctionType>()) {
+        auto parameters = fnType->getParams();
+        for (auto index : indices(parameters)) {
+          if (parameters[index].getType()->isEqual(type)) {
+            atParameterPos = index;
+            break;
+          }
+        }
+      }
+    }
+
+    if (type->isExistentialType()) {
+      auto diagnostic = diag::protocol_does_not_conform_objc;
+      if (type->isObjCExistentialType())
+        diagnostic = diag::protocol_does_not_conform_static;
+
+      TC.diagnose(anchor->getLoc(), diagnostic, type, protocolType);
+    } else if (atParameterPos) {
+      // Requirement comes from one of the parameter types,
+      // let's try to point diagnostic to the argument expression.
+      auto *argExpr = getArgumentAt(applyExpr, *atParameterPos);
+      TC.diagnose(argExpr->getLoc(),
+                  diag::cannot_convert_argument_value_protocol, type,
+                  protocolType);
+    } else {
+      TC.diagnose(anchor->getLoc(), diag::type_does_not_conform_owner, owner,
+                  type, protocolType);
+    }
+    return true;
+  }
   }
 
   // FIXME: It would be really nice to emit a follow-up note showing where

lib/Sema/CSSimplify.cpp

@@ -2766,6 +2766,28 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyConformsToConstraint(
     }
     return result;
   }
+
+  // If this is a generic requirement let's try to record that
+  // conformance is missing and consider this a success, which
+  // makes it much easier to diagnose problems like that.
+  {
+    SmallVector<LocatorPathElt, 4> path;
+    auto *anchor = locator.getLocatorParts(path);
+
+    if (!path.empty() && path.back().getKind() ==
+        ConstraintLocator::PathElementKind::TypeParameterRequirement) {
+      auto typeRequirement = path.back();
+      std::pair<Expr *, unsigned> reqLoc = {anchor, typeRequirement.getValue()};
+      MissingConformances[reqLoc] = {type.getPointer(), protocol};
+      // 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));
+      if (!recordFix({FixKind::AddConformance},
+                     requirement.withPathElement(typeRequirement)))
+        return SolutionKind::Solved;
+    }
+  }
 
   // There's nothing more we can do; fail.
   return SolutionKind::Error;
@@ -5004,6 +5026,7 @@ ConstraintSystem::simplifyFixConstraint(Fix fix, Type type1, Type type2,
   case FixKind::ExplicitlyEscapingToAny:
   case FixKind::CoerceToCheckedCast:
   case FixKind::RelabelArguments:
+  case FixKind::AddConformance:
     llvm_unreachable("handled elsewhere");
   }
 

lib/Sema/Constraint.cpp

@@ -542,6 +542,8 @@ StringRef Fix::getName(FixKind kind) {
     return "fix: add @escaping";
   case FixKind::RelabelArguments:
     return "fix: re-label argument(s)";
+  case FixKind::AddConformance:
+    return "fix: add missing protocol conformance";
   }
 
   llvm_unreachable("Unhandled FixKind in switch.");

lib/Sema/Constraint.h

@@ -253,6 +253,9 @@ enum class FixKind : uint8_t {
   /// Arguments have labeling failures - missing/extraneous or incorrect
   /// labels attached to the, fix it by suggesting proper labels.
   RelabelArguments,
+
+  /// Add a new conformance to the type to satisfy a requirement.
+  AddConformance,
 };
 
 /// Describes a fix that can be applied to a constraint before visiting it.

lib/Sema/ConstraintSystem.h

@@ -437,10 +437,10 @@ struct SelectedOverload {
 enum ScoreKind {
   // These values are used as indices into a Score value.
 
-  /// A reference to an @unavailable declaration.
-  SK_Unavailable,
   /// A fix needs to be applied to the source.
   SK_Fix,
+  /// A reference to an @unavailable declaration.
+  SK_Unavailable,
   /// An implicit force of an implicitly unwrapped optional value.
   SK_ForceUnchecked,
   /// A user-defined conversion.
@@ -1009,6 +1009,12 @@ class ConstraintSystem {
   /// Argument labels fixed by the constraint solver.
   SmallVector<std::vector<Identifier>, 4> FixedArgLabels;
 
+  /// Conformances which solver "fixed" to help with
+  /// diagnosing problems related to generic requirements.
+  llvm::SmallDenseMap<std::pair<Expr *, unsigned>,
+                      std::pair<TypeBase *, ProtocolDecl *>>
+      MissingConformances;
+
   /// \brief The set of remembered disjunction choices used to reach
   /// the current constraint system.
   SmallVector<std::pair<ConstraintLocator*, unsigned>, 32>

test/Constraints/closures.swift

@@ -709,6 +709,6 @@ extension Node {
         return item.key == key
         // expected-error@-1 {{binary operator '==' cannot be applied to operands of type '_' and 'Self.K'}}
         // expected-note@-2 {{overloads for '==' exist with these partially matching parameter lists:}}
-      })
+      })!
   }
 }

test/Constraints/conditionally_defined_types.swift

@@ -75,18 +75,18 @@ let _ = Conforms<X>.Decl3.self
 let _ = Conforms<X>.Decl4<X>.self
 let _ = Conforms<X>.Decl5<X>.self
 
-let _ = Conforms<Y>.TypeAlias1.self // expected-error {{type 'Y' does not conform to protocol 'P'}}
-let _ = Conforms<Y>.TypeAlias2.self // expected-error {{type 'Y' does not conform to protocol 'P'}}
+let _ = Conforms<Y>.TypeAlias1.self // expected-error {{'Conforms<Y>.TypeAlias1' (aka 'Y') requires that 'Y' conform to 'P'}}
+let _ = Conforms<Y>.TypeAlias2.self // expected-error {{'Conforms<Y>.TypeAlias2' (aka 'Y') requires that 'Y' conform to 'P'}}
 let _ = Conforms<Y>.TypeAlias3<X>.self // expected-error {{type 'Y' does not conform to protocol 'P'}}
-let _ = Conforms<Y>.Decl1.self // expected-error {{type 'Y' does not conform to protocol 'P'}}
-let _ = Conforms<Y>.Decl2.self // expected-error {{type 'Y' does not conform to protocol 'P'}}
-let _ = Conforms<Y>.Decl3.self // expected-error {{type 'Y' does not conform to protocol 'P'}}
+let _ = Conforms<Y>.Decl1.self // expected-error {{'Conforms<Y>.Decl1' requires that 'Y' conform to 'P'}}
+let _ = Conforms<Y>.Decl2.self // expected-error {{'Conforms<Y>.Decl2' requires that 'Y' conform to 'P'}}
+let _ = Conforms<Y>.Decl3.self // expected-error {{'Conforms<Y>.Decl3' requires that 'Y' conform to 'P'}}
 let _ = Conforms<Y>.Decl4<X>.self // expected-error {{type 'Y' does not conform to protocol 'P'}}
 let _ = Conforms<Y>.Decl5<X>.self // expected-error {{type 'Y' does not conform to protocol 'P'}}
 
 extension Conforms: AssociatedType where T: P {}
 
-let _ = Conforms<Y>.T.self // expected-error {{type 'Y' does not conform to protocol 'P'}}
+let _ = Conforms<Y>.T.self // expected-error {{'Conforms<Y>.T' (aka 'Y') requires that 'Y' conform to 'P'}}
 
 let _ = Conforms<X>.T.self
 
@@ -196,20 +196,35 @@ let _ = Conforms<X>.Decl4<Z1>.Decl5<X>.self
 
 // Two different forms of badness, corresponding to the two requirements:
 
-let _ = Conforms<X>.Decl4<Y>.TypeAlias1.self // expected-error {{type 'Y' does not conform to protocol 'AssociatedType'}}
-let _ = Conforms<X>.Decl4<Y>.TypeAlias2.self // expected-error {{type 'Y' does not conform to protocol 'AssociatedType'}}
+let _ = Conforms<X>.Decl4<Y>.TypeAlias1.self
+// expected-error@-1 {{'Conforms<X>.Decl4<Y>.TypeAlias1' (aka 'X') requires that 'Y.T' conform to 'P'}}
+// expected-error@-2 {{'Conforms<X>.Decl4<Y>.TypeAlias1' (aka 'X') requires that 'Y' conform to 'AssociatedType'}}
+
+let _ = Conforms<X>.Decl4<Y>.TypeAlias2.self
+// expected-error@-1 {{'Conforms<X>.Decl4<Y>.TypeAlias2' (aka 'Y') requires that 'Y.T' conform to 'P'}}
+// expected-error@-2 {{'Conforms<X>.Decl4<Y>.TypeAlias2' (aka 'Y') requires that 'Y' conform to 'AssociatedType'}}
+
 let _ = Conforms<X>.Decl4<Y>.TypeAlias3<X>.self // expected-error {{type 'Y' does not conform to protocol 'AssociatedType'}}
-let _ = Conforms<X>.Decl4<Y>.Decl1.self // expected-error {{type 'Y' does not conform to protocol 'AssociatedType'}}
-let _ = Conforms<X>.Decl4<Y>.Decl2.self // expected-error {{type 'Y' does not conform to protocol 'AssociatedType'}}
-let _ = Conforms<X>.Decl4<Y>.Decl3.self // expected-error {{type 'Y' does not conform to protocol 'AssociatedType'}}
+let _ = Conforms<X>.Decl4<Y>.Decl1.self
+// expected-error@-1 {{'Conforms<X>.Decl4<Y>.Decl1' requires that 'Y.T' conform to 'P'}}
+// expected-error@-2 {{'Conforms<X>.Decl4<Y>.Decl1' requires that 'Y' conform to 'AssociatedType'}}
+
+let _ = Conforms<X>.Decl4<Y>.Decl2.self
+// expected-error@-1 {{'Conforms<X>.Decl4<Y>.Decl2' requires that 'Y.T' conform to 'P'}}
+// expected-error@-2 {{'Conforms<X>.Decl4<Y>.Decl2' requires that 'Y' conform to 'AssociatedType'}}
+
+let _ = Conforms<X>.Decl4<Y>.Decl3.self
+// expected-error@-1 {{'Conforms<X>.Decl4<Y>.Decl3' requires that 'Y.T' conform to 'P'}}
+// expected-error@-2 {{'Conforms<X>.Decl4<Y>.Decl3' requires that 'Y' conform to 'AssociatedType'}}
+
 let _ = Conforms<X>.Decl4<Y>.Decl4<X>.self // expected-error {{type 'Y' does not conform to protocol 'AssociatedType'}}
 let _ = Conforms<X>.Decl4<Y>.Decl5<X>.self // expected-error {{type 'Y' does not conform to protocol 'AssociatedType'}}
 
-let _ = Conforms<X>.Decl4<Z2>.TypeAlias1.self // expected-error {{type 'Z2.T' (aka 'Y') does not conform to protocol 'P'}}
-let _ = Conforms<X>.Decl4<Z2>.TypeAlias2.self // expected-error {{type 'Z2.T' (aka 'Y') does not conform to protocol 'P'}}
+let _ = Conforms<X>.Decl4<Z2>.TypeAlias1.self // expected-error {{'Conforms<X>.Decl4<Z2>.TypeAlias1' (aka 'X') requires that 'Z2.T' (aka 'Y') conform to 'P'}}
+let _ = Conforms<X>.Decl4<Z2>.TypeAlias2.self // expected-error {{'Conforms<X>.Decl4<Z2>.TypeAlias2' (aka 'Y') requires that 'Z2.T' (aka 'Y') conform to 'P'}}
 let _ = Conforms<X>.Decl4<Z2>.TypeAlias3<X>.self // expected-error {{type 'Z2.T' (aka 'Y') does not conform to protocol 'P'}}
-let _ = Conforms<X>.Decl4<Z2>.Decl1.self // expected-error {{type 'Z2.T' (aka 'Y') does not conform to protocol 'P'}}
-let _ = Conforms<X>.Decl4<Z2>.Decl2.self // expected-error {{type 'Z2.T' (aka 'Y') does not conform to protocol 'P'}}
-let _ = Conforms<X>.Decl4<Z2>.Decl3.self // expected-error {{type 'Z2.T' (aka 'Y') does not conform to protocol 'P'}}
+let _ = Conforms<X>.Decl4<Z2>.Decl1.self // expected-error {{'Conforms<X>.Decl4<Z2>.Decl1' requires that 'Z2.T' (aka 'Y') conform to 'P'}}
+let _ = Conforms<X>.Decl4<Z2>.Decl2.self // expected-error {{'Conforms<X>.Decl4<Z2>.Decl2' requires that 'Z2.T' (aka 'Y') conform to 'P'}}
+let _ = Conforms<X>.Decl4<Z2>.Decl3.self // expected-error {{'Conforms<X>.Decl4<Z2>.Decl3' requires that 'Z2.T' (aka 'Y') conform to 'P'}}
 let _ = Conforms<X>.Decl4<Z2>.Decl4<X>.self // expected-error {{type 'Z2.T' (aka 'Y') does not conform to protocol 'P'}}
 let _ = Conforms<X>.Decl4<Z2>.Decl5<X>.self // expected-error {{type 'Z2.T' (aka 'Y') does not conform to protocol 'P'}}

test/Constraints/diagnostics.swift

@@ -94,10 +94,10 @@ func f7() -> (c: Int, v: A) {
 }
 
 func f8<T:P2>(_ n: T, _ f: @escaping (T) -> T) {}
-f8(3, f4) // expected-error {{in argument type '(Int) -> Int', 'Int' does not conform to expected type 'P2'}}
+f8(3, f4) // expected-error {{argument type 'Int' does not conform to expected type 'P2'}}
 typealias Tup = (Int, Double)
 func f9(_ x: Tup) -> Tup { return x }
-f8((1,2.0), f9) // expected-error {{in argument type '(Tup) -> Tup' (aka '((Int, Double)) -> (Int, Double)'), 'Tup' (aka '(Int, Double)') does not conform to expected type 'P2'}}
+f8((1,2.0), f9) // expected-error {{'(Tup, @escaping (Tup) -> Tup) -> ()' (aka '((Int, Double), @escaping ((Int, Double)) -> (Int, Double)) -> ()') requires that '(_, _)' conform to 'P2'}}
 
 // <rdar://problem/19658691> QoI: Incorrect diagnostic for calling nonexistent members on literals
 1.doesntExist(0)  // expected-error {{value of type 'Int' has no member 'doesntExist'}}
@@ -657,8 +657,7 @@ example21890157.property = "confusing"  // expected-error {{value of optional ty
 
 struct UnaryOp {}
 
-_ = -UnaryOp() // expected-error {{unary operator '-' cannot be applied to an operand of type 'UnaryOp'}}
-// expected-note @-1 {{overloads for '-' exist with these partially matching parameter lists: (Float), (Double)}}
+_ = -UnaryOp() // expected-error {{argument type 'UnaryOp' does not conform to expected type 'SignedNumeric'}}
 
 
 // <rdar://problem/23433271> Swift compiler segfault in failure diagnosis
@@ -1190,10 +1189,9 @@ func rdar17170728() {
 // https://bugs.swift.org/browse/SR-5934 - failure to emit diagnostic for bad
 // generic constraints
 func elephant<T, U>(_: T) where T : Collection, T.Element == U, T.Element : Hashable {}
-// expected-note@-1 {{in call to function 'elephant'}}
 
 func platypus<T>(a: [T]) {
-    _ = elephant(a) // expected-error {{generic parameter 'U' could not be inferred}}
+    _ = elephant(a) // expected-error {{'([T]) -> ()' requires that 'T' conform to 'Hashable'}}
 }
 
 // Another case of the above.