Skip to content

[5.10] Enable noncopyable resilient types. #69287

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/swift/Basic/Features.def
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ LANGUAGE_FEATURE(
LANGUAGE_FEATURE(AttachedMacros, 389, "Attached macros", hasSwiftSwiftParser)
LANGUAGE_FEATURE(ExtensionMacros, 402, "Extension macros", hasSwiftSwiftParser)
LANGUAGE_FEATURE(MoveOnly, 390, "noncopyable types", true)
LANGUAGE_FEATURE(MoveOnlyResilientTypes, 390, "non-@frozen noncopyable types with library evolution", true)
LANGUAGE_FEATURE(ParameterPacks, 393, "Value and type parameter packs", true)
SUPPRESSIBLE_LANGUAGE_FEATURE(LexicalLifetimes, 0, "@_eagerMove/@_noEagerMove/@_lexicalLifetimes annotations", true)
LANGUAGE_FEATURE(FreestandingMacros, 397, "freestanding declaration macros", true)
Expand Down Expand Up @@ -140,7 +141,6 @@ EXPERIMENTAL_FEATURE(NoImplicitCopy, true)
EXPERIMENTAL_FEATURE(OldOwnershipOperatorSpellings, true)
EXPERIMENTAL_FEATURE(MoveOnlyEnumDeinits, true)
EXPERIMENTAL_FEATURE(MoveOnlyTuples, true)
EXPERIMENTAL_FEATURE(MoveOnlyResilientTypes, true)
EXPERIMENTAL_FEATURE(MoveOnlyPartialConsumption, true)

EXPERIMENTAL_FEATURE(OneWayClosureParameters, false)
Expand Down
15 changes: 6 additions & 9 deletions include/swift/SIL/OwnershipUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ class BorrowingOperandKind {
enum Kind : uint8_t {
Invalid = 0,
BeginBorrow,
StoreBorrow,
BeginApply,
Branch,
Apply,
Expand All @@ -277,6 +278,8 @@ class BorrowingOperandKind {
return Kind::Invalid;
case SILInstructionKind::BeginBorrowInst:
return Kind::BeginBorrow;
case SILInstructionKind::StoreBorrowInst:
return Kind::StoreBorrow;
case SILInstructionKind::BeginApplyInst:
return Kind::BeginApply;
case SILInstructionKind::BranchInst:
Expand Down Expand Up @@ -386,6 +389,7 @@ struct BorrowingOperand {
case BorrowingOperandKind::Invalid:
llvm_unreachable("Using invalid case?!");
case BorrowingOperandKind::BeginBorrow:
case BorrowingOperandKind::StoreBorrow:
case BorrowingOperandKind::BeginApply:
case BorrowingOperandKind::Apply:
case BorrowingOperandKind::TryApply:
Expand Down Expand Up @@ -418,6 +422,7 @@ struct BorrowingOperand {
case BorrowingOperandKind::BeginBorrow:
case BorrowingOperandKind::Branch:
return true;
case BorrowingOperandKind::StoreBorrow:
case BorrowingOperandKind::BeginApply:
case BorrowingOperandKind::Apply:
case BorrowingOperandKind::TryApply:
Expand Down Expand Up @@ -790,7 +795,6 @@ class InteriorPointerOperandKind {
RefTailAddr,
OpenExistentialBox,
ProjectBox,
StoreBorrow,
};

private:
Expand Down Expand Up @@ -819,8 +823,6 @@ class InteriorPointerOperandKind {
return Kind::OpenExistentialBox;
case SILInstructionKind::ProjectBoxInst:
return Kind::ProjectBox;
case SILInstructionKind::StoreBorrowInst:
return Kind::StoreBorrow;
}
}

Expand All @@ -839,8 +841,6 @@ class InteriorPointerOperandKind {
return Kind::OpenExistentialBox;
case ValueKind::ProjectBoxInst:
return Kind::ProjectBox;
case ValueKind::StoreBorrowInst:
return Kind::StoreBorrow;
}
}

Expand Down Expand Up @@ -897,8 +897,7 @@ struct InteriorPointerOperand {
case InteriorPointerOperandKind::RefElementAddr:
case InteriorPointerOperandKind::RefTailAddr:
case InteriorPointerOperandKind::OpenExistentialBox:
case InteriorPointerOperandKind::ProjectBox:
case InteriorPointerOperandKind::StoreBorrow: {
case InteriorPointerOperandKind::ProjectBox: {
// Ok, we have a valid instruction. Return the relevant operand.
auto *op =
&cast<SingleValueInstruction>(resultValue)->getAllOperands()[0];
Expand Down Expand Up @@ -941,8 +940,6 @@ struct InteriorPointerOperand {
return cast<OpenExistentialBoxInst>(operand->getUser());
case InteriorPointerOperandKind::ProjectBox:
return cast<ProjectBoxInst>(operand->getUser());
case InteriorPointerOperandKind::StoreBorrow:
return cast<StoreBorrowInst>(operand->getUser());
}
llvm_unreachable("Covered switch isn't covered?!");
}
Expand Down
2 changes: 1 addition & 1 deletion lib/SIL/IR/OperandOwnership.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ OperandOwnership OperandOwnershipClassifier::visitBranchInst(BranchInst *bi) {
OperandOwnership
OperandOwnershipClassifier::visitStoreBorrowInst(StoreBorrowInst *i) {
if (getValue() == i->getSrc()) {
return OperandOwnership::InteriorPointer;
return OperandOwnership::Borrow;
}
return OperandOwnership::TrivialUse;
}
Expand Down
20 changes: 20 additions & 0 deletions lib/SIL/Utils/OwnershipUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -600,6 +600,9 @@ void BorrowingOperandKind::print(llvm::raw_ostream &os) const {
case Kind::BeginBorrow:
os << "BeginBorrow";
return;
case Kind::StoreBorrow:
os << "StoreBorrow";
return;
case Kind::BeginApply:
os << "BeginApply";
return;
Expand Down Expand Up @@ -649,6 +652,7 @@ bool BorrowingOperand::hasEmptyRequiredEndingUses() const {
case BorrowingOperandKind::Invalid:
llvm_unreachable("Using invalid case");
case BorrowingOperandKind::BeginBorrow:
case BorrowingOperandKind::StoreBorrow:
case BorrowingOperandKind::BeginApply:
case BorrowingOperandKind::BeginAsyncLet:
case BorrowingOperandKind::PartialApplyStack: {
Expand Down Expand Up @@ -687,6 +691,20 @@ bool BorrowingOperand::visitScopeEndingUses(
// false.
return !deadBorrow;
}
case BorrowingOperandKind::StoreBorrow: {
bool deadBorrow = true;
for (auto *use : cast<StoreBorrowInst>(op->getUser())->getUses()) {
if (isa<EndBorrowInst>(use->getUser())) {
deadBorrow = false;
if (!func(use))
return false;
}
}
// FIXME: special case for dead borrows. This is dangerous because clients
// only expect visitScopeEndingUses to return false if the visitor returned
// false.
return !deadBorrow;
}
case BorrowingOperandKind::BeginApply: {
bool deadApply = true;
auto *user = cast<BeginApplyInst>(op->getUser());
Expand Down Expand Up @@ -763,6 +781,7 @@ BorrowedValue BorrowingOperand::getBorrowIntroducingUserResult() const {
case BorrowingOperandKind::Yield:
case BorrowingOperandKind::PartialApplyStack:
case BorrowingOperandKind::BeginAsyncLet:
case BorrowingOperandKind::StoreBorrow:
return BorrowedValue();

case BorrowingOperandKind::BeginBorrow: {
Expand Down Expand Up @@ -792,6 +811,7 @@ SILValue BorrowingOperand::getScopeIntroducingUserResult() {
case BorrowingOperandKind::BeginAsyncLet:
case BorrowingOperandKind::PartialApplyStack:
case BorrowingOperandKind::BeginBorrow:
case BorrowingOperandKind::StoreBorrow:
return cast<SingleValueInstruction>(op->getUser());

case BorrowingOperandKind::BeginApply:
Expand Down
23 changes: 13 additions & 10 deletions lib/SILGen/SILGenApply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6517,9 +6517,9 @@ ArgumentSource AccessorBaseArgPreparer::prepareAccessorObjectBaseArg() {
if (selfParam.isConsumed() && !base.hasCleanup()) {
base = base.copyUnmanaged(SGF, loc);
}

// If the parameter is indirect, we need to drop the value into
// temporary memory.
// If the parameter is indirect, we'll need to drop the value into
// temporary memory. Make a copy scoped to the current formal access that
// we can materialize later.
if (SGF.silConv.isSILIndirect(selfParam)) {
// It's a really bad idea to materialize when we're about to
// pass a value to an inout argument, because it's a really easy
Expand All @@ -6529,14 +6529,17 @@ ArgumentSource AccessorBaseArgPreparer::prepareAccessorObjectBaseArg() {
// itself).
assert(!selfParam.isIndirectMutating() &&
"passing unmaterialized r-value as inout argument");

base = base.formallyMaterialize(SGF, loc);
auto shouldTake = IsTake_t(base.hasCleanup());
base = SGF.emitFormalAccessLoad(loc, base.forward(SGF),
SGF.getTypeLowering(baseLoweredType),
SGFContext(), shouldTake);

// Avoid copying the base if it's move-only. It should be OK to do in this
// case since the move-only base will be accessed in a formal access scope
// as well. This isn't always true for copyable bases so be less aggressive
// in that case.
if (base.getType().isMoveOnly()) {
base = base.formalAccessBorrow(SGF, loc);
} else {
base = base.formalAccessCopy(SGF, loc);
}
}

return ArgumentSource(loc, RValue(SGF, loc, baseFormalType, base));
}

Expand Down
38 changes: 33 additions & 5 deletions lib/SILGen/SILGenLValue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2935,10 +2935,7 @@ class LLVM_LIBRARY_VISIBILITY SILGenBorrowedBaseVisitor
SILGenBorrowedBaseVisitor(SILGenLValue &SGL, SILGenFunction &SGF)
: SGL(SGL), SGF(SGF) {}

/// Returns the subexpr
static bool isNonCopyableBaseBorrow(SILGenFunction &SGF, Expr *e) {
if (auto *le = dyn_cast<LoadExpr>(e))
return le->getType()->isPureMoveOnly();
if (auto *m = dyn_cast<MemberRefExpr>(e)) {
// If our m is a pure noncopyable type or our base is, we need to perform
// a noncopyable base borrow.
Expand All @@ -2949,8 +2946,39 @@ class LLVM_LIBRARY_VISIBILITY SILGenBorrowedBaseVisitor
return m->getType()->isPureMoveOnly() ||
m->getBase()->getType()->isPureMoveOnly();
}
if (auto *d = dyn_cast<DeclRefExpr>(e))
return e->getType()->isPureMoveOnly();

if (auto *le = dyn_cast<LoadExpr>(e)) {
// Noncopyable type is obviously noncopyable.
if (le->getType()->isPureMoveOnly()) {
return true;
}
// Otherwise, check if the thing we're loading from is a noncopyable
// param decl.
e = le->getSubExpr();
// fall through...
}

if (auto *de = dyn_cast<DeclRefExpr>(e)) {
// Noncopyable type is obviously noncopyable.
if (de->getType()->isPureMoveOnly()) {
return true;
}
// If the decl ref refers to a parameter with an explicit ownership
// modifier, it is not implicitly copyable.
if (auto pd = dyn_cast<ParamDecl>(de->getDecl())) {
switch (pd->getSpecifier()) {
case ParamSpecifier::Borrowing:
case ParamSpecifier::Consuming:
return true;
case ParamSpecifier::Default:
case ParamSpecifier::InOut:
case ParamSpecifier::LegacyShared:
case ParamSpecifier::LegacyOwned:
return false;
}
llvm_unreachable("unhandled switch case");
}
}
return false;
}

Expand Down
14 changes: 9 additions & 5 deletions lib/SILOptimizer/Mandatory/ClosureLifetimeFixup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -725,6 +725,9 @@ static SILValue tryRewriteToPartialApplyStack(

SmallSetVector<SILValue, 4> borrowedOriginals;

unsigned appliedArgStartIdx =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't fix this in this commit, but I think this is automatically fixed for you if you use callee conventions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, the API is actually on ApplySite.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's somewhere around here: https://github.com/apple/swift/blob/main/include/swift/SIL/ApplySite.h#L341. If there isn't an exact one for what you need, I would add it here. We do not want people to muck around with indices for partial apply since it is pretty easy to mess up (as you see here)

newPA->getOrigCalleeType()->getNumParameters() - newPA->getNumArguments();

for (unsigned i : indices(newPA->getArgumentOperands())) {
auto &arg = newPA->getArgumentOperands()[i];
SILValue copy = arg.get();
Expand All @@ -747,11 +750,12 @@ static SILValue tryRewriteToPartialApplyStack(
}

// Is the capture a borrow?
auto paramIndex = newPA
->getArgumentIndexForOperandIndex(i + newPA->getArgumentOperandNumber())
.value();
if (!newPA->getOrigCalleeType()->getParameters()[paramIndex]
.isIndirectInGuaranteed()) {

auto paramIndex = i + appliedArgStartIdx;
auto param = newPA->getOrigCalleeType()->getParameters()[paramIndex];
LLVM_DEBUG(param.print(llvm::dbgs());
llvm::dbgs() << '\n');
if (!param.isIndirectInGuaranteed()) {
LLVM_DEBUG(llvm::dbgs() << "-- not an in_guaranteed parameter\n";
newPA->getOrigCalleeType()->getParameters()[paramIndex]
.print(llvm::dbgs());
Expand Down
2 changes: 2 additions & 0 deletions lib/SILOptimizer/Mandatory/DiagnoseLifetimeIssues.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,8 @@ visitUses(SILValue def, bool updateLivenessAndWeakStores, int callDepth) {
// Try to get information from the called function.
switch (getArgumentState(ai, use, callDepth)) {
case DoesNotEscape:
if (updateLivenessAndWeakStores)
liveness->updateForUse(user, /*lifetimeEnding*/ false);
break;
case CanEscape:
return CanEscape;
Expand Down
Loading