Skip to content

Commit b4fb25f

Browse files
xedinxedin
authored
Merge pull request swiftlang#22050 from theblixguy/fix/SR-9201
[Diag] Fix incorrect diagnostic when unwrapping double optional
2 parents 05b6ace + 614f72f commit b4fb25f

File tree

11 files changed

+124
-38
lines changed

11 files changed

+124
-38
lines changed

‎lib/Sema/CSDiagnostics.cpp

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -510,12 +510,18 @@ class VarDeclMultipleReferencesChecker : public ASTWalker {
510510
int referencesCount() { return count; }
511511
};
512512

513-
static bool diagnoseUnwrap(ConstraintSystem &CS, Expr *expr, Type type) {
514-
Type unwrappedType = type->getOptionalObjectType();
515-
if (!unwrappedType)
513+
static bool diagnoseUnwrap(ConstraintSystem &CS, Expr *expr, Type baseType,
514+
Type unwrappedType) {
515+
516+
assert(!baseType->hasTypeVariable() &&
517+
"Base type must not be a type variable");
518+
assert(!unwrappedType->hasTypeVariable() &&
519+
"Unwrapped type must not be a type variable");
520+
521+
if (baseType && !baseType->getOptionalObjectType())
516522
return false;
517523

518-
CS.TC.diagnose(expr->getLoc(), diag::optional_not_unwrapped, type,
524+
CS.TC.diagnose(expr->getLoc(), diag::optional_not_unwrapped, baseType,
519525
unwrappedType);
520526

521527
// If the expression we're unwrapping is the only reference to a
@@ -541,7 +547,8 @@ static bool diagnoseUnwrap(ConstraintSystem &CS, Expr *expr, Type type) {
541547
Expr *initializer = varDecl->getParentInitializer();
542548
if (auto declRefExpr = dyn_cast<DeclRefExpr>(initializer)) {
543549
if (declRefExpr->getDecl()->getAttrs().hasAttribute<ImplicitlyUnwrappedOptionalAttr>()) {
544-
CS.TC.diagnose(declRefExpr->getLoc(), diag::unwrap_iuo_initializer, type);
550+
CS.TC.diagnose(declRefExpr->getLoc(), diag::unwrap_iuo_initializer,
551+
baseType);
545552
}
546553
}
547554

@@ -580,12 +587,18 @@ bool MissingOptionalUnwrapFailure::diagnoseAsError() {
580587
anchor = assignExpr->getSrc();
581588

582589
auto *unwrapped = anchor->getValueProvidingExpr();
583-
auto type = getType(anchor)->getRValueType();
590+
Type type = getType(anchor)->getRValueType();
584591

585592
auto *tryExpr = dyn_cast<OptionalTryExpr>(unwrapped);
586-
if (!tryExpr)
587-
return diagnoseUnwrap(getConstraintSystem(), unwrapped, type);
588-
593+
if (!tryExpr) {
594+
auto resolvedBaseTy =
595+
resolveType(BaseType)->reconstituteSugar(/*recursive=*/true);
596+
auto resolvedUnwrappedTy =
597+
resolveType(UnwrappedType)->reconstituteSugar(/*recursive=*/true);
598+
return diagnoseUnwrap(getConstraintSystem(), unwrapped, resolvedBaseTy,
599+
resolvedUnwrappedTy);
600+
}
601+
589602
bool isSwift5OrGreater = getTypeChecker().getLangOpts().isSwiftVersionAtLeast(5);
590603
auto subExprType = getType(tryExpr->getSubExpr());
591604
bool subExpressionIsOptional = (bool)subExprType->getOptionalObjectType();

‎lib/Sema/CSDiagnostics.h

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -480,10 +480,14 @@ class MemberAccessOnOptionalBaseFailure final : public FailureDiagnostic {
480480
/// Diagnose failures related to use of the unwrapped optional types,
481481
/// which require some type of force-unwrap e.g. "!" or "try!".
482482
class MissingOptionalUnwrapFailure final : public FailureDiagnostic {
483+
Type BaseType;
484+
Type UnwrappedType;
485+
483486
public:
484-
MissingOptionalUnwrapFailure(Expr *expr, ConstraintSystem &cs,
485-
ConstraintLocator *locator)
486-
: FailureDiagnostic(expr, cs, locator) {}
487+
MissingOptionalUnwrapFailure(Expr *expr, ConstraintSystem &cs, Type baseType,
488+
Type unwrappedType, ConstraintLocator *locator)
489+
: FailureDiagnostic(expr, cs, locator), BaseType(baseType),
490+
UnwrappedType(unwrappedType) {}
487491

488492
bool diagnoseAsError() override;
489493
};

‎lib/Sema/CSFix.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,14 +65,17 @@ ForceDowncast *ForceDowncast::create(ConstraintSystem &cs, Type toType,
6565
}
6666

6767
bool ForceOptional::diagnose(Expr *root, bool asNote) const {
68-
MissingOptionalUnwrapFailure failure(root, getConstraintSystem(),
69-
getLocator());
68+
MissingOptionalUnwrapFailure failure(root, getConstraintSystem(), BaseType,
69+
UnwrappedType, getLocator());
7070
return failure.diagnose(asNote);
7171
}
7272

73-
ForceOptional *ForceOptional::create(ConstraintSystem &cs,
73+
ForceOptional *ForceOptional::create(ConstraintSystem &cs, Type baseType,
74+
Type unwrappedType,
7475
ConstraintLocator *locator) {
75-
return new (cs.getAllocator()) ForceOptional(cs, locator);
76+
return new (cs.getAllocator()) ForceOptional(
77+
cs, baseType, unwrappedType,
78+
cs.getConstraintLocator(simplifyLocatorToAnchor(cs, locator)));
7679
}
7780

7881
bool UnwrapOptionalBase::diagnose(Expr *root, bool asNote) const {

‎lib/Sema/CSFix.h

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -166,16 +166,21 @@ class ForceDowncast final : public ConstraintFix {
166166

167167
/// Introduce a '!' to force an optional unwrap.
168168
class ForceOptional final : public ConstraintFix {
169-
ForceOptional(ConstraintSystem &cs, ConstraintLocator *locator)
170-
: ConstraintFix(cs, FixKind::ForceOptional, locator) {}
169+
Type BaseType;
170+
Type UnwrappedType;
171+
172+
ForceOptional(ConstraintSystem &cs, Type baseType, Type unwrappedType,
173+
ConstraintLocator *locator)
174+
: ConstraintFix(cs, FixKind::ForceOptional, locator), BaseType(baseType),
175+
UnwrappedType(unwrappedType) {}
171176

172177
public:
173178
std::string getName() const override { return "force optional"; }
174179

175180
bool diagnose(Expr *root, bool asNote = false) const override;
176181

177-
static ForceOptional *create(ConstraintSystem &cs,
178-
ConstraintLocator *locator);
182+
static ForceOptional *create(ConstraintSystem &cs, Type baseType,
183+
Type unwrappedType, ConstraintLocator *locator);
179184
};
180185

181186
/// Unwrap an optional base when we have a member access.

‎lib/Sema/CSSimplify.cpp

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2390,11 +2390,11 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
23902390
forceUnwrapPossible = false;
23912391
}
23922392
}
2393-
23942393

23952394
if (forceUnwrapPossible) {
2396-
conversionsOrFixes.push_back(
2397-
ForceOptional::create(*this, getConstraintLocator(locator)));
2395+
conversionsOrFixes.push_back(ForceOptional::create(
2396+
*this, objectType1, objectType1->getOptionalObjectType(),
2397+
getConstraintLocator(locator)));
23982398
}
23992399
}
24002400

@@ -2743,7 +2743,8 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyConformsToConstraint(
27432743
locator.withPathElement(LocatorPathElt::getGenericArgument(0)),
27442744
subflags);
27452745
if (result == SolutionKind::Solved) {
2746-
auto *fix = ForceOptional::create(*this, getConstraintLocator(locator));
2746+
auto *fix = ForceOptional::create(*this, type, optionalObjectType,
2747+
getConstraintLocator(locator));
27472748
if (recordFix(fix)) {
27482749
return SolutionKind::Error;
27492750
}
@@ -2996,6 +2997,7 @@ ConstraintSystem::simplifyFunctionComponentConstraint(
29962997
TypeMatchOptions flags,
29972998
ConstraintLocatorBuilder locator) {
29982999
auto simplified = simplifyType(first);
3000+
auto simplifiedCopy = simplified;
29993001

30003002
unsigned unwrapCount = 0;
30013003
if (shouldAttemptFixes()) {
@@ -3038,7 +3040,9 @@ ConstraintSystem::simplifyFunctionComponentConstraint(
30383040
}
30393041

30403042
if (unwrapCount > 0) {
3041-
auto *fix = ForceOptional::create(*this, getConstraintLocator(locator));
3043+
auto *fix = ForceOptional::create(*this, simplifiedCopy,
3044+
simplifiedCopy->getOptionalObjectType(),
3045+
getConstraintLocator(locator));
30423046
while (unwrapCount-- > 0) {
30433047
if (recordFix(fix))
30443048
return SolutionKind::Error;
@@ -4662,7 +4666,9 @@ ConstraintSystem::simplifyApplicableFnConstraint(
46624666
return SolutionKind::Error;
46634667

46644668
// Record any fixes we attempted to get to the correct solution.
4665-
auto *fix = ForceOptional::create(*this, getConstraintLocator(locator));
4669+
auto *fix = ForceOptional::create(*this, origType2,
4670+
origType2->getOptionalObjectType(),
4671+
getConstraintLocator(locator));
46664672
while (unwrapCount-- > 0) {
46674673
if (recordFix(fix))
46684674
return SolutionKind::Error;
@@ -4685,7 +4691,9 @@ ConstraintSystem::simplifyApplicableFnConstraint(
46854691

46864692
// Record any fixes we attempted to get to the correct solution.
46874693
if (simplified == SolutionKind::Solved) {
4688-
auto *fix = ForceOptional::create(*this, getConstraintLocator(locator));
4694+
auto *fix = ForceOptional::create(*this, origType2,
4695+
origType2->getOptionalObjectType(),
4696+
getConstraintLocator(locator));
46894697
while (unwrapCount-- > 0) {
46904698
if (recordFix(fix))
46914699
return SolutionKind::Error;

‎test/ClangImporter/cfuncs_parse.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ func test_cfunc2(_ i: Int) {
1717

1818
func test_cfunc3_a() {
1919
let b = cfunc3( { (a : Double, b : Double) -> Double in a + b } )
20-
_ = b(1.5, 2.5) as Double // expected-error{{value of optional type 'double_bin_op_block?' (aka 'Optional<(Double, Double) -> Double>') must be unwrapped}}
20+
_ = b(1.5, 2.5) as Double // expected-error{{value of optional type 'double_bin_op_block?' (aka 'Optional<(Double, Double) -> Double>') must be unwrapped to a value of type 'double_bin_op_block' (aka '(Double, Double) -> Double')}}
2121
// expected-note@-1{{coalesce}}
2222
// expected-note@-2{{force-unwrap}}
2323
_ = b!(1.5, 2.5) as Double
@@ -26,7 +26,7 @@ func test_cfunc3_a() {
2626

2727
func test_cfunc3_b() {
2828
let b = cfunc3( { a, b in a + b } )
29-
_ = b(1.5, 2.5) as Double // expected-error{{value of optional type 'double_bin_op_block?' (aka 'Optional<(Double, Double) -> Double>') must be unwrapped}}
29+
_ = b(1.5, 2.5) as Double // expected-error{{value of optional type 'double_bin_op_block?' (aka 'Optional<(Double, Double) -> Double>') must be unwrapped to a value of type 'double_bin_op_block' (aka '(Double, Double) -> Double')}}
3030
// expected-note@-1{{coalesce}}
3131
// expected-note@-2{{force-unwrap}}
3232
_ = b!(1.5, 2.5) as Double
@@ -35,7 +35,7 @@ func test_cfunc3_b() {
3535

3636
func test_cfunc3_c() {
3737
let b = cfunc3({ $0 + $1 })
38-
_ = b(1.5, 2.5) as Double // expected-error{{value of optional type 'double_bin_op_block?' (aka 'Optional<(Double, Double) -> Double>') must be unwrapped}}
38+
_ = b(1.5, 2.5) as Double // expected-error{{value of optional type 'double_bin_op_block?' (aka 'Optional<(Double, Double) -> Double>') must be unwrapped to a value of type 'double_bin_op_block' (aka '(Double, Double) -> Double')}}
3939
// expected-note@-1{{coalesce}}
4040
// expected-note@-2{{force-unwrap}}
4141
_ = b!(1.5, 2.5) as Double

‎test/ClangImporter/objc_parse.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ extension Wobbler2 : NSMaybeInitWobble { // expected-error{{type 'Wobbler2' does
289289

290290
func optionalMemberAccess(_ w: NSWobbling) {
291291
w.wobble()
292-
w.wibble() // expected-error{{value of optional type '(() -> Void)?' must be unwrapped}}
292+
w.wibble() // expected-error{{value of optional type '(() -> Void)?' must be unwrapped to a value of type '() -> Void'}}
293293
// expected-note@-1{{coalesce}}
294294
// expected-note@-2{{force-unwrap}}
295295
let x = w[5]!!

‎test/Constraints/fixes.swift

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ func moreComplexUnwrapFixes() {
205205
// expected-note@-1{{force-unwrap using '!'}}{{13-14=!}}
206206
// expected-note@-2{{coalesce}}
207207
takeNon(os?.optValue) // expected-error{{value of optional type 'Int?' must be unwrapped to a value of type 'Int'}}
208-
// expected-note@-1{{force-unwrap using '!'}}{{11-11=(}}{{23-23=)!}}
208+
// expected-note@-1{{force-unwrap using '!'}}{{11-11=(}} {{23-23=)!}}
209209
// expected-note@-2{{coalesce}}
210210

211211
func sample(a: Int?, b: Int!) {
@@ -243,3 +243,47 @@ func moreComplexUnwrapFixes() {
243243

244244
}
245245
}
246+
247+
struct FooStruct {
248+
func a() -> Int?? { return 10 }
249+
250+
var b: Int?? {
251+
return 15
252+
}
253+
254+
func c() -> Int??? { return 20 }
255+
256+
var d: Int??? {
257+
return 25
258+
}
259+
260+
let e: BarStruct? = BarStruct()
261+
}
262+
263+
struct BarStruct {
264+
func a() -> Int? { return 30 }
265+
var b: Int?? {
266+
return 35
267+
}
268+
}
269+
270+
let thing: FooStruct? = FooStruct()
271+
272+
let _: Int? = thing?.a() // expected-error {{value of optional type 'Int??' must be unwrapped to a value of type 'Int?'}}
273+
// expected-note@-1{{coalesce}}
274+
// expected-note@-2{{force-unwrap}}
275+
let _: Int? = thing?.b // expected-error {{value of optional type 'Int??' must be unwrapped to a value of type 'Int?'}}
276+
// expected-note@-1{{coalesce}}
277+
// expected-note@-2{{force-unwrap}}
278+
let _: Int?? = thing?.c() // expected-error {{value of optional type 'Int???' must be unwrapped to a value of type 'Int??'}}
279+
// expected-note@-1{{coalesce}}
280+
// expected-note@-2{{force-unwrap}}
281+
let _: Int?? = thing?.d // expected-error {{value of optional type 'Int???' must be unwrapped to a value of type 'Int??'}}
282+
// expected-note@-1{{coalesce}}
283+
// expected-note@-2{{force-unwrap}}
284+
let _: Int = thing?.e?.a() // expected-error {{value of optional type 'Int?' must be unwrapped to a value of type 'Int'}}
285+
// expected-note@-1{{coalesce}}
286+
// expected-note@-2{{force-unwrap}}
287+
let _: Int? = thing?.e?.b // expected-error {{value of optional type 'Int??' must be unwrapped to a value of type 'Int?'}}
288+
// expected-note@-1{{coalesce}}
289+
// expected-note@-2{{force-unwrap}}

‎test/Constraints/patterns.swift

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,9 @@ switch staticMembers {
291291
case .init(0): break
292292
case .init(_): break // expected-error{{'_' can only appear in a pattern}}
293293
case .init(let x): break // expected-error{{cannot appear in an expression}}
294-
case .init(opt: 0): break // expected-error{{pattern cannot match values of type 'StaticMembers'}}
294+
case .init(opt: 0): break // expected-error{{value of optional type 'StaticMembers?' must be unwrapped to a value of type 'StaticMembers'}}
295+
// expected-note@-1{{coalesce}}
296+
// expected-note@-2{{force-unwrap}}
295297

296298
case .prop: break
297299
// TODO: repeated error message
@@ -308,7 +310,9 @@ switch staticMembers {
308310
case .method(withLabel: let x): break // expected-error{{cannot appear in an expression}}
309311

310312
case .optMethod: break // expected-error{{cannot match}}
311-
case .optMethod(0): break // expected-error{{pattern cannot match values of type 'StaticMembers'}}
313+
case .optMethod(0): break // expected-error{{value of optional type 'StaticMembers?' must be unwrapped to a value of type 'StaticMembers'}}
314+
// expected-note@-1{{coalesce}}
315+
// expected-note@-2{{force-unwrap}}
312316
}
313317

314318
_ = 0

‎test/expr/delayed-ident/static_var.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,5 +46,7 @@ struct HasClosure {
4646
var _: HasClosure = .factoryNormal(0)
4747
var _: HasClosure = .factoryReturnOpt(1)!
4848
var _: HasClosure = .factoryIUO(2)
49-
var _: HasClosure = .factoryOpt(3) // expected-error {{static property 'factoryOpt' is not a function}}
49+
var _: HasClosure = .factoryOpt(3) // expected-error {{value of optional type '((Int) -> HasClosure)?' must be unwrapped to a value of type '(Int) -> HasClosure'}}
50+
// expected-note@-1{{coalesce}}
51+
// expected-note@-2{{force-unwrap}}
5052
var _: HasClosure = .factoryOpt!(4) // expected-error {{type of expression is ambiguous without more context}}

0 commit comments

Comments
 (0)