Skip to content

Commit e58c45f

Browse files
committed
[move-only] Add support for ref_element_addr with AssignableButNotConsumable semantics.
rdar://104874497
1 parent b0a3167 commit e58c45f

10 files changed

+236
-260
lines changed

include/swift/AST/DiagnosticsSIL.def

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -748,6 +748,25 @@ ERROR(sil_moveonlychecker_exclusivity_violation, none,
748748
ERROR(sil_moveonlychecker_moveonly_field_consumed, none,
749749
"'%0' has a move only field that was consumed before later uses", (StringRef))
750750

751+
ERROR(sil_moveonlychecker_notconsumable_but_assignable_was_consumed_classfield_let, none,
752+
"'%0' was consumed but it is illegal to consume a noncopyable class let field. One can only read from it",
753+
(StringRef))
754+
ERROR(sil_moveonlychecker_notconsumable_but_assignable_was_consumed_classfield_var, none,
755+
"'%0' was consumed but it is illegal to consume a noncopyable class var field. One can only read from it or assign to it",
756+
(StringRef))
757+
ERROR(sil_moveonlychecker_notconsumable_but_assignable_was_consumed_global_var, none,
758+
"'%0' was consumed but it is illegal to consume a noncopyable global var. One can only read from it or assign to it",
759+
(StringRef))
760+
ERROR(sil_moveonlychecker_notconsumable_but_assignable_was_consumed_global_let, none,
761+
"'%0' was consumed but it is illegal to consume a noncopyable global let. One can only read from it",
762+
(StringRef))
763+
ERROR(sil_moveonlychecker_notconsumable_but_assignable_was_consumed_escaping_let, none,
764+
"'%0' was consumed but it is illegal to consume a noncopyable escaping immutable capture. One can only read from it",
765+
(StringRef))
766+
ERROR(sil_moveonlychecker_notconsumable_but_assignable_was_consumed_escaping_var, none,
767+
"'%0' was consumed but it is illegal to consume a noncopyable escaping mutable capture. One can only read from it or assign over it",
768+
(StringRef))
769+
751770
NOTE(sil_moveonlychecker_moveonly_field_consumed_here, none,
752771
"move only field consumed here", ())
753772
NOTE(sil_moveonlychecker_boundary_use, none,

lib/SILGen/SILGenLValue.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -788,6 +788,17 @@ namespace {
788788
SGF.B.createRefElementAddr(loc, base.getUnmanagedValue(),
789789
Field, SubstFieldType);
790790

791+
// If we have a move only type...
792+
if (result->getType().isMoveOnly()) {
793+
auto checkKind =
794+
MarkMustCheckInst::CheckKind::AssignableButNotConsumable;
795+
if (isReadAccess(getAccessKind())) {
796+
// Add a mark_must_check [no_consume_or_assign].
797+
checkKind = MarkMustCheckInst::CheckKind::NoConsumeOrAssign;
798+
}
799+
result = SGF.B.createMarkMustCheckInst(loc, result, checkKind);
800+
}
801+
791802
// Avoid emitting access markers completely for non-accesses or immutable
792803
// declarations. Access marker verification is aware of these cases.
793804
if (!IsNonAccessing && !Field->isLet()) {

lib/SILOptimizer/Mandatory/MoveOnlyAddressChecker.cpp

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -541,16 +541,19 @@ void UseState::initializeLiveness(
541541
FieldSensitiveMultiDefPrunedLiveRange &liveness) {
542542
// We begin by initializing all of our init uses.
543543
for (auto initInstAndValue : initInsts) {
544+
LLVM_DEBUG(llvm::dbgs() << "Found def: " << *initInstAndValue.first);
544545
liveness.initializeDef(initInstAndValue.first, initInstAndValue.second);
545546
}
546547

547548
// If we have a reinitInstAndValue that we are going to be able to convert
548549
// into a simple init, add it as an init. We are going to consider the rest of
549550
// our reinit uses to be liveness uses.
550551
for (auto reinitInstAndValue : reinitInsts) {
551-
if (isReinitToInitConvertibleInst(reinitInstAndValue.first))
552+
if (isReinitToInitConvertibleInst(reinitInstAndValue.first)) {
553+
LLVM_DEBUG(llvm::dbgs() << "Found def: " << *reinitInstAndValue.first);
552554
liveness.initializeDef(reinitInstAndValue.first,
553555
reinitInstAndValue.second);
556+
}
554557
}
555558

556559
// Then check if our markedValue is from an argument that is in,
@@ -603,6 +606,15 @@ void UseState::initializeLiveness(
603606
}
604607
}
605608

609+
// Check if our address is from a ref_element_addr. In such a case, we treat
610+
// the mark_must_check as the initialization.
611+
if (auto *refEltAddr = dyn_cast<RefElementAddrInst>(address->getOperand())) {
612+
LLVM_DEBUG(llvm::dbgs() << "Found ref_element_addr use... "
613+
"adding mark_must_check as init!\n");
614+
initInsts.insert({address, liveness.getTopLevelSpan()});
615+
liveness.initializeDef(address, liveness.getTopLevelSpan());
616+
}
617+
606618
// Now that we have finished initialization of defs, change our multi-maps
607619
// from their array form to their map form.
608620
liveness.finishedInitializationOfDefs();
@@ -1157,15 +1169,17 @@ bool GatherUsesVisitor::visitUse(Operand *op, AccessUseType useTy) {
11571169
// Canonicalize the lifetime of the load [take], load [copy].
11581170
moveChecker.changed |= moveChecker.canonicalizer.canonicalize(li);
11591171

1160-
// If we are asked to perform guaranteed checking, emit an error if we
1161-
// have /any/ consuming uses. This is a case that can always be converted
1162-
// to a load_borrow if we pass the check.
1172+
// If we are asked to perform no_consume_or_assign checking or
1173+
// assignable_but_not_consumable checking, if we found any consumes of our
1174+
// load, then we need to emit an error.
11631175
if (markedValue->getCheckKind() ==
1164-
MarkMustCheckInst::CheckKind::NoConsumeOrAssign) {
1165-
if (!moveChecker.canonicalizer.foundAnyConsumingUses()) {
1176+
MarkMustCheckInst::CheckKind::NoConsumeOrAssign ||
1177+
markedValue->getCheckKind() ==
1178+
MarkMustCheckInst::CheckKind::AssignableButNotConsumable) {
1179+
if (moveChecker.canonicalizer.foundAnyConsumingUses()) {
11661180
LLVM_DEBUG(llvm::dbgs()
11671181
<< "Found mark must check [nocopy] error: " << *user);
1168-
moveChecker.diagnosticEmitter.emitObjectGuaranteedDiagnostic(
1182+
moveChecker.diagnosticEmitter.emitAddressInstLoadedAndConsumed(
11691183
markedValue);
11701184
emittedEarlyDiagnostic = true;
11711185
return true;
@@ -1856,14 +1870,18 @@ void MoveOnlyChecker::rewriteUses(
18561870
// destroy_value and use then to create a new load_borrow scope.
18571871
SILBuilderWithScope builder(li);
18581872
auto *lbi = builder.createLoadBorrow(li->getLoc(), li->getOperand());
1859-
1873+
// We use this auxillary list to avoid iterator invalidation of
1874+
// li->getConsumingUse();
1875+
StackList<DestroyValueInst *> toDelete(lbi->getFunction());
18601876
for (auto *consumeUse : li->getConsumingUses()) {
18611877
auto *dvi = cast<DestroyValueInst>(consumeUse->getUser());
18621878
SILBuilderWithScope destroyBuilder(dvi);
18631879
destroyBuilder.createEndBorrow(dvi->getLoc(), lbi);
1864-
dvi->eraseFromParent();
1880+
toDelete.push_back(dvi);
18651881
changed = true;
18661882
}
1883+
while (!toDelete.empty())
1884+
toDelete.pop_back_val()->eraseFromParent();
18671885

18681886
li->replaceAllUsesWith(lbi);
18691887
li->eraseFromParent();
@@ -1903,7 +1921,7 @@ bool MoveOnlyChecker::performSingleCheck(MarkMustCheckInst *markedAddress) {
19031921
diagnosticEmitter, gatherUsesLiveness);
19041922
SWIFT_DEFER { visitor.clear(); };
19051923
visitor.reset(markedAddress);
1906-
if (!visitAccessPathUses(visitor, accessPath, fn)) {
1924+
if (!visitAccessPathBaseUses(visitor, accessPathWithBase, fn)) {
19071925
LLVM_DEBUG(llvm::dbgs() << "Failed access path visit: " << *markedAddress);
19081926
return false;
19091927
}

lib/SILOptimizer/Mandatory/MoveOnlyDiagnostics.cpp

Lines changed: 50 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -91,13 +91,6 @@ static void getVariableNameForValue(MarkMustCheckInst *mmci,
9191
continue;
9292
}
9393

94-
// Single value instructions we should look through.
95-
if (isa<BeginBorrowInst>(value) || isa<LoadInst>(value) ||
96-
isa<BeginAccessInst>(value) || isa<MarkMustCheckInst>(value)) {
97-
value = cast<SingleValueInstruction>(value)->getOperand(0);
98-
continue;
99-
}
100-
10194
// If we do not do an exact match, see if we can find a debug_var inst. If
10295
// we do, we always break since we have a root value.
10396
if (auto *use = getSingleDebugUse(value)) {
@@ -108,6 +101,14 @@ static void getVariableNameForValue(MarkMustCheckInst *mmci,
108101
}
109102
}
110103

104+
// Otherwise, try to see if we have a single value instruction we can look
105+
// through.
106+
if (isa<BeginBorrowInst>(value) || isa<LoadInst>(value) ||
107+
isa<BeginAccessInst>(value) || isa<MarkMustCheckInst>(value)) {
108+
value = cast<SingleValueInstruction>(value)->getOperand(0);
109+
continue;
110+
}
111+
111112
// If we do not pattern match successfully, just set resulting string to
112113
// unknown and return early.
113114
resultingString += "unknown";
@@ -619,3 +620,45 @@ void DiagnosticEmitter::emitObjectInstConsumesAndUsesValue(
619620
diag::sil_moveonlychecker_consuming_and_non_consuming_uses_here);
620621
registerDiagnosticEmitted(markedValue);
621622
}
623+
624+
void DiagnosticEmitter::emitAddressInstLoadedAndConsumed(
625+
MarkMustCheckInst *markedValue) {
626+
SmallString<64> varName;
627+
getVariableNameForValue(markedValue, varName);
628+
629+
using DiagType =
630+
decltype(diag::
631+
sil_moveonlychecker_notconsumable_but_assignable_was_consumed_classfield_let);
632+
Optional<DiagType> diag;
633+
if (auto *reai = dyn_cast<RefElementAddrInst>(markedValue->getOperand())) {
634+
auto *field = reai->getField();
635+
if (field->isLet()) {
636+
diag = diag::
637+
sil_moveonlychecker_notconsumable_but_assignable_was_consumed_classfield_let;
638+
} else {
639+
diag = diag::
640+
sil_moveonlychecker_notconsumable_but_assignable_was_consumed_classfield_var;
641+
}
642+
} else if (auto *globalAddr =
643+
dyn_cast<GlobalAddrInst>(markedValue->getOperand())) {
644+
auto inst = VarDeclCarryingInst(globalAddr);
645+
if (auto *decl = inst.getDecl()) {
646+
if (decl->isLet()) {
647+
diag = diag::
648+
sil_moveonlychecker_notconsumable_but_assignable_was_consumed_global_let;
649+
} else {
650+
diag = diag::
651+
sil_moveonlychecker_notconsumable_but_assignable_was_consumed_global_var;
652+
}
653+
}
654+
}
655+
656+
if (!diag) {
657+
llvm::report_fatal_error(
658+
"Unknown address assignable but not consumable case!");
659+
}
660+
661+
diagnose(markedValue->getModule().getASTContext(), markedValue, *diag,
662+
varName);
663+
registerDiagnosticEmitted(markedValue);
664+
}

lib/SILOptimizer/Mandatory/MoveOnlyDiagnostics.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,8 @@ class DiagnosticEmitter {
103103
Operand *consumingUse,
104104
Operand *nonConsumingUse);
105105

106+
void emitAddressInstLoadedAndConsumed(MarkMustCheckInst *markedValue);
107+
106108
private:
107109
/// Emit diagnostics for the final consuming uses and consuming uses needing
108110
/// copy. If filter is non-null, allow for the caller to pre-process operands

0 commit comments

Comments
 (0)