Merge pull request #11236 from xedin/rdar-33477726-4.0

[4.0] [QoI] Improve diagnostics of unsatisfied generic requirements

Author: ematejska

include/swift/AST/DiagnosticsSema.def

@@ -1387,6 +1387,14 @@ NOTE(type_does_not_inherit_requirement,none,
 ERROR(types_not_equal,none,
       "%0 requires the types %1 and %2 be equivalent",
       (Type, Type, Type))
+NOTE(candidate_types_equal_requirement,none,
+     "candidate requires that the types %0 and %1 be equivalent "
+     "(requirement specified as %2 == %3%4)",
+     (Type, Type, Type, Type, StringRef))
+NOTE(candidate_types_inheritance_requirement,none,
+     "candidate requires that %1 inherit from %2 "
+     "(requirement specified as %2 : %3%4)",
+     (Type, Type, Type, Type, StringRef))
 NOTE(types_not_equal_requirement,none,
      "requirement specified as %0 == %1%2", (Type, Type, StringRef))
 ERROR(non_class_cannot_conform_to_class_protocol,none,

lib/Sema/CSDiag.cpp

@@ -2152,8 +2152,8 @@ class FailureDiagnosis :public ASTVisitor<FailureDiagnosis, /*exprresult*/bool>{
 
   /// Produce diagnostic for failures related to unfulfilled requirements
   /// of the generic parameters used as arguments.
-  bool diagnoseArgumentGenericRequirements(TypeChecker &TC, Expr *fnExpr,
-                                           Expr *argExpr,
+  bool diagnoseArgumentGenericRequirements(TypeChecker &TC, Expr *callExpr,
+                                           Expr *fnExpr, Expr *argExpr,
                                            CalleeCandidateInfo &candidates,
                                            ArrayRef<Identifier> argLabels);
 
@@ -5886,16 +5886,18 @@ bool FailureDiagnosis::diagnoseMethodAttributeFailures(
 }
 
 bool FailureDiagnosis::diagnoseArgumentGenericRequirements(
-    TypeChecker &TC, Expr *fnExpr, Expr *argExpr,
+    TypeChecker &TC, Expr *callExpr, Expr *fnExpr, Expr *argExpr,
     CalleeCandidateInfo &candidates, ArrayRef<Identifier> argLabels) {
   if (candidates.closeness != CC_ExactMatch || candidates.size() != 1)
     return false;
 
-  auto DRE = dyn_cast<DeclRefExpr>(fnExpr);
-  if (!DRE)
-    return false;
+  AbstractFunctionDecl *AFD = nullptr;
+  if (auto *DRE = dyn_cast<DeclRefExpr>(fnExpr)) {
+    AFD = dyn_cast<AbstractFunctionDecl>(DRE->getDecl());
+  } else if (auto *candidate = candidates[0].getDecl()) {
+    AFD = dyn_cast<AbstractFunctionDecl>(candidate);
+  }
 
-  auto AFD = dyn_cast<AbstractFunctionDecl>(DRE->getDecl());
   if (!AFD || !AFD->isGeneric() || !AFD->hasInterfaceType())
     return false;
 
@@ -5920,7 +5922,9 @@ bool FailureDiagnosis::diagnoseArgumentGenericRequirements(
   // requirements e.g. <A, B where A.Element == B.Element>.
   for (unsigned i = 0, e = bindings.size(); i != e; ++i) {
     auto param = params[i];
-    auto archetype = param.Ty->getAs<ArchetypeType>();
+    auto paramType = param.Ty->getInOutObjectType();
+
+    auto archetype = paramType->getAs<ArchetypeType>();
     if (!archetype)
       continue;
 
@@ -5948,18 +5952,88 @@ bool FailureDiagnosis::diagnoseArgumentGenericRequirements(
     return false;
 
   class RequirementsListener : public GenericRequirementsCheckListener {
+    TypeChecker &TC;
+    AbstractFunctionDecl *Candidate;
+    TypeSubstitutionFn Substitutions;
+
+    Expr *CallExpr;
+    Expr *FnExpr;
+    Expr *ArgExpr;
+
   public:
+    RequirementsListener(TypeChecker &TC, AbstractFunctionDecl *AFD,
+                         TypeSubstitutionFn subs,
+                         Expr *callExpr, Expr *fnExpr, Expr *argExpr)
+        : TC(TC), Candidate(AFD), Substitutions(subs), CallExpr(callExpr),
+          FnExpr(fnExpr), ArgExpr(argExpr) {}
+
     bool shouldCheck(RequirementKind kind, Type first, Type second) override {
       // This means that we have encountered requirement which references
       // generic parameter not used in the arguments, we can't diagnose it here.
       return !(first->hasTypeParameter() || first->isTypeVariableOrMember());
     }
+
+    bool diagnoseUnsatisfiedRequirement(const Requirement &req, Type first,
+                                        Type second) override {
+      Diag<Type, Type, Type, Type, StringRef> note;
+      switch (req.getKind()) {
+      case RequirementKind::Conformance:
+      case RequirementKind::Layout:
+        return false;
+
+      case RequirementKind::Superclass:
+        note = diag::candidate_types_inheritance_requirement;
+        break;
+
+      case RequirementKind::SameType:
+        note = diag::candidate_types_equal_requirement;
+        break;
+      }
+
+      SmallVector<char, 8> scratch;
+      auto overloadName = Candidate->getFullName().getString(scratch);
+
+      if (isa<BinaryExpr>(CallExpr) && isa<TupleExpr>(ArgExpr)) {
+        auto argTuple = cast<TupleExpr>(ArgExpr);
+        auto lhsExpr = argTuple->getElement(0),
+             rhsExpr = argTuple->getElement(1);
+        auto lhsType = lhsExpr->getType()->getRValueType();
+        auto rhsType = rhsExpr->getType()->getRValueType();
+
+        TC.diagnose(FnExpr->getLoc(), diag::cannot_apply_binop_to_args,
+                    overloadName, lhsType, rhsType)
+            .highlight(lhsExpr->getSourceRange())
+            .highlight(rhsExpr->getSourceRange());
+      } else if (isa<PrefixUnaryExpr>(CallExpr) ||
+                 isa<PostfixUnaryExpr>(CallExpr)) {
+        TC.diagnose(ArgExpr->getLoc(), diag::cannot_apply_unop_to_arg,
+                    overloadName, ArgExpr->getType());
+      } else {
+        bool isInitializer = isa<ConstructorDecl>(Candidate);
+        TC.diagnose(ArgExpr->getLoc(), diag::cannot_call_with_params,
+                    overloadName, getTypeListString(ArgExpr->getType()),
+                    isInitializer);
+      }
+
+      auto rawFirstType = req.getFirstType();
+      auto rawSecondType = req.getSecondType();
+      auto *genericSig = Candidate->getGenericSignature();
+
+      TC.diagnose(Candidate, note, first, second,
+                  rawFirstType, rawSecondType,
+                  genericSig->gatherGenericParamBindingsText(
+                                 {rawFirstType, rawSecondType}, Substitutions));
+      return true;
+    }
   };
 
-  auto dc = env->getOwningDeclContext();
-  RequirementsListener genericReqListener;
+  auto *dc = env->getOwningDeclContext();
+  auto substitutionFn = QueryTypeSubstitutionMap{substitutions};
+  RequirementsListener genericReqListener(TC, AFD, substitutionFn,
+                                          callExpr, fnExpr, argExpr);
+
   auto result = TC.checkGenericArguments(
-      dc, argExpr->getLoc(), AFD->getLoc(), AFD->getInterfaceType(),
+      dc, callExpr->getLoc(), fnExpr->getLoc(), AFD->getInterfaceType(),
       env->getGenericSignature(), QueryTypeSubstitutionMap{substitutions},
       LookUpConformanceInModule{dc->getParentModule()}, nullptr,
       ConformanceCheckFlags::SuppressDependencyTracking, &genericReqListener);
@@ -6662,6 +6736,10 @@ bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
       }
     }
 
+    if (diagnoseArgumentGenericRequirements(CS->TC, callExpr, fnExpr, argExpr,
+                                            calleeInfo, argLabels))
+      return true;
+
     if (isContextualConversionFailure(argTuple))
       return false;
 
@@ -6695,13 +6773,13 @@ bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
     return true;
   }
 
-  // If all of the arguments are a perfect match, so let's check if there
+  // If all of the arguments are a perfect match, let's check if there
   // are problems with requirements placed on generic parameters, because
   // CalleeCandidateInfo validates only conformance of the parameters
   // to their protocol types (if any) but it doesn't check additional
   // requirements placed on e.g. nested types or between parameters.
-  if (diagnoseArgumentGenericRequirements(CS->TC, fnExpr, argExpr, calleeInfo,
-                                          argLabels))
+  if (diagnoseArgumentGenericRequirements(CS->TC, callExpr, fnExpr, argExpr,
+                                          calleeInfo, argLabels))
     return true;
 
   if (isContextualConversionFailure(argExpr))

lib/Sema/TypeCheckConstraints.cpp

@@ -1639,6 +1639,11 @@ void GenericRequirementsCheckListener::satisfiedConformance(
                                           ProtocolConformanceRef conformance) {
 }
 
+bool GenericRequirementsCheckListener::diagnoseUnsatisfiedRequirement(
+    const Requirement &req, Type first, Type second) {
+  return false;
+}
+
 bool TypeChecker::
 solveForExpression(Expr *&expr, DeclContext *dc, Type convertType,
                    FreeTypeVariableBinding allowFreeTypeVariables,

lib/Sema/TypeCheckGeneric.cpp

@@ -1300,9 +1300,13 @@ RequirementCheckResult TypeChecker::checkGenericArguments(
       secondType = req->getSecondType();
     }
 
+    bool requirementFailure = false;
     if (listener && !listener->shouldCheck(kind, firstType, secondType))
       continue;
 
+    Diag<Type, Type, Type> diagnostic;
+    Diag<Type, Type, StringRef> diagnosticNote;
+
     switch (kind) {
     case RequirementKind::Conformance: {
       // Protocol conformance requirements.
@@ -1311,6 +1315,8 @@ RequirementCheckResult TypeChecker::checkGenericArguments(
       // or non-private dependency.
       // FIXME: Do we really need "used" at this point?
       // FIXME: Poor location information. How much better can we do here?
+      // FIXME: This call should support listener to be able to properly
+      //        diagnose problems with conformances.
       auto result =
           conformsToProtocol(firstType, proto->getDecl(), dc,
                              conformanceOptions, loc, unsatisfiedDependency);
@@ -1343,37 +1349,37 @@ RequirementCheckResult TypeChecker::checkGenericArguments(
     case RequirementKind::Superclass:
       // Superclass requirements.
       if (!isSubclassOf(firstType, secondType, dc)) {
-        if (loc.isValid()) {
-          // FIXME: Poor source-location information.
-          diagnose(loc, diag::type_does_not_inherit, owner, firstType,
-                   secondType);
-
-          diagnose(noteLoc, diag::type_does_not_inherit_requirement,
-                   rawFirstType, rawSecondType,
-                   genericSig->gatherGenericParamBindingsText(
-                       {rawFirstType, rawSecondType}, substitutions));
-        }
-
-        return RequirementCheckResult::Failure;
+        diagnostic = diag::type_does_not_inherit;
+        diagnosticNote = diag::type_does_not_inherit_requirement;
+        requirementFailure = true;
       }
-      continue;
+      break;
 
     case RequirementKind::SameType:
       if (!firstType->isEqual(secondType)) {
-        if (loc.isValid()) {
-          // FIXME: Better location info for both diagnostics.
-          diagnose(loc, diag::types_not_equal, owner, firstType, secondType);
-
-          diagnose(noteLoc, diag::types_not_equal_requirement, rawFirstType,
-                   rawSecondType,
-                   genericSig->gatherGenericParamBindingsText(
-                       {rawFirstType, rawSecondType}, substitutions));
-        }
-
-        return RequirementCheckResult::Failure;
+        diagnostic = diag::types_not_equal;
+        diagnosticNote = diag::types_not_equal_requirement;
+        requirementFailure = true;
       }
+      break;
+    }
+
+    if (!requirementFailure)
       continue;
+
+    if (listener &&
+        listener->diagnoseUnsatisfiedRequirement(rawReq, firstType, secondType))
+      return RequirementCheckResult::Failure;
+
+    if (loc.isValid()) {
+      // FIXME: Poor source-location information.
+      diagnose(loc, diagnostic, owner, firstType, secondType);
+      diagnose(noteLoc, diagnosticNote, rawFirstType, rawSecondType,
+               genericSig->gatherGenericParamBindingsText(
+                   {rawFirstType, rawSecondType}, substitutions));
     }
+
+    return RequirementCheckResult::Failure;
   }
 
   if (valid)

lib/Sema/TypeChecker.h

@@ -372,6 +372,20 @@ class GenericRequirementsCheckListener {
   /// \param conformance The conformance itself.
   virtual void satisfiedConformance(Type depTy, Type replacementTy,
                                     ProtocolConformanceRef conformance);
+
+  /// Callback to diagnose problem with unsatisfied generic requirement.
+  ///
+  /// \param req The unsatisfied generic requirement.
+  ///
+  /// \param first The left-hand side type assigned to the requirement,
+  /// possibly represented by its generic substitute.
+  ///
+  /// \param second The right-hand side type assigned to the requirement,
+  /// possibly represented by its generic substitute.
+  ///
+  /// \returns true if problem has been diagnosed, false otherwise.
+  virtual bool diagnoseUnsatisfiedRequirement(const Requirement &req,
+                                              Type first, Type second);
 };
 
 /// The result of `checkGenericRequirement`.

test/Constraints/generics.swift

@@ -248,10 +248,12 @@ struct S27515965 : P27515965 {
 
 struct V27515965 {
   init<T : P27515965>(_ tp: T) where T.R == Float {}
+  // expected-note@-1 {{candidate requires that the types 'Any' and 'Float' be equivalent (requirement specified as 'T.R' == 'Float' [with T = S27515965])}}
 }
 
 func test(x: S27515965) -> V27515965 {
-  return V27515965(x) // expected-error {{generic parameter 'T' could not be inferred}}
+  return V27515965(x)
+  // expected-error@-1 {{cannot invoke initializer for type 'init(_:)' with an argument list of type '(S27515965)'}}
 }
 
 protocol BaseProto {}

test/Generics/deduction.swift

@@ -216,11 +216,12 @@ func callRangeOfIsBefore(_ ia: [Int], da: [Double]) {
 }
 
 func testEqualIterElementTypes<A: IteratorProtocol, B: IteratorProtocol>(_ a: A, _ b: B) where A.Element == B.Element {}
-// expected-note@-1 {{requirement specified as 'A.Element' == 'B.Element' [with A = IndexingIterator<[Int]>, B = IndexingIterator<[Double]>]}}
+// expected-note@-1 {{candidate requires that the types 'Int' and 'Double' be equivalent (requirement specified as 'A.Element' == 'B.Element' [with A = IndexingIterator<[Int]>, B = IndexingIterator<[Double]>])}}
 func compareIterators() {
   var a: [Int] = []
   var b: [Double] = []
-  testEqualIterElementTypes(a.makeIterator(), b.makeIterator()) // expected-error {{'<A, B where A : IteratorProtocol, B : IteratorProtocol, A.Element == B.Element> (A, B) -> ()' requires the types 'Int' and 'Double' be equivalent}}
+  testEqualIterElementTypes(a.makeIterator(), b.makeIterator())
+  // expected-error@-1 {{cannot invoke 'testEqualIterElementTypes(_:_:)' with an argument list of type '(IndexingIterator<[Int]>, IndexingIterator<[Double]>)'}}
 }
 
 protocol P_GI {
@@ -233,9 +234,9 @@ class C_GI : P_GI {
 
 class GI_Diff {}
 func genericInheritsA<T>(_ x: T) where T : P_GI, T.Y : GI_Diff {}
-// expected-note@-1 {{requirement specified as 'T.Y' : 'GI_Diff' [with T = C_GI]}}
-genericInheritsA(C_GI()) // expected-error {{<T where T : P_GI, T.Y : GI_Diff> (T) -> ()' requires that 'C_GI.Y' (aka 'Double') inherit from 'GI_Diff'}}
-
+// expected-note@-1 {{candidate requires that 'GI_Diff' inherit from 'T.Y' (requirement specified as 'T.Y' : 'GI_Diff' [with T = C_GI])}}
+genericInheritsA(C_GI())
+// expected-error@-1 {{cannot invoke 'genericInheritsA(_:)' with an argument list of type '(C_GI)'}}
 
 //===----------------------------------------------------------------------===//
 // Deduction for member operators
@@ -318,3 +319,24 @@ func foo() {
     let l = min(3, oi) // expected-error{{value of optional type 'Int?' not unwrapped; did you mean to use '!' or '?'?}}
 }
 
+infix operator +&
+func +&<R, S>(lhs: inout R, rhs: S) where R : RangeReplaceableCollection, S : Sequence, R.Element == S.Element {}
+// expected-note@-1 {{candidate requires that the types 'String' and 'String.Element' (aka 'Character') be equivalent (requirement specified as 'R.Element' == 'S.Element' [with R = [String], S = String])}}
+
+func rdar33477726_1() {
+  var arr: [String] = []
+  arr +& "hello"
+  // expected-error@-1 {{binary operator '+&(_:_:)' cannot be applied to operands of type '[String]' and 'String'}}
+}
+
+func rdar33477726_2<R, S>(_: R, _: S) where R: Sequence, S == R.Element {}
+// expected-note@-1 {{candidate requires that the types 'Int' and 'String.Element' (aka 'Character') be equivalent (requirement specified as 'S' == 'R.Element' [with R = String, S = Int])}}
+rdar33477726_2("answer", 42)
+// expected-error@-1 {{cannot invoke 'rdar33477726_2(_:_:)' with an argument list of type '(String, Int)'}}
+
+prefix operator +-
+prefix func +-<T>(_: T) where T: Sequence, T.Element == Int {}
+// expected-note@-1 {{candidate requires that the types 'String.Element' (aka 'Character') and 'Int' be equivalent (requirement specified as 'T.Element' == 'Int' [with T = String])}}
+
++-"hello"
+// expected-error@-1 {{unary operator '+-(_:)' cannot be applied to an operand of type 'String'}}

test/decl/protocol/req/recursion.swift

@@ -48,7 +48,7 @@ public struct S<A: P> where A.T == S<A> {
 // expected-error@-2 {{generic struct 'S' references itself}}
   func f(a: A.T) {
     g(a: id(t: a))
-    // expected-error@-1 {{cannot convert value of type 'A.T' to expected argument type 'S<_>'}}
+    // expected-error@-1 {{generic parameter 'T' could not be inferred}}
     _ = A.T.self
   }