Skip to content

Commit 310ff65

Browse files
authored
Merge pull request #65920 from hamishknight/off-the-chain-5.9
2 parents 780e3ec + 6641fa4 commit 310ff65

File tree

4 files changed

+516
-16
lines changed

4 files changed

+516
-16
lines changed

lib/AST/ASTScopeCreation.cpp

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -100,13 +100,14 @@ class ScopeCreator final : public ASTAllocated<ScopeCreator> {
100100
ASTScopeAssert(expr,
101101
"If looking for closures, must have an expression to search.");
102102

103-
/// AST walker that finds top-level closures in an expression.
104-
class ClosureFinder : public ASTWalker {
103+
/// AST walker that finds nested scopes in expressions. This handles both
104+
/// closures and if/switch expressions.
105+
class NestedExprScopeFinder : public ASTWalker {
105106
ScopeCreator &scopeCreator;
106107
ASTScopeImpl *parent;
107108

108109
public:
109-
ClosureFinder(ScopeCreator &scopeCreator, ASTScopeImpl *parent)
110+
NestedExprScopeFinder(ScopeCreator &scopeCreator, ASTScopeImpl *parent)
110111
: scopeCreator(scopeCreator), parent(parent) {}
111112

112113
PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
@@ -122,6 +123,13 @@ class ScopeCreator final : public ASTAllocated<ScopeCreator> {
122123
parent, capture);
123124
return Action::SkipChildren(E);
124125
}
126+
127+
// If we have a single value statement expression, we need to add any
128+
// scopes in the underlying statement.
129+
if (auto *SVE = dyn_cast<SingleValueStmtExpr>(E)) {
130+
scopeCreator.addToScopeTree(SVE->getStmt(), parent);
131+
return Action::SkipChildren(E);
132+
}
125133
return Action::Continue(E);
126134
}
127135
PreWalkResult<Stmt *> walkToStmtPre(Stmt *S) override {
@@ -148,7 +156,7 @@ class ScopeCreator final : public ASTAllocated<ScopeCreator> {
148156
}
149157
};
150158

151-
expr->walk(ClosureFinder(*this, parent));
159+
expr->walk(NestedExprScopeFinder(*this, parent));
152160
}
153161

154162
public:
@@ -518,11 +526,6 @@ class NodeAdder
518526
if (!expr)
519527
return p;
520528

521-
// If we have a single value statement expression, we expand scopes based
522-
// on the underlying statement.
523-
if (auto *SVE = dyn_cast<SingleValueStmtExpr>(expr))
524-
return visit(SVE->getStmt(), p, scopeCreator);
525-
526529
scopeCreator.addExprToScopeTree(expr, p);
527530
return p;
528531
}

lib/Sema/MiscDiagnostics.cpp

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3833,6 +3833,23 @@ class SingleValueStmtUsageChecker final : public ASTWalker {
38333833
return MacroWalking::Expansion;
38343834
}
38353835

