Skip to content

Commit 3e38655

Browse files
LucianoPAlmeidaLucianoPAlmeida
authored
Merge pull request #34883 from LucianoPAlmeida/SR-13899-coerce-to-checked
[SR-13899] [Sema] Adjustments on coerce to checked cast diagnostics
2 parents 5266b99 + ac65e6f commit 3e38655

24 files changed

+174
-74
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1112,9 +1112,13 @@ ERROR(missing_unwrap_optional_try,none,
11121112
"value of optional type %0 not unwrapped; did you mean to use 'try!' "
11131113
"or chain with '?'?",
11141114
(Type))
1115-
ERROR(missing_forced_downcast,none,
1116-
"%0 is not convertible to %1; "
1117-
"did you mean to use 'as!' to force downcast?", (Type, Type))
1115+
ERROR(cannot_coerce_to_type, none,
1116+
"%0 is not convertible to %1", (Type, Type))
1117+
NOTE(missing_forced_downcast, none,
1118+
"did you mean to use 'as!' to force downcast?", ())
1119+
NOTE(missing_optional_downcast, none,
1120+
"did you mean to use 'as?' to conditionally downcast?", ())
1121+
11181122
WARNING(coercion_may_fail_warning,none,
11191123
"coercion from %0 to %1 may fail; use 'as?' or 'as!' instead",
11201124
(Type, Type))

‎include/swift/Sema/CSFix.h

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1728,20 +1728,25 @@ class UseRawValue final : public ConstraintFix {
17281728
Type expectedType, ConstraintLocator *locator);
17291729
};
17301730

1731-
/// Replace a coercion ('as') with a forced checked cast ('as!').
1731+
/// Replace a coercion ('as') with runtime checked cast ('as!' or 'as?').
17321732
class CoerceToCheckedCast final : public ContextualMismatch {
17331733
CoerceToCheckedCast(ConstraintSystem &cs, Type fromType, Type toType,
1734-
ConstraintLocator *locator)
1734+
bool useConditionalCast, ConstraintLocator *locator)
17351735
: ContextualMismatch(cs, FixKind::CoerceToCheckedCast, fromType, toType,
1736-
locator) {}
1736+
locator),
1737+
UseConditionalCast(useConditionalCast) {}
1738+
bool UseConditionalCast = false;
17371739

17381740
public:
1739-
std::string getName() const override { return "as to as!"; }
1741+
std::string getName() const override {
1742+
return UseConditionalCast ? "as to as?" : "as to as!";
1743+
}
17401744

17411745
bool diagnose(const Solution &solution, bool asNote = false) const override;
17421746

17431747
static CoerceToCheckedCast *attempt(ConstraintSystem &cs, Type fromType,
1744-
Type toType, ConstraintLocator *locator);
1748+
Type toType, bool useConditionalCast,
1749+
ConstraintLocator *locator);
17451750
};
17461751

