Merge pull request swiftlang#38082 from hborla/tuple-locator-refactoring

[ConstraintSystem] Store tuple types in the locator when matching element types.

Author: hborla

include/swift/Sema/ConstraintLocator.h

@@ -581,6 +581,20 @@ class LocatorPathElt::SynthesizedArgument final : public StoredIntegerElement<2>
   }
 };
 
+class LocatorPathElt::TupleType : public StoredPointerElement<TypeBase> {
+public:
+  TupleType(Type type)
+      : StoredPointerElement(PathElementKind::TupleType, type.getPointer()) {
+    assert(type->getDesugaredType()->is<swift::TupleType>());
+  }
+
+  Type getType() const { return getStoredPointer(); }
+
+  static bool classof(const LocatorPathElt *elt) {
+    return elt->getKind() == PathElementKind::TupleType;
+  }
+};
+
 /// Abstract superclass for any kind of tuple element.
 class LocatorPathElt::AnyTupleElement : public StoredIntegerElement<1> {
 protected:

include/swift/Sema/ConstraintLocatorPathElts.def

@@ -164,6 +164,10 @@ SIMPLE_LOCATOR_PATH_ELT(SubscriptMember)
 /// The missing argument synthesized by the solver.
 CUSTOM_LOCATOR_PATH_ELT(SynthesizedArgument)
 
+/// A tuple type, which provides context for subsequent tuple element
+/// path components.
+CUSTOM_LOCATOR_PATH_ELT(TupleType)
+
 /// Tuple elements.
 ABSTRACT_LOCATOR_PATH_ELT(AnyTupleElement)
   /// A tuple element referenced by position.

lib/Sema/CSDiagnostics.cpp

@@ -3177,7 +3177,7 @@ bool TupleContextualFailure::diagnoseAsError() {
   auto purpose = getContextualTypePurpose();
   if (isNumElementsMismatch())
     diagnostic = diag::tuple_types_not_convertible_nelts;
-  else if ((purpose == CTP_Initialization) && !getContextualType(getAnchor()))
+  else if (purpose == CTP_Unused)
     diagnostic = diag::tuple_types_not_convertible;
   else if (auto diag = getDiagnosticFor(purpose, getToType()))
     diagnostic = *diag;

lib/Sema/CSFix.cpp

@@ -453,26 +453,19 @@ bool AllowTupleTypeMismatch::coalesceAndDiagnose(
     indices.push_back(*tupleFix->Index);
   }
 
-  auto &cs = getConstraintSystem();
   auto *locator = getLocator();
   ContextualTypePurpose purpose;
-  Type fromType;
-  Type toType;
-
-  if (getFromType()->is<TupleType>() && getToType()->is<TupleType>()) {
-    purpose = cs.getContextualTypePurpose(locator->getAnchor());
-    fromType = getFromType();
-    toType = getToType();
-  } else if (auto contextualTypeInfo =
-                 getStructuralTypeContext(solution, locator)) {
-    std::tie(purpose, fromType, toType) = *contextualTypeInfo;
+  if (isExpr<CoerceExpr>(locator->getAnchor())) {
+    purpose = CTP_CoerceOperand;
+  } else if (auto *assignExpr = getAsExpr<AssignExpr>(locator->getAnchor())) {
+    purpose = isa<SubscriptExpr>(assignExpr->getDest()) ? CTP_SubscriptAssignSource
+                                                        : CTP_AssignSource;
   } else {
-    return false;
+    auto &cs = getConstraintSystem();
+    purpose = cs.getContextualTypePurpose(locator->getAnchor());
   }
 
-  TupleContextualFailure failure(solution, purpose,
-                                 fromType->lookThroughAllOptionalTypes(),
-                                 toType->lookThroughAllOptionalTypes(),
+  TupleContextualFailure failure(solution, purpose, getFromType(), getToType(),
                                  indices, locator);
   return failure.diagnose(asNote);
 }

lib/Sema/CSSimplify.cpp

@@ -1547,6 +1547,9 @@ ConstraintSystem::matchTupleTypes(TupleType *tuple1, TupleType *tuple2,
       SmallVector<LocatorPathElt, 4> path;
       (void)locator.getLocatorParts(path);
 
+      while (!path.empty() && path.back().is<LocatorPathElt::TupleType>())
+        path.pop_back();
+
       if (!path.empty()) {
         // Direct pattern matching between tuple pattern and tuple type.
         if (path.back().is<LocatorPathElt::PatternMatch>()) {
@@ -4723,6 +4726,15 @@ bool ConstraintSystem::repairFailures(
     // mismatches within the same tuple type can be coalesced later.
     auto index = elt.getAs<LocatorPathElt::TupleElement>()->getIndex();
     path.pop_back();
+
+    // Drop the tuple type path elements too, but extract each tuple type first.
+    if (path.back().is<LocatorPathElt::TupleType>()) {
+      rhs = path.back().getAs<LocatorPathElt::TupleType>()->getType();
+      path.pop_back();
+      lhs = path.back().getAs<LocatorPathElt::TupleType>()->getType();
+      path.pop_back();
+    }
+
     auto *tupleLocator = getConstraintLocator(locator.getAnchor(), path);
 
     // Let this fail if it's a contextual mismatch with sequence element types,
@@ -5225,9 +5237,15 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
     }
 
     case TypeKind::Tuple: {
+      // Add each tuple type to the locator before matching the element types.
+      // This is useful for diagnostics, because the error message can use the
+      // full tuple type for several element mismatches. Use the original types
+      // to preserve sugar such as typealiases.
+      auto tmpTupleLoc = locator.withPathElement(LocatorPathElt::TupleType(type1));
+      auto tupleLoc = tmpTupleLoc.withPathElement(LocatorPathElt::TupleType(type2));
       auto result = matchTupleTypes(cast<TupleType>(desugar1),
                                     cast<TupleType>(desugar2),
-                                    kind, subflags, locator);
+                                    kind, subflags, tupleLoc);
       if (result != SolutionKind::Error)
         return result;
 

lib/Sema/ConstraintLocator.cpp

@@ -63,6 +63,7 @@ unsigned LocatorPathElt::getNewSummaryFlags() const {
   case ConstraintLocator::WrappedValue:
   case ConstraintLocator::GenericParameter:
   case ConstraintLocator::GenericArgument:
+  case ConstraintLocator::TupleType:
   case ConstraintLocator::NamedTupleElement:
   case ConstraintLocator::TupleElement:
   case ConstraintLocator::ProtocolRequirement:
@@ -356,6 +357,12 @@ void ConstraintLocator::dump(SourceManager *sm, raw_ostream &out) const {
       out << "member reference base";
       break;
 
+    case TupleType: {
+      auto tupleElt = elt.castTo<LocatorPathElt::TupleType>();
+      out << "tuple type '" << tupleElt.getType()->getString(PO) << "'";
+      break;
+    }
+
     case NamedTupleElement: {
       auto tupleElt = elt.castTo<LocatorPathElt::NamedTupleElement>();
       out << "named tuple element #" << llvm::utostr(tupleElt.getIndex());

lib/Sema/ConstraintSystem.cpp

@@ -4273,6 +4273,10 @@ void constraints::simplifyLocator(ASTNode &anchor,
       path = path.slice(1);
       continue;
 
+   case ConstraintLocator::TupleType:
+      path = path.slice(1);
+      continue;
+
     case ConstraintLocator::NamedTupleElement:
     case ConstraintLocator::TupleElement: {
       // Extract tuple element.

test/Constraints/diagnostics.swift

@@ -573,9 +573,17 @@ func r22263468(_ a : String?) {
   // TODO(diagnostics): This is a regression from diagnosing missing optional unwrap for `a`, we have to
   // re-think the way errors in tuple elements are detected because it's currently impossible to detect
   // exactly what went wrong here and aggregate fixes for different elements at the same time.
-  _ = MyTuple(42, a) // expected-error {{tuple type 'MyTuple' (aka '(Int, String)') is not convertible to tuple type '(Int, String?)'}}
+  _ = MyTuple(42, a) // expected-error {{tuple type '(Int, String?)' is not convertible to tuple type 'MyTuple' (aka '(Int, String)')}}
 }
 
+// rdar://71829040 - "ambiguous without more context" error for tuple type mismatch.
+func r71829040() {
+  func object(forKey: String) -> Any? { nil }
+
+  let flags: [String: String]
+  // expected-error@+1 {{tuple type '(String, Bool)' is not convertible to tuple type '(String, String)'}}
+  flags = Dictionary(uniqueKeysWithValues: ["keyA", "keyB"].map { ($0, object(forKey: $0) as? Bool ?? false) })
+}
 
 // rdar://22470302 - Crash with parenthesized call result.
 class r22470302Class {