Merge pull request #35281 from xedin/rdar-72819046

[Diagnostics] Special case requirement failures related to `return` statement/expression

Author: xedin

include/swift/Sema/ConstraintLocator.h

@@ -762,6 +762,20 @@ class LocatorPathElt::ArgumentAttribute final : public StoredIntegerElement<1> {
   }
 };
 
+class LocatorPathElt::ConformanceRequirement final
+    : public StoredPointerElement<ProtocolDecl> {
+public:
+  ConformanceRequirement(ProtocolDecl *protocol)
+      : StoredPointerElement(PathElementKind::ConformanceRequirement,
+                             protocol) {}
+
+  ProtocolDecl *getRequirement() const { return getStoredPointer(); }
+
+  static bool classof(const LocatorPathElt *elt) {
+    return elt->getKind() == ConstraintLocator::ConformanceRequirement;
+  }
+};
+
 namespace details {
   template <typename CustomPathElement>
   class PathElement {

include/swift/Sema/ConstraintLocatorPathElts.def

@@ -195,6 +195,9 @@ CUSTOM_LOCATOR_PATH_ELT(ArgumentAttribute)
 /// The result of a chain of member accesses off an UnresolvedMemberExpr
 SIMPLE_LOCATOR_PATH_ELT(UnresolvedMemberChainResult)
 
+/// The conformance requirement associated with a type.
+CUSTOM_LOCATOR_PATH_ELT(ConformanceRequirement)
+
 #undef LOCATOR_PATH_ELT
 #undef CUSTOM_LOCATOR_PATH_ELT
 #undef SIMPLE_LOCATOR_PATH_ELT

lib/Sema/CSApply.cpp

@@ -7972,10 +7972,12 @@ static Optional<SolutionApplicationTarget> applySolutionToForEachStmt(
   auto stmt = forEachStmtInfo.stmt;
   auto sequenceProto = TypeChecker::getProtocol(
       cs.getASTContext(), stmt->getForLoc(), KnownProtocolKind::Sequence);
-  auto contextualLocator = cs.getConstraintLocator(
-      target.getAsExpr(), LocatorPathElt::ContextualType());
-  auto sequenceConformance = solution.resolveConformance(
-      contextualLocator, sequenceProto);
+  auto contextualLocator = solution.getConstraintLocator(
+      target.getAsExpr(),
+      {LocatorPathElt::ContextualType(),
+       LocatorPathElt::ConformanceRequirement(sequenceProto)});
+  auto sequenceConformance =
+      solution.resolveConformance(contextualLocator, sequenceProto);
   assert(!sequenceConformance.isInvalid() &&
          "Couldn't find sequence conformance");
 
@@ -8386,32 +8388,30 @@ Expr *Solution::coerceToType(Expr *expr, Type toType,
 
 ProtocolConformanceRef Solution::resolveConformance(
     ConstraintLocator *locator, ProtocolDecl *proto) {
-  for (const auto &conformance : Conformances) {
-    if (conformance.first != locator)
-      continue;
-    if (conformance.second.getRequirement() != proto)
-      continue;
-
-    // If the conformance doesn't require substitution, return it immediately.
-    auto conformanceRef = conformance.second;
-    if (conformanceRef.isAbstract())
-      return conformanceRef;
-
-    auto concrete = conformanceRef.getConcrete();
-    auto conformingType = concrete->getType();
-    if (!conformingType->hasTypeVariable())
-      return conformanceRef;
-
-    // Substitute into the conformance type, then look for a conformance
-    // again.
-    // FIXME: Should be able to perform the substitution using the Solution
-    // itself rather than another conforms-to-protocol check.
-    Type substConformingType = simplifyType(conformingType);
-    return TypeChecker::conformsToProtocol(
-        substConformingType, proto, constraintSystem->DC);
-  }
-
-  return ProtocolConformanceRef::forInvalid();
+  auto conformance = llvm::find_if(Conformances, [&locator](const auto &elt) {
+    return elt.first == locator;
+  });
+
+  if (conformance == Conformances.end())
+    return ProtocolConformanceRef::forInvalid();
+
+  // If the conformance doesn't require substitution, return it immediately.
+  auto conformanceRef = conformance->second;
+  if (conformanceRef.isAbstract())
+    return conformanceRef;
+
+  auto concrete = conformanceRef.getConcrete();
+  auto conformingType = concrete->getType();
+  if (!conformingType->hasTypeVariable())
+    return conformanceRef;
+
+  // Substitute into the conformance type, then look for a conformance
+  // again.
+  // FIXME: Should be able to perform the substitution using the Solution
+  // itself rather than another conforms-to-protocol check.
+  Type substConformingType = simplifyType(conformingType);
+  return TypeChecker::conformsToProtocol(substConformingType, proto,
+                                         constraintSystem->DC);
 }
 
 bool Solution::hasType(ASTNode node) const {

lib/Sema/CSDiagnostics.cpp

@@ -211,12 +211,12 @@ ProtocolConformance *RequirementFailure::getConformanceForConditionalReq(
     return nullptr;
 
   auto path = locator->getPath();
-  auto *typeReqLoc = getConstraintLocator(getRawAnchor(), path.drop_back());
+  auto *conformanceLoc = getConstraintLocator(getRawAnchor(), path.drop_back());
 
   auto result = llvm::find_if(
       solution.Conformances,
       [&](const std::pair<ConstraintLocator *, ProtocolConformanceRef>
-              &conformance) { return conformance.first == typeReqLoc; });
+              &conformance) { return conformance.first == conformanceLoc; });
   assert(result != solution.Conformances.end());
 
   auto conformance = result->second;
@@ -249,8 +249,28 @@ ValueDecl *RequirementFailure::getDeclRef() const {
     return getAffectedDeclFromType(func->getResultInterfaceType());
   }
 
-  if (isFromContextualType())
-    return getAffectedDeclFromType(getContextualType(getRawAnchor()));
+  if (isFromContextualType()) {
+    auto anchor = getRawAnchor();
+    auto contextualPurpose = getContextualTypePurpose(anchor);
+    auto contextualTy = getContextualType(anchor);
+
+    // If the issue is a mismatch between `return` statement/expression
+    // and its contextual requirements, it means that affected declaration
+    // is a declarer of a contextual "result" type e.g. member of a
+    // type, local function etc.
+    if (contextualPurpose == CTP_ReturnStmt ||
+        contextualPurpose == CTP_ReturnSingleExpr) {
+      // In case of opaque result type, let's point to the declaration
+      // associated with the type itself (since it has one) instead of
+      // declarer.
+      if (auto *opaque = contextualTy->getAs<OpaqueTypeArchetypeType>())
+        return opaque->getDecl();
+
+      return cast<ValueDecl>(getDC()->getAsDecl());
+    }
+
+    return getAffectedDeclFromType(contextualTy);
+  }
 
   if (auto overload = getCalleeOverloadChoiceIfAvailable(getLocator())) {
     // If there is a declaration associated with this

lib/Sema/CSSimplify.cpp

@@ -5716,21 +5716,23 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyConformsToConstraint(
   /// Record the given conformance as the result, adding any conditional
   /// requirements if necessary.
   auto recordConformance = [&](ProtocolConformanceRef conformance) {
+    auto *conformanceLoc = getConstraintLocator(
+        loc, LocatorPathElt::ConformanceRequirement(protocol));
     // Record the conformance.
-    CheckedConformances.push_back({loc, conformance});
+    CheckedConformances.push_back({conformanceLoc, conformance});
 
-    if (isConformanceUnavailable(conformance, loc))
+    if (isConformanceUnavailable(conformance, conformanceLoc))
       increaseScore(SK_Unavailable);
 
     // This conformance may be conditional, in which case we need to consider
     // those requirements as constraints too.
     if (conformance.isConcrete()) {
       unsigned index = 0;
       for (const auto &req : conformance.getConditionalRequirements()) {
-        addConstraint(req,
-                      locator.withPathElement(
-                          LocatorPathElt::ConditionalRequirement(
-                              index++, req.getKind())));
+        addConstraint(
+            req, getConstraintLocator(conformanceLoc,
+                                      LocatorPathElt::ConditionalRequirement(
+                                          index++, req.getKind())));
       }
     }
 

lib/Sema/ConstraintLocator.cpp

@@ -66,6 +66,7 @@ unsigned LocatorPathElt::getNewSummaryFlags() const {
   case ConstraintLocator::KeyPathComponent:
   case ConstraintLocator::ConditionalRequirement:
   case ConstraintLocator::TypeParameterRequirement:
+  case ConstraintLocator::ConformanceRequirement:
   case ConstraintLocator::ImplicitlyUnwrappedDisjunctionChoice:
   case ConstraintLocator::DynamicLookupResult:
   case ConstraintLocator::ContextualType:
@@ -397,6 +398,14 @@ void ConstraintLocator::dump(SourceManager *sm, raw_ostream &out) const {
       break;
     }
 
+    case ConformanceRequirement: {
+      auto reqElt = elt.castTo<LocatorPathElt::ConformanceRequirement>();
+      out << "conformance requirement (";
+      reqElt.getRequirement()->dumpRef(out);
+      out << ")";
+      break;
+    }
+
     case ImplicitlyUnwrappedDisjunctionChoice:
       out << "implicitly unwrapped disjunction choice";
       break;

lib/Sema/ConstraintSystem.cpp

@@ -5181,15 +5181,15 @@ static Optional<Requirement> getRequirement(ConstraintSystem &cs,
 
   if (reqLoc->isConditionalRequirement()) {
     auto path = reqLocator->getPath();
-    auto *typeReqLoc =
+    auto *conformanceLoc =
         cs.getConstraintLocator(reqLocator->getAnchor(), path.drop_back());
 
     auto conformances = cs.getCheckedConformances();
     auto result = llvm::find_if(
         conformances,
-        [&typeReqLoc](
+        [&conformanceLoc](
             const std::pair<ConstraintLocator *, ProtocolConformanceRef>
-                &conformance) { return conformance.first == typeReqLoc; });
+                &conformance) { return conformance.first == conformanceLoc; });
     assert(result != conformances.end());
 
     auto conformance = result->second;

test/Constraints/sr13992.swift

@@ -0,0 +1,29 @@
+// RUN: %target-typecheck-verify-swift
+
+// SR-13992
+
+protocol V {}
+
+protocol P1 {}
+protocol P2 {
+  func bar() -> V
+}
+protocol P3 {}
+
+struct S<T> {
+  var foo: T
+}
+
+extension S : P1 {}
+extension S : P2 where T: P3 { // expected-note {{requirement from conditional conformance of 'S<V>' to 'P2'}}
+  func bar() -> V { fatalError() }
+}
+
+struct Q {
+  var foo: V
+
+  func test() -> P1 & P2 {
+    S(foo: foo) // expected-error {{protocol 'V' as a type cannot conform to 'P3'}}
+    // expected-note@-1 {{only concrete types such as structs, enums and classes can conform to protocols}}
+  }
+}