Skip to content

Commit 8f41ee7

Browse files
gregomnigregomni
authored
Merge pull request #18324 from gregomni/opty2
[Sema] More unwrap fixits
2 parents 5ea6eae + ba96db6 commit 8f41ee7

File tree

7 files changed

+179
-45
lines changed

7 files changed

+179
-45
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/CSDiagnostics.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,7 @@ bool MissingOptionalUnwrapFailure::diagnose() {
377377

378378
auto *tryExpr = dyn_cast<OptionalTryExpr>(unwrapped);
379379
if (!tryExpr)
380-
return diagnoseUnwrap(getTypeChecker(), getDC(), unwrapped, type);
380+
return diagnoseUnwrap(getConstraintSystem(), unwrapped, type);
381381

382382
emitDiagnostic(tryExpr->getTryLoc(), diag::missing_unwrap_optional_try, type)
383383
.fixItReplace({tryExpr->getTryLoc(), tryExpr->getQuestionLoc()}, "try!");

‎lib/Sema/CalleeCandidateInfo.cpp

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

‎lib/Sema/ConstraintSystem.h

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

35683568
/// Diagnose an attempt to recover from a member access into a value of
35693569
/// 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)