Skip to content

Commit 1dd896d

Browse files
gottesmmgottesmm
committed
[move-only] Implement escaping closure semantics.
NOTE: A few of the test patterns need to be made better, but this patch series is large enough, I want to get it into tree and iterate.
1 parent 50af8fd commit 1dd896d

20 files changed

+487
-359
lines changed

‎include/swift/AST/DiagnosticsSIL.def

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -760,12 +760,11 @@ ERROR(sil_moveonlychecker_notconsumable_but_assignable_was_consumed_global_var,
760760
ERROR(sil_moveonlychecker_notconsumable_but_assignable_was_consumed_global_let, none,
761761
"'%0' was consumed but it is illegal to consume a noncopyable global let. One can only read from it",
762762
(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))
766763
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",
764+
"'%0' was consumed but it is illegal to consume a noncopyable mutable capture of an escaping closure. One can only read from it or assign over it",
768765
(StringRef))
766+
ERROR(sil_moveonlychecker_let_capture_consumed, none,
767+
"'%0' was consumed but it is illegal to consume a noncopyable immutable capture of an escaping closure. One can only read from it", (StringRef))
769768

770769
NOTE(sil_moveonlychecker_moveonly_field_consumed_here, none,
771770
"move only field consumed here", ())

‎include/swift/AST/SemanticAttrs.def

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,5 +120,9 @@ SEMANTICS_ATTR(LIFETIMEMANAGEMENT_COPY, "lifetimemanagement.copy")
120120

121121
SEMANTICS_ATTR(NO_PERFORMANCE_ANALYSIS, "no_performance_analysis")
122122

123+
// A flag used to turn off moveonly diagnostics on functions that allocbox to
124+
// stack specialized.
125+
SEMANTICS_ATTR(NO_MOVEONLY_DIAGNOSTICS, "sil.optimizer.moveonly.diagnostic.ignore")
126+
123127
#undef SEMANTICS_ATTR
124128

