Skip to content

Commit a14f36f

Browse files
committed
[move-only] Ported rest of no implicit copy tests to move only and fixed resulting issues.
This is just to get code coverage. A few interesting things I noticed: 1. I am seeing a false diagnostic on this pattern: let x = ... var x2 = x print(x2) // I get that print(x2) is a consuming use of x. I think it is predictable mem opts maybe forwarding in an interesting way. I am not sure. It doesn't happen with no implicit copy types since x2 would not be no implicit copy so we would leave the no implicit copy monad. 2. I think that SILGen creates a real lexical scope for temporary variables meaning that compiler generated code would actually hit the move checker. I had to add an extra code pattern to handle this.
1 parent 3c1e8bb commit a14f36f

File tree

4 files changed

+2371
-134
lines changed

4 files changed

+2371
-134
lines changed

lib/SILGen/SILGenDecl.cpp

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -584,29 +584,36 @@ class LetValueInitialization : public Initialization {
584584
// that acts as a consuming use of the value. The reason why we want this is
585585
// even if we are only performing a borrow for our lexical lifetime, we want
586586
// to ensure that our defs see this initialization as consuming this value.
587-
if (value->getOwnershipKind() == OwnershipKind::Owned) {
587+
if (value->getOwnershipKind() == OwnershipKind::Owned &&
588+
value->getType().isMoveOnlyWrapped()) {
588589
assert(wasPlusOne);
589590
// NOTE: If our type is trivial when not wrapped in a
590591
// SILMoveOnlyWrappedType, this will return a trivial value. We rely
591592
// on the checker to determine if this is an acceptable use of the
592593
// value.
593-
if (value->getType().isMoveOnly()) {
594-
if (value->getType().isMoveOnlyWrapped()) {
595-
value = SGF.B.createOwnedMoveOnlyWrapperToCopyableValue(PrologueLoc,
596-
value);
597-
} else {
598-
// Change this to be lexical and get rid of borrow scope?
599-
value = SGF.B.createMoveValue(PrologueLoc, value);
600-
}
601-
}
594+
value =
595+
SGF.B.createOwnedMoveOnlyWrapperToCopyableValue(PrologueLoc, value);
602596
}
603597

604598
// If we still have a trivial thing, just return that.
605599
if (value->getType().isTrivial(SGF.F))
606600
return value;
607601

602+
// Check if we have a move only type. In that case, we perform a lexical
603+
// move and insert a mark_must_check.
604+
//
605+
// We do this before the begin_borrow "normal" path below since move only
606+
// types do not have no implicit copy attr on them.
607+
if (value->getOwnershipKind() == OwnershipKind::Owned &&
608+
value->getType().isMoveOnly() &&
609+
!value->getType().isMoveOnlyWrapped()) {
610+
value = SGF.B.createMoveValue(PrologueLoc, value, true /*isLexical*/);
611+
return SGF.B.createMarkMustCheckInst(
612+
PrologueLoc, value, MarkMustCheckInst::CheckKind::NoImplicitCopy);
613+
}
614+
608615
// Otherwise, if we do not have a no implicit copy variable, just do a
609-
// borrow lexical.
616+
// borrow lexical. This is the "normal path".
610617
if (!vd->getAttrs().hasAttribute<NoImplicitCopyAttr>())
611618
return SGF.B.createBeginBorrow(PrologueLoc, value, /*isLexical*/ true);
612619

lib/SILOptimizer/Mandatory/MoveOnlyChecker.cpp

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -117,16 +117,26 @@ bool MoveOnlyChecker::searchForCandidateMarkMustChecks() {
117117
if (!mmci || !mmci->hasMoveCheckerKind())
118118
continue;
119119

120-
// Handle guaranteed/owned move only typed arguments.
120+
// Handle guaranteed/owned move arguments and values.
121121
//
122122
// We are pattern matching against these patterns:
123123
//
124124
// bb0(%0 : @guaranteed $T):
125125
// %1 = copy_value %0
126126
// %2 = mark_must_check [no_copy] %1
127127
// bb0(%0 : @owned $T):
128-
// %1 = copy_value %0
129-
// %2 = mark_must_check [no_copy] %1
128+
// %1 = mark_must_check [no_copy] %2
129+
//
130+
// This is forming a let or an argument.
131+
// bb0:
132+
// %1 = move_value [lexical] %0
133+
// %2 = mark_must_check [no_implicit_copy] %1
134+
//
135+
// This occurs when SILGen materializes a temporary move only value?
136+
// bb0:
137+
// %1 = begin_borrow [lexical] %0
138+
// %2 = copy_value %1
139+
// %3 = mark_must_check [no_copy] %2
130140
if (mmci->getOperand()->getType().isMoveOnly() &&
131141
!mmci->getOperand()->getType().isMoveOnlyWrapped()) {
132142
if (auto *cvi = dyn_cast<CopyValueInst>(mmci->getOperand())) {
@@ -136,16 +146,20 @@ bool MoveOnlyChecker::searchForCandidateMarkMustChecks() {
136146
continue;
137147
}
138148
}
149+
150+
if (auto *bbi = dyn_cast<BeginBorrowInst>(cvi->getOperand())) {
151+
if (bbi->isLexical()) {
152+
moveIntroducersToProcess.insert(mmci);
153+
continue;
154+
}
155+
}
139156
}
140157

158+
// Any time we have a lexical move_value, we can process it.
141159
if (auto *mvi = dyn_cast<MoveValueInst>(mmci->getOperand())) {
142160
if (mvi->isLexical()) {
143-
if (auto *arg = dyn_cast<SILFunctionArgument>(mvi->getOperand())) {
144-
if (arg->getOwnershipKind() == OwnershipKind::Owned) {
145-
moveIntroducersToProcess.insert(mmci);
146-
continue;
147-
}
148-
}
161+
moveIntroducersToProcess.insert(mmci);
162+
continue;
149163
}
150164
}
151165

@@ -247,6 +261,8 @@ bool MoveOnlyChecker::searchForCandidateMarkMustChecks() {
247261
// %2 = copy_value %1
248262
// %3 = copyable_to_moveonlywrapper [owned] %2
249263
// %4 = mark_must_check [no_implicit_copy]
264+
//
265+
// Or for a move only type, we look for a move_value [lexical].
250266
if (auto *mvi = dyn_cast<CopyableToMoveOnlyWrapperValueInst>(
251267
mmci->getOperand())) {
252268
if (auto *cvi = dyn_cast<CopyValueInst>(mvi->getOperand())) {

test/SILGen/moveonly_diagnostics.swift

Lines changed: 0 additions & 114 deletions
This file was deleted.

0 commit comments

Comments
 (0)