Merge pull request #14246 from rudkx/rdar36913150-5.0

[5.0] Allow inout arguments that differ in optionality than the expected pa…

Author: rudkx

lib/AST/ASTVerifier.cpp

@@ -1069,8 +1069,17 @@ class Verifier : public ASTWalker {
       Type srcObj = checkLValue(E->getSubExpr()->getType(),
                                 "result of InOutExpr");
       auto DestTy = E->getType()->castTo<InOutType>()->getObjectType();
-      
-      checkSameType(DestTy, srcObj, "object types for InOutExpr");
+
+      // HACK: Allow differences in optionality of the source and
+      // result types. When IUO is gone from the type system we'll no
+      // longer need this.
+      auto srcOptObjTy = srcObj->getAnyOptionalObjectType();
+      auto dstOptObjTy = DestTy->getAnyOptionalObjectType();
+      if (srcOptObjTy && dstOptObjTy) {
+        checkSameType(srcOptObjTy, dstOptObjTy, "object types for InOutExpr");
+      } else {
+        checkSameType(DestTy, srcObj, "object types for InOutExpr");
+      }
       verifyCheckedBase(E);
     }
 

lib/Sema/CSApply.cpp

@@ -5447,6 +5447,38 @@ static unsigned computeCallLevel(ConstraintSystem &cs, ConcreteDeclRef callee,
   return 0;
 }
 
+// HACK: Support calling functions with inouts of differing kinds of
+// optionality so that we can warn about overloading by IUO vs. plain
+// Optional and allow users to correct the issue by removing an
+// overload and passing the differently-optional value to the
+// remaining overload.
+bool inOutOptionalityDifferenceHack(Expr *arg, Type paramType,
+                                    ConstraintSystem &cs) {
+  auto *inOutArgTy = cs.getType(arg)->getAs<InOutType>();
+  if (!inOutArgTy)
+    return false;
+
+  auto *inOutParamTy = paramType->getAs<InOutType>();
+  if (!inOutParamTy)
+    return false;
+
+  OptionalTypeKind argOTK;
+  OptionalTypeKind paramOTK;
+  auto argObjTy = inOutArgTy->getObjectType()->getAnyOptionalObjectType(argOTK);
+  auto paramObjTy =
+      inOutParamTy->getObjectType()->getAnyOptionalObjectType(paramOTK);
+
+  if (argOTK == paramOTK || argOTK == OTK_None || paramOTK == OTK_None)
+    return false;
+
+  if (!argObjTy->isEqual(paramObjTy))
+    return false;
+
+  // Hammer over the argument type with the expected parameter type.
+  cs.setType(arg, paramType);
+  return true;
+}
+
 Expr *ExprRewriter::coerceCallArguments(
     Expr *arg, AnyFunctionType *funcType,
     ApplyExpr *apply,
@@ -5677,11 +5709,16 @@ Expr *ExprRewriter::coerceCallArguments(
       continue;
     }
 
-    // Convert the argument.
-    auto convertedArg = coerceToType(arg, paramType,
-                                     getArgLocator(argIdx, paramIdx));
-    if (!convertedArg)
-      return nullptr;
+    Expr *convertedArg;
+    if (inOutOptionalityDifferenceHack(arg, paramType, cs)) {
+      convertedArg = arg;
+    } else {
+      // Convert the argument.
+      convertedArg =
+          coerceToType(arg, paramType, getArgLocator(argIdx, paramIdx));
+      if (!convertedArg)
+        return nullptr;
+    }
 
     // Add the converted argument.
     fromTupleExpr[argIdx] = convertedArg;

lib/Sema/CSRanking.cpp

@@ -80,6 +80,8 @@ void ConstraintSystem::increaseScore(ScoreKind kind, unsigned value) {
     case SK_ValueToPointerConversion:
       log << "value-to-pointer conversion";
       break;
+    case SK_InOutOptionalityConversion:
+      log << "inout optionality conversion";
     }
     log << ")\n";
   }

lib/Sema/CSSimplify.cpp

@@ -1906,16 +1906,32 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
                         locator.withPathElement(
                           ConstraintLocator::ArrayElementType));
 
