Skip to content

Commit 36cf24f

Browse files
committed
When we diagnose an unwrap, if the expression being unwrapped is a sole reference to a local let/var, and the local's type is inferred, offer fixits on the initializer as well. In this position we can also reasonably offer 'guard let'. Since we're identifying the implicitly typed initializer now, we can give a specific explanation of how IUOs get turned into optional types with the new IUO implementation.
Pass constraint system down into offering force unwrap fixits so that we can identify the type of the last member chosen for an optional chain. If there's a chain and the last member's return type isn't optional, then it's cleaner to offer to change that last '?' into a '!' to perform the force, rather than parenthesize the whole expression and force the result.
1 parent 3f4227e commit 36cf24f

File tree

6 files changed

+178
-44
lines changed

6 files changed

+178
-44
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -929,6 +929,12 @@ NOTE(unwrap_with_default_value,none,
929929
NOTE(unwrap_with_force_value,none,
930930
"force-unwrap using '!' to abort execution if the optional value contains "
931931
"'nil'", ())
932+
NOTE(unwrap_iuo_initializer,none,
933+
"value inferred to be type %0 when initialized with an implicitly "
934+
"unwrapped value", (Type))
935+
NOTE(unwrap_with_guard,none,
936+
"short-circuit using 'guard' to exit this function early "
937+
"if the optional value contains 'nil'", ())
932938
ERROR(optional_base_not_unwrapped,none,
933939
"value of optional type %0 must be unwrapped to refer to member %1 of "
934940
"wrapped base type %2", (Type, DeclName, Type))

lib/Sema/CSDiag.cpp

Lines changed: 118 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -9108,57 +9108,136 @@ bool ConstraintSystem::salvage(SmallVectorImpl<Solution> &viable, Expr *expr) {
91089108
return true;
91099109
}
91109110

9111-
bool swift::diagnoseUnwrap(TypeChecker &TC, DeclContext *DC,
9112-
Expr *expr, Type type) {
9111+
// Suggest a default value via ?? <default value>
9112+
static void offerDefaultValueUnwrapFixit(TypeChecker &TC, DeclContext *DC, Expr *expr) {
9113+
auto diag =
9114+
TC.diagnose(expr->getLoc(), diag::unwrap_with_default_value);
9115+
9116+
// Figure out what we need to parenthesize.
9117+
bool needsParensInside =
9118+
exprNeedsParensBeforeAddingNilCoalescing(TC, DC, expr);
9119+
bool needsParensOutside =
9120+
exprNeedsParensAfterAddingNilCoalescing(TC, DC, expr, expr);
9121+
9122+
llvm::SmallString<2> insertBefore;
9123+
llvm::SmallString<32> insertAfter;
9124+
if (needsParensOutside) {
9125+
insertBefore += "(";
9126+
}
9127+
if (needsParensInside) {
9128+
insertBefore += "(";
9129+
insertAfter += ")";
9130+
}
9131+
insertAfter += " ?? <" "#default value#" ">";
9132+
if (needsParensOutside)
9133+
insertAfter += ")";
9134+
9135+
if (!insertBefore.empty()) {
9136+
diag.fixItInsert(expr->getStartLoc(), insertBefore);
9137+
}
9138+
diag.fixItInsertAfter(expr->getEndLoc(), insertAfter);
9139+
}
9140+
9141+
// Suggest a force-unwrap.
9142+
static void offerForceUnwrapFixit(ConstraintSystem &CS, Expr *expr) {
9143+
auto diag = CS.TC.diagnose(expr->getLoc(), diag::unwrap_with_force_value);
9144+
9145+
// If expr is optional as the result of an optional chain and this last
9146+
// dot isn't a member returning optional, then offer to force the last
9147+
// link in the chain, rather than an ugly parenthesized postfix force.
9148+
if (auto optionalChain = dyn_cast<OptionalEvaluationExpr>(expr)) {
9149+
if (auto dotExpr =
9150+
dyn_cast<UnresolvedDotExpr>(optionalChain->getSubExpr())) {
9151+
auto bind = dyn_cast<BindOptionalExpr>(dotExpr->getBase());
9152+
if (bind && !CS.getType(dotExpr)->getOptionalObjectType()) {
9153+
diag.fixItReplace(SourceRange(bind->getLoc()), "!");
9154+
return;
9155+
}
9156+
}
9157+
}
9158+
9159+
if (expr->canAppendPostfixExpression(true)) {
9160+
diag.fixItInsertAfter(expr->getEndLoc(), "!");
9161+
} else {
9162+
diag.fixItInsert(expr->getStartLoc(), "(")
9163+
.fixItInsertAfter(expr->getEndLoc(), ")!");
9164+
}
9165+
}
9166+
9167+
class VarDeclMultipleReferencesChecker : public ASTWalker {
9168+
VarDecl *varDecl;
9169+
int count;
9170+
9171+
std::pair<bool, Expr *> walkToExprPre(Expr *E) {
9172+
if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
9173+
if (DRE->getDecl() == varDecl)
9174+
count++;
9175+
}
9176+
return { true, E };
9177+
}
9178+
9179+
public:
9180+
VarDeclMultipleReferencesChecker(VarDecl *varDecl) : varDecl(varDecl),count(0) {}
9181+
int referencesCount() { return count; }
9182+
};
9183+
9184+
bool swift::diagnoseUnwrap(ConstraintSystem &CS, Expr *expr, Type type) {
91139185
Type unwrappedType = type->getOptionalObjectType();
91149186
if (!unwrappedType)
91159187
return false;
91169188

9117-
TC.diagnose(expr->getLoc(), diag::optional_not_unwrapped, type,
9118-
unwrappedType);
9189+
CS.TC.diagnose(expr->getLoc(), diag::optional_not_unwrapped, type,
9190+
unwrappedType);
9191+
9192+
// If the expression we're unwrapping is the only reference to a
9193+
// local variable whose type isn't explicit in the source, then
9194+
// offer unwrapping fixits on the initializer as well.
9195+
if (auto declRef = dyn_cast<DeclRefExpr>(expr)) {
9196+
if (auto varDecl = dyn_cast<VarDecl>(declRef->getDecl())) {
9197+
9198+
bool singleUse = false;
9199+
AbstractFunctionDecl *AFD = nullptr;
9200+
if (auto contextDecl = varDecl->getDeclContext()->getAsDeclOrDeclExtensionContext()) {
9201+
if ((AFD = dyn_cast<AbstractFunctionDecl>(contextDecl))) {
9202+
auto checker = VarDeclMultipleReferencesChecker(varDecl);
9203+
AFD->getBody()->walk(checker);
9204+
singleUse = checker.referencesCount() == 1;
9205+
}
9206+
}
91199207

9120-
// Suggest a default value via ?? <default value>
9121-
{
9122-
auto diag =
9123-
TC.diagnose(expr->getLoc(), diag::unwrap_with_default_value);
9208+
PatternBindingDecl *binding = varDecl->getParentPatternBinding();
9209+
if (singleUse && binding && binding->getNumPatternEntries() == 1 &&
9210+
varDecl->getTypeSourceRangeForDiagnostics().isInvalid()) {
91249211

9125-
// Figure out what we need to parenthesize.
9126-
bool needsParensInside =
9127-
exprNeedsParensBeforeAddingNilCoalescing(TC, DC, expr);
9128-
bool needsParensOutside =
9129-
exprNeedsParensAfterAddingNilCoalescing(TC, DC, expr, expr);
9212+
Expr *initializer = varDecl->getParentInitializer();
9213+
if (auto declRefExpr = dyn_cast<DeclRefExpr>(initializer)) {
9214+
if (declRefExpr->getDecl()->getAttrs().hasAttribute<ImplicitlyUnwrappedOptionalAttr>()) {
9215+
CS.TC.diagnose(declRefExpr->getLoc(), diag::unwrap_iuo_initializer, type);
9216+
}
9217+
}
91309218

9131-
llvm::SmallString<2> insertBefore;
9132-
llvm::SmallString<32> insertAfter;
9133-
if (needsParensOutside) {
9134-
insertBefore += "(";
9135-
}
9136-
if (needsParensInside) {
9137-
insertBefore += "(";
9138-
insertAfter += ")";
9139-
}
9140-
insertAfter += " ?? <" "#default value#" ">";
9141-
if (needsParensOutside)
9142-
insertAfter += ")";
9219+
auto fnTy = AFD->getInterfaceType()->castTo<AnyFunctionType>();
9220+
bool voidReturn = fnTy->getResult()->isEqual(TupleType::getEmpty(CS.DC->getASTContext()));
91439221

9144-
if (!insertBefore.empty()) {
9145-
diag.fixItInsert(expr->getStartLoc(), insertBefore);
9146-
}
9147-
diag.fixItInsertAfter(expr->getEndLoc(), insertAfter);
9148-
}
9222+
auto diag = CS.TC.diagnose(varDecl->getLoc(), diag::unwrap_with_guard);
9223+
diag.fixItInsert(binding->getStartLoc(), "guard ");
9224+
if (voidReturn) {
9225+
diag.fixItInsertAfter(binding->getEndLoc(), " else { return }");
9226+
} else {
9227+
diag.fixItInsertAfter(binding->getEndLoc(), " else { return <"
9228+
"#default value#" "> }");
9229+
}
9230+
diag.flush();
91499231

9150-
// Suggest a force-unwrap.
9151-
{
9152-
auto diag =
9153-
TC.diagnose(expr->getLoc(), diag::unwrap_with_force_value);
9154-
if (expr->canAppendPostfixExpression(true)) {
9155-
diag.fixItInsertAfter(expr->getEndLoc(), "!");
9156-
} else {
9157-
diag.fixItInsert(expr->getStartLoc(), "(")
9158-
.fixItInsertAfter(expr->getEndLoc(), ")!");
9232+
offerDefaultValueUnwrapFixit(CS.TC, varDecl->getDeclContext(),
9233+
initializer);
9234+
offerForceUnwrapFixit(CS, initializer);
9235+
}
91599236
}
91609237
}
91619238

9239+
offerDefaultValueUnwrapFixit(CS.TC, CS.DC, expr);
9240+
offerForceUnwrapFixit(CS, expr);
91629241
return true;
91639242
}
91649243

lib/Sema/CalleeCandidateInfo.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1032,7 +1032,7 @@ bool CalleeCandidateInfo::diagnoseGenericParameterErrors(Expr *badArgExpr) {
10321032
// Check for optional near miss.
10331033
if (auto argOptType = substitution->getOptionalObjectType()) {
10341034
if (isSubstitutableFor(argOptType, paramArchetype, CS.DC)) {
1035-
if (diagnoseUnwrap(CS.TC, CS.DC, badArgExpr, substitution)) {
1035+
if (diagnoseUnwrap(CS, badArgExpr, substitution)) {
10361036
foundFailure = true;
10371037
break;
10381038
}

lib/Sema/ConstraintSystem.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3587,7 +3587,7 @@ class InputMatcher {
35873587
/// that needs to be unwrapped.
35883588
///
35893589
/// \returns true if a diagnostic was produced.
3590-
bool diagnoseUnwrap(TypeChecker &TC, DeclContext *DC, Expr *expr, Type type);
3590+
bool diagnoseUnwrap(constraints::ConstraintSystem &CS, Expr *expr, Type type);
35913591

35923592
/// Diagnose an attempt to recover from a member access into a value of
35933593
/// optional type which needed to be unwrapped for the member to be found.

test/Constraints/fixes.swift

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,10 @@ var s1116 = Set(1...10).subtracting(a1116.map({ $0.s })) // expected-error {{can
163163

164164

165165
func moreComplexUnwrapFixes() {
166-
struct S { let value: Int }
166+
struct S {
167+
let value: Int
168+
let optValue: Int? = nil
169+
}
167170
struct T {
168171
let s: S
169172
let optS: S?
@@ -196,4 +199,46 @@ func moreComplexUnwrapFixes() {
196199
// expected-error@-2{{value of optional type 'S?' must be unwrapped to refer to member 'value' of wrapped base type 'S'}}
197200
// expected-note@-3{{chain the optional using '?'}}{{12-12=?}}
198201
// expected-note@-4{{force-unwrap using '!'}}{{17-17=!}}
202+
203+
takeNon(os?.value) // expected-error{{value of optional type 'Int?' must be unwrapped to a value of type 'Int'}}
204+
// expected-note@-1{{force-unwrap using '!'}}{{13-14=!}}
205+
// expected-note@-2{{coalesce}}
206+
takeNon(os?.optValue) // expected-error{{value of optional type 'Int?' must be unwrapped to a value of type 'Int'}}
207+
// expected-note@-1{{force-unwrap using '!'}}{{11-11=(}}{{23-23=)!}}
208+
// expected-note@-2{{coalesce}}
209+
210+
func sample(a: Int?, b: Int!) {
211+
let aa = a
212+
// expected-note@-1{{short-circuit using 'guard'}}{{5-5=guard }} {{15-15= else \{ return \}}}
213+
// expected-note@-2{{force-unwrap}}
214+
// expected-note@-3{{coalesce}}
215+
216+
let bb = b // expected-note{{value inferred to be type 'Int?' when initialized with an implicitly unwrapped value}}
217+
// expected-note@-1{{short-circuit using 'guard'}}{{5-5=guard }} {{15-15= else \{ return \}}}
218+
// expected-note@-2{{force-unwrap}}
219+
// expected-note@-3{{coalesce}}
220+
221+
let cc = a
222+
223+
takeNon(aa) // expected-error{{value of optional type 'Int?' must be unwrapped to a value of type 'Int'}}
224+
// expected-note@-1{{force-unwrap}} expected-note@-1{{coalesce}}
225+
takeNon(bb) // expected-error{{value of optional type 'Int?' must be unwrapped to a value of type 'Int'}}
226+
// expected-note@-1{{force-unwrap}} expected-note@-1{{coalesce}}
227+
228+
_ = [].map { takeNon(cc) } // expected-error{{value of optional type 'Int?' must be unwrapped to a value of type 'Int'}}
229+
// expected-note@-1{{force-unwrap}} expected-note@-1{{coalesce}}
230+
231+
takeOpt(cc)
232+
}
233+
234+
func sample2(a: Int?) -> Int {
235+
let aa = a
236+
// expected-note@-1{{short-circuit using 'guard'}}{{5-5=guard }} {{15-15= else \{ return <#default value#> \}}}
237+
// expected-note@-2{{force-unwrap}}
238+
// expected-note@-3{{coalesce}}
239+
240+
return aa // expected-error{{value of optional type 'Int?' must be unwrapped to a value of type 'Int'}}
241+
// expected-note@-1{{force-unwrap}} expected-note@-1{{coalesce}}
242+
243+
}
199244
}

test/NameBinding/dynamic-member-lookup.swift

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,8 +149,12 @@ func test_iuo_result(x : IUOResultTest) {
149149
x.foo?.negate() // Test mutating writeback.
150150

151151
let _ : Int = x.bar // Test implicitly forced optional
152-
let b = x.bar // Should promote to 'Int?'
153-
let _ : Int = b // expected-error {{value of optional type 'Int?' must be unwrapped}}
152+
let b = x.bar
153+
// expected-note@-1{{short-circuit}}
154+
// expected-note@-2{{coalesce}}
155+
// expected-note@-3{{force-unwrap}}
156+
157+
let _ : Int = b // expected-error {{value of optional type 'Int?' must be unwrapped to a value of type 'Int'}}
154158
// expected-note@-1{{coalesce}}
155159
// expected-note@-2{{force-unwrap}}
156160
}

0 commit comments

Comments
 (0)