17471752
class RemoveInvalidCall final : public ConstraintFix {

‎lib/Sema/CSDiagnostics.cpp

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -964,20 +964,29 @@ bool NoEscapeFuncToTypeConversionFailure::diagnoseParameterUse() const {
964964
return true;
965965
}
966966

967-
ASTNode MissingForcedDowncastFailure::getAnchor() const {
967+
ASTNode InvalidCoercionFailure::getAnchor() const {
968968
auto anchor = FailureDiagnostic::getAnchor();
969969
if (auto *assignExpr = getAsExpr<AssignExpr>(anchor))
970970
return assignExpr->getSrc();
971971
return anchor;
972972
}
973973

974-
bool MissingForcedDowncastFailure::diagnoseAsError() {
974+
bool InvalidCoercionFailure::diagnoseAsError() {
975975
auto fromType = getFromType();
976976
auto toType = getToType();
977977

978-
emitDiagnostic(diag::missing_forced_downcast, fromType, toType)
979-
.highlight(getSourceRange())
980-
.fixItReplace(getLoc(), "as!");
978+
emitDiagnostic(diag::cannot_coerce_to_type, fromType, toType);
979+
980+
if (UseConditionalCast) {
981+
emitDiagnostic(diag::missing_optional_downcast)
982+
.highlight(getSourceRange())
983+
.fixItReplace(getLoc(), "as?");
984+
} else {
985+
emitDiagnostic(diag::missing_forced_downcast)
986+
.highlight(getSourceRange())
987+
.fixItReplace(getLoc(), "as!");
988+
}
989+
981990
return true;
982991
}
983992

@@ -1051,10 +1060,19 @@ bool MissingExplicitConversionFailure::diagnoseAsError() {
10511060
if (needsParensOutside)
10521061
insertAfter += ")";
10531062

1054-
auto diagID =
1055-
useAs ? diag::missing_explicit_conversion : diag::missing_forced_downcast;
1056-
auto diag = emitDiagnostic(diagID, fromType, toType);
1063+
auto diagnose = [&]() {
1064+
if (useAs) {
1065+
return emitDiagnostic(diag::missing_explicit_conversion, fromType,
1066+
toType);
1067+
} else {
1068+
// Emit error diagnostic.
1069+
emitDiagnostic(diag::cannot_coerce_to_type, fromType, toType);
1070+
// Emit and return note suggesting as! where the fix-it will be placed.
1071+
return emitDiagnostic(diag::missing_forced_downcast);
1072+
}
1073+
};
10571074

1075+
auto diag = diagnose();
10581076
if (!insertBefore.empty()) {
10591077
diag.fixItInsert(getSourceRange().Start, insertBefore);
10601078
}

‎lib/Sema/CSDiagnostics.h

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1939,12 +1939,15 @@ class ArgumentMismatchFailure : public ContextualFailure {
19391939
bool diagnoseMisplacedMissingArgument() const;
19401940
};
19411941

1942-
/// Replace a coercion ('as') with a forced checked cast ('as!').
1943-
class MissingForcedDowncastFailure final : public ContextualFailure {
1942+
/// Replace a coercion ('as') with a runtime checked cast ('as!' or 'as?').
1943+
class InvalidCoercionFailure final : public ContextualFailure {
1944+
bool UseConditionalCast;
1945+
19441946
public:
1945-
MissingForcedDowncastFailure(const Solution &solution, Type fromType,
1946-
Type toType, ConstraintLocator *locator)
1947-
: ContextualFailure(solution, fromType, toType, locator) {}
1947+
InvalidCoercionFailure(const Solution &solution, Type fromType, Type toType,
1948+
bool useConditionalCast, ConstraintLocator *locator)
1949+
: ContextualFailure(solution, fromType, toType, locator),
1950+
UseConditionalCast(useConditionalCast) {}
19481951

19491952
ASTNode getAnchor() const override;
19501953

‎lib/Sema/CSFix.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -131,13 +131,14 @@ TreatRValueAsLValue *TreatRValueAsLValue::create(ConstraintSystem &cs,
131131

132132
bool CoerceToCheckedCast::diagnose(const Solution &solution,
133133
bool asNote) const {
134-
MissingForcedDowncastFailure failure(solution, getFromType(), getToType(),
135-
getLocator());
134+
InvalidCoercionFailure failure(solution, getFromType(), getToType(),
135+
UseConditionalCast, getLocator());
136136
return failure.diagnose(asNote);
137137
}
138138

139139
CoerceToCheckedCast *CoerceToCheckedCast::attempt(ConstraintSystem &cs,
140140
Type fromType, Type toType,
141+
bool useConditionalCast,
141142
ConstraintLocator *locator) {
142143
// If any of the types has a type variable, don't add the fix.
143144
if (fromType->hasTypeVariable() || toType->hasTypeVariable())
@@ -159,7 +160,7 @@ CoerceToCheckedCast *CoerceToCheckedCast::attempt(ConstraintSystem &cs,
159160
return nullptr;
160161

161162
return new (cs.getAllocator())
162-
CoerceToCheckedCast(cs, fromType, toType, locator);
163+
CoerceToCheckedCast(cs, fromType, toType, useConditionalCast, locator);
163164
}
164165

165166
bool TreatArrayLiteralAsDictionary::diagnose(const Solution &solution,

‎lib/Sema/CSSimplify.cpp

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3551,9 +3551,26 @@ bool ConstraintSystem::repairFailures(
35513551
getConstraintLocator(coercion->getSubExpr())))
35523552
return true;
35533553

3554-
// Repair a coercion ('as') with a forced checked cast ('as!').
3555-
if (auto *coerceToCheckCastFix = CoerceToCheckedCast::attempt(
3556-
*this, lhs, rhs, getConstraintLocator(locator))) {
3554+
// If the result type of the coercion has an value to optional conversion
3555+
// we can instead suggest the conditional downcast as it is safer in
3556+
// situations like conditional binding.
3557+
auto useConditionalCast = llvm::any_of(
3558+
ConstraintRestrictions,
3559+
[&](std::tuple<Type, Type, ConversionRestrictionKind> restriction) {
3560+
ConversionRestrictionKind restrictionKind;
3561+
Type type1, type2;
3562+
std::tie(type1, type2, restrictionKind) = restriction;
3563+
3564+
if (restrictionKind != ConversionRestrictionKind::ValueToOptional)
3565+
return false;
3566+
3567+
return rhs->isEqual(type1);
3568+
});
3569+
3570+
// Repair a coercion ('as') with a runtime checked cast ('as!' or 'as?').
3571+
if (auto *coerceToCheckCastFix =
3572+
CoerceToCheckedCast::attempt(*this, lhs, rhs, useConditionalCast,
3573+
getConstraintLocator(locator))) {
35573574
conversionsOrFixes.push_back(coerceToCheckCastFix);
35583575
return true;
35593576
}
@@ -9560,8 +9577,8 @@ ConstraintSystem::simplifyRestrictedConstraintImpl(
95609577
loc->isLastElement<LocatorPathElt::ApplyArgToParam>() ||
95619578
loc->isForOptionalTry()) {
95629579
if (restriction == ConversionRestrictionKind::Superclass) {
9563-
if (auto *fix =
9564-
CoerceToCheckedCast::attempt(*this, fromType, toType, loc))
9580+
if (auto *fix = CoerceToCheckedCast::attempt(
9581+
*this, fromType, toType, /*useConditionalCast*/ false, loc))
95659582
return !recordFix(fix, impact);
95669583
}
95679584

‎test/ClangImporter/Dispatch_test.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ func test2(_ queue: DispatchQueue) {
1515

1616
// Make sure the dispatch types are actually distinct types!
1717
let _ = queue as DispatchSource // expected-error {{cannot convert value of type 'DispatchQueue' to type 'DispatchSource' in coercion}}
18-
let _ = base as DispatchSource // expected-error {{'NSObjectProtocol' is not convertible to 'DispatchSource'; did you mean to use 'as!' to force downcast?}}
18+
let _ = base as DispatchSource // expected-error {{'NSObjectProtocol' is not convertible to 'DispatchSource'}}
19+
// expected-note@-1 {{did you mean to use 'as!' to force downcast?}} {{16-18=as!}}
1920
}
2021

2122
extension dispatch_queue_t {} // expected-error {{'dispatch_queue_t' is unavailable}}

‎test/ClangImporter/Security_test.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@ import Security
66

77
_ = kSecClass as CFString
88
_ = kSecClassGenericPassword as CFString
9-
_ = kSecClassGenericPassword as CFDictionary // expected-error {{'CFString?' is not convertible to 'CFDictionary'}} {{30-32=as!}}
9+
_ = kSecClassGenericPassword as CFDictionary // expected-error {{'CFString?' is not convertible to 'CFDictionary'}}
10+
// expected-note@-1 {{did you mean to use 'as!' to force downcast?}} {{30-32=as!}}
1011

1112
func testIntegration() {
1213
// Based on code in <rdar://problem/17162475>.

‎test/ClangImporter/objc_bridging_custom.swift

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -218,11 +218,16 @@ func testExplicitConversion(objc: APPManufacturerInfo<NSString>,
218218
swift: ManufacturerInfo<NSString>) {
219219
// Bridging to Swift
220220
let _ = objc as ManufacturerInfo<NSString>
221-
let _ = objc as ManufacturerInfo<NSNumber> // expected-error{{'APPManufacturerInfo<NSString>' is not convertible to 'ManufacturerInfo<NSNumber>'; did you mean to use 'as!' to force downcast?}}
222-
let _ = objc as ManufacturerInfo<NSObject> // expected-error{{'APPManufacturerInfo<NSString>' is not convertible to 'ManufacturerInfo<NSObject>'; did you mean to use 'as!' to force downcast?}}
221+
let _ = objc as ManufacturerInfo<NSNumber> // expected-error{{'APPManufacturerInfo<NSString>' is not convertible to 'ManufacturerInfo<NSNumber>'}}
222+
// expected-note@-1 {{did you mean to use 'as!' to force downcast?}} {{16-18=as!}}
223+
let _ = objc as ManufacturerInfo<NSObject> // expected-error{{'APPManufacturerInfo<NSString>' is not convertible to 'ManufacturerInfo<NSObject>'}}
224+
// expected-note@-1 {{did you mean to use 'as!' to force downcast?}} {{16-18=as!}}
223225

224226
// Bridging to Objective-C
225227
let _ = swift as APPManufacturerInfo<NSString>
226-
let _ = swift as APPManufacturerInfo<NSNumber> // expected-error{{'ManufacturerInfo<NSString>' is not convertible to 'APPManufacturerInfo<NSNumber>'; did you mean to use 'as!' to force downcast?}}
227-
let _ = swift as APPManufacturerInfo<NSObject> // expected-error{{'ManufacturerInfo<NSString>' is not convertible to 'APPManufacturerInfo<NSObject>'; did you mean to use 'as!' to force downcast?}}
228+
let _ = swift as APPManufacturerInfo<NSNumber> // expected-error{{'ManufacturerInfo<NSString>' is not convertible to 'APPManufacturerInfo<NSNumber>'}}
229+
// expected-note@-1 {{did you mean to use 'as!' to force downcast?}} {{17-19=as!}}
230+
let _ = swift as APPManufacturerInfo<NSObject> // expected-error{{'ManufacturerInfo<NSString>' is not convertible to 'APPManufacturerInfo<NSObject>'}}
231+
// expected-note@-1 {{did you mean to use 'as!' to force downcast?}} {{17-19=as!}}
232+
228233
}

‎test/ClangImporter/objc_parse.swift

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -540,10 +540,12 @@ func testStrangeSelectors(obj: StrangeSelectors) {
540540

541541
func testProtocolQualified(_ obj: CopyableNSObject, cell: CopyableSomeCell,
542542
plainObj: NSObject, plainCell: SomeCell) {
543-
_ = obj as NSObject // expected-error {{'CopyableNSObject' (aka 'NSCopying & NSObjectProtocol') is not convertible to 'NSObject'; did you mean to use 'as!' to force downcast?}} {{11-13=as!}}
543+
_ = obj as NSObject // expected-error {{'CopyableNSObject' (aka 'NSCopying & NSObjectProtocol') is not convertible to 'NSObject'}}
544+
// expected-note@-1 {{did you mean to use 'as!' to force downcast?}} {{11-13=as!}}
544545
_ = obj as NSObjectProtocol
545546
_ = obj as NSCopying
546-
_ = obj as SomeCell // expected-error {{'CopyableNSObject' (aka 'NSCopying & NSObjectProtocol') is not convertible to 'SomeCell'; did you mean to use 'as!' to force downcast?}} {{11-13=as!}}
547+
_ = obj as SomeCell // expected-error {{'CopyableNSObject' (aka 'NSCopying & NSObjectProtocol') is not convertible to 'SomeCell'}}
548+
// expected-note@-1 {{did you mean to use 'as!' to force downcast?}} {{11-13=as!}}
547549

548550
_ = cell as NSObject
549551
_ = cell as NSObjectProtocol

0 commit comments

Comments
 (0)