Skip to content

Commit 1666ebb

Browse files
rjmccallrjmccall
authored
Merge pull request swiftlang#9039 from rjmccall/optional-to-optional
Fix a semantic bug in CSApply's optional-to-optional application.
2 parents af6548d + 7bb2631 commit 1666ebb

File tree

3 files changed

+69
-17
lines changed

3 files changed

+69
-17
lines changed

‎include/swift/AST/Expr.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2516,6 +2516,9 @@ class BindOptionalExpr : public Expr {
25162516
SourceLoc getQuestionLoc() const { return QuestionLoc; }
25172517

25182518
unsigned getDepth() const { return BindOptionalExprBits.Depth; }
2519+
void setDepth(unsigned depth) {
2520+
BindOptionalExprBits.Depth = depth;
2521+
}
25192522

25202523
Expr *getSubExpr() const { return SubExpr; }
25212524
void setSubExpr(Expr *expr) { SubExpr = expr; }

‎lib/Sema/CSApply.cpp

Lines changed: 44 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5064,6 +5064,41 @@ Expr *ExprRewriter::coerceExistential(Expr *expr, Type toType,
50645064
return cs.cacheType(new (ctx) ErasureExpr(expr, toType, conformances));
50655065
}
50665066

5067+
/// Given that the given expression is an implicit conversion added
5068+
/// to the target by coerceToType, find out how many OptionalEvaluationExprs
5069+
/// it includes and the target.
5070+
static unsigned getOptionalEvaluationDepth(Expr *expr, Expr *target) {
5071+
unsigned depth = 0;
5072+
while (true) {
5073+
// Look through sugar expressions.
5074+
expr = expr->getSemanticsProvidingExpr();
5075+
5076+
// If we find the target expression, we're done.
5077+
if (expr == target) return depth;
5078+
5079+
// If we see an optional evaluation, the depth goes up.
5080+
if (auto optEval = dyn_cast<OptionalEvaluationExpr>(expr)) {
5081+
depth++;
5082+
expr = optEval->getSubExpr();
5083+
5084+
// We have to handle any other expressions that can be introduced by
5085+
// coerceToType.
5086+
} else if (auto bind = dyn_cast<BindOptionalExpr>(expr)) {
5087+
expr = bind->getSubExpr();
5088+
} else if (auto force = dyn_cast<ForceValueExpr>(expr)) {
5089+
expr = force->getSubExpr();
5090+
} else if (auto open = dyn_cast<OpenExistentialExpr>(expr)) {
5091+
depth += getOptionalEvaluationDepth(open->getSubExpr(),
5092+
open->getOpaqueValue());
5093+
expr = open->getExistentialValue();
5094+
5095+
// Otherwise, look through implicit conversions.
5096+
} else {
5097+
expr = cast<ImplicitConversionExpr>(expr)->getSubExpr();
5098+
}
5099+
}
5100+
}
5101+
50675102
Expr *ExprRewriter::coerceOptionalToOptional(Expr *expr, Type toType,
50685103
ConstraintLocatorBuilder locator,
50695104
Optional<Pattern*> typeFromPattern) {
@@ -5101,15 +5136,18 @@ Expr *ExprRewriter::coerceOptionalToOptional(Expr *expr, Type toType,
51015136
Type fromValueType = fromType->getAnyOptionalObjectType();
51025137
Type toValueType = toType->getAnyOptionalObjectType();
51035138

5104-
// FIXME: this depth is wrong, it needs to be the depth of
5105-
// OptionalEvaluationExprs introduced by the call to coerceToType below.
5106-
expr =
5107-
cs.cacheType(new (tc.Context) BindOptionalExpr(expr,
5108-
expr->getSourceRange().End,
5109-
/*depth*/ 0, fromValueType));
5139+
// The depth we use here will get patched after we apply the coercion.
5140+
auto bindOptional =
5141+
new (tc.Context) BindOptionalExpr(expr, expr->getSourceRange().End,
5142+
/*depth*/ 0, fromValueType);
5143+
5144+
expr = cs.cacheType(bindOptional);
51105145
expr->setImplicit(true);
51115146
expr = coerceToType(expr, toValueType, locator, typeFromPattern);
51125147
if (!expr) return nullptr;
5148+
5149+
unsigned depth = getOptionalEvaluationDepth(expr, bindOptional);
5150+
bindOptional->setDepth(depth);
51135151

51145152
expr = cs.cacheType(new (tc.Context) InjectIntoOptionalExpr(expr, toType));
51155153

‎test/SILGen/pointer_conversion.swift

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -319,15 +319,14 @@ func optOptArrayToOptOptPointer(array: [Int]??) {
319319
// CHECK: [[SIDE1:%.*]] = function_ref @_T018pointer_conversion11sideEffect1SiyF
320320
// CHECK: [[RESULT1:%.*]] = apply [[SIDE1]]()
321321
// CHECK: [[T0:%.*]] = select_enum [[COPY]]
322-
// FIXME: this should really go somewhere that will make nil, not some(nil)
323322
// CHECK: cond_br [[T0]], [[SOME_BB:bb[0-9]+]], [[NONE_BB:bb[0-9]+]]
324323
// CHECK: [[SOME_BB]]:
325324
// CHECK: [[SOME_VALUE:%.*]] = unchecked_enum_data [[COPY]]
326325
// CHECK: [[T0:%.*]] = select_enum [[SOME_VALUE]]
327326
// CHECK: cond_br [[T0]], [[SOME_SOME_BB:bb[0-9]+]], [[SOME_NONE_BB:bb[0-9]+]]
328327
// CHECK: [[SOME_NONE_BB]]:
329328
// CHECK: destroy_value [[SOME_VALUE]]
330-
// CHECK: br [[NONE_BB]]
329+
// CHECK: br [[SOME_NONE_BB2:bb[0-9]+]]
331330
// CHECK: [[SOME_SOME_BB]]:
332331
// CHECK: [[SOME_SOME_VALUE:%.*]] = unchecked_enum_data [[SOME_VALUE]]
333332
// CHECK: [[CONVERT:%.*]] = function_ref @_T0s35_convertConstArrayToPointerArguments9AnyObject_pSg_q_tSayxGs01_E0R_r0_lF
@@ -337,19 +336,25 @@ func optOptArrayToOptOptPointer(array: [Int]??) {
337336
// CHECK: [[DEP:%.*]] = mark_dependence [[PTR]] : $UnsafePointer<Int> on [[OWNER]]
338337
// CHECK: [[OPTPTR:%.*]] = enum $Optional<UnsafePointer<Int>>, #Optional.some!enumelt.1, [[DEP]]
339338
// CHECK: dealloc_stack [[TEMP]]
340-
// CHECK: br [[SOME_CONT_BB:bb[0-9]+]]([[OPTPTR]] : $Optional<UnsafePointer<Int>>, [[OWNER]] : $Optional<AnyObject>)
341-
// CHECK: [[SOME_CONT_BB]]([[OPTPTR:%.*]] : $Optional<UnsafePointer<Int>>, [[OWNER:%.*]] : $Optional<AnyObject>):
339+
// CHECK: br [[SOME_SOME_CONT_BB:bb[0-9]+]]([[OPTPTR]] : $Optional<UnsafePointer<Int>>, [[OWNER]] : $Optional<AnyObject>)
340+
// CHECK: [[SOME_SOME_CONT_BB]]([[OPTPTR:%.*]] : $Optional<UnsafePointer<Int>>, [[OWNER:%.*]] : $Optional<AnyObject>):
342341
// CHECK: [[OPTDEP:%.*]] = mark_dependence [[OPTPTR]] : $Optional<UnsafePointer<Int>> on [[OWNER]]
343342
// CHECK: [[OPTOPTPTR:%.*]] = enum $Optional<Optional<UnsafePointer<Int>>>, #Optional.some!enumelt.1, [[OPTDEP]]
343+
// CHECK: br [[SOME_CONT_BB:bb[0-9]+]]([[OPTOPTPTR]] : $Optional<Optional<UnsafePointer<Int>>>, [[OWNER]] : $Optional<AnyObject>)
344+
// CHECK: [[SOME_CONT_BB]]([[OPTOPTPTR:%.*]] : $Optional<Optional<UnsafePointer<Int>>>, [[OWNER:%.*]] : $Optional<AnyObject>):
344345
// CHECK: [[OPTOPTDEP:%.*]] = mark_dependence [[OPTOPTPTR]] : $Optional<Optional<UnsafePointer<Int>>> on [[OWNER]]
345346
// CHECK: apply [[TAKES]]([[OPTOPTDEP]], [[RESULT1]])
346347
// CHECK: destroy_value [[OWNER]]
347348
// CHECK: end_borrow [[BORROW]]
348349
// CHECK: destroy_value %0
349-
// CHECK: [[NONE_BB]]:
350+
// CHECK: [[SOME_NONE_BB2]]:
350351
// CHECK: [[NO_VALUE:%.*]] = enum $Optional<UnsafePointer<Int>>, #Optional.none
351352
// CHECK: [[NO_OWNER:%.*]] = enum $Optional<AnyObject>, #Optional.none
352-
// CHECK: br [[SOME_CONT_BB]]([[NO_VALUE]] : $Optional<UnsafePointer<Int>>, [[NO_OWNER]] : $Optional<AnyObject>)
353+
// CHECK: br [[SOME_SOME_CONT_BB]]([[NO_VALUE]] : $Optional<UnsafePointer<Int>>, [[NO_OWNER]] : $Optional<AnyObject>)
354+
// CHECK: [[NONE_BB]]:
355+
// CHECK: [[NO_VALUE:%.*]] = enum $Optional<Optional<UnsafePointer<Int>>>, #Optional.none
356+
// CHECK: [[NO_OWNER:%.*]] = enum $Optional<AnyObject>, #Optional.none
357+
// CHECK: br [[SOME_CONT_BB]]([[NO_VALUE]] : $Optional<Optional<UnsafePointer<Int>>>, [[NO_OWNER]] : $Optional<AnyObject>)
353358
takesOptOptConstPointer(array, and: sideEffect1())
354359
}
355360

@@ -401,7 +406,7 @@ func optOptStringToOptOptPointer(string: String??) {
401406
// CHECK: cond_br [[T0]], [[SOME_SOME_BB:bb[0-9]+]], [[SOME_NONE_BB:bb[0-9]+]]
402407
// CHECK: [[SOME_NONE_BB]]:
403408
// CHECK: destroy_value [[SOME_VALUE]]
404-
// CHECK: br [[NONE_BB]]
409+
// CHECK: br [[SOME_NONE_BB2:bb[0-9]+]]
405410
// CHECK: [[SOME_SOME_BB]]:
406411
// CHECK: [[SOME_SOME_VALUE:%.*]] = unchecked_enum_data [[SOME_VALUE]]
407412
// CHECK: [[CONVERT:%.*]] = function_ref @_T0s40_convertConstStringToUTF8PointerArguments9AnyObject_pSg_xtSSs01_F0RzlF
@@ -411,18 +416,24 @@ func optOptStringToOptOptPointer(string: String??) {
411416
// CHECK: [[DEP:%.*]] = mark_dependence [[PTR]] : $UnsafeRawPointer on [[OWNER]]
412417
// CHECK: [[OPTPTR:%.*]] = enum $Optional<UnsafeRawPointer>, #Optional.some!enumelt.1, [[DEP]]
413418
// CHECK: dealloc_stack [[TEMP]]
414-
// CHECK: br [[SOME_CONT_BB:bb[0-9]+]]([[OPTPTR]] : $Optional<UnsafeRawPointer>, [[OWNER]] : $Optional<AnyObject>)
415-
// CHECK: [[SOME_CONT_BB]]([[OPTPTR:%.*]] : $Optional<UnsafeRawPointer>, [[OWNER:%.*]] : $Optional<AnyObject>):
419+
// CHECK: br [[SOME_SOME_CONT_BB:bb[0-9]+]]([[OPTPTR]] : $Optional<UnsafeRawPointer>, [[OWNER]] : $Optional<AnyObject>)
420+
// CHECK: [[SOME_SOME_CONT_BB]]([[OPTPTR:%.*]] : $Optional<UnsafeRawPointer>, [[OWNER:%.*]] : $Optional<AnyObject>):
416421
// CHECK: [[OPTDEP:%.*]] = mark_dependence [[OPTPTR]] : $Optional<UnsafeRawPointer> on [[OWNER]]
417422
// CHECK: [[OPTOPTPTR:%.*]] = enum $Optional<Optional<UnsafeRawPointer>>, #Optional.some!enumelt.1, [[OPTDEP]]
423+
// CHECK: br [[SOME_CONT_BB:bb[0-9]+]]([[OPTOPTPTR]] : $Optional<Optional<UnsafeRawPointer>>, [[OWNER]] : $Optional<AnyObject>)
424+
// CHECK: [[SOME_CONT_BB]]([[OPTOPTPTR:%.*]] : $Optional<Optional<UnsafeRawPointer>>, [[OWNER:%.*]] : $Optional<AnyObject>):
418425
// CHECK: [[OPTOPTDEP:%.*]] = mark_dependence [[OPTOPTPTR]] : $Optional<Optional<UnsafeRawPointer>> on [[OWNER]]
419426
// CHECK: apply [[TAKES]]([[OPTOPTDEP]], [[RESULT1]])
420427
// CHECK: destroy_value [[OWNER]]
421428
// CHECK: end_borrow [[BORROW]]
422429
// CHECK: destroy_value %0
423-
// CHECK: [[NONE_BB]]:
430+
// CHECK: [[SOME_NONE_BB2]]:
424431
// CHECK: [[NO_VALUE:%.*]] = enum $Optional<UnsafeRawPointer>, #Optional.none
425432
// CHECK: [[NO_OWNER:%.*]] = enum $Optional<AnyObject>, #Optional.none
426-
// CHECK: br [[SOME_CONT_BB]]([[NO_VALUE]] : $Optional<UnsafeRawPointer>, [[NO_OWNER]] : $Optional<AnyObject>)
433+
// CHECK: br [[SOME_SOME_CONT_BB]]([[NO_VALUE]] : $Optional<UnsafeRawPointer>, [[NO_OWNER]] : $Optional<AnyObject>)
434+
// CHECK: [[NONE_BB]]:
435+
// CHECK: [[NO_VALUE:%.*]] = enum $Optional<Optional<UnsafeRawPointer>>, #Optional.none
436+
// CHECK: [[NO_OWNER:%.*]] = enum $Optional<AnyObject>, #Optional.none
437+
// CHECK: br [[SOME_CONT_BB]]([[NO_VALUE]] : $Optional<Optional<UnsafeRawPointer>>, [[NO_OWNER]] : $Optional<AnyObject>)
427438
takesOptOptConstRawPointer(string, and: sideEffect1())
428439
}

0 commit comments

Comments
 (0)