Skip to content

Commit 4d862d5

Browse files
gregomnigregomni
committed
In tryTypeVariableBindings, if T? fails, try T for all binding kinds (previously just allowed for Subtypes). This allows us to always find MissingOptionalUnwrapFailures, so that all the unwrap fixit code can be moved into CSDiagnostics and made static.
1 parent 8f41ee7 commit 4d862d5

File tree

5 files changed

+148
-165
lines changed

5 files changed

+148
-165
lines changed

‎lib/Sema/CSDiag.cpp

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

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) {
9185-
Type unwrappedType = type->getOptionalObjectType();
9186-
if (!unwrappedType)
9187-
return false;
9188-
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-
}
9207-
9208-
PatternBindingDecl *binding = varDecl->getParentPatternBinding();
9209-
if (singleUse && binding && binding->getNumPatternEntries() == 1 &&
9210-
varDecl->getTypeSourceRangeForDiagnostics().isInvalid()) {
9211-
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-
}
9218-
9219-
auto fnTy = AFD->getInterfaceType()->castTo<AnyFunctionType>();
9220-
bool voidReturn = fnTy->getResult()->isEqual(TupleType::getEmpty(CS.DC->getASTContext()));
9221-
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();
9231-
9232-
offerDefaultValueUnwrapFixit(CS.TC, varDecl->getDeclContext(),
9233-
initializer);
9234-
offerForceUnwrapFixit(CS, initializer);
9235-
}
9236-
}
9237-
}
9238-
9239-
offerDefaultValueUnwrapFixit(CS.TC, CS.DC, expr);
9240-
offerForceUnwrapFixit(CS, expr);
9241-
return true;
9242-
}
9243-
92449111
bool swift::diagnoseBaseUnwrapForMemberAccess(Expr *baseExpr, Type baseType,
92459112
DeclName memberName,
92469113
bool resultOptional,
@@ -9257,7 +9124,8 @@ bool swift::diagnoseBaseUnwrapForMemberAccess(Expr *baseExpr, Type baseType,
92579124
// FIXME: It would be nice to immediately offer "base?.member ?? defaultValue"
92589125
// for non-optional results where that would be appropriate. For the moment
92599126
// always offering "?" means that if the user chooses chaining, we'll end up
9260-
// in diagnoseUnwrap() to offer a default value during the next compile.
9127+
// in MissingOptionalUnwrapFailure:diagnose() to offer a default value during
9128+
// the next compile.
92619129
diags.diagnose(baseExpr->getLoc(), diag::optional_base_chain, memberName)
92629130
.fixItInsertAfter(baseExpr->getEndLoc(), "?");
92639131

‎lib/Sema/CSDiagnostics.cpp

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,139 @@ bool MemberAccessOnOptionalBaseFailure::diagnose() {
367367
resultIsOptional, SourceRange());
368368
}
369369

370+
// Suggest a default value via ?? <default value>
371+
static void offerDefaultValueUnwrapFixit(TypeChecker &TC, DeclContext *DC, Expr *expr) {
372+
auto diag =
373+
TC.diagnose(expr->getLoc(), diag::unwrap_with_default_value);
374+
375+
// Figure out what we need to parenthesize.
376+
bool needsParensInside =
377+
exprNeedsParensBeforeAddingNilCoalescing(TC, DC, expr);
378+
bool needsParensOutside =
379+
exprNeedsParensAfterAddingNilCoalescing(TC, DC, expr, expr);
380+
381+
llvm::SmallString<2> insertBefore;
382+
llvm::SmallString<32> insertAfter;
383+
if (needsParensOutside) {
384+
insertBefore += "(";
385+
}
386+
if (needsParensInside) {
387+
insertBefore += "(";
388+
insertAfter += ")";
389+
}
390+
insertAfter += " ?? <" "#default value#" ">";
391+
if (needsParensOutside)
392+
insertAfter += ")";
393+
394+
if (!insertBefore.empty()) {
395+
diag.fixItInsert(expr->getStartLoc(), insertBefore);
396+
}
397+
diag.fixItInsertAfter(expr->getEndLoc(), insertAfter);
398+
}
399+
400+
// Suggest a force-unwrap.
401+
static void offerForceUnwrapFixit(ConstraintSystem &CS, Expr *expr) {
402+
auto diag = CS.TC.diagnose(expr->getLoc(), diag::unwrap_with_force_value);
403+
404+
// If expr is optional as the result of an optional chain and this last
405+
// dot isn't a member returning optional, then offer to force the last
406+
// link in the chain, rather than an ugly parenthesized postfix force.
407+
if (auto optionalChain = dyn_cast<OptionalEvaluationExpr>(expr)) {
408+
if (auto dotExpr =
409+
dyn_cast<UnresolvedDotExpr>(optionalChain->getSubExpr())) {
410+
auto bind = dyn_cast<BindOptionalExpr>(dotExpr->getBase());
411+
if (bind && !CS.getType(dotExpr)->getOptionalObjectType()) {
412+
diag.fixItReplace(SourceRange(bind->getLoc()), "!");
413+
return;
414+
}
415+
}
416+
}
417+
418+
if (expr->canAppendPostfixExpression(true)) {
419+
diag.fixItInsertAfter(expr->getEndLoc(), "!");
420+
} else {
421+
diag.fixItInsert(expr->getStartLoc(), "(")
422+
.fixItInsertAfter(expr->getEndLoc(), ")!");
423+
}
424+
}
425+
426+
class VarDeclMultipleReferencesChecker : public ASTWalker {
427+
VarDecl *varDecl;
428+
int count;
429+
430+
std::pair<bool, Expr *> walkToExprPre(Expr *E) {
431+
if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
432+
if (DRE->getDecl() == varDecl)
433+
count++;
434+
}
435+
return { true, E };
436+
}
437+
438+
public:
439+
VarDeclMultipleReferencesChecker(VarDecl *varDecl) : varDecl(varDecl),count(0) {}
440+
int referencesCount() { return count; }
441+
};
442+
443+
static bool diagnoseUnwrap(ConstraintSystem &CS, Expr *expr, Type type) {
444+
Type unwrappedType = type->getOptionalObjectType();
445+
if (!unwrappedType)
446+
return false;
447+
448+
CS.TC.diagnose(expr->getLoc(), diag::optional_not_unwrapped, type,
449+
unwrappedType);
450+
451+
// If the expression we're unwrapping is the only reference to a
452+
// local variable whose type isn't explicit in the source, then
453+
// offer unwrapping fixits on the initializer as well.
454+
if (auto declRef = dyn_cast<DeclRefExpr>(expr)) {
455+
if (auto varDecl = dyn_cast<VarDecl>(declRef->getDecl())) {
456+
457+
bool singleUse = false;
458+
AbstractFunctionDecl *AFD = nullptr;
459+
if (auto contextDecl = varDecl->getDeclContext()->getAsDeclOrDeclExtensionContext()) {
460+
if ((AFD = dyn_cast<AbstractFunctionDecl>(contextDecl))) {
461+
auto checker = VarDeclMultipleReferencesChecker(varDecl);
462+
AFD->getBody()->walk(checker);
463+
singleUse = checker.referencesCount() == 1;
464+
}
465+
}
466+
467+
PatternBindingDecl *binding = varDecl->getParentPatternBinding();
468+
if (singleUse && binding && binding->getNumPatternEntries() == 1 &&
469+
varDecl->getTypeSourceRangeForDiagnostics().isInvalid()) {
470+
471+
Expr *initializer = varDecl->getParentInitializer();
472+
if (auto declRefExpr = dyn_cast<DeclRefExpr>(initializer)) {
473+
if (declRefExpr->getDecl()->getAttrs().hasAttribute<ImplicitlyUnwrappedOptionalAttr>()) {
474+
CS.TC.diagnose(declRefExpr->getLoc(), diag::unwrap_iuo_initializer, type);
475+
}
476+
}
477+
478+
auto fnTy = AFD->getInterfaceType()->castTo<AnyFunctionType>();
479+
bool voidReturn = fnTy->getResult()->isEqual(TupleType::getEmpty(CS.DC->getASTContext()));
480+
481+
auto diag = CS.TC.diagnose(varDecl->getLoc(), diag::unwrap_with_guard);
482+
diag.fixItInsert(binding->getStartLoc(), "guard ");
483+
if (voidReturn) {
484+
diag.fixItInsertAfter(binding->getEndLoc(), " else { return }");
485+
} else {
486+
diag.fixItInsertAfter(binding->getEndLoc(), " else { return <"
487+
"#default value#" "> }");
488+
}
489+
diag.flush();
490+
491+
offerDefaultValueUnwrapFixit(CS.TC, varDecl->getDeclContext(),
492+
initializer);
493+
offerForceUnwrapFixit(CS, initializer);
494+
}
495+
}
496+
}
497+
498+
offerDefaultValueUnwrapFixit(CS.TC, CS.DC, expr);
499+
offerForceUnwrapFixit(CS, expr);
500+
return true;
501+
}
502+
370503
bool MissingOptionalUnwrapFailure::diagnose() {
371504
if (hasComplexLocator())
372505
return false;

‎lib/Sema/CSSolver.cpp

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -713,24 +713,22 @@ bool ConstraintSystem::tryTypeVariableBindings(
713713
newBindings.push_back({subtype, binding.Kind, binding.BindingSource});
714714
}
715715

716-
if (binding.Kind == AllowedBindingKind::Subtypes) {
717-
// If we were unsuccessful solving for T?, try solving for T.
718-
if (auto objTy = type->getOptionalObjectType()) {
719-
if (exploredTypes.insert(objTy->getCanonicalType()).second) {
720-
// If T is a type variable, only attempt this if both the
721-
// type variable we are trying bindings for, and the type
722-
// variable we will attempt to bind, both have the same
723-
// polarity with respect to being able to bind lvalues.
724-
if (auto otherTypeVar = objTy->getAs<TypeVariableType>()) {
725-
if (typeVar->getImpl().canBindToLValue() ==
726-
otherTypeVar->getImpl().canBindToLValue()) {
727-
newBindings.push_back(
728-
{objTy, binding.Kind, binding.BindingSource});
729-
}
730-
} else {
716+
// If we were unsuccessful solving for T?, try solving for T.
717+
if (auto objTy = type->getOptionalObjectType()) {
718+
if (exploredTypes.insert(objTy->getCanonicalType()).second) {
719+
// If T is a type variable, only attempt this if both the
720+
// type variable we are trying bindings for, and the type
721+
// variable we will attempt to bind, both have the same
722+
// polarity with respect to being able to bind lvalues.
723+
if (auto otherTypeVar = objTy->getAs<TypeVariableType>()) {
724+
if (typeVar->getImpl().canBindToLValue() ==
725+
otherTypeVar->getImpl().canBindToLValue()) {
731726
newBindings.push_back(
732727
{objTy, binding.Kind, binding.BindingSource});
733728
}
729+
} else {
730+
newBindings.push_back(
731+
{objTy, binding.Kind, binding.BindingSource});
734732
}
735733
}
736734
}

‎lib/Sema/CalleeCandidateInfo.cpp

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1032,16 +1032,6 @@ bool CalleeCandidateInfo::diagnoseGenericParameterErrors(Expr *badArgExpr) {
10321032

10331033
// FIXME: Add specific error for not subclass, if the archetype has a superclass?
10341034

1035-
// Check for optional near miss.
1036-
if (auto argOptType = substitution->getOptionalObjectType()) {
1037-
if (isSubstitutableFor(argOptType, paramArchetype, CS.DC)) {
1038-
if (diagnoseUnwrap(CS, badArgExpr, substitution)) {
1039-
foundFailure = true;
1040-
break;
1041-
}
1042-
}
1043-
}
1044-
10451035
for (auto proto : paramArchetype->getConformsTo()) {
10461036
if (!CS.TC.conformsToProtocol(substitution, proto, CS.DC,
10471037
ConformanceCheckFlags::InExpression)) {

‎lib/Sema/ConstraintSystem.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3559,12 +3559,6 @@ class InputMatcher {
35593559
size_t getNumSkippedParameters() const { return NumSkippedParameters; }
35603560
};
35613561

3562-
/// Diagnose an attempt to recover when we have a value of optional type
3563-
/// that needs to be unwrapped.
3564-
///
3565-
/// \returns true if a diagnostic was produced.
3566-
bool diagnoseUnwrap(constraints::ConstraintSystem &CS, Expr *expr, Type type);
3567-
35683562
/// Diagnose an attempt to recover from a member access into a value of
35693563
/// optional type which needed to be unwrapped for the member to be found.
35703564
///

0 commit comments

Comments
 (0)