Skip to content

Commit 67fcb1c

Browse files
gottesmmgottesmm
committed
[consume-operator] Change consume so that its result is not considered an lvalue.
Before this change it was possible to: 1. Call mutating methods on a consume result. 2. assign into a consume (e.x.: var x = ...; (consume x) = value. From an implementation perspective, this involved just taking the logic I already used for the CopyExpr and reusing it for ConsumeExpr with some small tweaks. rdar://109479440
1 parent fd25cf3 commit 67fcb1c

File tree

9 files changed

+175
-21
lines changed

9 files changed

+175
-21
lines changed

‎include/swift/AST/Expr.h

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2052,17 +2052,14 @@ class AwaitExpr final : public IdentityExpr {
20522052

20532053
/// ConsumeExpr - A 'consume' surrounding an lvalue expression marking the
20542054
/// lvalue as needing to be moved.
2055-
///
2056-
/// getSemanticsProvidingExpr() looks through this because it doesn't
2057-
/// provide the value and only very specific clients care where the
2058-
/// 'move' was written.
2059-
class ConsumeExpr final : public IdentityExpr {
2055+
class ConsumeExpr final : public Expr {
2056+
Expr *SubExpr;
20602057
SourceLoc ConsumeLoc;
20612058

20622059
public:
20632060
ConsumeExpr(SourceLoc consumeLoc, Expr *sub, Type type = Type(),
20642061
bool implicit = false)
2065-
: IdentityExpr(ExprKind::Consume, sub, type, implicit),
2062+
: Expr(ExprKind::Consume, implicit, type), SubExpr(sub),
20662063
ConsumeLoc(consumeLoc) {}
20672064

20682065
static ConsumeExpr *createImplicit(ASTContext &ctx, SourceLoc moveLoc,
@@ -2072,7 +2069,10 @@ class ConsumeExpr final : public IdentityExpr {
20722069

20732070
SourceLoc getLoc() const { return ConsumeLoc; }
20742071

2075-
SourceLoc getStartLoc() const { return ConsumeLoc; }
2072+
Expr *getSubExpr() const { return SubExpr; }
2073+
void setSubExpr(Expr *E) { SubExpr = E; }
2074+
2075+
SourceLoc getStartLoc() const { return getLoc(); }
20762076
SourceLoc getEndLoc() const { return getSubExpr()->getEndLoc(); }
20772077

20782078
static bool classof(const Expr *e) {

‎include/swift/AST/ExprNodes.def

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,11 +107,11 @@ ABSTRACT_EXPR(Identity, Expr)
107107
EXPR(Paren, IdentityExpr)
108108
EXPR(DotSelf, IdentityExpr)
109109
EXPR(Await, IdentityExpr)
110-
EXPR(Consume, IdentityExpr)
111110
EXPR(Borrow, IdentityExpr)
112111
EXPR(UnresolvedMemberChainResult, IdentityExpr)
113112
EXPR_RANGE(Identity, Paren, UnresolvedMemberChainResult)
114113
EXPR(Copy, Expr)
114+
EXPR(Consume, Expr)
115115
ABSTRACT_EXPR(AnyTry, Expr)
116116
EXPR(Try, AnyTryExpr)
117117
EXPR(ForceTry, AnyTryExpr)

‎lib/AST/ASTWalker.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -699,6 +699,14 @@ class Traversal : public ASTVisitor<Traversal, Expr*, Stmt*,
699699
return nullptr;
700700
}
701701

702+
Expr *visitConsumeExpr(ConsumeExpr *E) {
703+
if (Expr *subExpr = doIt(E->getSubExpr())) {
704+
E->setSubExpr(subExpr);
705+
return E;
706+
}
707+
return nullptr;
708+
}
709+
702710
Expr *visitTupleExpr(TupleExpr *E) {
703711
for (unsigned i = 0, e = E->getNumElements(); i != e; ++i)
704712
if (E->getElement(i)) {

‎lib/SILGen/SILGenExpr.cpp

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6116,27 +6116,47 @@ RValue RValueEmitter::visitErrorExpr(ErrorExpr *E, SGFContext C) {
61166116
}
61176117

61186118
RValue RValueEmitter::visitConsumeExpr(ConsumeExpr *E, SGFContext C) {
6119-
auto *subExpr = cast<DeclRefExpr>(E->getSubExpr());
6119+
auto *subExpr = E->getSubExpr();
61206120
auto subASTType = subExpr->getType()->getCanonicalType();
6121-
61226121
auto subType = SGF.getLoweredType(subASTType);
61236122

6123+
if (auto *li = dyn_cast<LoadExpr>(subExpr)) {
6124+
FormalEvaluationScope writeback(SGF);
6125+
LValue lv =
6126+
SGF.emitLValue(li->getSubExpr(), SGFAccessKind::ReadWrite);
6127+
auto address = SGF.emitAddressOfLValue(subExpr, std::move(lv));
6128+
auto optTemp = SGF.emitTemporary(E, SGF.getTypeLowering(subType));
6129+
SILValue toAddr = optTemp->getAddressForInPlaceInitialization(SGF, E);
6130+
if (address.getType().isMoveOnly()) {
6131+
SGF.B.createCopyAddr(subExpr, address.getLValueAddress(), toAddr, IsNotTake,
6132+
IsInitialization);
6133+
} else {
6134+
SGF.B.createMarkUnresolvedMoveAddr(subExpr, address.getLValueAddress(), toAddr);
6135+
}
6136+
optTemp->finishInitialization(SGF);
6137+
6138+
if (subType.isLoadable(SGF.F)) {
6139+
ManagedValue value = SGF.B.createLoadTake(E, optTemp->getManagedAddress());
6140+
if (value.getType().isTrivial(SGF.F))
6141+
return RValue(SGF, {value}, subType.getASTType());
6142+
return RValue(SGF, {value}, subType.getASTType());
6143+
}
6144+
6145+
return RValue(SGF, {optTemp->getManagedAddress()}, subType.getASTType());
6146+
}
6147+
61246148
if (subType.isLoadable(SGF.F)) {
6125-
auto mv = SGF.emitRValue(subExpr).getAsSingleValue(SGF, subExpr);
6149+
ManagedValue mv = SGF.emitRValue(subExpr).getAsSingleValue(SGF, subExpr);
61266150
if (mv.getType().isTrivial(SGF.F))
61276151
return RValue(SGF, {mv}, subType.getASTType());
61286152
mv = SGF.B.createMoveValue(E, mv);
6129-
auto *movedValue = cast<MoveValueInst>(mv.getValue());
6130-
movedValue->setAllowsDiagnostics(true /*set allows diagnostics*/);
6153+
// Set the flag so we check this.
6154+
cast<MoveValueInst>(mv.getValue())->setAllowsDiagnostics(true);
61316155
return RValue(SGF, {mv}, subType.getASTType());
61326156
}
61336157

61346158
// If we aren't loadable, then create a temporary initialization and
6135-
// mark_unresolved_move into that if we have a copyable type if we have a move
6136-
// only, just add a copy_addr init.
6137-
//
6138-
// The reason why we do this is that we only use mark_unresolved_move_addr and
6139-
// the move operator checker for copyable values.
6159+
// explicit_copy_addr into that.
61406160
std::unique_ptr<TemporaryInitialization> optTemp;
61416161
optTemp = SGF.emitTemporary(E, SGF.getTypeLowering(subType));
61426162
SILValue toAddr = optTemp->getAddressForInPlaceInitialization(SGF, E);
@@ -6163,7 +6183,6 @@ RValue RValueEmitter::visitCopyExpr(CopyExpr *E, SGFContext C) {
61636183
auto subType = SGF.getLoweredType(subASTType);
61646184

61656185
if (auto *li = dyn_cast<LoadExpr>(subExpr)) {
6166-
61676186
FormalEvaluationScope writeback(SGF);
61686187
LValue lv =
61696188
SGF.emitLValue(li->getSubExpr(), SGFAccessKind::BorrowedAddressRead);

‎lib/Sema/CSApply.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3546,6 +3546,27 @@ namespace {
35463546
return expr;
35473547
}
35483548

3549+
Expr *visitConsumeExpr(ConsumeExpr *expr) {
3550+
auto toType = simplifyType(cs.getType(expr));
3551+
cs.setType(expr, toType);
3552+
3553+
auto *subExpr = expr->getSubExpr();
3554+
auto type = simplifyType(cs.getType(subExpr));
3555+
3556+
// Let's load the value associated with this consume.
3557+
if (type->hasLValueType()) {
3558+
subExpr = coerceToType(subExpr, type->getRValueType(),
3559+
cs.getConstraintLocator(subExpr));
3560+
3561+
if (!subExpr)
3562+
return nullptr;
3563+
}
3564+
3565+
expr->setSubExpr(subExpr);
3566+
3567+
return expr;
3568+
}
3569+
35493570
Expr *visitAnyTryExpr(AnyTryExpr *expr) {
35503571
auto *subExpr = expr->getSubExpr();
35513572
auto type = simplifyType(cs.getType(subExpr));

‎lib/Sema/CSGen.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1899,6 +1899,16 @@ namespace {
18991899
return valueTy;
19001900
}
19011901

1902+
Type visitConsumeExpr(ConsumeExpr *expr) {
1903+
auto valueTy = CS.createTypeVariable(CS.getConstraintLocator(expr),
1904+
TVO_PrefersSubtypeBinding |
1905+
TVO_CanBindToNoEscape);
1906+
CS.addConstraint(ConstraintKind::Equal, valueTy,
1907+
CS.getType(expr->getSubExpr()),
1908+
CS.getConstraintLocator(expr));
1909+
return valueTy;
1910+
}
1911+
19021912
Type visitAnyTryExpr(AnyTryExpr *expr) {
19031913
return CS.getType(expr->getSubExpr());
19041914
}

‎lib/Sema/MiscDiagnostics.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -422,7 +422,10 @@ static void diagSyntacticUseRestrictions(const Expr *E, const DeclContext *DC,
422422
}
423423

424424
void checkConsumeExpr(ConsumeExpr *consumeExpr) {
425-
if (!isa<DeclRefExpr>(consumeExpr->getSubExpr())) {
425+
auto *subExpr = consumeExpr->getSubExpr();
426+
if (auto *li = dyn_cast<LoadExpr>(subExpr))
427+
subExpr = li->getSubExpr();
428+
if (!isa<DeclRefExpr>(subExpr)) {
426429
Ctx.Diags.diagnose(consumeExpr->getLoc(),
427430
diag::consume_expression_not_passed_lvalue);
428431
}

‎test/SILGen/consume_operator.swift

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
// RUN: %target-swift-emit-silgen -enable-copy-propagation=requested-passes-only -module-name consume %s -disable-objc-attr-requires-foundation-module | %FileCheck %s
2+
3+
class Klass {}
4+
5+
protocol P {
6+
static var value: Self { get }
7+
}
8+
9+
// CHECK-LABEL: sil hidden [ossa] @$s7consume15testLoadableLetyyF : $@convention(thin) () -> () {
10+
// CHECK: [[ORIG_VALUE:%.*]] = apply {{%.*}}({{%.*}}) : $@convention(method) (@thick Klass.Type) -> @owned Klass
11+
// CHECK: [[BORROWED_VALUE:%.*]] = begin_borrow [lexical] [[ORIG_VALUE]]
12+
// CHECK: [[COPY:%.*]] = copy_value [[BORROWED_VALUE:%.*]]
13+
// CHECK: move_value [allows_diagnostics] [[COPY]]
14+
// CHECK: } // end sil function '$s7consume15testLoadableLetyyF'
15+
func testLoadableLet() {
16+
let k = Klass()
17+
let _ = consume k
18+
}
19+
20+
// CHECK-LABEL: sil hidden [ossa] @$s7consume15testLoadableVaryyF : $@convention(thin) () -> () {
21+
// CHECK: [[BOX:%.*]] = alloc_box $
22+
// CHECK: [[BORROW_BOX:%.*]] = begin_borrow [lexical] [[BOX]]
23+
// CHECK: [[PROJECT:%.*]] = project_box [[BORROW_BOX]]
24+
//
25+
// CHECK: [[ACCESS:%.*]] = begin_access [modify] [unknown] [[PROJECT]]
26+
// CHECK: assign {{%.*}} to [[ACCESS]]
27+
// CHECK: end_access [[ACCESS]]
28+
//
29+
// CHECK: [[ACCESS:%.*]] = begin_access [modify] [unknown] [[PROJECT]]
30+
// CHECK: [[STACK:%.*]] = alloc_stack $Klass
31+
// CHECK: mark_unresolved_move_addr [[ACCESS]] to [[STACK]]
32+
// CHECK: [[VALUE:%.*]] = load [take] [[STACK]]
33+
// CHECK: end_access [[ACCESS]]
34+
//
35+
// CHECK: } // end sil function '$s7consume15testLoadableVaryyF'
36+
func testLoadableVar() {
37+
var k = Klass()
38+
k = Klass()
39+
let _ = consume k
40+
}
41+
42+
// CHECK-LABEL: sil hidden [ossa] @$s7consume18testAddressOnlyLetyyxmAA1PRzlF : $@convention(thin) <T where T : P> (@thick T.Type) -> () {
43+
// CHECK: [[BOX:%.*]] = alloc_stack [lexical] $T
44+
//
45+
// CHECK: [[STACK:%.*]] = alloc_stack $T
46+
// CHECK: mark_unresolved_move_addr [[BOX]] to [[STACK]]
47+
//
48+
// CHECK: } // end sil function '$s7consume18testAddressOnlyLetyyxmAA1PRzlF'
49+
func testAddressOnlyLet<T : P>(_ t: T.Type) {
50+
let k = T.value
51+
let _ = consume k
52+
}
53+
54+
// CHECK-LABEL: sil hidden [ossa] @$s7consume18testAddressOnlyVaryyxmAA1PRzlF : $@convention(thin) <T where T : P> (@thick T.Type) -> () {
55+
// CHECK: [[BOX:%.*]] = alloc_box $
56+
// CHECK: [[BORROW_BOX:%.*]] = begin_borrow [lexical] [[BOX]]
57+
// CHECK: [[PROJECT:%.*]] = project_box [[BORROW_BOX]]
58+
//
59+
// CHECK: [[ACCESS:%.*]] = begin_access [modify] [unknown] [[PROJECT]]
60+
// CHECK: copy_addr [take] {{%.*}} to [[ACCESS]]
61+
// CHECK: end_access [[ACCESS]]
62+
//
63+
// CHECK: [[ACCESS:%.*]] = begin_access [modify] [unknown] [[PROJECT]]
64+
// CHECK: [[STACK:%.*]] = alloc_stack $
65+
// CHECK: mark_unresolved_move_addr [[ACCESS]] to [[STACK]]
66+
// CHECK: end_access [[ACCESS]]
67+
//
68+
// CHECK: } // end sil function '$s7consume18testAddressOnlyVaryyxmAA1PRzlF'
69+
func testAddressOnlyVar<T : P>(_ t: T.Type) {
70+
var k = T.value
71+
k = T.value
72+
let _ = consume k
73+
}

‎test/Sema/move_expr.swift

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-typecheck-verify-swift -disable-availability-checking
1+
// RUN: %target-typecheck-verify-swift -disable-availability-checking -enable-experimental-feature NoImplicitCopy
22

33
class Klass {
44
var k: Klass? = nil
@@ -63,3 +63,23 @@ func testVarClassAccessField() {
6363
t = Klass()
6464
let _ = consume t.k // expected-error {{'consume' can only be applied to lvalues}}
6565
}
66+
67+
func testConsumeResultImmutable() {
68+
class Klass {}
69+
70+
struct Test {
71+
var k = Klass()
72+
mutating func mutatingTest() {}
73+
func borrowingTest() {}
74+
consuming func consumingTest() {}
75+
}
76+
77+
var t = Test()
78+
t.mutatingTest()
79+
consume t.borrowingTest() // expected-error {{'consume' can only be applied to lvalues}}
80+
(consume t).borrowingTest()
81+
(consume t).consumingTest()
82+
(consume t).mutatingTest() // expected-error {{cannot use mutating member on immutable value of type 'Test'}}
83+
(consume t) = Test() // expected-error {{cannot assign to immutable expression of type 'Test'}}
84+
consume t = Test() // expected-error {{cannot assign to immutable expression of type 'Test'}}
85+
}

0 commit comments

Comments
 (0)