Skip to content

Commit 1789d44

Browse files
committed
Don't consider fixups that won't do anything.
Since 'try?' no longer unconditionally adds a layer of optional, converting it to 'try!' will no longer unconditionally remove a layer of optional. So let's not suggest it. This leads to better diagnostics anyway.
1 parent 9422986 commit 1789d44

File tree

3 files changed

+38
-11
lines changed

3 files changed

+38
-11
lines changed

lib/Sema/CSDiagnostics.cpp

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -576,10 +576,22 @@ bool MissingOptionalUnwrapFailure::diagnoseAsError() {
576576
auto *tryExpr = dyn_cast<OptionalTryExpr>(unwrapped);
577577
if (!tryExpr)
578578
return diagnoseUnwrap(getConstraintSystem(), unwrapped, type);
579-
580-
emitDiagnostic(tryExpr->getTryLoc(), diag::missing_unwrap_optional_try, type)
581-
.fixItReplace({tryExpr->getTryLoc(), tryExpr->getQuestionLoc()}, "try!");
582-
return true;
579+
580+
bool isSwift5OrGreater = getTypeChecker().getLangOpts().isSwiftVersionAtLeast(5);
581+
auto subExprType = getType(tryExpr->getSubExpr());
582+
bool subExpressionIsOptional = (bool)subExprType->getOptionalObjectType();
583+
584+
585+
if (isSwift5OrGreater && subExpressionIsOptional) {
586+
// Using 'try!' won't change the type for a 'try?' with an optional sub-expr
587+
// under Swift 5+, so just report that a missing unwrap can't be handled here.
588+
return false;
589+
}
590+
else {
591+
emitDiagnostic(tryExpr->getTryLoc(), diag::missing_unwrap_optional_try, type)
592+
.fixItReplace({tryExpr->getTryLoc(), tryExpr->getQuestionLoc()}, "try!");
593+
return true;
594+
}
583595
}
584596

585597
bool RValueTreatedAsLValueFailure::diagnoseAsError() {

lib/Sema/CSSimplify.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2353,6 +2353,20 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
23532353
forceUnwrapPossible = false;
23542354
}
23552355
}
2356+
2357+
if (auto optTryExpr =
2358+
dyn_cast_or_null<OptionalTryExpr>(locator.trySimplifyToExpr())) {
2359+
auto subExprType = getType(optTryExpr->getSubExpr());
2360+
bool isSwift5OrGreater = TC.getLangOpts().isSwiftVersionAtLeast(5);
2361+
if (isSwift5OrGreater && (bool)subExprType->getOptionalObjectType()) {
2362+
// For 'try?' expressions, a ForceOptional fix converts 'try?'
2363+
// to 'try!'. If the sub-expression is optional, then a force-unwrap
2364+
// won't change anything in Swift 5+ because 'try?' already avoids
2365+
// adding an additional layer of Optional there.
2366+
forceUnwrapPossible = false;
2367+
}
2368+
}
2369+
23562370

23572371
if (forceUnwrapPossible) {
23582372
conversionsOrFixes.push_back(

test/Parse/try.swift

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -238,25 +238,26 @@ struct ThingProducer {
238238

239239
let optProducer: ThingProducer? = ThingProducer()
240240
let _: Int? = try? optProducer?.produceInt()
241-
let _: Int = try? optProducer?.produceInt() // expected-error {{value of optional type 'Int?' not unwrapped; did you mean to use 'try!' or chain with '?'?}}
241+
let _: Int = try? optProducer?.produceInt() // expected-error {{cannot convert value of type 'Int?' to specified type 'Int'}}
242242
let _: String = try? optProducer?.produceInt() // expected-error {{cannot convert value of type 'Int?' to specified type 'String'}}
243-
let _: Int? = try? optProducer?.produceIntNoThrowing() // expected-warning {{no calls to throwing functions occur within 'try' expression}}
244-
245243
let _: Int?? = try? optProducer?.produceInt() // This was the expected type before Swift 5, but this still works; just adds more optional-ness
246244

245+
let _: Int? = try? optProducer?.produceIntNoThrowing() // expected-warning {{no calls to throwing functions occur within 'try' expression}}
246+
247247
let _: Int? = (try? optProducer?.produceAny()) as? Int // good
248248
let _: Int? = try? optProducer?.produceAny() as? Int // good
249249
let _: Int?? = try? optProducer?.produceAny() as? Int // good
250250
let _: String = try? optProducer?.produceAny() as? Int // expected-error {{cannot convert value of type 'Int?' to specified type 'String'}}
251251

252-
let _: Int? = try? optProducer?.produceDoubleOptionalInt() // expected-error {{value of optional type 'Int??' not unwrapped; did you mean to use 'try!' or chain with '?'?}}
253-
let _: String = try? optProducer?.produceDoubleOptionalInt() // expected-error {{cannot convert value of type 'Int??' to specified type 'String'}}
254252
let _: String = try? optProducer?.produceDoubleOptionalInt() // expected-error {{cannot convert value of type 'Int??' to specified type 'String'}}
255253

256254
let producer = ThingProducer()
257255

258256
let _: Int = try? producer.produceDoubleOptionalInt() // expected-error {{cannot convert value of type 'Int??' to specified type 'Int'}}
259-
let _: Int? = try? producer.produceDoubleOptionalInt() // expected-error {{value of optional type 'Int??' not unwrapped; did you mean to use 'try!' or chain with '?'?}}
257+
258+
// We don't offer 'try!' here because it would not change the type of the expression in Swift 5+
259+
let _: Int? = try? producer.produceDoubleOptionalInt() // expected-error {{cannot convert value of type 'Int??' to specified type 'Int?'}}
260+
260261
let _: Int?? = try? producer.produceDoubleOptionalInt() // good
261262
let _: Int??? = try? producer.produceDoubleOptionalInt() // good
262-
let _: String = try? producer.produceDoubleOptionalInt() // expected-error {{cannot convert value of type 'Int??' to specified type 'String'}}
263+
let _: String = try? producer.produceDoubleOptionalInt() // expected-error {{cannot convert value of type 'Int??' to specified type 'String'}}

0 commit comments

Comments
 (0)