3836+
AssignExpr *findAssignment(Expr *E) const {
3837+
// Don't consider assignments if we have a parent expression (as otherwise
3838+
// this would be effectively allowing it in an arbitrary expression
3839+
// position).
3840+
if (Parent.getAsExpr())
3841+
return nullptr;
3842+
3843+
// Look through optional exprs, which are present for e.g x?.y = z, as
3844+
// we wrap the entire assign in the optional evaluation of the destination.
3845+
if (auto *OEE = dyn_cast<OptionalEvaluationExpr>(E)) {
3846+
E = OEE->getSubExpr();
3847+
while (auto *IIO = dyn_cast<InjectIntoOptionalExpr>(E))
3848+
E = IIO->getSubExpr();
3849+
}
3850+
return dyn_cast<AssignExpr>(E);
3851+
}
3852+
38363853
PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
38373854
if (auto *SVE = dyn_cast<SingleValueStmtExpr>(E)) {
38383855
// Diagnose a SingleValueStmtExpr in a context that we do not currently
@@ -3908,13 +3925,9 @@ class SingleValueStmtUsageChecker final : public ASTWalker {
39083925
return Action::Continue(E);
39093926
}
39103927

3911-
// Valid as the source of an assignment, as long as it's not a nested
3912-
// expression (as otherwise this would be effectively allowing it in an
3913-
// arbitrary expression position).
3914-
if (auto *AE = dyn_cast<AssignExpr>(E)) {
3915-
if (!Parent.getAsExpr())
3916-
markValidSingleValueStmt(AE->getSrc());
3917-
}
3928+
// Valid as the source of an assignment.
3929+
if (auto *AE = findAssignment(E))
3930+
markValidSingleValueStmt(AE->getSrc());
39183931

39193932
// Valid as a single expression body of a closure. This is needed in
39203933
// addition to ReturnStmt checking, as we will remove the return if the

test/SILGen/if_expr.swift

Lines changed: 237 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,3 +263,240 @@ func nestedType() throws -> Int {
263263
0
264264
}
265265
}
266+
267+
// MARK: Bindings
268+
269+
enum E {
270+
case e(Int)
271+
}
272+
273+
struct S {
274+
var i: Int
275+
var opt: Int?
276+
277+
var computed: Int {
278+
get { i }
279+
set { i = newValue }
280+
}
281+
var coroutined: Int {
282+
_read { yield i }
283+
_modify { yield &i }
284+
}
285+
286+
subscript(x: Int) -> Int {
287+
get { i }
288+
set { i = newValue }
289+
}
290+
291+
mutating func testAssign1(_ x: E) {
292+
i = if case .e(let y) = x { y } else { 0 }
293+
}
294+
295+
296+
mutating func testAssign2(_ x: E) {
297+
i = if case .e(let y) = x { Int(y) } else { 0 }
298+
}
299+
300+
func testAssign3(_ x: E) {
301+
var i = 0
302+
i = if case .e(let y) = x { y } else { 0 }
303+
_ = i
304+
}
305+
306+
func testAssign4(_ x: E) {
307+
var i = 0
308+
let _ = {
309+
i = if case .e(let y) = x { y } else { 0 }
310+
}
311+
_ = i
312+
}
313+
314+
mutating func testAssign5(_ x: E) {
315+
i = switch Bool.random() {
316+
case true:
317+
if case .e(let y) = x { y } else { 0 }
318+
case let z:
319+
z ? 0 : 1
320+
}
321+
}
322+
323+
mutating func testAssign6(_ x: E) {
324+
i = if case .e(let y) = x {
325+
switch Bool.random() {
326+
case true: y
327+
case false: y
328+
}
329+
} else {
330+
0
331+
}
332+
}
333+
334+
mutating func testAssign7(_ x: E?) {
335+
i = if let x = x {
336+
switch x {
337+
case .e(let y): y
338+
}
339+
} else {
340+
0
341+
}
342+
}
343+
344+
func testReturn1(_ x: E) -> Int {
345+
if case .e(let y) = x { y } else { 0 }
346+
}
347+
348+
func testReturn2(_ x: E) -> Int {
349+
return if case .e(let y) = x { y } else { 0 }
350+
}
351+
352+
func testReturn3(_ x: E) -> Int {
353+
{
354+
if case .e(let y) = x { y } else { 0 }
355+
}()
356+
}
357+
358+
func testReturn4(_ x: E) -> Int {
359+
return {
360+
if case .e(let y) = x { y } else { 0 }
361+
}()
362+
}
363+
364+
func testBinding1(_ x: E) -> Int {
365+
let i = if case .e(let y) = x { y } else { 0 }
366+
return i
367+
}
368+
369+
func testBinding2(_ x: E) -> Int {
370+
let i = {
371+
if case .e(let y) = x { y } else { 0 }
372+
}()
373+
return i
374+
}
375+
}
376+
377+
enum G {
378+
case e(Int)
379+
case f
380+
}
381+
382+
struct TestLValues {
383+
var s: S
384+
var opt: S?
385+
var optopt: S??
386+
387+
mutating func testOptPromote1() {
388+
opt = if .random() { s } else { s }
389+
}
390+
391+
mutating func testOptPromote2() {
392+
optopt = if .random() { s } else { s }
393+
}
394+
395+
mutating func testStored1() {
396+
s.i = if .random() { 1 } else { 0 }
397+
}
398+
399+
mutating func testStored2() throws {
400+
s.i = if .random() { 1 } else { throw Err() }
401+
}
402+
403+
mutating func testComputed1() {
404+
s.computed = if .random() { 1 } else { 0 }
405+
}
406+
407+
mutating func testComputed2() throws {
408+
s.computed = if .random() { 1 } else { throw Err() }
409+
}
410+
411+
mutating func testCoroutined1() {
412+
s.coroutined = if .random() { 1 } else { 0 }
413+
}
414+
415+
mutating func testCoroutined2() throws {
416+
s.coroutined = if .random() { 1 } else { throw Err() }
417+
}
418+
419+
mutating func testOptionalChain1() {
420+
opt?.i = if .random() { 1 } else { 0 }
421+
}
422+
423+
mutating func testOptionalChain2() throws {
424+
opt?.i = if .random() { throw Err() } else { 0 }
425+
}
426+
427+
mutating func testOptionalChain3(_ g: G) {
428+
opt?.i = if case .e(let i) = g { i } else { 0 }
429+
}
430+
431+
mutating func testOptionalChain4(_ g: G) throws {
432+
opt?.i = if case .e(let i) = g { i } else { throw Err() }
433+
}
434+
435+
mutating func testOptionalChain5(_ g: G) throws {
436+
opt?.computed = if case .e(let i) = g { i } else { throw Err() }
437+
}
438+
439+
mutating func testOptionalChain6(_ g: G) throws {
440+
opt?.coroutined = if case .e(let i) = g { i } else { throw Err() }
441+
}
442+
443+
mutating func testOptionalChain7() throws {
444+
optopt??.i = if .random() { 1 } else { throw Err() }
445+
}
446+
447+
mutating func testOptionalChain8() throws {
448+
optopt??.opt = if .random() { 1 } else { throw Err() }
449+
}
450+
451+
mutating func testOptionalChain9() throws {
452+
optopt??.opt? = if .random() { 1 } else { throw Err() }
453+
}
454+
455+
mutating func testOptionalForce1() throws {
456+
opt!.i = if .random() { throw Err() } else { 0 }
457+
}
458+
459+
mutating func testOptionalForce2() throws {
460+
opt!.computed = if .random() { throw Err() } else { 0 }
461+
}
462+
463+
mutating func testOptionalForce3(_ g: G) throws {
464+
opt!.coroutined = if case .e(let i) = g { i } else { throw Err() }
465+
}
466+
467+
mutating func testOptionalForce4() throws {
468+
optopt!!.i = if .random() { 1 } else { throw Err() }
469+
}
470+
471+
mutating func testOptionalForce5() throws {
472+
optopt!!.opt = if .random() { 1 } else { throw Err() }
473+
}
474+
475+
mutating func testOptionalForce6() throws {
476+
optopt!!.opt! = if .random() { 1 } else { throw Err() }
477+
}
478+
479+
mutating func testSubscript1() throws {
480+
s[5] = if .random() { 1 } else { throw Err() }
481+
}
482+
483+
mutating func testSubscript2() throws {
484+
opt?[5] = if .random() { 1 } else { throw Err() }
485+
}
486+
487+
mutating func testSubscript3() throws {
488+
opt![5] = if .random() { 1 } else { throw Err() }
489+
}
490+
491+
mutating func testKeyPath1(_ kp: WritableKeyPath<S, Int>) throws {
492+
s[keyPath: kp] = if .random() { 1 } else { throw Err() }
493+
}
494+
495+
mutating func testKeyPath2(_ kp: WritableKeyPath<S, Int>) throws {
496+
opt?[keyPath: kp] = if .random() { 1 } else { throw Err() }
497+
}
498+
499+
mutating func testKeyPath3(_ kp: WritableKeyPath<S, Int>) throws {
500+
opt![keyPath: kp] = if .random() { 1 } else { throw Err() }
501+
}
502+
}

0 commit comments

Comments
 (0)