Skip to content

Commit d909c89

Browse files
committed
[no-implicit-copy] When accessing a field from a no-implicit-copy struct, unwrap it. Also do not run the move only borrow to destructure transform error.
The reason to do the first is to ensure that when I enable -sil-verify-all, we do not have errors due to a copy_value of a move only type. The reason to do the second thing is that: 1. If we have a move only type that is no implicit copy, I am in a subsequent commit going to emit a type checker error saying that one cannot do this. When we implement consuming/borrowing bindings/etc, we can make this looser if it gets into the way. 2. If we have a copyable no implicit copy type, then any structural accesses that we may want to do that would require a destructure must be to a copyable type which is ok to copy as long as we do the unwrap from the first thing. rdar://104929957
1 parent e6ac700 commit d909c89

File tree

6 files changed

+95
-296
lines changed

6 files changed

+95
-296
lines changed

lib/SILGen/SILGenLValue.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "ExecutorBreadcrumb.h"
2222
#include "Initialization.h"
2323
#include "LValue.h"
24+
#include "ManagedValue.h"
2425
#include "RValue.h"
2526
#include "SILGen.h"
2627
#include "SILGenFunction.h"
@@ -4606,6 +4607,19 @@ RValue SILGenFunction::emitLoadOfLValue(SILLocation loc, LValue &&src,
46064607
substFormalType, rvalueTL, C, IsNotTake, isBaseGuaranteed);
46074608
} else if (isReadAccessResultOwned(src.getAccessKind()) &&
46084609
!projection.isPlusOne(*this)) {
4610+
4611+
// Before we copy, if we have a move only wrapped value, unwrap the
4612+
// value using a guaranteed moveonlywrapper_to_copyable.
4613+
if (projection.getType().isMoveOnlyWrapped()) {
4614+
// We are assuming we always get a guaranteed value here.
4615+
assert(projection.getValue()->getOwnershipKind() ==
4616+
OwnershipKind::Guaranteed);
4617+
// We use SILValues here to ensure we get a tight scope around our
4618+
// copy.
4619+
projection =
4620+
B.createGuaranteedMoveOnlyWrapperToCopyableValue(loc, projection);
4621+
}
4622+
46094623
projection = projection.copy(*this, loc);
46104624
}
46114625

