Skip to content

Commit 12e7b83

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 (cherry picked from commit 67fcb1c)
1 parent 9168605 commit 12e7b83

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
@@ -6120,27 +6120,47 @@ RValue RValueEmitter::visitErrorExpr(ErrorExpr *E, SGFContext C) {
61206120
}
61216121

61226122
RValue RValueEmitter::visitConsumeExpr(ConsumeExpr *E, SGFContext C) {
6123-
auto *subExpr = cast<DeclRefExpr>(E->getSubExpr());
6123+
auto *subExpr = E->getSubExpr();
61246124
auto subASTType = subExpr->getType()->getCanonicalType();
6125-
61266125
auto subType = SGF.getLoweredType(subASTType);
61276126

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

61386162
// If we aren't loadable, then create a temporary initialization and
6139-
// mark_unresolved_move into that if we have a copyable type if we have a move
6140-
// only, just add a copy_addr init.
6141-
//
6142-
// The reason why we do this is that we only use mark_unresolved_move_addr and
6143-
// the move operator checker for copyable values.
6163+
// explicit_copy_addr into that.
61446164
std::unique_ptr<TemporaryInitialization> optTemp;
61456165
optTemp = SGF.emitTemporary(E, SGF.getTypeLowering(subType));
61466166
SILValue toAddr = optTemp->getAddressForInPlaceInitialization(SGF, E);
@@ -6167,7 +6187,6 @@ RValue RValueEmitter::visitCopyExpr(CopyExpr *E, SGFContext C) {
61676187
auto subType = SGF.getLoweredType(subASTType);
61686188

61696189
if (auto *li = dyn_cast<LoadExpr>(subExpr)) {
6170-
61716190
FormalEvaluationScope writeback(SGF);
61726191
LValue lv =
61736192
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)