Skip to content

Commit 55b8d95

Browse files
committed
[CSSimplify] Rework how/when mismatches between optional types are fixed
- Don't attempt to insert fixes if there are restrictions present, they'd inform the failures. Inserting fixes too early doesn't help the solver because restriction matching logic would record the same fixes. - Adjust impact of the fixes. Optional conversions shouldn't impact the score in any way because they are not the source of the issue. - Look through one level of optional when failure is related to optional injection. The diagnostic is going to be about underlying type, so there is no reason to print optional on right-hand side.
1 parent 854298a commit 55b8d95

27 files changed

+105
-139
lines changed

lib/Sema/CSSimplify.cpp

Lines changed: 31 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -3978,8 +3978,19 @@ ConstraintSystem::matchExistentialTypes(Type type1, Type type2,
39783978
path.back().is<LocatorPathElt::GenericArgument>()))
39793979
path.pop_back();
39803980

3981+
auto *fixLoc = getConstraintLocator(anchor, path);
3982+
// If after looking through optionals and generic arguments
3983+
// we end up directly on assignment this is a source/destination
3984+
// type mismatch.
3985+
if (fixLoc->directlyAt<AssignExpr>()) {
3986+
auto *fix = IgnoreAssignmentDestinationType::create(
3987+
*this, type1, type2, fixLoc);
3988+
return recordFix(fix) ? getTypeMatchFailure(locator)
3989+
: getTypeMatchSuccess();
3990+
}
3991+
39813992
auto *fix = AllowNonClassTypeToConvertToAnyObject::create(
3982-
*this, type1, getConstraintLocator(anchor, path));
3993+
*this, type1, fixLoc);
39833994