lib/SILOptimizer/Mandatory/MoveOnlyBorrowToDestructureTransform.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,20 @@ void BorrowToDestructureTransform::checkDestructureUsesOnBoundary() const {
345345

346346
bool BorrowToDestructureTransform::gatherBorrows(
347347
MarkMustCheckInst *mmci, StackList<BeginBorrowInst *> &borrowWorklist) {
348+
// If we have a no implicit copy mark_must_check, we do not run the borrow to
349+
// destructure transform since:
350+
//
351+
// 1. If we have a move only type, we should have emitted an earlier error
352+
// saying that move only types should not be marked as no implicit copy.
353+
//
354+
// 2. If we do not have a move only type, then we know that all fields that we
355+
// access directly and would cause a need to destructure must be copyable,
356+
// so no transformation/error is needed.
357+
if (mmci->getType().isMoveOnlyWrapped()) {
358+
LLVM_DEBUG(llvm::dbgs() << "Skipping move only wrapped inst: " << *mmci);
359+
return true;
360+
}
361+
348362
LLVM_DEBUG(llvm::dbgs() << "Searching for borrows for inst: " << *mmci);
349363

350364
StackList<Operand *> worklist(mmci->getFunction());

test/SILGen/noimplicitcopy.swift

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,15 @@ class Klass {
2222
func increment() { i += 1 }
2323
}
2424

25+
struct Trivial {
26+
var value = 5
27+
}
28+
29+
struct NonTrivial {
30+
var value = Klass()
31+
}
32+
33+
2534
///////////
2635
// Tests //
2736
///////////
@@ -461,3 +470,24 @@ func callPrintKlassOwnedOwnedArgThrows() throws {
461470
y = Klass()
462471
try printKlassOwnedArgThrows(y)
463472
}
473+
474+
//////////////
475+
// Gep Test //
476+
//////////////
477+
478+
// CHECK-LABEL: sil hidden [ossa] @$s14noimplicitcopy19test_nontrivial_gepyyAA7TrivialVF : $@convention(thin) (Trivial) -> () {
479+
// CHECK: bb0([[ARG:%.*]] : $Trivial):
480+
// CHECK: [[WRAPPED:%.*]] = copyable_to_moveonlywrapper [owned] [[ARG]]
481+
// CHECK: [[MV_WRAPPED:%.*]] = move_value [lexical] [[WRAPPED]]
482+
// CHECK: [[MARKED:%.*]] = mark_must_check [no_implicit_copy] [[MV_WRAPPED]]
483+
// CHECK: [[BORROW:%.*]] = begin_borrow [[MARKED]]
484+
// CHECK: [[EXT:%.*]] = struct_extract [[BORROW]]
485+
// CHECK: [[UNWRAPPED:%.*]] = moveonlywrapper_to_copyable [guaranteed] [[EXT]]
486+
// CHECK: apply {{%.*}}([[UNWRAPPED]])
487+
// CHECK: end_borrow [[BORROW]]
488+
// CHECK: destroy_value [[MARKED]]
489+
// CHECK: } // end sil function '$s14noimplicitcopy19test_nontrivial_gepyyAA7TrivialVF'
490+
func test_nontrivial_gep(_ x: Trivial) {
491+
@_noImplicitCopy let y = x
492+
print2(y.value)
493+
}

test/SILOptimizer/noimplicitcopy.swift

Lines changed: 25 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -866,26 +866,23 @@ public func aggStructAccessFieldOwnedArg(@_noImplicitCopy _ x2: __owned AggStruc
866866

867867
public func aggStructConsumeField(_ x: AggStruct) {
868868
@_noImplicitCopy let x2 = x
869-
// expected-error @-1 {{'x2' has a move only field that was consumed before later uses}}
870869
classConsume(x2.lhs)
871870
for _ in 0..<1024 {
872-
classConsume(x2.lhs) // expected-note {{consuming use here}}
871+
classConsume(x2.lhs)
873872
}
874873
}
875874

876875
public func aggStructConsumeFieldArg(@_noImplicitCopy _ x2: AggStruct) {
877-
// expected-error @-1 {{'x2' has a move only field that was consumed before later uses}}
878876
classConsume(x2.lhs)
879877
for _ in 0..<1024 {
880-
classConsume(x2.lhs) // expected-note {{consuming use here}}
878+
classConsume(x2.lhs)
881879
}
882880
}
883881

884882
public func aggStructConsumeFieldOwnedArg(@_noImplicitCopy _ x2: __owned AggStruct) {
885-
// expected-error @-1 {{'x2' has a move only field that was consumed before later uses}}
886883
classConsume(x2.lhs)
887884
for _ in 0..<1024 {
888-
classConsume(x2.lhs) // expected-note {{consuming use here}}
885+
classConsume(x2.lhs)
889886
}
890887
}
891888

@@ -913,26 +910,25 @@ public func aggStructAccessGrandFieldOwnedArg(@_noImplicitCopy _ x2: __owned Agg
913910

914911
public func aggStructConsumeGrandField(_ x: AggStruct) {
915912
@_noImplicitCopy let x2 = x
916-
// expected-error @-1 {{'x2' has a move only field that was consumed before later uses}}
913+
917914
classConsume(x2.pair.lhs)
918915
for _ in 0..<1024 {
919-
classConsume(x2.pair.lhs) // expected-note {{consuming use here}}
916+
classConsume(x2.pair.lhs)
920917
}
921918
}
922919

923920
public func aggStructConsumeGrandFieldArg(@_noImplicitCopy _ x2: AggStruct) {
924-
// expected-error @-1 {{'x2' has a move only field that was consumed before later uses}}
925921
classConsume(x2.pair.lhs)
926922
for _ in 0..<1024 {
927-
classConsume(x2.pair.lhs) // expected-note {{consuming use here}}
923+
classConsume(x2.pair.lhs)
928924
}
929925
}
930926

931927
public func aggStructConsumeGrandFieldOwnedArg(@_noImplicitCopy _ x2: __owned AggStruct) {
932-
// expected-error @-1 {{'x2' has a move only field that was consumed before later uses}}
928+
933929
classConsume(x2.pair.lhs)
934930
for _ in 0..<1024 {
935-
classConsume(x2.pair.lhs) // expected-note {{consuming use here}}
931+
classConsume(x2.pair.lhs)
936932
}
937933
}
938934

@@ -1136,26 +1132,24 @@ public func aggGenericStructAccessFieldOwnedArg(@_noImplicitCopy _ x2: __owned A
11361132

11371133
public func aggGenericStructConsumeField(_ x: AggGenericStruct<Klass>) {
11381134
@_noImplicitCopy let x2 = x
1139-
// expected-error @-1 {{'x2' has a move only field that was consumed before later uses}}
1135+
11401136
classConsume(x2.lhs)
11411137
for _ in 0..<1024 {
1142-
classConsume(x2.lhs) // expected-note {{consuming use here}}
1138+
classConsume(x2.lhs)
11431139
}
11441140
}
11451141

11461142
public func aggGenericStructConsumeFieldArg(@_noImplicitCopy _ x2: AggGenericStruct<Klass>) {
1147-
// expected-error @-1 {{'x2' has a move only field that was consumed before later uses}}
11481143
classConsume(x2.lhs)
11491144
for _ in 0..<1024 {
1150-
classConsume(x2.lhs) // expected-note {{consuming use here}}
1145+
classConsume(x2.lhs)
11511146
}
11521147
}
11531148

11541149
public func aggGenericStructConsumeFieldOwnedArg(@_noImplicitCopy _ x2: __owned AggGenericStruct<Klass>) {
1155-
// expected-error @-1 {{'x2' has a move only field that was consumed before later uses}}
11561150
classConsume(x2.lhs)
11571151
for _ in 0..<1024 {
1158-
classConsume(x2.lhs) // expected-note {{consuming use here}}
1152+
classConsume(x2.lhs)
11591153
}
11601154
}
11611155

@@ -1183,26 +1177,24 @@ public func aggGenericStructAccessGrandFieldOwnedArg(@_noImplicitCopy _ x2: __ow
11831177

11841178
public func aggGenericStructConsumeGrandField(_ x: AggGenericStruct<Klass>) {
11851179
@_noImplicitCopy let x2 = x
1186-
// expected-error @-1 {{'x2' has a move only field that was consumed before later uses}}
1180+
11871181
classConsume(x2.pair.lhs)
11881182
for _ in 0..<1024 {
1189-
classConsume(x2.pair.lhs) // expected-note {{consuming use here}}
1183+
classConsume(x2.pair.lhs)
11901184
}
11911185
}
11921186

11931187
public func aggGenericStructConsumeGrandFieldArg(@_noImplicitCopy _ x2: AggGenericStruct<Klass>) {
1194-
// expected-error @-1 {{'x2' has a move only field that was consumed before later uses}}
11951188
classConsume(x2.pair.lhs)
11961189
for _ in 0..<1024 {
1197-
classConsume(x2.pair.lhs) // expected-note {{consuming use here}}
1190+
classConsume(x2.pair.lhs)
11981191
}
11991192
}
12001193

12011194
public func aggGenericStructConsumeGrandFieldOwnedArg(@_noImplicitCopy _ x2: __owned AggGenericStruct<Klass>) {
1202-
// expected-error @-1 {{'x2' has a move only field that was consumed before later uses}}
12031195
classConsume(x2.pair.lhs)
12041196
for _ in 0..<1024 {
1205-
classConsume(x2.pair.lhs) // expected-note {{consuming use here}}
1197+
classConsume(x2.pair.lhs)
12061198
}
12071199
}
12081200

@@ -1400,26 +1392,24 @@ public func aggGenericStructAccessFieldOwnedArg<T>(@_noImplicitCopy _ x2: __owne
14001392

14011393
public func aggGenericStructConsumeField<T>(_ x: AggGenericStruct<T>) {
14021394
@_noImplicitCopy let x2 = x
1403-
// expected-error @-1 {{'x2' has a move only field that was consumed before later uses}}
1395+
14041396
classConsume(x2.lhs)
14051397
for _ in 0..<1024 {
1406-
classConsume(x2.lhs) // expected-note {{consuming use here}}
1398+
classConsume(x2.lhs)
14071399
}
14081400
}
14091401

14101402
public func aggGenericStructConsumeFieldArg<T>(@_noImplicitCopy _ x2: AggGenericStruct<T>) {
1411-
// expected-error @-1 {{'x2' has a move only field that was consumed before later uses}}
14121403
classConsume(x2.lhs)
14131404
for _ in 0..<1024 {
1414-
classConsume(x2.lhs) // expected-note {{consuming use here}}
1405+
classConsume(x2.lhs)
14151406
}
14161407
}
14171408

14181409
public func aggGenericStructConsumeFieldOwnedArg<T>(@_noImplicitCopy _ x2: __owned AggGenericStruct<T>) {
1419-
// expected-error @-1 {{'x2' has a move only field that was consumed before later uses}}
14201410
classConsume(x2.lhs)
14211411
for _ in 0..<1024 {
1422-
classConsume(x2.lhs) // expected-note {{consuming use here}}
1412+
classConsume(x2.lhs)
14231413
}
14241414
}
14251415

@@ -1447,26 +1437,25 @@ public func aggGenericStructAccessGrandFieldOwnedArg<T>(@_noImplicitCopy _ x2: _
14471437

14481438
public func aggGenericStructConsumeGrandField<T>(_ x: AggGenericStruct<T>) {
14491439
@_noImplicitCopy let x2 = x
1450-
// expected-error @-1 {{'x2' has a move only field that was consumed before later uses}}
1440+
14511441
classConsume(x2.pair.lhs)
14521442
for _ in 0..<1024 {
1453-
classConsume(x2.pair.lhs) // expected-note {{consuming use here}}
1443+
classConsume(x2.pair.lhs)
14541444
}
14551445
}
14561446

14571447
public func aggGenericStructConsumeGrandFieldArg<T>(@_noImplicitCopy _ x2: AggGenericStruct<T>) {
1458-
// expected-error @-1 {{'x2' has a move only field that was consumed before later uses}}
1448+
14591449
classConsume(x2.pair.lhs)
14601450
for _ in 0..<1024 {
1461-
classConsume(x2.pair.lhs) // expected-note {{consuming use here}}
1451+
classConsume(x2.pair.lhs)
14621452
}
14631453
}
14641454

14651455
public func aggGenericStructConsumeGrandFieldOwnedArg<T>(@_noImplicitCopy _ x2: __owned AggGenericStruct<T>) {
1466-
// expected-error @-1 {{'x2' has a move only field that was consumed before later uses}}
14671456
classConsume(x2.pair.lhs)
14681457
for _ in 0..<1024 {
1469-
classConsume(x2.pair.lhs) // expected-note {{consuming use here}}
1458+
classConsume(x2.pair.lhs)
14701459
}
14711460
}
14721461

0 commit comments

Comments
 (0)