Skip to content

Commit cb23187

Browse files
committed
SILGen: Emit move-only bases for lvalues in a borrow scope.
Normally, if we project from a mutable class ivar or global variable, we'll load a copy in a tight access scope and then project from the copy, in order to minimize the potential for exclusivity violations while working with classes and copyable values. However, this is undesirable when a value is move-only, since copying is not allowed; borrowing the value in place is the expected and only possible behavior. rdar://105794506
1 parent 159385e commit cb23187

File tree

5 files changed

+108
-34
lines changed

5 files changed

+108
-34
lines changed

lib/SILGen/LValue.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ class PathComponent {
105105
CoroutineAccessorKind, // coroutine accessor
106106
ValueKind, // random base pointer as an lvalue
107107
PhysicalKeyPathApplicationKind, // applying a key path
108+
BorrowValueKind, // load_borrow the base rvalue for a projection
108109

109110
// Logical LValue kinds
110111
GetterSetterKind, // property or subscript getter/setter
@@ -114,6 +115,7 @@ class PathComponent {
114115
WritebackPseudoKind, // a fake component to customize writeback
115116
OpenNonOpaqueExistentialKind, // opened class or metatype existential
116117
LogicalKeyPathApplicationKind, // applying a key path
118+
117119
// Translation LValue kinds (a subtype of logical)
118120
OrigToSubstKind, // generic type substitution
119121
SubstToOrigKind, // generic type substitution

lib/SILGen/SILGenLValue.cpp

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1148,6 +1148,28 @@ namespace {
11481148
Value.dump(OS, indent + 2);
11491149
}
11501150
};
1151+
1152+
class BorrowValueComponent : public PhysicalPathComponent {
1153+
public:
1154+
BorrowValueComponent(LValueTypeData typeData)
1155+
: PhysicalPathComponent(typeData, BorrowValueKind, None) {}
1156+
1157+
virtual bool isLoadingPure() const override { return true; }
1158+
1159+
ManagedValue project(SILGenFunction &SGF, SILLocation loc,
1160+
ManagedValue base) && override {
1161+
assert(base
1162+
&& base.getType().isAddress()
1163+
&& "should have an address base to borrow from");
1164+
auto result = SGF.B.createLoadBorrow(loc, base.getValue());
1165+
return SGF.emitFormalEvaluationManagedBorrowedRValueWithCleanup(loc,
1166+
base.getValue(), result);
1167+
}
1168+
1169+
void dump(raw_ostream &OS, unsigned indent) const override {
1170+
OS.indent(indent) << "BorrowValueComponent\n";
1171+
}
1172+
};
11511173
} // end anonymous namespace
11521174

