Skip to content

Commit 8685ee0

Browse files
committed
Allow inout arguments that differ in optionality than the expected parameter.
Allow passing Optional<T> as inout where ImplicitlyUnwrappedOptional<T> is expected, and vice-versa. Swift 4.1 added a warning that overloading inouts by kind of optional was deprecated and would be removed, but we didn't actually allow people to remove an overload and pass arguments of the other kind of optional to the remaining function. Fixes rdar://problem/36913150
1 parent 8dad899 commit 8685ee0

File tree

6 files changed

+106
-14
lines changed

6 files changed

+106
-14
lines changed

lib/AST/ASTVerifier.cpp

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1069,8 +1069,17 @@ class Verifier : public ASTWalker {
10691069
Type srcObj = checkLValue(E->getSubExpr()->getType(),
10701070
"result of InOutExpr");
10711071
auto DestTy = E->getType()->castTo<InOutType>()->getObjectType();
1072-
1073-
checkSameType(DestTy, srcObj, "object types for InOutExpr");
1072+
1073+
// HACK: Allow differences in optionality of the source and
1074+
// result types. When IUO is gone from the type system we'll no
1075+
// longer need this.
1076+
auto srcOptObjTy = srcObj->getAnyOptionalObjectType();
1077+
auto dstOptObjTy = DestTy->getAnyOptionalObjectType();
1078+
if (srcOptObjTy && dstOptObjTy) {
1079+
checkSameType(srcOptObjTy, dstOptObjTy, "object types for InOutExpr");
1080+
} else {
1081+
checkSameType(DestTy, srcObj, "object types for InOutExpr");
1082+
}
10741083
verifyCheckedBase(E);
10751084
}
10761085

lib/Sema/CSApply.cpp

Lines changed: 42 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5517,6 +5517,38 @@ static unsigned computeCallLevel(ConstraintSystem &cs, ConcreteDeclRef callee,
55175517
return 0;
55185518
}
55195519

5520+
// HACK: Support calling functions with inouts of differing kinds of
5521+
// optionality so that we can warn about overloading by IUO vs. plain
5522+
// Optional and allow users to correct the issue by removing an
5523+
// overload and passing the differently-optional value to the
5524+
// remaining overload.
5525+
bool inOutOptionalityDifferenceHack(Expr *arg, Type paramType,
5526+
ConstraintSystem &cs) {
5527+
auto *inOutArgTy = cs.getType(arg)->getAs<InOutType>();
5528+
if (!inOutArgTy)
5529+
return false;
5530+
5531+
auto *inOutParamTy = paramType->getAs<InOutType>();
5532+
if (!inOutParamTy)
5533+
return false;
5534+
5535+
OptionalTypeKind argOTK;
5536+
OptionalTypeKind paramOTK;
5537+
auto argObjTy = inOutArgTy->getObjectType()->getAnyOptionalObjectType(argOTK);
5538+
auto paramObjTy =
5539+
inOutParamTy->getObjectType()->getAnyOptionalObjectType(paramOTK);
5540+
5541+
if (argOTK == paramOTK || argOTK == OTK_None || paramOTK == OTK_None)
5542+
return false;
5543+
5544+
if (!argObjTy->isEqual(paramObjTy))
5545+
return false;
5546+
5547+
// Hammer over the argument type with the expected parameter type.
5548+
cs.setType(arg, paramType);
5549+
return true;
5550+
}
5551+
55205552
Expr *ExprRewriter::coerceCallArguments(
55215553
Expr *arg, AnyFunctionType *funcType,
55225554
ApplyExpr *apply,
@@ -5747,11 +5779,16 @@ Expr *ExprRewriter::coerceCallArguments(
57475779
continue;
57485780
}
57495781

5750-
// Convert the argument.
5751-
auto convertedArg = coerceToType(arg, paramType,
5752-
getArgLocator(argIdx, paramIdx));
5753-
if (!convertedArg)
5754-
return nullptr;
5782+
Expr *convertedArg;
5783+
if (inOutOptionalityDifferenceHack(arg, paramType, cs)) {
5784+
convertedArg = arg;
5785+
} else {
5786+
// Convert the argument.
5787+
convertedArg =
5788+
coerceToType(arg, paramType, getArgLocator(argIdx, paramIdx));
5789+
if (!convertedArg)
5790+
return nullptr;
5791+
}
57555792

57565793
// Add the converted argument.
57575794
fromTupleExpr[argIdx] = convertedArg;

lib/Sema/CSRanking.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,8 @@ void ConstraintSystem::increaseScore(ScoreKind kind, unsigned value) {
8080
case SK_ValueToPointerConversion:
8181
log << "value-to-pointer conversion";
8282
break;
83+
case SK_InOutOptionalityConversion:
84+
log << "inout optionality conversion";
8385
}
8486
log << ")\n";
8587
}

