Merge pull request swiftlang#29672 from xedin/eliminate-leftover-contextual-diagnostics

[Diagnostics] Fix all of the edge cases and remove `diagnoseContextualConversionError`

Author: xedin

lib/Sema/CSDiag.cpp

@@ -195,12 +195,6 @@ class FailureDiagnosis :public ASTVisitor<FailureDiagnosis, /*exprresult*/bool>{
   /// Emit an ambiguity diagnostic about the specified expression.
   void diagnoseAmbiguity(Expr *E);
 
-  /// Attempt to produce a diagnostic for a mismatch between an expression's
-  /// type and its assumed contextual type.
-  bool diagnoseContextualConversionError(Expr *expr, Type contextualType,
-                                         ContextualTypePurpose CTP,
-                                         Type suggestedType = Type());
-
 private:
   /// Validate potential contextual type for type-checking one of the
   /// sub-expressions, usually correct/valid types are the ones which
@@ -500,51 +494,6 @@ DeclContext *FailureDiagnosis::findDeclContext(Expr *subExpr) const {
   return finder.DC;
 }
 
-bool FailureDiagnosis::diagnoseContextualConversionError(
-    Expr *expr, Type contextualType, ContextualTypePurpose CTP,
-    Type suggestedType) {
-  // If the constraint system has a contextual type, then we can test to see if
-  // this is the problem that prevents us from solving the system.
-  if (!contextualType)
-    return false;
-
-  // Try re-type-checking the expression without the contextual type to see if
-  // it can work without it.  If so, the contextual type is the problem.  We
-  // force a recheck, because "expr" is likely in our table with the extra
-  // contextual constraint that we know we are relaxing.
-  TCCOptions options = TCC_ForceRecheck;
-  if (contextualType->is<InOutType>())
-    options |= TCC_AllowLValue;
-
-  auto *recheckedExpr = typeCheckChildIndependently(expr, options);
-  auto exprType = recheckedExpr ? CS.getType(recheckedExpr) : Type();
-
-  // If there is a suggested type and re-typecheck failed, let's use it.
-  if (!exprType)
-    exprType = suggestedType;
-
-  // If it failed and diagnosed something, then we're done.
-  if (!exprType)
-    return CS.getASTContext().Diags.hadAnyError();
-
-  // If we don't have a type for the expression, then we cannot use it in
-  // conversion constraint diagnostic generation.  If the types match, then it
-  // must not be the contextual type that is the problem.
-  if (isUnresolvedOrTypeVarType(exprType) || exprType->isEqual(contextualType))
-    return false;
-
-  // Don't attempt fixits if we have an unsolved type variable, since
-  // the recovery path's recursion into the type checker via typeCheckCast()
-  // will confuse matters.
-  if (exprType->hasTypeVariable())
-    return false;
-
-  ContextualFailure failure(
-      CS, CTP, exprType, contextualType,
-      CS.getConstraintLocator(expr, LocatorPathElt::ContextualType()));
-  return failure.diagnoseAsError();
-}
-
 //===----------------------------------------------------------------------===//
 // Diagnose assigning variable to itself.
 //===----------------------------------------------------------------------===//
@@ -1256,13 +1205,6 @@ void ConstraintSystem::diagnoseFailureFor(SolutionApplicationTarget target) {
     if (diagnosis.diagnoseExprFailure())
       return;
 
-    // If this is a contextual conversion problem, dig out some information.
-    if (diagnosis.diagnoseContextualConversionError(
-            expr,
-            getContextualType(expr),
-            getContextualTypePurpose(expr)))
-      return;
-
     // If no one could find a problem with this expression or constraint system,
     // then it must be well-formed... but is ambiguous.  Handle this by diagnostic
     // various cases that come up.

lib/Sema/CSDiagnostics.cpp

@@ -1891,9 +1891,6 @@ bool ContextualFailure::diagnoseAsError() {
   if (diagnoseMissingFunctionCall())
     return true;
 
-  if (diagnoseConversionToDictionary())
-    return true;
-
   // Special case of some common conversions involving Swift.String
   // indexes, catching cases where people attempt to index them with an integer.
   if (isIntegerToStringIndexConversion()) {
@@ -1958,6 +1955,17 @@ bool ContextualFailure::diagnoseAsError() {
     if (diagnoseYieldByReferenceMismatch())
       return true;
 
+    if (isa<OptionalTryExpr>(anchor) || isa<OptionalEvaluationExpr>(anchor)) {
+      auto objectType = fromType->getOptionalObjectType();
+      if (objectType->isEqual(toType)) {
+        auto &cs = getConstraintSystem();
+        MissingOptionalUnwrapFailure failure(cs, getType(anchor), toType,
+                                             cs.getConstraintLocator(anchor));
+        if (failure.diagnoseAsError())
+          return true;
+      }
+    }
+
     if (CTP == CTP_ForEachStmt) {
       if (fromType->isAnyExistentialType()) {
         emitDiagnostic(anchor->getLoc(), diag::type_cannot_conform,
@@ -2392,61 +2400,6 @@ bool ContextualFailure::diagnoseConversionToBool() const {
   return false;
 }
 
-bool ContextualFailure::isInvalidDictionaryConversion(
-    ConstraintSystem &cs, Expr *anchor, Type contextualType) {
-  auto *arrayExpr = dyn_cast<ArrayExpr>(anchor);
-  if (!arrayExpr)
-    return false;
-
-  auto type = contextualType->lookThroughAllOptionalTypes();
-  if (!conformsToKnownProtocol(
-        cs, type, KnownProtocolKind::ExpressibleByDictionaryLiteral))
-    return false;
-
-  return (arrayExpr->getNumElements() & 1) == 0;
-}
-
-bool ContextualFailure::diagnoseConversionToDictionary() const {
-  auto &cs = getConstraintSystem();
-  auto toType = getToType()->lookThroughAllOptionalTypes();
-
-  if (!isInvalidDictionaryConversion(cs, getAnchor(), toType))
-    return false;
-
-  auto *arrayExpr = cast<ArrayExpr>(getAnchor());
-
-  // If the contextual type conforms to ExpressibleByDictionaryLiteral and
-  // this is an empty array, then they meant "[:]".
-  auto numElements = arrayExpr->getNumElements();
-  if (numElements == 0) {
-    emitDiagnostic(arrayExpr->getStartLoc(),
-                   diag::should_use_empty_dictionary_literal)
-        .fixItInsert(arrayExpr->getEndLoc(), ":");
-    return true;
-  }
-
-  // If the contextual type conforms to ExpressibleByDictionaryLiteral, then
-  // they wrote "x = [1,2]" but probably meant "x = [1:2]".
-  bool isIniting = getContextualTypePurpose() == CTP_Initialization;
-  emitDiagnostic(arrayExpr->getStartLoc(), diag::should_use_dictionary_literal,
-                 toType, isIniting);
-
-  auto diagnostic =
-      emitDiagnostic(arrayExpr->getStartLoc(), diag::meant_dictionary_lit);
-
-  // Change every other comma into a colon, only if the number
-  // of commas present matches the number of elements, because
-  // otherwise it might a structural problem with the expression
-  // e.g. ["a""b": 1].
-  const auto commaLocs = arrayExpr->getCommaLocs();
-  if (commaLocs.size() == numElements - 1) {
-    for (unsigned i = 0, e = numElements / 2; i != e; ++i)
-      diagnostic.fixItReplace(commaLocs[i * 2], ":");
-  }
-
-  return true;
-}
-
 bool ContextualFailure::diagnoseThrowsTypeMismatch() const {
   // If this is conversion failure due to a return statement with an argument
   // that cannot be coerced to the result type of the function, emit a
@@ -4089,7 +4042,12 @@ bool MissingArgumentsFailure::diagnoseClosure(ClosureExpr *closure) {
   if (locator->isForContextualType()) {
     funcType = cs.getContextualType(locator->getAnchor())->getAs<FunctionType>();
   } else if (auto info = cs.getFunctionArgApplyInfo(locator)) {
-    funcType = info->getParamType()->getAs<FunctionType>();
+    auto paramType = info->getParamType();
+    // Drop a single layer of optionality because argument could get injected
+    // into optional and that doesn't contribute to the problem.
+    if (auto objectType = paramType->getOptionalObjectType())
+      paramType = objectType;
+    funcType = paramType->getAs<FunctionType>();
   } else if (locator->isLastElement<LocatorPathElt::ClosureResult>()) {
     // Based on the locator we know this this is something like this:
     // `let _: () -> ((Int) -> Void) = { return {} }`.
@@ -4841,24 +4799,6 @@ bool CollectionElementContextualFailure::diagnoseAsError() {
   auto *anchor = getAnchor();
   auto *locator = getLocator();
 
-  // Check whether this is situation like `let _: [String: Int] = ["A", 0]`
-  // which attempts to convert an array into a dictionary. We have a tailored
-  // contextual diagnostic for that, so no need to diagnose element mismatches
-  // as well.
-  auto &cs = getConstraintSystem();
-  auto *rawAnchor = getRawAnchor();
-  if (llvm::any_of(cs.getFixes(), [&](const ConstraintFix *fix) -> bool {
-        auto *locator = fix->getLocator();
-        if (!(fix->getKind() == FixKind::ContextualMismatch &&
-              locator->getAnchor() == rawAnchor))
-          return false;
-
-        auto *mismatch = static_cast<const ContextualMismatch *>(fix);
-        return isInvalidDictionaryConversion(cs, rawAnchor,
-                                             mismatch->getToType());
-      }))
-    return false;
-
   auto eltType = getFromType();
   auto contextualType = getToType();
 

lib/Sema/CSDiagnostics.h

@@ -566,10 +566,6 @@ class ContextualFailure : public FailureDiagnostic {
   /// Produce a specialized diagnostic if this is an invalid conversion to Bool.
   bool diagnoseConversionToBool() const;
 
-  /// Produce a specialized diagnostic if this is an attempt to initialize
-  /// or convert an array literal to a dictionary e.g. `let _: [String: Int] = ["A", 0]`
-  bool diagnoseConversionToDictionary() const;
-
   /// Produce a specialized diagnostic if this is an attempt to throw
   /// something with doesn't conform to `Error`.
   bool diagnoseThrowsTypeMismatch() const;
@@ -625,11 +621,6 @@ class ContextualFailure : public FailureDiagnostic {
   /// protocol
   bool tryProtocolConformanceFixIt(InFlightDiagnostic &diagnostic) const;
 
-  /// Check whether this contextual failure represents an invalid
-  /// conversion from array literal to dictionary.
-  static bool isInvalidDictionaryConversion(ConstraintSystem &cs, Expr *anchor,
-                                            Type contextualType);
-
 private:
   Type resolve(Type rawType) const {
     return resolveType(rawType)->getWithoutSpecifierType();

lib/Sema/CSFix.cpp

@@ -262,11 +262,15 @@ ContextualMismatch *ContextualMismatch::create(ConstraintSystem &cs, Type lhs,
 /// and the contextual type.
 static Optional<std::tuple<ContextualTypePurpose, Type, Type>>
 getStructuralTypeContext(ConstraintSystem &cs, ConstraintLocator *locator) {
-  if (auto contextualType = cs.getContextualType(locator->getAnchor())) {
-    if (auto *anchor = simplifyLocatorToAnchor(locator))
-      return std::make_tuple(cs.getContextualTypePurpose(locator->getAnchor()),
-                             cs.getType(anchor),
-                             contextualType);
+  if (locator->findLast<LocatorPathElt::ContextualType>()) {
+    assert(locator->isLastElement<LocatorPathElt::ContextualType>() ||
+           locator->isLastElement<LocatorPathElt::FunctionArgument>());
+
+    auto *anchor = locator->getAnchor();
+    auto contextualType = cs.getContextualType(anchor);
+    auto exprType = cs.getType(anchor);
+    return std::make_tuple(cs.getContextualTypePurpose(anchor), exprType,
+                           contextualType);
   } else if (auto argApplyInfo = cs.getFunctionArgApplyInfo(locator)) {
     return std::make_tuple(CTP_CallArgument,
                            argApplyInfo->getArgType(),

lib/Sema/CSGen.cpp

@@ -1852,7 +1852,56 @@ namespace {
 
         return contextualArrayType;
       }
-      
+
+      // Produce a specialized diagnostic if this is an attempt to initialize
+      // or convert an array literal to a dictionary e.g.
+      // `let _: [String: Int] = ["A", 0]`
+      auto isDictionaryContextualType = [&](Type contextualType) -> bool {
+        if (!contextualType)
+          return false;
+
+        auto type = contextualType->lookThroughAllOptionalTypes();
+        if (conformsToKnownProtocol(
+                CS, type, KnownProtocolKind::ExpressibleByArrayLiteral))
+          return false;
+
+        return conformsToKnownProtocol(
+            CS, type, KnownProtocolKind::ExpressibleByDictionaryLiteral);
+      };
+
+      if (isDictionaryContextualType(contextualType)) {
+        auto &DE = CS.getASTContext().Diags;
+        auto numElements = expr->getNumElements();
+
+        if (numElements == 0) {
+          DE.diagnose(expr->getStartLoc(),
+                      diag::should_use_empty_dictionary_literal)
+              .fixItInsert(expr->getEndLoc(), ":");
+          return nullptr;
+        }
+
+        bool isIniting =
+            CS.getContextualTypePurpose(expr) == CTP_Initialization;
+        DE.diagnose(expr->getStartLoc(), diag::should_use_dictionary_literal,
+                    contextualType->lookThroughAllOptionalTypes(), isIniting);
+
+        auto diagnostic =
+            DE.diagnose(expr->getStartLoc(), diag::meant_dictionary_lit);
+
+        // If there is an even number of elements in the array, let's produce
+        // a fix-it which suggests to replace "," with ":" to form a dictionary
+        // literal.
+        if ((numElements & 1) == 0) {
+          const auto commaLocs = expr->getCommaLocs();
+          if (commaLocs.size() == numElements - 1) {
+            for (unsigned i = 0, e = numElements / 2; i != e; ++i)
+              diagnostic.fixItReplace(commaLocs[i * 2], ":");
+          }
+        }
+
+        return nullptr;
+      }
+
       auto arrayTy = CS.createTypeVariable(locator,
                                            TVO_PrefersSubtypeBinding |
                                            TVO_CanBindToNoEscape);