11531175
static bool isReadNoneFunction(const Expr *e) {
@@ -2760,7 +2782,7 @@ static ManagedValue visitRecNonInOutBase(SILGenLValue &SGL, Expr *e,
27602782
}
27612783
}
27622784
}
2763-
2785+
27642786
if (SGF.SGM.Types.isIndirectPlusZeroSelfParameter(e->getType())) {
27652787
ctx = SGFContext::AllowGuaranteedPlusZero;
27662788
}
@@ -2788,6 +2810,22 @@ LValue SILGenLValue::visitRec(Expr *e, SGFAccessKind accessKind,
27882810
if (e->getType()->is<LValueType>() || e->isSemanticallyInOutExpr()) {
27892811
return visitRecInOut(*this, e, accessKind, options, orig);
27902812
}
2813+
2814+
// If the base is a load of a noncopyable type (or, eventually, when we have
2815+
// a `borrow x` operator, the operator is used on the base here), we want to
2816+
// apply the lvalue within a formal access to the original value instead of
2817+
// an actual loaded copy.
2818+
if (e->getType()->isPureMoveOnly()) {
2819+
if (auto load = dyn_cast<LoadExpr>(e)) {
2820+
LValue lv = visitRec(load->getSubExpr(), SGFAccessKind::BorrowedAddressRead,
2821+
options, orig);
2822+
CanType formalType = getSubstFormalRValueType(e);
2823+
LValueTypeData typeData{accessKind, AbstractionPattern(formalType),
2824+
formalType, lv.getTypeOfRValue().getASTType()};
2825+
lv.add<BorrowValueComponent>(typeData);
2826+
return lv;
2827+
}
2828+
}
27912829

27922830
// Otherwise we have a non-lvalue type (references, values, metatypes,
27932831
// etc). These act as the root of a logical lvalue. Compute the root value,
@@ -3493,7 +3531,7 @@ LValue SILGenLValue::visitMemberRefExpr(MemberRefExpr *e,
34933531
SGF.F.getResilienceExpansion());
34943532
}
34953533
}
3496-
3534+
34973535
LValue lv = visitRec(e->getBase(),
34983536
getBaseAccessKind(SGF.SGM, var, accessKind, strategy,
34993537
getBaseFormalType(e->getBase())),
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// RUN: %target-run-simple-swift
2+
// REQUIRES: executable_test
3+
@_moveOnly
4+
struct FileDescriptor {
5+
let desc: Int
6+
7+
var empty: Bool { return desc == Int.min }
8+
func isEmpty() -> Bool { return empty }
9+
}
10+
11+
final class Wrapper {
12+
var val: FileDescriptor = FileDescriptor(desc: 0)
13+
14+
func isEmpty_bug() -> Bool {
15+
// error: 'self.val' has consuming use that cannot
16+
// be eliminated due to a tight exclusivity scope
17+
return val.empty // note: consuming use here
18+
}
19+
20+
func isEmpty_ok() -> Bool {
21+
return val.isEmpty()
22+
}
23+
}
24+
25+
let w = Wrapper()
26+
// CHECK: is not empty
27+
print(w.isEmpty_bug() ? "is empty" : "is not empty")
28+
w.val = FileDescriptor(desc: Int.min)
29+
// CHECK: is empty
30+
print(w.isEmpty_bug() ? "is empty" : "is not empty")

test/SILOptimizer/moveonly_addresschecker_diagnostics.swift

Lines changed: 15 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -377,47 +377,38 @@ public func classAssignToVar5Arg2(_ x: __shared Klass, _ x2: inout Klass) { // e
377377

378378
public func classAccessAccessField(_ x: __shared Klass) { // expected-error {{'x' has guaranteed ownership but was consumed}}
379379
var x2 = x // expected-note {{consuming use here}}
380-
// expected-error @-1 {{'x2' has consuming use that cannot be eliminated due to a tight exclusivity scope}}
381-
// expected-error @-2 {{'x2' has consuming use that cannot be eliminated due to a tight exclusivity scope}}
382380
x2 = Klass()
383-
borrowVal(x2.k) // expected-note {{consuming use here}}
381+
borrowVal(x2.k)
384382
for _ in 0..<1024 {
385-
borrowVal(x2.k) // expected-note {{consuming use here}}
383+
borrowVal(x2.k)
386384
}
387385
}
388386

389387
public func classAccessAccessFieldArg(_ x2: inout Klass) {
390-
// expected-error @-1 {{'x2' has consuming use that cannot be eliminated due to a tight exclusivity scope}}
391-
// expected-error @-2 {{'x2' has consuming use that cannot be eliminated due to a tight exclusivity scope}}
392-
borrowVal(x2.k) // expected-note {{consuming use here}}
393388
for _ in 0..<1024 {
394-
borrowVal(x2.k) // expected-note {{consuming use here}}
389+
borrowVal(x2.k)
395390
}
396391
}
397392

398393
public func classAccessConsumeField(_ x: __shared Klass) { // expected-error {{'x' has guaranteed ownership but was consumed}}
399394
var x2 = x // expected-note {{consuming use here}}
400-
// expected-error @-1 {{'x2' has consuming use that cannot be eliminated due to a tight exclusivity scope}}
401-
// expected-error @-2 {{'x2' has consuming use that cannot be eliminated due to a tight exclusivity scope}}
402395
x2 = Klass()
403396
// Since a class is a reference type, we do not emit an error here.
404-
consumeVal(x2.k) // expected-note {{consuming use here}}
397+
consumeVal(x2.k)
405398
// expected-error @-1 {{'x2.k' was consumed but it is illegal to consume a noncopyable class var field. One can only read from it or assign to it}}
406399
for _ in 0..<1024 {
407-
consumeVal(x2.k) // expected-note {{consuming use here}}
400+
consumeVal(x2.k)
408401
// expected-error @-1 {{'x2.k' was consumed but it is illegal to consume a noncopyable class var field. One can only read from it or assign to it}}
409402
}
410403
}
411404

412405
public func classAccessConsumeFieldArg(_ x2: inout Klass) {
413-
// expected-error @-1 {{'x2' has consuming use that cannot be eliminated due to a tight exclusivity scope}}
414-
// expected-error @-2 {{'x2' has consuming use that cannot be eliminated due to a tight exclusivity scope}}
415406
// Since a class is a reference type, we do not emit an error here.
416-
consumeVal(x2.k) // expected-note {{consuming use here}}
407+
consumeVal(x2.k)
417408
// expected-error @-1 {{'x2.k' was consumed but it is illegal to consume a noncopyable class var field. One can only read from it or assign to it}}
418409

419410
for _ in 0..<1024 {
420-
consumeVal(x2.k) // expected-note {{consuming use here}}
411+
consumeVal(x2.k)
421412
// expected-error @-1 {{'x2.k' was consumed but it is illegal to consume a noncopyable class var field. One can only read from it or assign to it}}
422413
}
423414
}
@@ -682,45 +673,37 @@ public func finalClassAssignToVar5Arg2(_ x2: inout FinalKlass) {
682673

683674
public func finalClassAccessField() {
684675
var x2 = FinalKlass()
685-
// expected-error @-1 {{'x2' has consuming use that cannot be eliminated due to a tight exclusivity scope}}
686-
// expected-error @-2 {{'x2' has consuming use that cannot be eliminated due to a tight exclusivity scope}}
687676
x2 = FinalKlass()
688-
borrowVal(x2.k) // expected-note {{consuming use here}}
677+
borrowVal(x2.k)
689678
for _ in 0..<1024 {
690-
borrowVal(x2.k) // expected-note {{consuming use here}}
679+
borrowVal(x2.k)
691680
}
692681
}
693682

694683
public func finalClassAccessFieldArg(_ x2: inout FinalKlass) {
695-
// expected-error @-1 {{'x2' has consuming use that cannot be eliminated due to a tight exclusivity scope}}
696-
// expected-error @-2 {{'x2' has consuming use that cannot be eliminated due to a tight exclusivity scope}}
697-
borrowVal(x2.k) // expected-note {{consuming use here}}
684+
borrowVal(x2.k)
698685
for _ in 0..<1024 {
699-
borrowVal(x2.k) // expected-note {{consuming use here}}
686+
borrowVal(x2.k)
700687
}
701688
}
702689

703690
public func finalClassConsumeField() {
704691
var x2 = FinalKlass()
705-
// expected-error @-1 {{'x2' has consuming use that cannot be eliminated due to a tight exclusivity scope}}
706-
// expected-error @-2 {{'x2' has consuming use that cannot be eliminated due to a tight exclusivity scope}}
707692
x2 = FinalKlass()
708693

709-
consumeVal(x2.k) // expected-note {{consuming use here}}
694+
consumeVal(x2.k)
710695
// expected-error @-1 {{'x2.k' was consumed but it is illegal to consume a noncopyable class var field. One can only read from it or assign to it}}
711696
for _ in 0..<1024 {
712-
consumeVal(x2.k) // expected-note {{consuming use here}}
697+
consumeVal(x2.k)
713698
// expected-error @-1 {{'x2.k' was consumed but it is illegal to consume a noncopyable class var field. One can only read from it or assign to it}}
714699
}
715700
}
716701

717702
public func finalClassConsumeFieldArg(_ x2: inout FinalKlass) {
718-
// expected-error @-1 {{'x2' has consuming use that cannot be eliminated due to a tight exclusivity scope}}
719-
// expected-error @-2 {{'x2' has consuming use that cannot be eliminated due to a tight exclusivity scope}}
720-
consumeVal(x2.k) // expected-note {{consuming use here}}
703+
consumeVal(x2.k)
721704
// expected-error @-1 {{'x2.k' was consumed but it is illegal to consume a noncopyable class var field. One can only read from it or assign to it}}
722705
for _ in 0..<1024 {
723-
consumeVal(x2.k) // expected-note {{consuming use here}}
706+
consumeVal(x2.k)
724707
// expected-error @-1 {{'x2.k' was consumed but it is illegal to consume a noncopyable class var field. One can only read from it or assign to it}}
725708
}
726709
}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// RUN: %target-swift-frontend -emit-sil -verify %s
2+
3+
// Applying a computed property to a move-only field in a class should occur
4+
// entirely within a formal access to the class property. rdar://105794506
5+
6+
@_moveOnly
7+
struct FileDescriptor {
8+
private let desc: Int
9+
10+
var empty: Bool { return desc == Int.min }
11+
}
12+
13+
final class Wrapper {
14+
private var val: FileDescriptor
15+
16+
func isEmpty_bug() -> Bool {
17+
return val.empty
18+
}
19+
20+
init() { fatalError() }
21+
}

0 commit comments

Comments
 (0)