-    case TypeKind::InOut:
+    case TypeKind::InOut: {
       // If the RHS is an inout type, the LHS must be an @lvalue type.
       if (kind == ConstraintKind::BindParam ||
           kind >= ConstraintKind::OperatorArgumentConversion)
         return SolutionKind::Error;
-      
-      return matchTypes(cast<InOutType>(desugar1)->getObjectType(),
-                        cast<InOutType>(desugar2)->getObjectType(),
-                        ConstraintKind::Equal, subflags,
-                  locator.withPathElement(ConstraintLocator::ArrayElementType));
+
+      auto inoutObjTy1 = cast<InOutType>(desugar1)->getObjectType();
+      auto inoutObjTy2 = cast<InOutType>(desugar2)->getObjectType();
+
+      OptionalTypeKind OTK1;
+      OptionalTypeKind OTK2;
+      auto optionalObjTy1 = inoutObjTy1->getAnyOptionalObjectType(OTK1);
+      auto optionalObjTy2 = inoutObjTy2->getAnyOptionalObjectType(OTK2);
+      if (OTK1 != OTK2 && optionalObjTy1 && optionalObjTy2) {
+        increaseScore(ScoreKind::SK_InOutOptionalityConversion);
+        return matchTypes(inoutObjTy1,
+                          inoutObjTy2,
+                          ConstraintKind::ArgumentConversion, subflags,
+                          locator.withPathElement(ConstraintLocator::ArrayElementType));
+      } else {
+        return matchTypes(inoutObjTy1,
+                          inoutObjTy2,
+                          ConstraintKind::Equal, subflags,
+                          locator.withPathElement(ConstraintLocator::ArrayElementType));
+      }
+    }
 
     case TypeKind::UnboundGeneric:
       llvm_unreachable("Unbound generic type should have been opened");

lib/Sema/ConstraintSystem.h

@@ -462,8 +462,12 @@ enum ScoreKind {
   SK_KeyPathSubscript,
   /// A conversion from a string, array, or inout to a pointer.
   SK_ValueToPointerConversion,
+  /// A conversion from 'inout Optional<T>' to 'inout
+  /// ImplicitlyUnwrappedOptional<T>' or vice-versa.
+  /// FIXME: This goes away when IUO-as-a-type goes away.
+  SK_InOutOptionalityConversion,
 
-  SK_LastScoreKind = SK_ValueToPointerConversion,
+  SK_LastScoreKind = SK_InOutOptionalityConversion,
 };
 
 /// The number of score kinds.

test/Constraints/iuo.swift

@@ -159,3 +159,27 @@ func cast<T : P>(_ t: T) {
   let _: (Bool) -> T? = id(T.iuoResultStatic as (Bool) -> T?)
   let _: T! = id(T.iuoResultStatic(true))
 }
+
+func takesInOutIUO(_ i: inout Int!) {}
+func takesInOutOpt(_ o: inout Int?) {}
+
+func overloadedByOptionality(_ a: inout Int!) {}
+// expected-note@-1 {{'overloadedByOptionality' previously declared here}}
+// expected-note@-2 {{'overloadedByOptionality' previously declared here}}
+func overloadedByOptionality(_ a: inout Int?) {}
+// expected-warning@-1 {{invalid redeclaration of 'overloadedByOptionality' which differs only by the kind of optional passed as an inout argument ('Int?' vs. 'Int!')}}
+// expected-warning@-2 {{invalid redeclaration of 'overloadedByOptionality' which differs only by the kind of optional passed as an inout argument ('Int?' vs. 'Int!')}}
+// expected-note@-3 {{overloading by kind of optional is deprecated and will be removed in a future release}}
+// expected-note@-4 {{overloading by kind of optional is deprecated and will be removed in a future release}}
+
+func testInOutOptionality() {
+  var i: Int! = 1
+  var o: Int? = 2
+
+  takesInOutIUO(&i)
+  takesInOutOpt(&i)
+  takesInOutIUO(&o)
+  takesInOutOpt(&o)
+
+  overloadedByOptionality(&o)
+}