[Diagnostics] Improving logic and removing old checks on RemoveUnnecessaryCoercion fix and adjusting some tests

Author: LucianoPAlmeida

lib/Sema/CSDiagnostics.cpp

@@ -753,6 +753,11 @@ bool GenericArgumentsMismatchFailure::diagnoseAsError() {
       break;
     }
 
+    case ConstraintLocator::ExplicitTypeCoercion: {
+      diagnostic = diag::cannot_convert_coerce;
+      break;
+    }
+
     default:
       break;
     }
@@ -2069,7 +2074,8 @@ bool ContextualFailure::diagnoseAsError() {
     diagnostic = diag::cannot_convert_condition_value;
     break;
   }
-      
+  
+  case ConstraintLocator::ExplicitTypeCoercion:
   case ConstraintLocator::InstanceType: {
     if (diagnoseCoercionToUnrelatedType())
       return true;
@@ -5556,16 +5562,13 @@ bool UnnecessaryCoercionFailure::diagnoseAsError() {
                      diag::unnecessary_same_typealias_type_coercion,
                      getFromType(), castType)
           .fixItRemove(sourceRange);
-    } else {
-      emitDiagnostic(expr->getLoc(), diag::unnecessary_same_type_coercion,
-                     castType)
-          .fixItRemove(sourceRange);
+      return true;
     }
-  } else {
-    emitDiagnostic(expr->getLoc(), diag::unnecessary_same_type_coercion,
-                   castType)
-        .fixItRemove(sourceRange);
   }
+  
+  emitDiagnostic(expr->getLoc(), diag::unnecessary_same_type_coercion,
+                 castType)
+      .fixItRemove(sourceRange);
   return true;
 }
 

lib/Sema/CSFix.cpp