‎lib/SIL/IR/TypeLowering.cpp

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -112,15 +112,25 @@ CaptureKind TypeConverter::getDeclCaptureKind(CapturedValue capture,
112112
assert(var->hasStorage() &&
113113
"should not have attempted to directly capture this variable");
114114

115+
auto &lowering = getTypeLowering(
116+
var->getType(), TypeExpansionContext::noOpaqueTypeArchetypesSubstitution(
117+
expansion.getResilienceExpansion()));
118+
119+
// If this is a noncopyable 'let' constant that is not a shared paramdecl,
120+
// then we know it is boxed and want to pass it in its boxed form so we can
121+
// obey Swift's capture reference semantics.
122+
if (!var->supportsMutation() && lowering.getLoweredType().isPureMoveOnly()) {
123+
auto *param = dyn_cast<ParamDecl>(var);
124+
if (!param || param->getValueOwnership() != ValueOwnership::Shared) {
125+
return CaptureKind::ImmutableBox;
126+
}
127+
}
128+
115129
// If this is a non-address-only stored 'let' constant, we can capture it
116130
// by value. If it is address-only, then we can't load it, so capture it
117131
// by its address (like a var) instead.
118-
if (!var->supportsMutation() &&
119-
!getTypeLowering(var->getType(),
120-
TypeExpansionContext::noOpaqueTypeArchetypesSubstitution(
121-
expansion.getResilienceExpansion()))
122-
.isAddressOnly())
123-
return CaptureKind::Constant;
132+
if (!var->supportsMutation() && !lowering.isAddressOnly())
133+
return CaptureKind::Constant;
124134

125135
// In-out parameters are captured by address.
126136
if (auto *param = dyn_cast<ParamDecl>(var)) {

‎lib/SILGen/SILGenDecl.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -351,13 +351,13 @@ class LocalVariableInitialization : public SingleBufferInitialization {
351351
"can't emit a local var for a non-local var decl");
352352
assert(decl->hasStorage() && "can't emit storage for a computed variable");
353353
assert(!SGF.VarLocs.count(decl) && "Already have an entry for this decl?");
354+
354355
// The box type's context is lowered in the minimal resilience domain.
356+
auto instanceType = SGF.SGM.Types.getLoweredRValueType(
357+
TypeExpansionContext::minimal(), decl->getType());
355358
auto boxType = SGF.SGM.Types.getContextBoxTypeForCapture(
356-
decl,
357-
SGF.SGM.Types.getLoweredRValueType(TypeExpansionContext::minimal(),
358-
decl->getType()),
359-
SGF.F.getGenericEnvironment(),
360-
/*mutable*/ true);
359+
decl, instanceType, SGF.F.getGenericEnvironment(),
360+
/*mutable*/ !instanceType->isPureMoveOnly() || !decl->isLet());
361361

362362
// The variable may have its lifetime extended by a closure, heap-allocate
363363
// it using a box.

‎lib/SILGen/SILGenLValue.cpp

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3164,18 +3164,9 @@ RValue SILGenFunction::emitRValueForNonMemberVarDecl(SILLocation loc,
31643164
SILAccessKind::Read);
31653165

31663166
if (accessAddr->getType().isMoveOnly()) {
3167-
bool needCheck = true;
3168-
SILValue tmp = destAddr;
3169-
if (auto *mmci = dyn_cast<MarkMustCheckInst>(tmp))
3170-
tmp = mmci->getOperand();
3171-
if (auto *pbi = dyn_cast<ProjectBoxInst>(tmp)) {
3172-
auto boxType = pbi->getOperand()->getType().castTo<SILBoxType>();
3173-
needCheck = boxType->getLayout()->getFields()[0].isMutable();
3174-
}
3175-
if (needCheck)
3176-
accessAddr = B.createMarkMustCheckInst(
3177-
loc, accessAddr,
3178-
MarkMustCheckInst::CheckKind::AssignableButNotConsumable);
3167+
accessAddr = B.createMarkMustCheckInst(
3168+
loc, accessAddr,
3169+
MarkMustCheckInst::CheckKind::AssignableButNotConsumable);
31793170
}
31803171

31813172
auto propagateRValuePastAccess = [&](RValue &&rvalue) {

‎lib/SILGen/SILGenProlog.cpp

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -339,7 +339,7 @@ struct ArgumentInitHelper {
339339
SGF.SGM.Types.getLoweredRValueType(TypeExpansionContext::minimal(),
340340
pd->getType()),
341341
SGF.F.getGenericEnvironment(),
342-
/*mutable*/ true);
342+
/*mutable*/ false);
343343

344344
auto *box = SGF.B.createAllocBox(loc, boxType, varinfo);
345345
SILValue destAddr = SGF.B.createProjectBox(loc, box, 0);
@@ -355,6 +355,13 @@ struct ArgumentInitHelper {
355355
return;
356356
}
357357

358+
// If we have a guaranteed noncopyable argument, we do something a little
359+
// different. Specifically, we emit it as normal and do a non-consume or
360+
// assign. The reason why we do this is that a guaranteed argument cannot
361+
// be used in an escaping closure. So today, we leave it with the
362+
// misleading consuming message. We still are able to pass it to
363+
// non-escaping closures though since the onstack partial_apply does not
364+
// consume the value.
358365
assert(value->getOwnershipKind() == OwnershipKind::Guaranteed);
359366
value = SGF.B.createCopyValue(loc, value);
360367
value = SGF.B.createMarkMustCheckInst(
@@ -601,10 +608,10 @@ static void emitCaptureArguments(SILGenFunction &SGF,
601608
case CaptureKind::Box: {
602609
// LValues are captured as a retained @box that owns
603610
// the captured value.
611+
bool isMutable = captureKind == CaptureKind::Box;
604612
auto type = getVarTypeInCaptureContext();
605613
// Get the content for the box in the minimal resilience domain because we
606614
// are declaring a type.
607-
bool isMutable = captureKind != CaptureKind::ImmutableBox;
608615
auto boxTy = SGF.SGM.Types.getContextBoxTypeForCapture(
609616
VD,
610617
SGF.SGM.Types.getLoweredRValueType(TypeExpansionContext::minimal(),
@@ -613,18 +620,14 @@ static void emitCaptureArguments(SILGenFunction &SGF,
613620
auto *box = SGF.F.begin()->createFunctionArgument(
614621
SILType::getPrimitiveObjectType(boxTy), VD);
615622
box->setClosureCapture(true);
616-
SILValue addr = SGF.B.createProjectBox(VD, box, 0);
617-
if (addr->getType().isMoveOnly()) {
618-
if (isMutable)
619-
addr = SGF.B.createMarkMustCheckInst(
620-
VD, addr, MarkMustCheckInst::CheckKind::ConsumableAndAssignable);
621-
else
622-
addr = SGF.B.createMarkMustCheckInst(
623-
VD, addr, MarkMustCheckInst::CheckKind::NoConsumeOrAssign);
623+
if (box->getType().getSILBoxFieldType(&SGF.F, 0).isMoveOnly()) {
624+
SGF.VarLocs[VD] = SILGenFunction::VarLoc::getForBox(box);
625+
} else {
626+
SILValue addr = SGF.B.createProjectBox(VD, box, 0);
627+
SGF.VarLocs[VD] = SILGenFunction::VarLoc::get(addr, box);
628+
SILDebugVariable DbgVar(VD->isLet(), ArgNo);
629+
SGF.B.createDebugValueAddr(Loc, addr, DbgVar);
624630
}
625-
SGF.VarLocs[VD] = SILGenFunction::VarLoc::get(addr, box);
626-
SILDebugVariable DbgVar(VD->isLet(), ArgNo);
627-
SGF.B.createDebugValueAddr(Loc, addr, DbgVar);
628631
break;
629632
}
630633
case CaptureKind::Immutable:

‎lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

Lines changed: 12 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -387,28 +387,6 @@ void swift::siloptimizer::searchForCandidateAddressMarkMustChecks(
387387
if (!mmci || !mmci->hasMoveCheckerKind() || !mmci->getType().isAddress())
388388
continue;
389389

390-
// Skip any alloc_box due to heap to stack failing on a box capture. This
391-
// will just cause an error.
392-
if (auto *pbi = dyn_cast<ProjectBoxInst>(mmci->getOperand())) {
393-
if (isa<AllocBoxInst>(pbi->getOperand())) {
394-
LLVM_DEBUG(
395-
llvm::dbgs()
396-
<< "Early emitting diagnostic for unsupported alloc box!\n");
397-
diagnosticEmitter.emitCheckerDoesntUnderstandDiagnostic(mmci);
398-
continue;
399-
}
400-
401-
if (auto *bbi = dyn_cast<BeginBorrowInst>(pbi->getOperand())) {
402-
if (isa<AllocBoxInst>(bbi->getOperand())) {
403-
LLVM_DEBUG(
404-
llvm::dbgs()
405-
<< "Early emitting diagnostic for unsupported alloc box!\n");
406-
diagnosticEmitter.emitCheckerDoesntUnderstandDiagnostic(mmci);
407-
continue;
408-
}
409-
}
410-
}
411-
412390
moveIntroducersToProcess.insert(mmci);
413391
}
414392
}
@@ -1368,15 +1346,21 @@ bool GatherUsesVisitor::visitUse(Operand *op, AccessUseType useTy) {
13681346
// If we are asked to perform no_consume_or_assign checking or
13691347
// assignable_but_not_consumable checking, if we found any consumes of our
13701348
// load, then we need to emit an error.
1371-
if (markedValue->getCheckKind() ==
1372-
MarkMustCheckInst::CheckKind::NoConsumeOrAssign ||
1373-
markedValue->getCheckKind() ==
1374-
MarkMustCheckInst::CheckKind::AssignableButNotConsumable) {
1349+
auto checkKind = markedValue->getCheckKind();
1350+
if (checkKind != MarkMustCheckInst::CheckKind::ConsumableAndAssignable) {
13751351
if (moveChecker.canonicalizer.foundAnyConsumingUses()) {
13761352
LLVM_DEBUG(llvm::dbgs()
13771353
<< "Found mark must check [nocopy] error: " << *user);
1378-
moveChecker.diagnosticEmitter.emitAddressInstLoadedAndConsumed(
1379-
markedValue);
1354+
auto *fArg = dyn_cast<SILFunctionArgument>(
1355+
stripAccessMarkers(markedValue->getOperand()));
1356+
if (fArg && fArg->isClosureCapture() && fArg->getType().isAddress()) {
1357+
moveChecker.diagnosticEmitter.emitPromotedBoxArgumentError(
1358+
markedValue, fArg);
1359+
} else {
1360+
moveChecker.diagnosticEmitter
1361+
.emitAddressEscapingClosureCaptureLoadedAndConsumed(
1362+
markedValue);
1363+
}
13801364
emittedEarlyDiagnostic = true;
13811365
return true;
13821366
}

‎lib/SILOptimizer/Mandatory/MoveOnlyChecker.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "swift/AST/AccessScope.h"
1616
#include "swift/AST/DiagnosticEngine.h"
1717
#include "swift/AST/DiagnosticsSIL.h"
18+
#include "swift/AST/SemanticAttrs.h"
1819
#include "swift/Basic/Debug.h"
1920
#include "swift/Basic/Defer.h"
2021
#include "swift/Basic/FrozenMultiMap.h"
@@ -156,6 +157,14 @@ class MoveOnlyCheckerPass : public SILFunctionTransform {
156157
assert(fn->getModule().getStage() == SILStage::Raw &&
157158
"Should only run on Raw SIL");
158159

160+
// If allocbox to stack told use to not emit diagnostics for this function,
161+
// clean up any copies, invalidate the analysis, and return early.
162+
if (getFunction()->hasSemanticsAttr(semantics::NO_MOVEONLY_DIAGNOSTICS)) {
163+
if (cleanupNonCopyableCopiesAfterEmittingDiagnostic(getFunction()))
164+
invalidateAnalysis(SILAnalysis::InvalidationKind::Instructions);
165+
return;
166+
}
167+
159168
LLVM_DEBUG(llvm::dbgs()
160169
<< "===> MoveOnly Checker. Visiting: " << fn->getName() << '\n');
161170

‎lib/SILOptimizer/Mandatory/MoveOnlyDiagnostics.cpp

Lines changed: 78 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,17 @@ static void diagnose(ASTContext &context, SILInstruction *inst, Diag<T...> diag,
5959
context.Diags.diagnose(loc.getSourceLoc(), diag, std::forward<U>(args)...);
6060
}
6161

62+
template <typename... T, typename... U>
63+
static void diagnose(ASTContext &context, SourceLoc loc, Diag<T...> diag,
64+
U &&...args) {
65+
// If for testing reasons we want to return that we emitted an error but not
66+
// emit the actual error itself, return early.
67+
if (SilentlyEmitDiagnostics)
68+
return;
69+
70+
context.Diags.diagnose(loc, diag, std::forward<U>(args)...);
71+
}
72+
6273
/// Helper function that actually implements getVariableNameForValue. Do not
6374
/// call it directly! Call the unary variants instead.
6475
static void getVariableNameForValue(SILValue value2,
@@ -75,7 +86,8 @@ static void getVariableNameForValue(SILValue value2,
7586
}
7687

7788
// Otherwise, we need to look at our mark_must_check's operand.
78-
StackList<SILInstruction *> variableNamePath(value2->getFunction());
89+
StackList<llvm::PointerUnion<SILInstruction *, SILValue>> variableNamePath(
90+
value2->getFunction());
7991
while (true) {
8092
if (auto *allocInst = dyn_cast<AllocationInst>(searchValue)) {
8193
variableNamePath.push_back(allocInst);
@@ -93,6 +105,11 @@ static void getVariableNameForValue(SILValue value2,
93105
continue;
94106
}
95107

108+
if (auto *fArg = dyn_cast<SILFunctionArgument>(searchValue)) {
109+
variableNamePath.push_back({fArg});
110+
break;
111+
}
112+
96113
// If we do not do an exact match, see if we can find a debug_var inst. If
97114
// we do, we always break since we have a root value.
98115
if (auto *use = getSingleDebugUse(searchValue)) {
@@ -121,12 +138,18 @@ static void getVariableNameForValue(SILValue value2,
121138

122139
// Walk backwards, constructing our string.
123140
while (true) {
124-
auto *next = variableNamePath.pop_back_val();
141+
auto next = variableNamePath.pop_back_val();
125142

126-
if (auto i = DebugVarCarryingInst(next)) {
127-
resultingString += i.getName();
128-
} else if (auto i = VarDeclCarryingInst(next)) {
129-
resultingString += i.getName();
143+
if (auto *inst = next.dyn_cast<SILInstruction *>()) {
144+
if (auto i = DebugVarCarryingInst(inst)) {
145+
resultingString += i.getName();
146+
} else if (auto i = VarDeclCarryingInst(inst)) {
147+
resultingString += i.getName();
148+
}
149+
} else {
150+
auto value = next.get<SILValue>();
151+
if (auto *fArg = dyn_cast<SILFunctionArgument>(value))
152+
resultingString += fArg->getDecl()->getBaseName().userFacingName();
130153
}
131154

132155
if (variableNamePath.empty())
@@ -637,7 +660,7 @@ void DiagnosticEmitter::emitObjectInstConsumesAndUsesValue(
637660
registerDiagnosticEmitted(markedValue);
638661
}
639662

640-
void DiagnosticEmitter::emitAddressInstLoadedAndConsumed(
663+
void DiagnosticEmitter::emitAddressEscapingClosureCaptureLoadedAndConsumed(
641664
MarkMustCheckInst *markedValue) {
642665
SmallString<64> varName;
643666
getVariableNameForValue(markedValue, varName);
@@ -648,7 +671,13 @@ void DiagnosticEmitter::emitAddressInstLoadedAndConsumed(
648671
decltype(diag::
649672
sil_moveonlychecker_notconsumable_but_assignable_was_consumed_classfield_let);
650673
Optional<DiagType> diag;
651-
if (auto *reai = dyn_cast<RefElementAddrInst>(operand)) {
674+
675+
if (markedValue->getCheckKind() ==
676+
MarkMustCheckInst::CheckKind::NoConsumeOrAssign) {
677+
// We only use no consume or assign if we have a promoted let box.
678+
diag = diag::
679+
sil_moveonlychecker_notconsumable_but_assignable_was_consumed_classfield_let;
680+
} else if (auto *reai = dyn_cast<RefElementAddrInst>(operand)) {
652681
auto *field = reai->getField();
653682
if (field->isLet()) {
654683
diag = diag::
@@ -657,8 +686,7 @@ void DiagnosticEmitter::emitAddressInstLoadedAndConsumed(
657686
diag = diag::
658687
sil_moveonlychecker_notconsumable_but_assignable_was_consumed_classfield_var;
659688
}
660-
} else if (auto *globalAddr =
661-
dyn_cast<GlobalAddrInst>(operand)) {
689+
} else if (auto *globalAddr = dyn_cast<GlobalAddrInst>(operand)) {
662690
auto inst = VarDeclCarryingInst(globalAddr);
663691
if (auto *decl = inst.getDecl()) {
664692
if (decl->isLet()) {
@@ -675,8 +703,21 @@ void DiagnosticEmitter::emitAddressInstLoadedAndConsumed(
675703
diag = diag::
676704
sil_moveonlychecker_notconsumable_but_assignable_was_consumed_escaping_var;
677705
} else {
706+
diag = diag::sil_moveonlychecker_let_capture_consumed;
707+
}
708+
} else if (auto *fArg = dyn_cast<SILFunctionArgument>(operand)) {
709+
if (auto boxType = fArg->getType().getAs<SILBoxType>()) {
710+
if (boxType->getLayout()->isMutable()) {
711+
diag = diag::
712+
sil_moveonlychecker_notconsumable_but_assignable_was_consumed_escaping_var;
713+
} else {
714+
diag = diag::sil_moveonlychecker_let_capture_consumed;
715+
}
716+
} else if (fArg->getType().isAddress() &&
717+
markedValue->getCheckKind() ==
718+
MarkMustCheckInst::CheckKind::AssignableButNotConsumable) {
678719
diag = diag::
679-
sil_moveonlychecker_notconsumable_but_assignable_was_consumed_escaping_let;
720+
sil_moveonlychecker_notconsumable_but_assignable_was_consumed_escaping_var;
680721
}
681722
}
682723

@@ -690,3 +731,29 @@ void DiagnosticEmitter::emitAddressInstLoadedAndConsumed(
690731
varName);
691732
registerDiagnosticEmitted(markedValue);
692733
}
734+
735+
void DiagnosticEmitter::emitPromotedBoxArgumentError(
736+
MarkMustCheckInst *markedValue, SILFunctionArgument *arg) {
737+
auto &astContext = fn->getASTContext();
738+
SmallString<64> varName;
739+
getVariableNameForValue(markedValue, varName);
740+
741+
registerDiagnosticEmitted(markedValue);
742+
743+
auto diag = diag::sil_moveonlychecker_let_capture_consumed;
744+
if (markedValue->getCheckKind() ==
745+
MarkMustCheckInst::CheckKind::AssignableButNotConsumable)
746+
diag = diag::
747+
sil_moveonlychecker_notconsumable_but_assignable_was_consumed_escaping_var;
748+
749+
diagnose(astContext, arg->getDecl()->getLoc(), diag, varName);
750+
751+
// Now for each consuming use that needs a copy...
752+
for (auto *user : getCanonicalizer().consumingUsesNeedingCopy) {
753+
diagnose(astContext, user, diag::sil_moveonlychecker_consuming_use_here);
754+
}
755+
756+
for (auto *user : getCanonicalizer().consumingBoundaryUsers) {
757+
diagnose(astContext, user, diag::sil_moveonlychecker_consuming_use_here);
758+
}
759+
}

0 commit comments

Comments
 (0)