Merge pull request #81936 from xedin/improve-optional-wrappedValue-mismatch-diagnostics

[CSSimplify] Fix `matchDeepEqualityTypes` to allow fixing of optionals

Author: xedin

include/swift/Sema/CSFix.h

@@ -560,6 +560,11 @@ class ConstraintFix {
     return false;
   }
 
+  template <typename E>
+  bool directlyAt() const {
+    return getLocator()->directlyAt<E>();
+  }
+
   void print(llvm::raw_ostream &Out) const;
 
   SWIFT_DEBUG_DUMP;

lib/Sema/CSDiagnostics.cpp

@@ -1114,7 +1114,12 @@ bool GenericArgumentsMismatchFailure::diagnoseAsError() {
   if (!diagnostic)
     return false;
 
-  emitDiagnosticAt(::getLoc(anchor), *diagnostic, fromType, toType);
+  {
+    auto diag =
+        emitDiagnosticAt(::getLoc(anchor), *diagnostic, fromType, toType);
+    (void)tryFixIts(diag);
+  }
+
   emitNotesForMismatches();
   return true;
 }

lib/Sema/CSSimplify.cpp

@@ -4039,6 +4039,12 @@ ConstraintSystem::matchDeepEqualityTypes(Type type1, Type type2,
       getConstraintLocator(locator, {LocatorPathElt::GenericType(type1),
           LocatorPathElt::GenericType(type2)});
 
+    // Optionals have a lot of special diagnostics and only one
+    // generic argument so if we're dealing with one, let's allow
+    // `repairFailures` to take care of it.
+    if (bound1->getDecl()->isOptionalDecl())
+      return matchDeepTypeArguments(*this, subflags, args1, args2, baseLoc);
+
     auto argMatchingFlags = subflags;
     // Allow the solver to produce separate fixes while matching
     // key path's root/value to a contextual type instead of the
@@ -4049,13 +4055,6 @@ ConstraintSystem::matchDeepEqualityTypes(Type type1, Type type2,
       argMatchingFlags |= TMF_MatchingGenericArguments;
     }
 
-    // Optionals have a lot of special diagnostics and only one
-    // generic argument so if we' re dealing with one, don't produce generic
-    // arguments mismatch fixes.
-    if (bound1->getDecl()->isOptionalDecl())
-      return matchDeepTypeArguments(*this, argMatchingFlags, args1, args2,
-                                    baseLoc);
-
     SmallVector<unsigned, 4> mismatches;
     auto result = matchDeepTypeArguments(
         *this, argMatchingFlags, args1, args2, baseLoc,
@@ -6943,12 +6942,19 @@ bool ConstraintSystem::repairFailures(
     path.pop_back();
 
     ConstraintFix *fix = nullptr;
-    if (!path.empty() && path.back().is<LocatorPathElt::AnyRequirement>()) {
+    auto *fixLoc = getConstraintLocator(anchor, path);
+
+    if (fixLoc->isLastElement<LocatorPathElt::AnyRequirement>()) {
       fix = fixRequirementFailure(*this, fromType, toType, anchor, path);
+    } else if (fixLoc->isLastElement<LocatorPathElt::TupleElement>()) {
+      return repairFailures(lhs, rhs, matchKind, flags, conversionsOrFixes,
+                            fixLoc);
+    } else if (!lhs->mayHaveSuperclass() && rhs->isAnyObject()) {
+      fix = AllowNonClassTypeToConvertToAnyObject::create(*this, fromType,
+                                                          fixLoc);
     } else {
       fix = GenericArgumentsMismatch::create(
-          *this, fromType, toType, {genericArgElt.getIndex()},
-          getConstraintLocator(anchor, path));
+          *this, fromType, toType, {genericArgElt.getIndex()}, fixLoc);
     }
 
     if (!fix)
@@ -15688,14 +15694,31 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
   case FixKind::AllowFunctionSpecialization:
   case FixKind::IgnoreGenericSpecializationArityMismatch:
   case FixKind::IgnoreKeyPathSubscriptIndexMismatch:
-  case FixKind::AllowMemberRefOnExistential: {
+  case FixKind::AllowMemberRefOnExistential:
+  case FixKind::AllowNonClassTypeToConvertToAnyObject: {
     return recordFix(fix) ? SolutionKind::Error : SolutionKind::Solved;
   }
 
   case FixKind::GenericArgumentsMismatch: {
     unsigned impact = 1;
     if (type1->isMarkerExistential() || type2->isMarkerExistential())
       ++impact;
+
+    // If generic arguments mismatch ends up being recorded on the result
+    // of the chain or a try expression it means that there is a contextual 
+    // conversion mismatch.
+    //
+    // For optional conversions the solver currently generates a disjunction
+    // with two choices - bind and optional-to-optional conversion which is
+    // anchored on the contextual expression. If we can get a fix recorded
+    // there that would result in a better diagnostic. It's only possible
+    // for optional-to-optional choice because it doesn't bind the
+    // variable immediately, so we need to downgrade direct fixes to prevent
+    // `bind` choice from considered better.
+    if (fix->directlyAt<OptionalEvaluationExpr>() ||
+        fix->directlyAt<AnyTryExpr>())
+      impact += 2;
+
     return recordFix(fix, impact) ? SolutionKind::Error : SolutionKind::Solved;
   }
 
@@ -15923,7 +15946,6 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
   case FixKind::DefaultGenericArgument:
   case FixKind::AllowMutatingMemberOnRValueBase:
   case FixKind::AllowTupleSplatForSingleParameter:
-  case FixKind::AllowNonClassTypeToConvertToAnyObject:
   case FixKind::SpecifyClosureParameterType:
   case FixKind::SpecifyClosureReturnType:
   case FixKind::AddQualifierToAccessTopLevelName:

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

@@ -66,11 +66,13 @@ func testProtocolNamingConflict() {
   let a: ConflictingName1?
   var b: ConflictingName1Protocol?
   b = a // expected-error {{cannot assign value of type 'ConflictingName1?' to type '(any ConflictingName1Protocol)?'}}
+  // expected-note@-1 {{arguments to generic parameter 'Wrapped' ('ConflictingName1' and 'any ConflictingName1Protocol') are expected to be equal}}
   _ = b
 
   let c: ConflictingName2?
   var d: ConflictingName2Protocol?
   d = c // expected-error {{cannot assign value of type 'ConflictingName2?' to type '(any ConflictingName2Protocol)?'}}
+  // expected-note@-1 {{arguments to generic parameter 'Wrapped' ('ConflictingName2' and 'any ConflictingName2Protocol') are expected to be equal}}
   _ = d
 }
 

test/ClangImporter/cf.swift

@@ -118,6 +118,7 @@ func testOutParametersGood() {
 func testOutParametersBad() {
   let fridge: CCRefrigerator?
   CCRefrigeratorCreateIndirect(fridge) // expected-error {{cannot convert value of type 'CCRefrigerator?' to expected argument type 'UnsafeMutablePointer<CCRefrigerator?>?'}}
+  // expected-note@-1 {{arguments to generic parameter 'Wrapped' ('CCRefrigerator' and 'UnsafeMutablePointer<CCRefrigerator?>') are expected to be equal}}
 
   let power: CCPowerSupply?
   CCRefrigeratorGetPowerSupplyIndirect(0, power)
@@ -128,21 +129,26 @@ func testOutParametersBad() {
   CCRefrigeratorGetItemUnaudited(0, 0, item)
   // expected-error@-1:34 {{cannot convert value of type 'Int' to expected argument type 'CCRefrigerator'}}
   // expected-error@-2:40 {{cannot convert value of type 'CCItem?' to expected argument type 'UnsafeMutablePointer<Unmanaged<CCItem>?>?'}}
+  // expected-note@-3 {{arguments to generic parameter 'Wrapped' ('CCItem' and 'UnsafeMutablePointer<Unmanaged<CCItem>?>') are expected to be equal}}
 }
 
 func nameCollisions() {
   var objc: MyProblematicObject?
   var cf: MyProblematicObjectRef?
   cf = objc // expected-error {{cannot assign value of type 'MyProblematicObject?' to type 'MyProblematicObjectRef?'}}
+  // expected-note@-1 {{arguments to generic parameter 'Wrapped' ('MyProblematicObject' and 'MyProblematicObjectRef') are expected to be equal}}
   objc = cf // expected-error {{cannot assign value of type 'MyProblematicObjectRef?' to type 'MyProblematicObject?'}}
+  // expected-note@-1 {{arguments to generic parameter 'Wrapped' ('MyProblematicObjectRef' and 'MyProblematicObject') are expected to be equal}}
 
   var cfAlias: MyProblematicAliasRef?
   cfAlias = cf // okay
   cf = cfAlias // okay
 
   var otherAlias: MyProblematicAlias?
   otherAlias = cfAlias // expected-error {{cannot assign value of type 'MyProblematicAliasRef?' (aka 'Optional<MyProblematicObjectRef>') to type 'MyProblematicAlias?' (aka 'Optional<Float>')}}
+  // expected-note@-1 {{arguments to generic parameter 'Wrapped' ('MyProblematicAliasRef' (aka 'MyProblematicObjectRef') and 'MyProblematicAlias' (aka 'Float')) are expected to be equal}}
   cfAlias = otherAlias // expected-error {{cannot assign value of type 'MyProblematicAlias?' (aka 'Optional<Float>') to type 'MyProblematicAliasRef?' (aka 'Optional<MyProblematicObjectRef>')}}
+  // expected-note@-1 {{arguments to generic parameter 'Wrapped' ('MyProblematicAlias' (aka 'Float') and 'MyProblematicAliasRef' (aka 'MyProblematicObjectRef')) are expected to be equal}}
 
   func isOptionalFloat(_: inout Optional<Float>) {}
   isOptionalFloat(&otherAlias) // okay

test/ClangImporter/ctypes_parse.swift

@@ -214,6 +214,7 @@ func testFunctionPointers() {
 
   useFunctionPointer2(anotherFP)
   sizedFP = fp // expected-error {{cannot assign value of type 'fptr?' (aka 'Optional<@convention(c) (Int32) -> Int32>') to type '(@convention(c) (CInt, CInt, UnsafeMutableRawPointer?) -> Void)?'}}
+  // expected-note@-1 {{arguments to generic parameter 'Wrapped' ('fptr' (aka '@convention(c) (Int32) -> Int32') and '@convention(c) (CInt, CInt, UnsafeMutableRawPointer?) -> Void'}}
 }
 
 func testStructDefaultInit() {

test/Constraints/bridging-nsnumber-and-nsvalue.swift.gyb

@@ -62,7 +62,7 @@ func bridgeNSNumberBackToSpecificType(object: ${ObjectType},
   _ = object as? ${Type}
   _ = object as! ${Type}
 
-  _ = optional as ${Type}? // expected-error{{is not convertible to}} expected-note {{use 'as!'}}
+  _ = optional as ${Type}? // expected-error{{cannot convert value of type '${ObjectType}?' to type '${Type}?' in coercion}} expected-note {{arguments to generic parameter 'Wrapped' ('${ObjectType}' and '${Type}') are expected to be equal}}
   _ = optional is ${Type}?
   _ = optional as? ${Type}?
   _ = optional as! ${Type}?

test/Constraints/bridging.swift

@@ -283,13 +283,17 @@ func rdar19831698() {
 // <rdar://problem/19836341> Incorrect fixit for NSString? to String? conversions
 func rdar19836341(_ ns: NSString?, vns: NSString?) {
   var vns = vns
-  let _: String? = ns // expected-error{{cannot convert value of type 'NSString?' to specified type 'String?'}}{{22-22= as String?}}
-  var _: String? = ns // expected-error{{cannot convert value of type 'NSString?' to specified type 'String?'}}{{22-22= as String?}}
+  let _: String? = ns // expected-error{{cannot assign value of type 'NSString?' to type 'String?'}}{{22-22= as String?}}
+  // expected-note@-1 {{arguments to generic parameter 'Wrapped' ('NSString' and 'String') are expected to be equal}}
+  var _: String? = ns // expected-error{{cannot assign value of type 'NSString?' to type 'String?'}}{{22-22= as String?}}
+  // expected-note@-1 {{arguments to generic parameter 'Wrapped' ('NSString' and 'String') are expected to be equal}}
 
   // Important part about below diagnostic is that from-type is described as
   // 'NSString?' and not '@lvalue NSString?':
-  let _: String? = vns // expected-error{{cannot convert value of type 'NSString?' to specified type 'String?'}}{{23-23= as String?}}
-  var _: String? = vns // expected-error{{cannot convert value of type 'NSString?' to specified type 'String?'}}{{23-23= as String?}}
+  let _: String? = vns // expected-error{{cannot assign value of type 'NSString?' to type 'String?'}}{{23-23= as String?}}
+  // expected-note@-1 {{arguments to generic parameter 'Wrapped' ('NSString' and 'String') are expected to be equal}}
+  var _: String? = vns // expected-error{{cannot assign value of type 'NSString?' to type 'String?'}}{{23-23= as String?}}
+  // expected-note@-1 {{arguments to generic parameter 'Wrapped' ('NSString' and 'String') are expected to be equal}}
 
   vns = ns
 }
@@ -300,15 +304,17 @@ func rdar20029786(_ ns: NSString?) {
   // expected-error@-1 {{cannot convert value of type 'NSString?' to expected argument type 'String?'}} {{21-21= as String?}}
   var s2 = ns ?? "str" as String as String // expected-error {{binary operator '??' cannot be applied to operands of type 'NSString?' and 'String'}}
 
-  let s3: NSString? = "str" as String? // expected-error {{cannot convert value of type 'String?' to specified type 'NSString?'}}{{39-39= as NSString?}}
+  let s3: NSString? = "str" as String? // expected-error {{cannot assign value of type 'String?' to type 'NSString?'}}{{39-39= as NSString?}}
+  // expected-note@-1 {{arguments to generic parameter 'Wrapped' ('String' and 'NSString') are expected to be equal}}
 
   var s4: String = ns ?? "str" // expected-error{{'NSString' is not implicitly convertible to 'String'; did you mean to use 'as' to explicitly convert?}} {{20-20=(}} {{31-31=) as String}}
   var s5: String = (ns ?? "str") as String // fixed version
 }
 
 // Make sure more complicated cast has correct parenthesization
 func castMoreComplicated(anInt: Int?) {
-  let _: (NSObject & NSCopying)? = anInt // expected-error{{cannot convert value of type 'Int?' to specified type '(any NSObject & NSCopying)?'}}{{41-41= as (any NSObject & NSCopying)?}}
+  let _: (NSObject & NSCopying)? = anInt // expected-error{{cannot assign value of type 'Int?' to type '(any NSObject & NSCopying)?'}}{{41-41= as (any NSObject & NSCopying)?}}
+  // expected-note@-1 {{arguments to generic parameter 'Wrapped' ('Int' and 'any NSObject & NSCopying') are expected to be equal}}
 }
 
 

test/Constraints/casts_objc.swift

@@ -82,8 +82,8 @@ func optionalityMismatchingCasts(f: CGFloat, n: NSNumber, fooo: CGFloat???,
                                  nooo: NSNumber???) {
   _ = f as NSNumber?
   _ = f as NSNumber??
-  let _ = fooo as NSNumber?? // expected-error{{'CGFloat???' is not convertible to 'NSNumber??'}}
-  //expected-note@-1 {{did you mean to use 'as!' to force downcast?}} {{16-18=as!}}
+  let _ = fooo as NSNumber?? // expected-error{{cannot convert value of type 'CGFloat???' to type 'NSNumber??' in coercion}}
+  //expected-note@-1 {{arguments to generic parameter 'Wrapped' ('CGFloat?' and 'NSNumber') are expected to be equal}}
   let _ = fooo as NSNumber???? // okay: injects extra optionals
 }
 

test/Constraints/fixes.swift

@@ -158,16 +158,14 @@ struct Q {
   let s: String?
 }
 let q = Q(s: nil)
-let a: Int? = q.s.utf8 // expected-error{{value of optional type 'String?' must be unwrapped to refer to member 'utf8' of wrapped base type 'String'}}
-// expected-error@-1 {{cannot convert value of type 'String.UTF8View?' to specified type 'Int?'}}
-// expected-note@-2{{chain the optional using '?'}}{{18-18=?}}
-let b: Int = q.s.utf8 // expected-error{{value of optional type 'String?' must be unwrapped to refer to member 'utf8' of wrapped base type 'String'}}
+let a: Int? = q.s.utf8 // expected-error{{value of optional type 'String?' must be unwrapped to refer to member 'utf8' of wrapped base type 'String'}} expected-note {{chain the optional using '?'}}{{18-18=?}}
+// expected-error@-1 {{cannot assign value of type 'String.UTF8View?' to type 'Int?'}}
+// expected-note@-2 {{arguments to generic parameter 'Wrapped' ('String.UTF8View' and 'Int') are expected to be equal}}
+let b: Int = q.s.utf8 // expected-error{{value of optional type 'String?' must be unwrapped to refer to member 'utf8' of wrapped base type 'String'}} expected-note {{chain the optional using '?'}}{{17-17=?}} expected-note {{force-unwrap using '!'}}{{17-17=!}}
 // expected-error@-1 {{cannot convert value of type 'String.UTF8View' to specified type 'Int'}}
-// expected-note@-2{{chain the optional using '?'}}{{17-17=?}}
-// expected-note@-3{{force-unwrap using '!'}}{{17-17=!}}
-let d: Int! = q.s.utf8 // expected-error{{value of optional type 'String?' must be unwrapped to refer to member 'utf8' of wrapped base type 'String'}}
-// expected-error@-1 {{cannot convert value of type 'String.UTF8View?' to specified type 'Int?'}}
-// expected-note@-2{{chain the optional using '?'}}{{18-18=?}}
+let d: Int! = q.s.utf8 // expected-error{{value of optional type 'String?' must be unwrapped to refer to member 'utf8' of wrapped base type 'String'}} expected-note {{chain the optional using '?'}}{{18-18=?}}
+// expected-error@-1 {{cannot assign value of type 'String.UTF8View?' to type 'Int?'}}
+// expected-note@-2 {{arguments to generic parameter 'Wrapped' ('String.UTF8View' and 'Int') are expected to be equal}}
 let c = q.s.utf8 // expected-error{{value of optional type 'String?' must be unwrapped to refer to member 'utf8' of wrapped base type 'String'}}
 // expected-note@-1{{chain the optional using '?' to access member 'utf8' only for non-'nil' base values}}{{12-12=?}}
 // expected-note@-2{{force-unwrap using '!' to abort execution if the optional value contains 'nil'}}{{12-12=!}}