@@ -1165,12 +1165,12 @@ bool RemoveUnnecessaryCoercion::attempt(ConstraintSystem &cs, Type fromType,
 
   // Check to ensure the from and to types are equal the cast type.
   // This is required to handle cases where the conversion is done
-  // using compiler intrinsics e.g. _HasCustomAnyHashableRepresentation
+  // using compiler intrinsics of _HasCustomAnyHashableRepresentation
   // to AnyHashable where if we coerce a generic type that conforms to
   // this protocol to AnyHashable we match equal types here, but the
   // explicit coercion is still required.
   auto castType = cs.getType(expr->getCastTypeLoc());
-  if (!fromType->isEqual(castType) && !castType->hasTypeVariable())
+  if (!fromType->isEqual(castType) && cs.isAnyHashableType(castType))
     return false;
 
   auto toTypeRepr = expr->getCastTypeLoc().getTypeRepr();
@@ -1196,22 +1196,13 @@ bool RemoveUnnecessaryCoercion::attempt(ConstraintSystem &cs, Type fromType,
 
   auto coercedType = cs.getType(expr->getSubExpr());
   if (auto *typeVariable = coercedType->getAs<TypeVariableType>()) {
-    // Only diagnose if we have all type variables resolved in the system.
-    if (cs.hasFreeTypeVariables())
-      return false;
-
     auto representative = cs.getRepresentative(typeVariable);
     if (auto *typeSourceLocator =
             cs.getTypeVariableBindingLocator(representative)) {
       // If the type variable binding source locator is the same the
       // contextual type equality is coming from this coercion.
       if (typeSourceLocator == cs.getConstraintLocator(locator))
         return false;
-
-      // Not warn if locator points to a TupleElement to avoid
-      // incorrect warnings.
-      if (typeSourceLocator->isLastElement<LocatorPathElt::TupleElement>())
-        return false;
     }
   }
 

lib/Sema/CSSimplify.cpp

@@ -10510,14 +10510,12 @@ void ConstraintSystem::addExplicitConversionConstraint(
   auto locatorPtr = getConstraintLocator(locator);
   ConstraintLocator *coerceLocator = locatorPtr;
 
-  if (allowFixes && shouldAttemptFixes()) {
-    auto *anchor = locator.getAnchor();
-    if (isa<CoerceExpr>(anchor) && !anchor->isImplicit()) {
-      coerceLocator =
-          getConstraintLocator(anchor, LocatorPathElt::ExplicitTypeCoercion());
-    }
+  auto *anchor = locator.getAnchor();
+  if (anchor && isa<CoerceExpr>(anchor) && !anchor->isImplicit()) {
+    coerceLocator =
+        getConstraintLocator(anchor, LocatorPathElt::ExplicitTypeCoercion());
   }
-
+  
   // Coercion (the common case).
   Constraint *coerceConstraint =
     Constraint::create(*this, ConstraintKind::Conversion,

lib/Sema/CSSolver.cpp

@@ -91,7 +91,7 @@ Solution ConstraintSystem::finalize() {
 
     case FreeTypeVariableBinding::UnresolvedType:
       assignFixedType(tv, ctx.TheUnresolvedType,
-                      tv->getImpl().getLocator());
+                      /*locator*/nullptr);
       break;
     }
   }
@@ -206,8 +206,7 @@ void ConstraintSystem::applySolution(const Solution &solution) {
     // If we don't already have a fixed type for this type variable,
     // assign the fixed type from the solution.
     if (!getFixedType(binding.first) && !binding.second->hasTypeVariable()) {
-      auto typeVar = binding.first;
-      assignFixedType(typeVar, binding.second, typeVar->getImpl().getLocator(),
+      assignFixedType(binding.first, binding.second, /*locator*/nullptr,
                       /*updateState=*/false);
     }
   }

lib/Sema/ConstraintSystem.cpp

@@ -198,9 +198,11 @@ void ConstraintSystem::assignFixedType(TypeVariableType *typeVar, Type type,
       }
     }
   }
-
-  // Recording fixed type source locator for type variable.
-  recordTypeVariableBindingLocator(typeVar, locator);
+  
+  if (locator) {
+    // Recording fixed type source locator for type variable.
+    recordTypeVariableBindingLocator(typeVar, locator);
+  }
 
   // Notify the constraint graph.
   CG.bindTypeVariable(typeVar, type);

test/expr/cast/as_coerce.swift

@@ -81,7 +81,7 @@ c3 as C4 // expected-error {{'C3' is not convertible to 'C4'; did you mean to us
 // <rdar://problem/19495142> Various incorrect diagnostics for explicit type conversions
 1 as Double as Float // expected-error{{cannot convert value of type 'Double' to type 'Float' in coercion}}
 1 as Int as String // expected-error{{cannot convert value of type 'Int' to type 'String' in coercion}}
-Double(1) as Double as String // expected-error{{cannot convert value of type 'Double' to type 'String' in coercion}}
+Double(1) as Double as String // expected-error{{cannot convert value of type 'Double' to type 'String' in coercion}} expected-warning {{redundant cast to 'Double' has no effect}} {{11-21=}}
 ["awd"] as [Int] // expected-error{{cannot convert value of type 'String' to expected element type 'Int'}}
 ([1, 2, 1.0], 1) as ([String], Int)
 // expected-error@-1 2 {{cannot convert value of type 'Int' to expected element type 'String'}}

test/expr/cast/dictionary_bridge.swift

@@ -1,4 +1,4 @@
-// RUN: %target-typecheck-verify-swift
+// RUN: %target-typecheck-verify-swift -disable-redundant-coercion-warning
 
 // REQUIRES: objc_interop
 

test/expr/cast/dictionary_coerce.swift

@@ -1,4 +1,4 @@
-// RUN: %target-typecheck-verify-swift
+// RUN: %target-typecheck-verify-swift -disable-redundant-coercion-warning
 
 class C : Hashable {
 	var x = 0

test/expr/cast/objc_coerce_array.swift

@@ -1,4 +1,4 @@
-// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -emit-silgen -verify %s
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -emit-silgen -verify %s -disable-redundant-coercion-warning
 // REQUIRES: objc_interop
 import Foundation