Skip to content

Commit 451489d

Browse files
jckarterjckarter
authored
Merge pull request #69287 from jckarter/enable-move-only-resilient-types-5.10
[5.10] Enable noncopyable resilient types.
2 parents eaac735 + ca6b7a4 commit 451489d

28 files changed

+317
-356
lines changed

‎include/swift/Basic/Features.def

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ LANGUAGE_FEATURE(
107107
LANGUAGE_FEATURE(AttachedMacros, 389, "Attached macros", hasSwiftSwiftParser)
108108
LANGUAGE_FEATURE(ExtensionMacros, 402, "Extension macros", hasSwiftSwiftParser)
109109
LANGUAGE_FEATURE(MoveOnly, 390, "noncopyable types", true)
110+
LANGUAGE_FEATURE(MoveOnlyResilientTypes, 390, "non-@frozen noncopyable types with library evolution", true)
110111
LANGUAGE_FEATURE(ParameterPacks, 393, "Value and type parameter packs", true)
111112
SUPPRESSIBLE_LANGUAGE_FEATURE(LexicalLifetimes, 0, "@_eagerMove/@_noEagerMove/@_lexicalLifetimes annotations", true)
112113
LANGUAGE_FEATURE(FreestandingMacros, 397, "freestanding declaration macros", true)
@@ -140,7 +141,6 @@ EXPERIMENTAL_FEATURE(NoImplicitCopy, true)
140141
EXPERIMENTAL_FEATURE(OldOwnershipOperatorSpellings, true)
141142
EXPERIMENTAL_FEATURE(MoveOnlyEnumDeinits, true)
142143
EXPERIMENTAL_FEATURE(MoveOnlyTuples, true)
143-
EXPERIMENTAL_FEATURE(MoveOnlyResilientTypes, true)
144144
EXPERIMENTAL_FEATURE(MoveOnlyPartialConsumption, true)
145145

146146
EXPERIMENTAL_FEATURE(OneWayClosureParameters, false)

‎include/swift/SIL/OwnershipUtils.h

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,7 @@ class BorrowingOperandKind {
254254
enum Kind : uint8_t {
255255
Invalid = 0,
256256
BeginBorrow,
257+
StoreBorrow,
257258
BeginApply,
258259
Branch,
259260
Apply,
@@ -277,6 +278,8 @@ class BorrowingOperandKind {
277278
return Kind::Invalid;
278279
case SILInstructionKind::BeginBorrowInst:
279280
return Kind::BeginBorrow;
281+
case SILInstructionKind::StoreBorrowInst:
282+
return Kind::StoreBorrow;
280283
case SILInstructionKind::BeginApplyInst:
281284
return Kind::BeginApply;
282285
case SILInstructionKind::BranchInst:
@@ -386,6 +389,7 @@ struct BorrowingOperand {
386389
case BorrowingOperandKind::Invalid:
387390
llvm_unreachable("Using invalid case?!");
388391
case BorrowingOperandKind::BeginBorrow:
392+
case BorrowingOperandKind::StoreBorrow:
389393
case BorrowingOperandKind::BeginApply:
390394
case BorrowingOperandKind::Apply:
391395
case BorrowingOperandKind::TryApply:
@@ -418,6 +422,7 @@ struct BorrowingOperand {
418422
case BorrowingOperandKind::BeginBorrow:
419423
case BorrowingOperandKind::Branch:
420424
return true;
425+
case BorrowingOperandKind::StoreBorrow:
421426
case BorrowingOperandKind::BeginApply:
422427
case BorrowingOperandKind::Apply:
423428
case BorrowingOperandKind::TryApply:
@@ -790,7 +795,6 @@ class InteriorPointerOperandKind {
790795
RefTailAddr,
791796
OpenExistentialBox,
792797
ProjectBox,
793-
StoreBorrow,
794798
};
795799

796800
private:
@@ -819,8 +823,6 @@ class InteriorPointerOperandKind {
819823
return Kind::OpenExistentialBox;
820824
case SILInstructionKind::ProjectBoxInst:
821825
return Kind::ProjectBox;
822-
case SILInstructionKind::StoreBorrowInst:
823-
return Kind::StoreBorrow;
824826
}
825827
}
826828

@@ -839,8 +841,6 @@ class InteriorPointerOperandKind {
839841
return Kind::OpenExistentialBox;
840842
case ValueKind::ProjectBoxInst:
841843
return Kind::ProjectBox;
842-
case ValueKind::StoreBorrowInst:
843-
return Kind::StoreBorrow;
844844
}
845845
}
846846

@@ -897,8 +897,7 @@ struct InteriorPointerOperand {
897897
case InteriorPointerOperandKind::RefElementAddr:
898898
case InteriorPointerOperandKind::RefTailAddr:
899899
case InteriorPointerOperandKind::OpenExistentialBox:
900-
case InteriorPointerOperandKind::ProjectBox:
901-
case InteriorPointerOperandKind::StoreBorrow: {
900+
case InteriorPointerOperandKind::ProjectBox: {
902901
// Ok, we have a valid instruction. Return the relevant operand.
903902
auto *op =
904903
&cast<SingleValueInstruction>(resultValue)->getAllOperands()[0];
@@ -941,8 +940,6 @@ struct InteriorPointerOperand {
941940
return cast<OpenExistentialBoxInst>(operand->getUser());
942941
case InteriorPointerOperandKind::ProjectBox:
943942
return cast<ProjectBoxInst>(operand->getUser());
944-
case InteriorPointerOperandKind::StoreBorrow:
945-
return cast<StoreBorrowInst>(operand->getUser());
946943
}
947944
llvm_unreachable("Covered switch isn't covered?!");
948945
}

‎lib/SIL/IR/OperandOwnership.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -452,7 +452,7 @@ OperandOwnership OperandOwnershipClassifier::visitBranchInst(BranchInst *bi) {
452452
OperandOwnership
453453
OperandOwnershipClassifier::visitStoreBorrowInst(StoreBorrowInst *i) {
454454
if (getValue() == i->getSrc()) {
455-
return OperandOwnership::InteriorPointer;
455+
return OperandOwnership::Borrow;
456456
}
457457
return OperandOwnership::TrivialUse;
458458
}

‎lib/SIL/Utils/OwnershipUtils.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -600,6 +600,9 @@ void BorrowingOperandKind::print(llvm::raw_ostream &os) const {
600600
case Kind::BeginBorrow:
601601
os << "BeginBorrow";
602602
return;
603+
case Kind::StoreBorrow:
604+
os << "StoreBorrow";
605+
return;
603606
case Kind::BeginApply:
604607
os << "BeginApply";
605608
return;
@@ -649,6 +652,7 @@ bool BorrowingOperand::hasEmptyRequiredEndingUses() const {
649652
case BorrowingOperandKind::Invalid:
650653
llvm_unreachable("Using invalid case");
651654
case BorrowingOperandKind::BeginBorrow:
655+
case BorrowingOperandKind::StoreBorrow:
652656
case BorrowingOperandKind::BeginApply:
653657
case BorrowingOperandKind::BeginAsyncLet:
654658
case BorrowingOperandKind::PartialApplyStack: {
@@ -687,6 +691,20 @@ bool BorrowingOperand::visitScopeEndingUses(
687691
// false.
688692
return !deadBorrow;
689693
}
694+
case BorrowingOperandKind::StoreBorrow: {
695+
bool deadBorrow = true;
696+
for (auto *use : cast<StoreBorrowInst>(op->getUser())->getUses()) {
697+
if (isa<EndBorrowInst>(use->getUser())) {
698+
deadBorrow = false;
699+
if (!func(use))
700+
return false;
701+
}
702+
}
703+
// FIXME: special case for dead borrows. This is dangerous because clients
704+
// only expect visitScopeEndingUses to return false if the visitor returned
705+
// false.
706+
return !deadBorrow;
707+
}
690708
case BorrowingOperandKind::BeginApply: {
691709
bool deadApply = true;
692710
auto *user = cast<BeginApplyInst>(op->getUser());
@@ -763,6 +781,7 @@ BorrowedValue BorrowingOperand::getBorrowIntroducingUserResult() const {
763781
case BorrowingOperandKind::Yield:
764782
case BorrowingOperandKind::PartialApplyStack:
765783
case BorrowingOperandKind::BeginAsyncLet:
784+
case BorrowingOperandKind::StoreBorrow:
766785
return BorrowedValue();
767786

768787
case BorrowingOperandKind::BeginBorrow: {
@@ -792,6 +811,7 @@ SILValue BorrowingOperand::getScopeIntroducingUserResult() {
792811
case BorrowingOperandKind::BeginAsyncLet:
793812
case BorrowingOperandKind::PartialApplyStack:
794813
case BorrowingOperandKind::BeginBorrow:
814+
case BorrowingOperandKind::StoreBorrow:
795815
return cast<SingleValueInstruction>(op->getUser());
796816

797817
case BorrowingOperandKind::BeginApply:

‎lib/SILGen/SILGenApply.cpp

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6517,9 +6517,9 @@ ArgumentSource AccessorBaseArgPreparer::prepareAccessorObjectBaseArg() {
65176517
if (selfParam.isConsumed() && !base.hasCleanup()) {
65186518
base = base.copyUnmanaged(SGF, loc);
65196519
}
6520-
6521-
// If the parameter is indirect, we need to drop the value into
6522-
// temporary memory.
6520+
// If the parameter is indirect, we'll need to drop the value into
6521+
// temporary memory. Make a copy scoped to the current formal access that
6522+
// we can materialize later.
65236523
if (SGF.silConv.isSILIndirect(selfParam)) {
65246524
// It's a really bad idea to materialize when we're about to
65256525
// pass a value to an inout argument, because it's a really easy
@@ -6529,14 +6529,17 @@ ArgumentSource AccessorBaseArgPreparer::prepareAccessorObjectBaseArg() {
65296529
// itself).
65306530
assert(!selfParam.isIndirectMutating() &&
65316531
"passing unmaterialized r-value as inout argument");
6532-
6533-
base = base.formallyMaterialize(SGF, loc);
6534-
auto shouldTake = IsTake_t(base.hasCleanup());
6535-
base = SGF.emitFormalAccessLoad(loc, base.forward(SGF),
6536-
SGF.getTypeLowering(baseLoweredType),
6537-
SGFContext(), shouldTake);
6532+
6533+
// Avoid copying the base if it's move-only. It should be OK to do in this
6534+
// case since the move-only base will be accessed in a formal access scope
6535+
// as well. This isn't always true for copyable bases so be less aggressive
6536+
// in that case.
6537+
if (base.getType().isMoveOnly()) {
6538+
base = base.formalAccessBorrow(SGF, loc);
6539+
} else {
6540+
base = base.formalAccessCopy(SGF, loc);
6541+
}
65386542
}
6539-
65406543
return ArgumentSource(loc, RValue(SGF, loc, baseFormalType, base));
65416544
}
65426545

‎lib/SILGen/SILGenLValue.cpp

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2935,10 +2935,7 @@ class LLVM_LIBRARY_VISIBILITY SILGenBorrowedBaseVisitor
29352935
SILGenBorrowedBaseVisitor(SILGenLValue &SGL, SILGenFunction &SGF)
29362936
: SGL(SGL), SGF(SGF) {}
29372937

2938-
/// Returns the subexpr
29392938
static bool isNonCopyableBaseBorrow(SILGenFunction &SGF, Expr *e) {
2940-
if (auto *le = dyn_cast<LoadExpr>(e))
2941-
return le->getType()->isPureMoveOnly();
29422939
if (auto *m = dyn_cast<MemberRefExpr>(e)) {
29432940
// If our m is a pure noncopyable type or our base is, we need to perform
29442941
// a noncopyable base borrow.
@@ -2949,8 +2946,39 @@ class LLVM_LIBRARY_VISIBILITY SILGenBorrowedBaseVisitor
29492946
return m->getType()->isPureMoveOnly() ||
29502947
m->getBase()->getType()->isPureMoveOnly();
29512948
}
2952-
if (auto *d = dyn_cast<DeclRefExpr>(e))
2953-
return e->getType()->isPureMoveOnly();
2949+
2950+
if (auto *le = dyn_cast<LoadExpr>(e)) {
2951+
// Noncopyable type is obviously noncopyable.
2952+
if (le->getType()->isPureMoveOnly()) {
2953+
return true;
2954+
}
2955+
// Otherwise, check if the thing we're loading from is a noncopyable
2956+
// param decl.
2957+
e = le->getSubExpr();
2958+
// fall through...
2959+
}
2960+
2961+
if (auto *de = dyn_cast<DeclRefExpr>(e)) {
2962+
// Noncopyable type is obviously noncopyable.
2963+
if (de->getType()->isPureMoveOnly()) {
2964+
return true;
2965+
}
2966+
// If the decl ref refers to a parameter with an explicit ownership
2967+
// modifier, it is not implicitly copyable.
2968+
if (auto pd = dyn_cast<ParamDecl>(de->getDecl())) {
2969+
switch (pd->getSpecifier()) {
2970+
case ParamSpecifier::Borrowing:
2971+
case ParamSpecifier::Consuming:
2972+
return true;
2973+
case ParamSpecifier::Default:
2974+
case ParamSpecifier::InOut:
2975+
case ParamSpecifier::LegacyShared:
2976+
case ParamSpecifier::LegacyOwned:
2977+
return false;
2978+
}
2979+
llvm_unreachable("unhandled switch case");
2980+
}
2981+
}
29542982
return false;
29552983
}
29562984

‎lib/SILOptimizer/Mandatory/ClosureLifetimeFixup.cpp

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -725,6 +725,9 @@ static SILValue tryRewriteToPartialApplyStack(
725725

726726
SmallSetVector<SILValue, 4> borrowedOriginals;
727727

728+
unsigned appliedArgStartIdx =
729+
newPA->getOrigCalleeType()->getNumParameters() - newPA->getNumArguments();
730+
728731
for (unsigned i : indices(newPA->getArgumentOperands())) {
729732
auto &arg = newPA->getArgumentOperands()[i];
730733
SILValue copy = arg.get();
@@ -747,11 +750,12 @@ static SILValue tryRewriteToPartialApplyStack(
747750
}
748751

749752
// Is the capture a borrow?
750-
auto paramIndex = newPA
751-
->getArgumentIndexForOperandIndex(i + newPA->getArgumentOperandNumber())
752-
.value();
753-
if (!newPA->getOrigCalleeType()->getParameters()[paramIndex]
754-
.isIndirectInGuaranteed()) {
753+
754+
auto paramIndex = i + appliedArgStartIdx;
755+
auto param = newPA->getOrigCalleeType()->getParameters()[paramIndex];
756+
LLVM_DEBUG(param.print(llvm::dbgs());
757+
llvm::dbgs() << '\n');
758+
if (!param.isIndirectInGuaranteed()) {
755759
LLVM_DEBUG(llvm::dbgs() << "-- not an in_guaranteed parameter\n";
756760
newPA->getOrigCalleeType()->getParameters()[paramIndex]
757761
.print(llvm::dbgs());

‎lib/SILOptimizer/Mandatory/DiagnoseLifetimeIssues.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,8 @@ visitUses(SILValue def, bool updateLivenessAndWeakStores, int callDepth) {
189189
// Try to get information from the called function.
190190
switch (getArgumentState(ai, use, callDepth)) {
191191
case DoesNotEscape:
192+
if (updateLivenessAndWeakStores)
193+
liveness->updateForUse(user, /*lifetimeEnding*/ false);
192194
break;
193195
case CanEscape:
194196
return CanEscape;

0 commit comments

Comments
 (0)