39843995
return recordFix(fix) ? getTypeMatchFailure(locator)
39853996
: getTypeMatchSuccess();
@@ -5341,6 +5352,9 @@ bool ConstraintSystem::repairFailures(
53415352
if (repairViaBridgingCast(*this, lhs, rhs, conversionsOrFixes, locator))
53425353
return true;
53435354

5355+
if (hasAnyRestriction())
5356+
return false;
5357+
53445358
// If destination is `AnyObject` it means that source doesn't conform.
53455359
if (rhs->getWithoutSpecifierType()
53465360
->lookThroughAllOptionalTypes()
@@ -5350,60 +5364,6 @@ bool ConstraintSystem::repairFailures(
53505364
return true;
53515365
}
53525366

5353-
// If we are trying to assign e.g. `Array<Int>` to `Array<Float>` let's
5354-
// give solver a chance to determine which generic parameters are
5355-
// mismatched and produce a fix for that.
5356-
if (hasConversionOrRestriction(ConversionRestrictionKind::DeepEquality))
5357-
return false;
5358-
5359-
// An attempt to assign `Int?` to `String?`.
5360-
if (hasConversionOrRestriction(
5361-
ConversionRestrictionKind::OptionalToOptional)) {
5362-
conversionsOrFixes.push_back(IgnoreAssignmentDestinationType::create(
5363-
*this, lhs, rhs, getConstraintLocator(locator)));
5364-
return true;
5365-
}
5366-
5367-
// If the situation has to do with protocol composition types and
5368-
// destination doesn't have one of the conformances e.g. source is
5369-
// `X & Y` but destination is only `Y` or vice versa, there is a
5370-
// tailored "missing conformance" fix for that.
5371-
if (hasConversionOrRestriction(ConversionRestrictionKind::Existential))
5372-
return false;
5373-
5374-
if (hasConversionOrRestriction(
5375-
ConversionRestrictionKind::MetatypeToExistentialMetatype) ||
5376-
hasConversionOrRestriction(
5377-
ConversionRestrictionKind::ExistentialMetatypeToMetatype) ||
5378-
hasConversionOrRestriction(ConversionRestrictionKind::Superclass)) {
5379-
conversionsOrFixes.push_back(IgnoreAssignmentDestinationType::create(
5380-
*this, lhs, rhs, getConstraintLocator(locator)));
5381-
return true;
5382-
}
5383-
5384-
if (hasConversionOrRestriction(
5385-
ConversionRestrictionKind::ValueToOptional)) {
5386-
lhs = lhs->lookThroughAllOptionalTypes();
5387-
rhs = rhs->lookThroughAllOptionalTypes();
5388-
5389-
// If both object types are functions, let's allow the solver to
5390-
// structurally compare them before trying to fix anything.
5391-
if (lhs->is<FunctionType>() && rhs->is<FunctionType>())
5392-
return false;
5393-
5394-
// If either object type is a generic, nominal or existential type
5395-
// it means that follow-up to value-to-optional is going to be:
5396-
//
5397-
// 1. "deep equality" check, which is handled by generic argument(s)
5398-
// or contextual mismatch fix, or
5399-
// 2. "existential" check, which is handled by a missing conformance
5400-
// fix.
5401-
if ((lhs->is<BoundGenericType>() && rhs->is<BoundGenericType>()) ||
5402-
(lhs->is<NominalType>() && rhs->is<NominalType>()) ||
5403-
rhs->isAnyExistentialType())
5404-
return false;
5405-
}
5406-
54075367
auto *destExpr = AE->getDest();
54085368
// Literal expression as well as call/operator application can't be
54095369
// used as an assignment destination because resulting type is immutable.
@@ -14216,14 +14176,23 @@ ConstraintSystem::simplifyRestrictedConstraintImpl(
1421614176
if (hasFixFor(loc, FixKind::AllowTupleTypeMismatch))
1421714177
return true;
1421814178

14219-
if (restriction == ConversionRestrictionKind::ValueToOptional ||
14220-
restriction == ConversionRestrictionKind::OptionalToOptional)
14221-
++impact;
14179+
if (restriction == ConversionRestrictionKind::ValueToOptional) {
14180+
// If this is an optional injection we can drop optional from
14181+
// "to" type since it's not significant for the diagnostic.
14182+
toType = toType->getOptionalObjectType();
14183+
}
1422214184

14223-
auto *fix =
14224-
loc->isLastElement<LocatorPathElt::ApplyArgToParam>()
14225-
? AllowArgumentMismatch::create(*this, fromType, toType, loc)
14226-
: ContextualMismatch::create(*this, fromType, toType, loc);
14185+
ConstraintFix *fix = nullptr;
14186+
if (loc->isLastElement<LocatorPathElt::ApplyArgToParam>()) {
14187+
fix = AllowArgumentMismatch::create(*this, fromType, toType, loc);
14188+
} else if (loc->isForAssignment()) {
14189+
fix = IgnoreAssignmentDestinationType::create(*this, fromType, toType,
14190+
loc);
14191+
} else {
14192+
fix = ContextualMismatch::create(*this, fromType, toType, loc);
14193+
}
14194+
14195+
assert(fix);
1422714196
return !recordFix(fix, impact);
1422814197
}
1422914198

test/APINotes/type_changes.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,6 @@ import APINotesFrameworkTest
77

88

99
func testChangedTypes(tc: TypeChanges, a: A, i: Int) {
10-
_ = tc.method(with: i) // expected-error{{cannot convert value of type 'Int' to expected argument type 'A?'}}
10+
_ = tc.method(with: i) // expected-error{{cannot convert value of type 'Int' to expected argument type 'A'}}
1111
let _: Int = tc.method(with: a) // expected-error{{cannot convert value of type 'A' to specified type 'Int'}}
1212
}

test/ClangImporter/MixedSource/import-mixed-framework-with-forward.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,4 +16,4 @@ BridgingHeader.takeForward(SwiftClass(x: 42))
1616
BridgingHeader.takeRenamedForward(CustomNameClass())
1717

1818
// Check that we're compiling at all.
19-
BridgingHeader.takeRenamedForward(SwiftClass(x: 42)) // expected-error {{cannot convert value of type 'SwiftClass' to expected argument type 'CustomNameClass?'}}
19+
BridgingHeader.takeRenamedForward(SwiftClass(x: 42)) // expected-error {{cannot convert value of type 'SwiftClass' to expected argument type 'CustomNameClass'}}

test/ClangImporter/cf.swift

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ func test0(_ fridge: CCRefrigerator) {
1616

1717
func test1(_ power: Unmanaged<CCPowerSupply>) {
1818
assertUnmanaged(power)
19-
let fridge = CCRefrigeratorCreate(power) // expected-error {{cannot convert value of type 'Unmanaged<CCPowerSupply>' to expected argument type 'CCPowerSupply?'}}
19+
let fridge = CCRefrigeratorCreate(power) // expected-error {{cannot convert value of type 'Unmanaged<CCPowerSupply>' to expected argument type 'CCPowerSupply'}}
2020
assertUnmanaged(fridge) // expected-error {{generic parameter 'T' could not be inferred}}
2121
}
2222

@@ -64,9 +64,9 @@ func test9() {
6464
CCRefrigeratorOpen(fridge)
6565
let item = CCRefrigeratorGet(fridge, 0).takeUnretainedValue()
6666
// TODO(diagnostics): In this case we should probably suggest to flip `item` and `fridge`
67-
CCRefrigeratorInsert(item, fridge) // expected-error {{cannot convert value of type 'CCItem' to expected argument type 'CCMutableRefrigerator?'}}
68-
// expected-error@-1 {{cannot convert value of type 'CCMutableRefrigerator' to expected argument type 'CCItem?'}}
69-
CCRefrigeratorInsert(constFridge, item) // expected-error {{cannot convert value of type 'CCRefrigerator' to expected argument type 'CCMutableRefrigerator?'}}
67+
CCRefrigeratorInsert(item, fridge) // expected-error {{cannot convert value of type 'CCItem' to expected argument type 'CCMutableRefrigerator'}}
68+
// expected-error@-1 {{cannot convert value of type 'CCMutableRefrigerator' to expected argument type 'CCItem'}}
69+
CCRefrigeratorInsert(constFridge, item) // expected-error {{cannot convert value of type 'CCRefrigerator' to expected argument type 'CCMutableRefrigerator'}}
7070
CCRefrigeratorInsert(fridge, item)
7171
CCRefrigeratorClose(fridge)
7272
}
@@ -121,12 +121,12 @@ func testOutParametersBad() {
121121

122122
let power: CCPowerSupply?
123123
CCRefrigeratorGetPowerSupplyIndirect(0, power)
124-
// expected-error@-1:40 {{cannot convert value of type 'Int' to expected argument type 'CCRefrigerator?'}}
124+
// expected-error@-1:40 {{cannot convert value of type 'Int' to expected argument type 'CCRefrigerator'}}
125125
// expected-error@-2:43 {{cannot convert value of type 'CCPowerSupply?' to expected argument type 'AutoreleasingUnsafeMutablePointer<CCPowerSupply?>'}}
126126

127127
let item: CCItem?
128128
CCRefrigeratorGetItemUnaudited(0, 0, item)
129-
// expected-error@-1:34 {{cannot convert value of type 'Int' to expected argument type 'CCRefrigerator?'}}
129+
// expected-error@-1:34 {{cannot convert value of type 'Int' to expected argument type 'CCRefrigerator'}}
130130
// expected-error@-2:40 {{cannot convert value of type 'CCItem?' to expected argument type 'UnsafeMutablePointer<Unmanaged<CCItem>?>?'}}
131131
}
132132

test/ClangImporter/cfuncs_parse.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -187,8 +187,8 @@ func test_nested_pointers() {
187187
nested_pointer_audited(nil)
188188
nested_pointer_audited2(nil) // expected-error {{'nil' is not compatible with expected argument type 'UnsafePointer<UnsafePointer<Int32>?>'}}
189189

190-
nested_pointer(0) // expected-error {{expected argument type 'UnsafePointer<UnsafePointer<Int32>?>?'}}
191-
nested_pointer_audited(0) // expected-error {{expected argument type 'UnsafePointer<UnsafePointer<Int32>>?'}}
190+
nested_pointer(0) // expected-error {{expected argument type 'UnsafePointer<UnsafePointer<Int32>?>'}}
191+
nested_pointer_audited(0) // expected-error {{expected argument type 'UnsafePointer<UnsafePointer<Int32>>'}}
192192
nested_pointer_audited2(0) // expected-error {{expected argument type 'UnsafePointer<UnsafePointer<Int32>?>'}}
193193
}
194194

test/ClangImporter/nullability.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,5 +66,5 @@ func testCF(_ fridge: CCRefrigerator) {
6666
CCRefrigeratorOpenMaybeDoSomething(fridge) // okay
6767
CCRefrigeratorOpenMaybeDoSomething(nil) // okay
6868

69-
CCRefrigeratorOpenMaybeDoSomething(5) // expected-error{{cannot convert value of type 'Int' to expected argument type 'CCRefrigerator?'}}
69+
CCRefrigeratorOpenMaybeDoSomething(5) // expected-error{{cannot convert value of type 'Int' to expected argument type 'CCRefrigerator'}}
7070
}

test/ClangImporter/objc_id_as_any.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ idLover.takesArray(ofId: &y) // expected-error{{cannot convert value of type 'Un
2727
idLover.takesId(x)
2828
idLover.takesId(y)
2929

30-
install_global_event_handler(idLover) // expected-error {{cannot convert value of type 'NSIdLover' to expected argument type 'event_handler?' (aka 'Optional<@convention(c) (Any) -> ()>')}}
30+
install_global_event_handler(idLover) // expected-error {{cannot convert value of type 'NSIdLover' to expected argument type 'event_handler' (aka '@convention(c) (Any) -> ()')}}
3131

3232
// FIXME: this should not type-check!
3333
// Function conversions are not legal when converting to a thin function type.

test/Concurrency/isolated_parameters.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -483,7 +483,7 @@ actor A2 {
483483
await { (self: isolated Self) in }(self)
484484
// expected-typechecker-error@-1 {{cannot convert value of type 'A2' to expected argument type 'Self'}}
485485
await { (self: isolated Self?) in }(self)
486-
// expected-typechecker-error@-1 {{cannot convert value of type 'A2' to expected argument type 'Self?'}}
486+
// expected-typechecker-error@-1 {{cannot convert value of type 'A2' to expected argument type 'Self'}}
487487
#endif
488488
}
489489
nonisolated func f2() async -> Self {

test/Constraints/assignment.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,6 @@ class С_56396 {
8585
var callback: ((С_56396) -> Void)!
8686

8787
func setCallback(_ callback: @escaping (Self) -> Void) {
88-
self.callback = callback // expected-error {{cannot assign value of type '(Self) -> Void' to type '((С_56396) -> Void)?'}}
88+
self.callback = callback // expected-error {{cannot assign value of type '(Self) -> Void' to type '(С_56396) -> Void'}}
8989
}
9090
}

test/Constraints/closures.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ var afterMessageCount : Int?
356356
func uintFunc() -> UInt {}
357357
func takeVoidVoidFn(_ a : () -> ()) {}
358358
takeVoidVoidFn { () -> Void in
359-
afterMessageCount = uintFunc() // expected-error {{cannot assign value of type 'UInt' to type 'Int?'}} {{23-23=Int(}} {{33-33=)}}
359+
afterMessageCount = uintFunc() // expected-error {{cannot assign value of type 'UInt' to type 'Int'}} {{23-23=Int(}} {{33-33=)}}
360360
}
361361

362362
// <rdar://problem/19997471> Swift: Incorrect compile error when calling a function inside a closure

0 commit comments

Comments
 (0)