Merge pull request #27880 from apple/revert-26710-SR-11295-warning-unecessary-casts

Revert "[Diagnostics][Qol] SR-11295 Emit diagnostics for same type coercion. "

Author: xedin

include/swift/AST/DiagnosticsSema.def

@@ -949,22 +949,18 @@ ERROR(super_initializer_not_in_initializer,none,
 WARNING(isa_is_always_true,none, "'%0' test is always true",
         (StringRef))
 WARNING(isa_is_foreign_check,none,
-        "'is' test is always true because %0 is a Core Foundation type",
-        (Type))
+      "'is' test is always true because %0 is a Core Foundation type",
+      (Type))
 WARNING(conditional_downcast_coercion,none,
-        "conditional cast from %0 to %1 always succeeds",
-        (Type, Type))
-WARNING(unnecessary_same_type_coercion,none,
-        "redundant cast to %0 has no effect",
-        (Type))
-WARNING(unnecessary_same_typealias_type_coercion,none,
-        "redundant cast from %0 to %1 has no effect",
-        (Type, Type))
+      "conditional cast from %0 to %1 always succeeds",
+      (Type, Type))
+
 WARNING(forced_downcast_noop,none,
         "forced cast of %0 to same type has no effect", (Type))
+
 WARNING(forced_downcast_coercion,none,
-        "forced cast from %0 to %1 always succeeds; did you mean to use 'as'?",
-        (Type, Type))
+      "forced cast from %0 to %1 always succeeds; did you mean to use 'as'?",
+      (Type, Type))
 
 // Note: the Boolean at the end indicates whether bridging is required after
 // the cast.

lib/Sema/CSDiagnostics.cpp

@@ -4427,7 +4427,9 @@ bool InaccessibleMemberFailure::diagnoseAsError() {
     auto &cs = getConstraintSystem();
     auto *locator =
         cs.getConstraintLocator(baseExpr, ConstraintLocator::Member);
-    if (cs.hasFixFor(locator))
+    if (llvm::any_of(cs.getFixes(), [&](const ConstraintFix *fix) {
+          return fix->getLocator() == locator;
+        }))
       return false;
   }
 
@@ -5056,36 +5058,6 @@ bool ThrowingFunctionConversionFailure::diagnoseAsError() {
   return true;
 }
 
