Skip to content

Commit 103882f

Browse files
tkremenektkremenek
authored
Merge pull request #7233 from rudkx/fix-rdar30311052-3.1
[Sema] Penalize conversions to Any.
2 parents d751c87 + bb0b264 commit 103882f

File tree

6 files changed

+64
-22
lines changed

6 files changed

+64
-22
lines changed

‎lib/Sema/CSRanking.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,9 @@ void ConstraintSystem::increaseScore(ScoreKind kind, unsigned value) {
7474
case SK_ScalarPointerConversion:
7575
log << "scalar-to-pointer conversion";
7676
break;
77+
case SK_EmptyExistentialConversion:
78+
log << "empty-existential conversion";
79+
break;
7780
}
7881
log << ")\n";
7982
}

‎lib/Sema/CSSimplify.cpp

Lines changed: 39 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1558,8 +1558,10 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
15581558
break;
15591559

15601560
case TypeKind::Tuple: {
1561+
assert(!type2->is<LValueType>() && "Unexpected lvalue type!");
15611562
// Try the tuple-to-tuple conversion.
1562-
conversionsOrFixes.push_back(ConversionRestrictionKind::TupleToTuple);
1563+
if (!type1->is<LValueType>())
1564+
conversionsOrFixes.push_back(ConversionRestrictionKind::TupleToTuple);
15631565
break;
15641566
}
15651567

