Skip to content

Commit 140968c

Browse files
committed
[move-only] Teach the object move operator checker to not check move only types.
move only types will already be checked by the move only checker and said checker will still see move_value as a consume so we are just eliminating an unnecessary double diagnostic. rdar://102056097
1 parent 938036b commit 140968c

File tree

3 files changed

+41
-9
lines changed

3 files changed

+41
-9
lines changed

lib/SILOptimizer/Mandatory/MoveKillsCopyableValuesChecker.cpp

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,8 @@ bool MoveKillsCopyableValuesChecker::check() {
400400
SmallSetVector<SILValue, 32> valuesToCheck;
401401

402402
for (auto *arg : fn->getEntryBlock()->getSILFunctionArguments()) {
403-
if (arg->getOwnershipKind() == OwnershipKind::Owned) {
403+
if (arg->getOwnershipKind() == OwnershipKind::Owned &&
404+
!arg->getType().isMoveOnly()) {
404405
LLVM_DEBUG(llvm::dbgs() << "Found owned arg to check: " << *arg);
405406
valuesToCheck.insert(arg);
406407
}
@@ -409,7 +410,7 @@ bool MoveKillsCopyableValuesChecker::check() {
409410
for (auto &block : *fn) {
410411
for (auto &ii : block) {
411412
if (auto *bbi = dyn_cast<BeginBorrowInst>(&ii)) {
412-
if (bbi->isLexical()) {
413+
if (bbi->isLexical() && !bbi->getType().isMoveOnly()) {
413414
LLVM_DEBUG(llvm::dbgs()
414415
<< "Found lexical lifetime to check: " << *bbi);
415416
valuesToCheck.insert(bbi);
@@ -551,19 +552,28 @@ class MoveKillsCopyableValuesCheckerPass : public SILFunctionTransform {
551552
SILAnalysis::InvalidationKind::BranchesAndInstructions);
552553
}
553554

554-
// Now search through our function one last time and any move_value
555-
// [allows_diagnostics] that remain are ones that we did not know how to
556-
// check so emit a diagnostic so the user doesn't assume that they have
557-
// guarantees.
555+
// Now search through our function one last time and:
556+
//
557+
// 1. Given any move_value on a move only type, just unset the allows
558+
// diagnostics flag. The move checker will have emitted any errors caused
559+
// by our move [allows_diagnostic] earlier in the compilation pipeline.
560+
//
561+
// 2. Any move_value [allows_diagnostics] that remain that are not on a move
562+
// only type are ones that we did not know how to check so emit a
563+
// diagnostic so the user doesn't assume that they have guarantees.
558564
//
559565
// TODO: Emit specific diagnostics here (e.x.: _move of global).
560-
if (DisableUnhandledMoveDiagnostic)
561-
return;
562566
for (auto &block : *fn) {
563567
for (auto &inst : block) {
564568
if (auto *mvi = dyn_cast<MoveValueInst>(&inst)) {
565569
if (mvi->getAllowDiagnostics()) {
566-
emitUnsupportedUseCaseError(mvi);
570+
if (mvi->getType().isMoveOnly()) {
571+
mvi->setAllowsDiagnostics(false);
572+
continue;
573+
}
574+
575+
if (!DisableUnhandledMoveDiagnostic)
576+
emitUnsupportedUseCaseError(mvi);
567577
}
568578
}
569579
}

test/SILOptimizer/moveonly_objectchecker_diagnostics.swift

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2326,3 +2326,14 @@ public func closureAndClosureCaptureClassOwnedArgUseAfterConsume2(_ x2: __owned
23262326
f()
23272327
print(x2) // expected-note {{consuming use}}
23282328
}
2329+
2330+
/////////////////////////////
2331+
// Tests For Move Operator //
2332+
/////////////////////////////
2333+
2334+
func moveOperatorTest(_ k: __owned Klass) {
2335+
let k2 = k // expected-error {{'k2' consumed more than once}}
2336+
let k3 = _move k2 // expected-note {{consuming use}}
2337+
let _ = _move k2 // expected-note {{consuming use}}
2338+
let _ = k3
2339+
}

test/SILOptimizer/noimplicitcopy.swift

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2212,3 +2212,14 @@ public func closureAndClosureCaptureClassOwnedArgUseAfterConsume2(@_noImplicitCo
22122212
f()
22132213
print(x2) // expected-note {{consuming use}}
22142214
}
2215+
2216+
/////////////////////////////
2217+
// Tests For Move Operator //
2218+
/////////////////////////////
2219+
2220+
func moveOperatorTest(_ k: __owned Klass) {
2221+
@_noImplicitCopy let k2 = k // expected-error {{'k2' consumed more than once}}
2222+
@_noImplicitCopy let k3 = _move k2 // expected-note {{consuming use}}
2223+
let _ = _move k2 // expected-note {{consuming use}}
2224+
let _ = k3
2225+
}

0 commit comments

Comments
 (0)