-bool UnnecessaryCoercionFailure::diagnoseAsError() {
-  auto expr = cast<CoerceExpr>(getAnchor());
-  auto sourceRange =
-      SourceRange(expr->getLoc(), expr->getCastTypeLoc().getSourceRange().End);
-
-  if (isa<TypeAliasType>(getFromType().getPointer()) &&
-      isa<TypeAliasType>(getToType().getPointer())) {
-    auto fromTypeAlias = cast<TypeAliasType>(getFromType().getPointer());
-    auto toTypeAlias = cast<TypeAliasType>(getToType().getPointer());
-    // If the typealias are different, we need a warning
-    // mentioning both types.
-    if (fromTypeAlias->getDecl() != toTypeAlias->getDecl()) {
-      emitDiagnostic(expr->getLoc(),
-                     diag::unnecessary_same_typealias_type_coercion,
-                     getFromType(), getToType())
-
-          .fixItRemove(sourceRange);
-    } else {
-      emitDiagnostic(expr->getLoc(), diag::unnecessary_same_type_coercion,
-                     getToType())
-          .fixItRemove(sourceRange);
-    }
-  } else {
-    emitDiagnostic(expr->getLoc(), diag::unnecessary_same_type_coercion,
-                   getToType())
-        .fixItRemove(sourceRange);
-  }
-  return true;
-}
-
 bool InOutConversionFailure::diagnoseAsError() {
   auto *anchor = getAnchor();
   auto *locator = getLocator();

lib/Sema/CSDiagnostics.h

@@ -709,8 +709,8 @@ class ContextualFailure : public FailureDiagnostic {
   /// If we're trying to convert something to `nil`.
   bool diagnoseConversionToNil() const;
 
-  /// If we're trying to convert something of type "() -> T" to T,
-  /// then we probably meant to call the value.
+  // If we're trying to convert something of type "() -> T" to T,
+  // then we probably meant to call the value.
   bool diagnoseMissingFunctionCall() const;
 
   /// Produce a specialized diagnostic if this is an invalid conversion to Bool.
@@ -1462,11 +1462,11 @@ class MutatingMemberRefOnImmutableBase final : public FailureDiagnostic {
   bool diagnoseAsError() override;
 };
 
-/// Diagnose an attempt to use AnyObject as the root type of a KeyPath
-///
-/// ```swift
-/// let keyPath = \AnyObject.bar
-/// ```
+// Diagnose an attempt to use AnyObject as the root type of a KeyPath
+//
+// ```swift
+// let keyPath = \AnyObject.bar
+// ```
 class AnyObjectKeyPathRootFailure final : public FailureDiagnostic {
 
 public:
@@ -1776,27 +1776,6 @@ class ExpandArrayIntoVarargsFailure final : public ContextualFailure {
   void tryDropArrayBracketsFixIt(Expr *anchor) const;
 };
 
-/// Diagnose a situation where there is an explicit type coercion
-/// to the same type e.g.:
-///
-/// ```swift
-/// Double(1) as Double // redundant cast to 'Double' has no effect
-/// 1 as Double as Double // redundant cast to 'Double' has no effect
-/// let string = "String"
-/// let s = string as String // redundant cast to 'String' has no effect
-/// ```
-class UnnecessaryCoercionFailure final
-    : public ContextualFailure {
-      
-public:
-  UnnecessaryCoercionFailure(Expr *root, ConstraintSystem &cs,
-                             Type fromType, Type toType,
-                             ConstraintLocator *locator)
-      : ContextualFailure(root, cs, fromType, toType, locator) {}
-  
-  bool diagnoseAsError() override;
-};
-
 /// Diagnose a situation there is a mismatch between argument and parameter
 /// types e.g.:
 ///

lib/Sema/CSFix.cpp

@@ -857,38 +857,6 @@ IgnoreContextualType *IgnoreContextualType::create(ConstraintSystem &cs,
       IgnoreContextualType(cs, resultTy, specifiedTy, locator);
 }
 
-bool RemoveUnnecessaryCoercion::diagnose(Expr *root, bool asNote) const {
-  auto &cs = getConstraintSystem();
-  UnnecessaryCoercionFailure failure(root, cs, getFromType(), getToType(),
-                                     getLocator());
-  return failure.diagnose(asNote);
-}
-
-bool RemoveUnnecessaryCoercion::attempt(ConstraintSystem &cs, Type fromType,
-                                        Type toType,
-                                        ConstraintLocatorBuilder locator) {
-  auto last = locator.last();
-  bool isExplicitCoercion =
-      last && last->is<LocatorPathElt::ExplicitTypeCoercion>();
-  if (!isExplicitCoercion)
-    return false;
-
-  auto expr = cast<CoerceExpr>(locator.getAnchor());
-  auto toTypeRepr = expr->getCastTypeLoc().getTypeRepr();
-
-  // only diagnosing for coercion where the left-side is a DeclRefExpr
-  // or a explicit/implicit coercion e.g. Double(1) as Double
-  if (!isa<ImplicitlyUnwrappedOptionalTypeRepr>(toTypeRepr) &&
-      (isa<DeclRefExpr>(expr->getSubExpr()) ||
-       isa<CoerceExpr>(expr->getSubExpr()))) {
-    auto *fix = new (cs.getAllocator()) RemoveUnnecessaryCoercion(
-        cs, fromType, toType, cs.getConstraintLocator(locator));
-
-    return cs.recordFix(fix);
-  }
-  return false;
-}
-
 bool IgnoreAssignmentDestinationType::diagnose(Expr *root, bool asNote) const {
   auto &cs = getConstraintSystem();
   auto *AE = cast<AssignExpr>(getAnchor());

lib/Sema/CSFix.h

@@ -196,10 +196,6 @@ enum class FixKind : uint8_t {
   /// Allow a single tuple parameter to be matched with N arguments
   /// by forming all of the given arguments into a single tuple.
   AllowTupleSplatForSingleParameter,
-  
-  /// Remove an unnecessary coercion ('as') if the types are already equal.
-  /// e.g. Double(1) as Double
-  RemoveUnnecessaryCoercion,
 
   /// Allow a single argument type mismatch. This is the most generic
   /// failure related to argument-to-parameter conversions.
@@ -1344,24 +1340,6 @@ class IgnoreContextualType : public ContextualMismatch {
                                       ConstraintLocator *locator);
 };
 
-class RemoveUnnecessaryCoercion : public ContextualMismatch {
-protected:
-  RemoveUnnecessaryCoercion(ConstraintSystem &cs, Type fromType, Type toType,
-                            ConstraintLocator *locator)
-      : ContextualMismatch(cs, FixKind::RemoveUnnecessaryCoercion, fromType,
-                           toType, locator, /*isWarning*/ true) {}
-
-public:
-  std::string getName() const override {
-    return "remove unnecessary explicit type coercion";
-  }
-
-  bool diagnose(Expr *root, bool asNote = false) const override;
-
-  static bool attempt(ConstraintSystem &cs, Type fromType, Type toType,
-                      ConstraintLocatorBuilder locator);
-};
-
 class IgnoreAssignmentDestinationType final : public ContextualMismatch {
   IgnoreAssignmentDestinationType(ConstraintSystem &cs, Type sourceTy,
                                   Type destTy, ConstraintLocator *locator)

lib/Sema/CSSimplify.cpp

@@ -2587,8 +2587,7 @@ bool ConstraintSystem::repairFailures(
                         });
   };
 
-  if (path.empty() ||
-      path.back().is<LocatorPathElt::ExplicitTypeCoercion>()) {
+  if (path.empty()) {
     if (!anchor)
       return false;
 
@@ -3219,16 +3218,9 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
   // let's defer it until later proper check.
   if (!(desugar1->is<DependentMemberType>() &&
         desugar2->is<DependentMemberType>())) {
-    if (desugar1->isEqual(desugar2)) {
-      if (kind >= ConstraintKind::Conversion &&
-          !flags.contains(TMF_ApplyingFix)) {
-        if (RemoveUnnecessaryCoercion::attempt(*this, type1, type2,
-                                               getConstraintLocator(locator))) {
-          return getTypeMatchFailure(locator);
-        }
-      }
-      if (!isa<InOutType>(desugar2))
-        return getTypeMatchSuccess();
+    // If the types are obviously equivalent, we're done.
+    if (desugar1->isEqual(desugar2) && !isa<InOutType>(desugar2)) {
+      return getTypeMatchSuccess();
     }
   }
 
@@ -8037,7 +8029,6 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
   case FixKind::GenericArgumentsMismatch:
   case FixKind::AllowMutatingMemberOnRValueBase:
   case FixKind::AllowTupleSplatForSingleParameter:
-  case FixKind::RemoveUnnecessaryCoercion:
     llvm_unreachable("handled elsewhere");
   }
 
@@ -8344,20 +8335,11 @@ void ConstraintSystem::addExplicitConversionConstraint(
   SmallVector<Constraint *, 3> constraints;
 
   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());
-    }
-  }
 
   // Coercion (the common case).
   Constraint *coerceConstraint =
     Constraint::create(*this, ConstraintKind::Conversion,
-                       fromType, toType, coerceLocator);
+                       fromType, toType, locatorPtr);
   coerceConstraint->setFavored();
   constraints.push_back(coerceConstraint);
 

lib/Sema/ConstraintLocator.cpp

@@ -86,7 +86,6 @@ void ConstraintLocator::Profile(llvm::FoldingSetNodeID &id, Expr *anchor,
     case KeyPathRoot:
     case KeyPathValue:
     case KeyPathComponentResult:
-    case ExplicitTypeCoercion:
     case Condition:
       auto numValues = numNumericValuesInPathElement(elt.getKind());
       for (unsigned i = 0; i < numValues; ++i)
@@ -134,7 +133,6 @@ unsigned LocatorPathElt::getNewSummaryFlags() const {
   case ConstraintLocator::KeyPathRoot:
   case ConstraintLocator::KeyPathValue:
   case ConstraintLocator::KeyPathComponentResult:
-  case ConstraintLocator::ExplicitTypeCoercion:
   case ConstraintLocator::Condition:
     return 0;
 
@@ -460,11 +458,7 @@ void ConstraintLocator::dump(SourceManager *sm, raw_ostream &out) {
     case KeyPathComponentResult:
       out << "key path component result";
       break;
-        
-    case ExplicitTypeCoercion:
-      out << "type coercion";
-      break;
-      
+
     case Condition:
       out << "condition expression";
       break;

lib/Sema/ConstraintLocator.h

@@ -89,7 +89,6 @@ class ConstraintLocator : public llvm::FoldingSetNode {
     case KeyPathRoot:
     case KeyPathValue:
     case KeyPathComponentResult:
-    case ExplicitTypeCoercion:
     case Condition:
       return 0;
 

lib/Sema/ConstraintLocatorPathElts.def

@@ -164,9 +164,6 @@ SIMPLE_LOCATOR_PATH_ELT(UnresolvedMember)
 /// The candidate witness during protocol conformance checking.
 CUSTOM_LOCATOR_PATH_ELT(Witness)
 
-/// An explicit type coercion.
-SIMPLE_LOCATOR_PATH_ELT(ExplicitTypeCoercion)
-
 /// The condition associated with 'if' expression or ternary operator.
 SIMPLE_LOCATOR_PATH_ELT(Condition)
 

lib/Sema/ConstraintSystem.cpp

@@ -3000,11 +3000,6 @@ void constraints::simplifyLocator(Expr *&anchor,
       path = path.slice(1);
       continue;
     }
-        
-    case ConstraintLocator::ExplicitTypeCoercion: {
-      path = path.slice(1);
-      continue;
-    }
 
     default:
       // FIXME: Lots of other cases to handle.

test/ClangImporter/Darwin_test.swift

@@ -9,8 +9,7 @@ _ = nil as Fract? // expected-error{{use of undeclared type 'Fract'}}
 _ = nil as Darwin.Fract? // okay
 
 _ = 0 as OSErr
-// noErr is from the overlay
-_ = noErr as OSStatus // expected-warning {{redundant cast to 'OSStatus' (aka 'Int32') has no effect}} {{11-23=}}
+_ = noErr as OSStatus // noErr is from the overlay
 _ = 0 as UniChar
 
 _ = ProcessSerialNumber()

test/ClangImporter/MixedSource/mixed-target-using-header.swift

@@ -53,13 +53,12 @@ func testSuppressed() {
 #endif
 
 func testMacro() {
-  _ = CONSTANT as CInt // expected-warning {{redundant cast to 'CInt' (aka 'Int32') has no effect}} {{16-24=}}
+  _ = CONSTANT as CInt
 }
 
 func testFoundationOverlay() {
   _ = NSUTF8StringEncoding // no ambiguity
-  // and we should get the overlay version
-  _ = NSUTF8StringEncoding as UInt // expected-warning {{redundant cast to 'UInt' has no effect}} {{28-36=}}
+  _ = NSUTF8StringEncoding as UInt // and we should get the overlay version
 }
 
 #if !SILGEN

test/ClangImporter/Security_test.swift

@@ -4,8 +4,8 @@
 
 import Security
 
-_ = kSecClass as CFString // expected-warning {{redundant cast to 'CFString' has no effect}} {{15-27=}}
-_ = kSecClassGenericPassword as CFString // expected-warning {{redundant cast to 'CFString' has no effect}} {{30-42=}}
+_ = kSecClass as CFString
+_ = kSecClassGenericPassword as CFString
 _ = kSecClassGenericPassword as CFDictionary // expected-error {{'CFString?' is not convertible to 'CFDictionary'}} {{30-32=as!}}
 
 func testIntegration() {

test/ClangImporter/attr-swift_private.swift

@@ -102,7 +102,7 @@ public func testTopLevel() {
 
 _ = __PrivAnonymousA
 _ = __E0PrivA
-_ = __PrivE1A as __PrivE1 // expected-warning {{redundant cast to '__PrivE1' has no effect}} {{15-27=}}
+_ = __PrivE1A as __PrivE1
 _ = NSEnum.__privA
 _ = NSEnum.B
 _ = NSOptions.__privA

test/ClangImporter/cf.swift

@@ -90,9 +90,9 @@ func testTollFree1(_ ccmduct: CCMutableDuct) {
 }
 
 func testChainedAliases(_ fridge: CCRefrigerator) {
-  _ = fridge as CCRefrigerator // expected-warning {{redundant cast to 'CCRefrigerator' has no effect}} {{14-32=}}
+  _ = fridge as CCRefrigerator
 
-  _ = fridge as CCFridge // expected-warning {{redundant cast to 'CCFridge' (aka 'CCRefrigerator') has no effect}} {{14-26=}}
+  _ = fridge as CCFridge
   _ = fridge as CCFridgeRef // expected-error{{'CCFridgeRef' has been renamed to 'CCFridge'}} {{17-28=CCFridge}}
 }
 

test/ClangImporter/cfuncs_parse.swift

@@ -15,7 +15,7 @@ func test_cfunc2(_ i: Int) {
 #else
   let f = cfunc2(i, 17)
 #endif
-  _ = f as Float // expected-warning {{redundant cast to 'Float' has no effect}} {{9-18=}}
+  _ = f as Float
   cfunc2(b:17, a:i) // expected-error{{extraneous argument labels 'b:a:' in call}}
   // expected-error@-1 {{cannot convert value of type 'Int' to expected argument type 'Int32'}}
   cfunc2(17, i) // expected-error{{cannot convert value of type 'Int' to expected argument type 'Int32'}}