@@ -1568,7 +1570,9 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
15681570
case TypeKind::Class: {
15691571
auto nominal1 = cast<NominalType>(desugar1);
15701572
auto nominal2 = cast<NominalType>(desugar2);
1571-
if (nominal1->getDecl() == nominal2->getDecl()) {
1573+
assert(!type2->is<LValueType>() && "Unexpected lvalue type!");
1574+
if (!type1->is<LValueType>() &&
1575+
nominal1->getDecl() == nominal2->getDecl()) {
15721576
conversionsOrFixes.push_back(ConversionRestrictionKind::DeepEquality);
15731577
}
15741578

@@ -1579,15 +1583,19 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
15791583
auto class2 = cast<ClassDecl>(nominal2->getDecl());
15801584

15811585
// CF -> Objective-C via toll-free bridging.
1582-
if (class1->getForeignClassKind() == ClassDecl::ForeignKind::CFType &&
1586+
assert(!type2->is<LValueType>() && "Unexpected lvalue type!");
1587+
if (!type1->is<LValueType>() &&
1588+
class1->getForeignClassKind() == ClassDecl::ForeignKind::CFType &&
15831589
class2->getForeignClassKind() != ClassDecl::ForeignKind::CFType &&
15841590
class1->getAttrs().hasAttribute<ObjCBridgedAttr>()) {
15851591
conversionsOrFixes.push_back(
15861592
ConversionRestrictionKind::CFTollFreeBridgeToObjC);
15871593
}
15881594

15891595
// Objective-C -> CF via toll-free bridging.
1590-
if (class2->getForeignClassKind() == ClassDecl::ForeignKind::CFType &&
1596+
assert(!type2->is<LValueType>() && "Unexpected lvalue type!");
1597+
if (!type1->is<LValueType>() &&
1598+
class2->getForeignClassKind() == ClassDecl::ForeignKind::CFType &&
15911599
class1->getForeignClassKind() != ClassDecl::ForeignKind::CFType &&
15921600
class2->getAttrs().hasAttribute<ObjCBridgedAttr>()) {
15931601
conversionsOrFixes.push_back(
@@ -1678,7 +1686,8 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
16781686
auto bound1 = cast<BoundGenericType>(desugar1);
16791687
auto bound2 = cast<BoundGenericType>(desugar2);
16801688

1681-
if (bound1->getDecl() == bound2->getDecl()) {
1689+
assert(!type2->is<LValueType>() && "Unexpected lvalue type!");
1690+
if (!type1->is<LValueType>() && bound1->getDecl() == bound2->getDecl()) {
16821691
conversionsOrFixes.push_back(ConversionRestrictionKind::DeepEquality);
16831692
}
16841693
break;
@@ -1713,12 +1722,13 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
17131722
// there is at most one non-defaulted element.
17141723
// For non-argument tuples, we can do the same conversion but not
17151724
// to a tuple with varargs.
1716-
if ((tuple2->getNumElements() == 1 &&
1717-
!tuple2->getElement(0).isVararg()) ||
1718-
(kind >= ConstraintKind::Conversion &&
1719-
tuple2->getElementForScalarInit() >= 0 &&
1720-
(isArgumentTupleConversion ||
1721-
!tuple2->getVarArgsBaseType()))) {
1725+
if (!type1->is<LValueType>() &&
1726+
((tuple2->getNumElements() == 1 &&
1727+
!tuple2->getElement(0).isVararg()) ||
1728+
(kind >= ConstraintKind::Conversion &&
1729+
tuple2->getElementForScalarInit() >= 0 &&
1730+
(isArgumentTupleConversion ||
1731+
!tuple2->getVarArgsBaseType())))) {
17221732
conversionsOrFixes.push_back(
17231733
ConversionRestrictionKind::ScalarToTuple);
17241734

@@ -1732,6 +1742,7 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
17321742
type2->getClassOrBoundGenericClass() &&
17331743
type1->getClassOrBoundGenericClass()
17341744
!= type2->getClassOrBoundGenericClass()) {
1745+
assert(!type2->is<LValueType>() && "Unexpected lvalue type!");
17351746
conversionsOrFixes.push_back(ConversionRestrictionKind::Superclass);
17361747
}
17371748

@@ -1740,7 +1751,9 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
17401751
// Don't allow this in operator contexts or we'll end up allowing
17411752
// 'T() == U()' for unrelated T and U that just happen to be Hashable.
17421753
// We can remove this special case when we implement operator hiding.
1743-
if (kind != ConstraintKind::OperatorArgumentConversion) {
1754+
if (!type1->is<LValueType>() &&
1755+
kind != ConstraintKind::OperatorArgumentConversion) {
1756+
assert(!type2->is<LValueType>() && "Unexpected lvalue type!");
17441757
conversionsOrFixes.push_back(
17451758
ConversionRestrictionKind::HashableToAnyHashable);
17461759
}
@@ -1800,16 +1813,20 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
18001813
}
18011814

18021815
// Special implicit nominal conversions.
1803-
if (kind >= ConstraintKind::Conversion) {
1816+
if (!type1->is<LValueType>() &&
1817+
kind >= ConstraintKind::Conversion) {
18041818
// Array -> Array.
18051819
if (isArrayType(desugar1) && isArrayType(desugar2)) {
1820+
assert(!type2->is<LValueType>() && "Unexpected lvalue type!");
18061821
conversionsOrFixes.push_back(ConversionRestrictionKind::ArrayUpcast);
18071822
// Dictionary -> Dictionary.
18081823
} else if (isDictionaryType(desugar1) && isDictionaryType(desugar2)) {
1824+
assert(!type2->is<LValueType>() && "Unexpected lvalue type!");
18091825
conversionsOrFixes.push_back(
18101826
ConversionRestrictionKind::DictionaryUpcast);
18111827
// Set -> Set.
18121828
} else if (isSetType(desugar1) && isSetType(desugar2)) {
1829+
assert(!type2->is<LValueType>() && "Unexpected lvalue type!");
18131830
conversionsOrFixes.push_back(
18141831
ConversionRestrictionKind::SetUpcast);
18151832
}
@@ -1981,7 +1998,13 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
19811998
// we hit commit_to_conversions below, but we have to add a token restriction
19821999
// to ensure we wrap the metatype value in a metatype erasure.
19832000
if (concrete && type2->isExistentialType() &&
2001+
!type1->is<LValueType>() &&
19842002
kind >= ConstraintKind::Subtype) {
2003+
2004+
// Penalize conversions to Any.
2005+
if (kind >= ConstraintKind::Conversion && type2->isAny())
2006+
increaseScore(ScoreKind::SK_EmptyExistentialConversion);
2007+
19852008
conversionsOrFixes.push_back(ConversionRestrictionKind::Existential);
19862009
}
19872010

@@ -1992,7 +2015,8 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
19922015
{
19932016
BoundGenericType *boundGenericType2;
19942017

1995-
if (concrete && kind >= ConstraintKind::Subtype &&
2018+
if (concrete && !type1->is<LValueType>() &&
2019+
kind >= ConstraintKind::Subtype &&
19962020
(boundGenericType2 = type2->getAs<BoundGenericType>())) {
19972021
auto decl2 = boundGenericType2->getDecl();
19982022
if (auto optionalKind2 = decl2->classifyAsOptionalType()) {
@@ -2025,6 +2049,7 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
20252049
ConversionRestrictionKind::ValueToOptional);
20262050
}
20272051
}
2052+
20282053
}
20292054

20302055
// A value of type T! can be (unsafely) forced to U if T

‎lib/Sema/ConstraintSystem.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -406,8 +406,10 @@ enum ScoreKind {
406406
SK_ScalarPointerConversion,
407407
/// A conversion from an array to a pointer of matching element type.
408408
SK_ArrayPointerConversion,
409+
/// A conversion to an empty existential type ('Any' or '{}').
410+
SK_EmptyExistentialConversion,
409411

410-
SK_LastScoreKind = SK_ArrayPointerConversion,
412+
SK_LastScoreKind = SK_EmptyExistentialConversion,
411413
};
412414

413415
/// The number of score kinds.

‎test/Constraints/array_literal.swift

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,3 +227,10 @@ let router = Company(
227227
}
228228
]
229229
)
230+
231+
// Infer [[Int]] for SR3786aa.
232+
// FIXME: As noted in SR-3786, this was the behavior in Swift 3, but
233+
// it seems like the wrong choice and is less by design than by
234+
// accident.
235+
let SR3786a: [Int] = [1, 2, 3]
236+
let SR3786aa = [SR3786a.reversed(), SR3786a]

‎test/Constraints/diag_ambiguities.swift

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,9 @@ func rdar29691909(o: AnyObject) -> Any? {
3636
return rdar29691909_callee(o) // expected-error{{ambiguous use of 'rdar29691909_callee'}}
3737
}
3838

39-
// FIXME: The fix for this broke other things. We want to get this
40-
// test case running again, though.
41-
// Ensure that we decay Any! to Any? rather than allowing Any!-to-Any
42-
// conversion directly and ending up with an ambiguity here.
43-
//func rdar29907555(_ value: Any!) -> String {
44-
// return "\(value)" // no error
45-
//}
39+
func rdar29907555(_ value: Any!) -> String {
40+
return "\(value)" // no error
41+
}
4642

4743
struct SR3715 {
4844
var overloaded: Int!

‎test/Constraints/overload.swift

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,3 +186,12 @@ func overloadedMethod<T>() {}
186186

187187
overloadedMethod()
188188
// expected-error@-1 {{missing argument for parameter 'n' in call}}
189+
190+
// Ensure we select the overload of '??' returning T? rather than T.
191+
func SR3817(_ d: [String : Any], _ s: String, _ t: String) -> Any {
192+
if let r = d[s] ?? d[t] {
193+
return r
194+
} else {
195+
return 0
196+
}
197+
}

0 commit comments

Comments
 (0)