Skip to content

Commit 79042f1

Browse files
committed
[no-implicit-copy] Emit an error diagnostic if we find a mark_must_check [no_implicit_copy] we don't know how to check.
We do the same thing already with the move function. If someone is using this functionality and the compiler fails, we don't want to succeed the compilation since the user is relying on the compiler for safety!
1 parent 167b08d commit 79042f1

File tree

2 files changed

+25
-3
lines changed

2 files changed

+25
-3
lines changed

include/swift/AST/DiagnosticsSIL.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -729,6 +729,9 @@ ERROR(sil_moveonlychecker_value_consumed_more_than_once, none,
729729
"'%0' consumed more than once", (StringRef))
730730
NOTE(sil_moveonlychecker_consuming_use_here, none,
731731
"consuming use", ())
732+
ERROR(sil_moveonlychecker_not_understand_mark_move, none,
733+
"Usage of @noImplicitCopy that the move checker does not know how to "
734+
"check!", ())
732735

733736
// move kills copyable values checker diagnostics
734737
ERROR(sil_movekillscopyablevalue_value_consumed_more_than_once, none,

lib/SILOptimizer/Mandatory/MoveOnlyChecker.cpp

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -484,9 +484,13 @@ struct MoveOnlyChecker {
484484

485485
bool MoveOnlyChecker::check(NonLocalAccessBlockAnalysis *accessBlockAnalysis,
486486
DominanceInfo *domTree) {
487+
bool changed = false;
488+
487489
for (auto &block : *fn) {
488-
for (auto &ii : block) {
489-
auto *mmci = dyn_cast<MarkMustCheckInst>(&ii);
490+
for (auto ii = block.begin(), ie = block.end(); ii != ie;) {
491+
auto *mmci = dyn_cast<MarkMustCheckInst>(&*ii);
492+
++ii;
493+
490494
if (!mmci || !mmci->isNoImplicitCopy())
491495
continue;
492496

@@ -496,6 +500,7 @@ bool MoveOnlyChecker::check(NonLocalAccessBlockAnalysis *accessBlockAnalysis,
496500
if (auto *bbi = dyn_cast<BeginBorrowInst>(cvi->getOperand())) {
497501
if (bbi->isLexical()) {
498502
moveIntroducersToProcess.insert(mmci);
503+
continue;
499504
}
500505
}
501506
}
@@ -505,9 +510,24 @@ bool MoveOnlyChecker::check(NonLocalAccessBlockAnalysis *accessBlockAnalysis,
505510
if (auto *bbi = dyn_cast<BeginBorrowInst>(cvi->getOperand())) {
506511
if (bbi->isLexical()) {
507512
moveIntroducersToProcess.insert(mmci);
513+
continue;
508514
}
509515
}
510516
}
517+
518+
// If we see a mark_must_check that is marked no implicit copy that we
519+
// don't understand, emit a diagnostic to fail the compilation. This
520+
// ensures that if someone marks something no implicit copy and we fail to
521+
// check it, we fail the compilation.
522+
//
523+
// We then RAUW the mark_must_check once we have emitted the error since
524+
// later passes expect that mark_must_check has been eliminated by
525+
// us. Since we are failing already, this is ok to do.
526+
diagnose(fn->getASTContext(), mmci->getLoc().getSourceLoc(),
527+
diag::sil_moveonlychecker_not_understand_mark_move);
528+
mmci->replaceAllUsesWith(mmci->getOperand());
529+
mmci->eraseFromParent();
530+
changed = true;
511531
}
512532
}
513533

@@ -518,7 +538,6 @@ bool MoveOnlyChecker::check(NonLocalAccessBlockAnalysis *accessBlockAnalysis,
518538
instToDelete->eraseFromParent();
519539
});
520540
InstructionDeleter deleter(std::move(callbacks));
521-
bool changed = false;
522541

523542
SmallVector<Operand *, 32> consumingUsesNeedingCopy;
524543
auto foundConsumingUseNeedingCopy = [&](Operand *use) {

0 commit comments

Comments
 (0)