lib/Sema/CSSimplify.cpp

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1870,16 +1870,32 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
18701870
locator.withPathElement(
18711871
ConstraintLocator::ArrayElementType));
18721872

1873-
case TypeKind::InOut:
1873+
case TypeKind::InOut: {
18741874
// If the RHS is an inout type, the LHS must be an @lvalue type.
18751875
if (kind == ConstraintKind::BindParam ||
18761876
kind >= ConstraintKind::OperatorArgumentConversion)
18771877
return getTypeMatchFailure(locator);
1878-
1879-
return matchTypes(cast<InOutType>(desugar1)->getObjectType(),
1880-
cast<InOutType>(desugar2)->getObjectType(),
1881-
ConstraintKind::Equal, subflags,
1882-
locator.withPathElement(ConstraintLocator::ArrayElementType));
1878+
1879+
auto inoutObjTy1 = cast<InOutType>(desugar1)->getObjectType();
1880+
auto inoutObjTy2 = cast<InOutType>(desugar2)->getObjectType();
1881+
1882+
OptionalTypeKind OTK1;
1883+
OptionalTypeKind OTK2;
1884+
auto optionalObjTy1 = inoutObjTy1->getAnyOptionalObjectType(OTK1);
1885+
auto optionalObjTy2 = inoutObjTy2->getAnyOptionalObjectType(OTK2);
1886+
if (OTK1 != OTK2 && optionalObjTy1 && optionalObjTy2) {
1887+
increaseScore(ScoreKind::SK_InOutOptionalityConversion);
1888+
return matchTypes(inoutObjTy1,
1889+
inoutObjTy2,
1890+
ConstraintKind::ArgumentConversion, subflags,
1891+
locator.withPathElement(ConstraintLocator::ArrayElementType));
1892+
} else {
1893+
return matchTypes(inoutObjTy1,
1894+
inoutObjTy2,
1895+
ConstraintKind::Equal, subflags,
1896+
locator.withPathElement(ConstraintLocator::ArrayElementType));
1897+
}
1898+
}
18831899

18841900
case TypeKind::UnboundGeneric:
18851901
llvm_unreachable("Unbound generic type should have been opened");

lib/Sema/ConstraintSystem.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -462,8 +462,12 @@ enum ScoreKind {
462462
SK_KeyPathSubscript,
463463
/// A conversion from a string, array, or inout to a pointer.
464464
SK_ValueToPointerConversion,
465+
/// A conversion from 'inout Optional<T>' to 'inout
466+
/// ImplicitlyUnwrappedOptional<T>' or vice-versa.
467+
/// FIXME: This goes away when IUO-as-a-type goes away.
468+
SK_InOutOptionalityConversion,
465469

466-
SK_LastScoreKind = SK_ValueToPointerConversion,
470+
SK_LastScoreKind = SK_InOutOptionalityConversion,
467471
};
468472

469473
/// The number of score kinds.

test/Constraints/iuo.swift

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,3 +159,27 @@ func cast<T : P>(_ t: T) {
159159
let _: (Bool) -> T? = id(T.iuoResultStatic as (Bool) -> T?)
160160
let _: T! = id(T.iuoResultStatic(true))
161161
}
162+
163+
func takesInOutIUO(_ i: inout Int!) {}
164+
func takesInOutOpt(_ o: inout Int?) {}
165+
166+
func overloadedByOptionality(_ a: inout Int!) {}
167+
// expected-note@-1 {{'overloadedByOptionality' previously declared here}}
168+
// expected-note@-2 {{'overloadedByOptionality' previously declared here}}
169+
func overloadedByOptionality(_ a: inout Int?) {}
170+
// expected-warning@-1 {{invalid redeclaration of 'overloadedByOptionality' which differs only by the kind of optional passed as an inout argument ('Int?' vs. 'Int!')}}
171+
// expected-warning@-2 {{invalid redeclaration of 'overloadedByOptionality' which differs only by the kind of optional passed as an inout argument ('Int?' vs. 'Int!')}}
172+
// expected-note@-3 {{overloading by kind of optional is deprecated and will be removed in a future release}}
173+
// expected-note@-4 {{overloading by kind of optional is deprecated and will be removed in a future release}}
174+
175+
func testInOutOptionality() {
176+
var i: Int! = 1
177+
var o: Int? = 2
178+
179+
takesInOutIUO(&i)
180+
takesInOutOpt(&i)
181+
takesInOutIUO(&o)
182+
takesInOutOpt(&o)
183+
184+
overloadedByOptionality(&o)
185+
}

0 commit comments

Comments
 (0)