[TypeChecker] Disambiguite cases of implicit pointer conversions with optionals

Currently `{inout, array, string}-to-pointer` conversion doesn't
track whether there was a difference in optionality between involved
types which leads to ambiguity when different overload choices
have different optionality requirements.

Let's fix that by increasing a score in cases if pointer type
is itself optional e.g.:

```swift
func foo(_ x: UnsafeMutablePointer<Int>) {}
func foo(_ x: UnsafeMutablePointer<Int>?) {}

foo(&foo) // Should pick the least optional overload choice.
```

Resolves: [SR-8411](https://bugs.swift.org/browse/SR-8411)

Author: xedin

lib/Sema/CSSimplify.cpp

@@ -2199,7 +2199,6 @@ static bool isStringCompatiblePointerBaseType(TypeChecker &TC,
 /// is potentially more optional than the second type with its number of
 /// optionals.
 static bool isPotentiallyMoreOptionalThan(Type type1, Type type2) {
-
   SmallVector<Type, 2> optionals1;
   Type objType1 = type1->lookThroughAllOptionalTypes(optionals1);
   auto numOptionals1 = optionals1.size();
@@ -7661,11 +7660,17 @@ ConstraintSystem::simplifyDynamicCallableApplicableFnConstraint(
   return SolutionKind::Solved;
 }
 
-static Type getBaseTypeForPointer(ConstraintSystem &cs, TypeBase *type) {
-  auto pointeeTy = type->lookThroughSingleOptionalType()
-                       ->getAnyPointerElementType();
+static llvm::PointerIntPair<Type, 3, unsigned>
+getBaseTypeForPointer(TypeBase *type) {
+  unsigned unwrapCount = 0;
+  while (auto objectTy = type->getOptionalObjectType()) {
+    type = objectTy.getPointer();
+    ++unwrapCount;
+  }
+
+  auto pointeeTy = type->getAnyPointerElementType();
   assert(pointeeTy);
-  return pointeeTy;
+  return {pointeeTy, unwrapCount};
 }
 
 void ConstraintSystem::addRestrictedConstraint(
@@ -7858,22 +7863,26 @@ ConstraintSystem::simplifyRestrictedConstraintImpl(
     auto t2 = type2->getDesugaredType();
 
     auto baseType1 = getFixedTypeRecursive(*isArrayType(obj1), false);
-    auto baseType2 = getBaseTypeForPointer(*this, t2);
+    auto ptr2 = getBaseTypeForPointer(t2);
 
-    return matchPointerBaseTypes(baseType1, baseType2);
+    increaseScore(SK_ValueToOptional, ptr2.getInt());
+
+    return matchPointerBaseTypes(baseType1, ptr2.getPointer());
   }
 
   // String ===> UnsafePointer<[U]Int8>
   case ConversionRestrictionKind::StringToPointer: {
     addContextualScore();
 
-    auto baseType2 = getBaseTypeForPointer(*this, type2->getDesugaredType());
-    
+    auto ptr2 = getBaseTypeForPointer(type2->getDesugaredType());
+
+    increaseScore(SK_ValueToOptional, ptr2.getInt());
+
     // The pointer element type must be void or a byte-sized type.
     // TODO: Handle different encodings based on pointer element type, such as
     // UTF16 for [U]Int16 or UTF32 for [U]Int32. For now we only interop with
     // Int8 pointers using UTF8 encoding.
-    baseType2 = getFixedTypeRecursive(baseType2, false);
+    auto baseType2 = getFixedTypeRecursive(ptr2.getPointer(), false);
     // If we haven't resolved the element type, generate constraints.
     if (baseType2->isTypeVariableOrMember()) {
       if (flags.contains(TMF_GenerateConstraints)) {
@@ -7910,22 +7919,24 @@ ConstraintSystem::simplifyRestrictedConstraintImpl(
     addContextualScore();
 
     auto t2 = type2->getDesugaredType();
-    
+
     auto baseType1 = type1->getInOutObjectType();
-    auto baseType2 = getBaseTypeForPointer(*this, t2);
+    auto ptr2 = getBaseTypeForPointer(t2);
 
-    return matchPointerBaseTypes(baseType1, baseType2);
+    increaseScore(SK_ValueToOptional, ptr2.getInt());
+
+    return matchPointerBaseTypes(baseType1, ptr2.getPointer());
   }
 
   // T <p U ===> UnsafeMutablePointer<T> <a UnsafeMutablePointer<U>
   case ConversionRestrictionKind::PointerToPointer: {
     auto t1 = type1->getDesugaredType();
     auto t2 = type2->getDesugaredType();
-    
-    Type baseType1 = getBaseTypeForPointer(*this, t1);
-    Type baseType2 = getBaseTypeForPointer(*this, t2);
 
-    return matchPointerBaseTypes(baseType1, baseType2);
+    auto ptr1 = getBaseTypeForPointer(t1);
+    auto ptr2 = getBaseTypeForPointer(t2);
+
+    return matchPointerBaseTypes(ptr1.getPointer(), ptr2.getPointer());
   }
 
   // T < U or T is bridged to V where V < U ===> Array<T> <c Array<U>

test/Constraints/optional.swift

@@ -373,3 +373,24 @@ func rdar_53238058() {
     // expected-note@-2 {{force-unwrap using '!' to abort execution if the optional value contains 'nil'}}
   }
 }
+
+// SR-8411 - Inconsistent ambiguity with optional and non-optional inout-to-pointer
+func sr8411() {
+  struct S {
+    init(_ x: UnsafeMutablePointer<Int>) {}
+    init(_ x: UnsafeMutablePointer<Int>?) {}
+
+    static func foo(_ x: UnsafeMutablePointer<Int>) {}
+    static func foo(_ x: UnsafeMutablePointer<Int>?) {}
+
+    static func bar(_ x: UnsafeMutablePointer<Int>, _ y: Int) {}
+    static func bar(_ x: UnsafeMutablePointer<Int>?, _ y: Int) {}
+  }
+
+  var foo = 0
+
+  _ = S(&foo)      // Ok
+  _ = S.init(&foo) // Ok
+  _ = S.foo(&foo)  // Ok
+  _ = S.bar(&foo, 42) // Ok
+}

test/stdlib/UnsafePointerDiagnostics.swift

@@ -152,18 +152,17 @@ func unsafePointerInitEphemeralConversions() {
   // expected-note@-1 {{implicit argument conversion from 'Int' to 'UnsafePointer<Int>' produces a pointer valid only for the duration of the call to '+'}}
   // expected-note@-2 {{use 'withUnsafePointer' in order to explicitly convert argument to pointer valid for a defined scope}}
 
-  // FIXME(SR-8411): This is currently ambiguous. The reason why we don't get
-  // the usual "no exact matches in call to initializer" error is we cannot
-  // currently diagnose ambiguities between solutions with multiple fixes, so we
-  // leave it up to CSDiag.
-  _ = UnsafePointer.init(&foo) // expected-error {{ambiguous reference to member 'init(_:)'}}
+  _ = UnsafePointer.init(&foo) // expected-error {{initialization of 'UnsafePointer<Int>' results in a dangling pointer}}
+  // expected-note@-1 {{implicit argument conversion from 'Int' to 'UnsafePointer<Int>' produces a pointer valid only for the duration of the call to 'init(_:)'}}
+  // expected-note@-2 {{use 'withUnsafePointer' in order to explicitly convert argument to pointer valid for a defined scope}}
 
   _ = UnsafePointer<Int8>("") // expected-error {{initialization of 'UnsafePointer<Int8>' results in a dangling pointer}}
   // expected-note@-1 {{implicit argument conversion from 'String' to 'UnsafePointer<Int8>' produces a pointer valid only for the duration of the call to 'init(_:)'}}
   // expected-note@-2 {{use the 'withCString' method on String in order to explicitly convert argument to pointer valid for a defined scope}}
 
-  // FIXME(SR-8411): This is currently ambiguous.
-  _ = UnsafePointer<Int8>.init("") // expected-error {{no exact matches in call to initializer}}
+  _ = UnsafePointer<Int8>.init("") // expected-error {{initialization of 'UnsafePointer<Int8>' results in a dangling pointer}}
+  // expected-note@-1 {{implicit argument conversion from 'String' to 'UnsafePointer<Int8>' produces a pointer valid only for the duration of the call to 'init(_:)'}}
+  // expected-note@-2 {{use the 'withCString' method on String in order to explicitly convert argument to pointer valid for a defined scope}}
 
   _ = UnsafePointer<Int8>(str) // expected-error {{initialization of 'UnsafePointer<Int8>' results in a dangling pointer}}
   // expected-note@-1 {{implicit argument conversion from 'String' to 'UnsafePointer<Int8>' produces a pointer valid only for the duration of the call to 'init(_:)'}}

test/stdlib/UnsafePointerDiagnostics_warning.swift

@@ -17,18 +17,17 @@ func unsafePointerInitEphemeralConversions() {
   // expected-note@-1 {{implicit argument conversion from 'Int' to 'UnsafePointer<Int>' produces a pointer valid only for the duration of the call to '+'}}
   // expected-note@-2 {{use 'withUnsafePointer' in order to explicitly convert argument to pointer valid for a defined scope}}
 
-  // FIXME(SR-8411): This is currently ambiguous. The reason why we don't get
-  // the usual "no exact matches in call to initializer" error is we cannot
-  // currently diagnose ambiguities between solutions with multiple fixes, so we
-  // leave it up to CSDiag.
-  _ = UnsafePointer.init(&foo) // expected-error {{ambiguous reference to member 'init(_:)'}}
+  _ = UnsafePointer.init(&foo) // expected-warning {{initialization of 'UnsafePointer<Int>' results in a dangling pointer}}
+  // expected-note@-1 {{implicit argument conversion from 'Int' to 'UnsafePointer<Int>' produces a pointer valid only for the duration of the call to 'init(_:)'}}
+  // expected-note@-2 {{use 'withUnsafePointer' in order to explicitly convert argument to pointer valid for a defined scope}}
 
   _ = UnsafePointer<Int8>("") // expected-warning {{initialization of 'UnsafePointer<Int8>' results in a dangling pointer}}
   // expected-note@-1 {{implicit argument conversion from 'String' to 'UnsafePointer<Int8>' produces a pointer valid only for the duration of the call to 'init(_:)'}}
   // expected-note@-2 {{use the 'withCString' method on String in order to explicitly convert argument to pointer valid for a defined scope}}
 
-  // FIXME(SR-8411): This is currently ambiguous.
-  _ = UnsafePointer<Int8>.init("") // expected-error {{no exact matches in call to initializer}}
+  _ = UnsafePointer<Int8>.init("") // expected-warning {{initialization of 'UnsafePointer<Int8>' results in a dangling pointer}}
+  // expected-note@-1 {{implicit argument conversion from 'String' to 'UnsafePointer<Int8>' produces a pointer valid only for the duration of the call to 'init(_:)'}}
+  // expected-note@-2 {{use the 'withCString' method on String in order to explicitly convert argument to pointer valid for a defined scope}}
 
   _ = UnsafePointer<Int8>(str) // expected-warning {{initialization of 'UnsafePointer<Int8>' results in a dangling pointer}}
   // expected-note@-1 {{implicit argument conversion from 'String' to 'UnsafePointer<Int8>' produces a pointer valid only for the duration of the call to 'init(_:)'}}