Skip to content

Commit a8a44eb

Browse files
kavonkavon
authored
Merge pull request #64106 from kavon/enable-moveonly-by-default
Enable moveonly / noncopyable types by default
2 parents 78a406a + 2c7d9a5 commit a8a44eb

File tree

89 files changed

+276
-230
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

89 files changed

+276
-230
lines changed

‎include/swift/AST/ASTContext.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -698,6 +698,9 @@ class ASTContext final {
698698
FuncDecl *getMakeInvocationEncoderOnDistributedActorSystem(
699699
AbstractFunctionDecl *thunk) const;
700700

701+
/// Indicates whether move-only / noncopyable types are supported.
702+
bool supportsMoveOnlyTypes() const;
703+
701704
// Retrieve the declaration of
702705
// DistributedInvocationEncoder.recordGenericSubstitution(_:).
703706
//

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6761,11 +6761,6 @@ ERROR(experimental_moveonly_feature_can_only_be_used_when_enabled,
67616761
none, "Can not use feature when experimental move only is disabled! Pass"
67626762
" the frontend flag -enable-experimental-move-only to swift to enable "
67636763
"the usage of this language feature", ())
6764-
ERROR(experimental_moveonly_feature_can_only_be_imported_when_enabled,
6765-
none, "Can not import module %0 that uses move only features when "
6766-
"experimental move only is disabled! Pass the frontend flag "
6767-
"-enable-experimental-move-only to swift to enable the usage of this "
6768-
"language feature", (Identifier))
67696764
ERROR(noimplicitcopy_attr_valid_only_on_local_let_params,
67706765
none, "'@_noImplicitCopy' attribute can only be applied to local lets and params", ())
67716766
ERROR(noimplicitcopy_attr_invalid_in_generic_context,
@@ -6848,6 +6843,10 @@ ERROR(move_expression_not_passed_lvalue,none,
68486843
ERROR(borrow_expression_not_passed_lvalue,none,
68496844
"'borrow' can only be applied to lvalues", ())
68506845

6846+
ERROR(moveOnly_requires_lexical_lifetimes,none,
6847+
"noncopyable types require lexical borrow scopes "
6848+
"(add -enable-lexical-borrow-scopes=true)", ())
6849+
68516850
//------------------------------------------------------------------------------
68526851
// MARK: #_hasSymbol
68536852
//------------------------------------------------------------------------------

‎include/swift/SIL/SILCloner.h

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1484,11 +1484,20 @@ template <typename ImplClass>
14841484
void SILCloner<ImplClass>::visitExplicitCopyAddrInst(
14851485
ExplicitCopyAddrInst *Inst) {
14861486
getBuilder().setCurrentDebugScope(getOpScope(Inst->getDebugScope()));
1487-
recordClonedInstruction(
1488-
Inst, getBuilder().createExplicitCopyAddr(
1489-
getOpLocation(Inst->getLoc()), getOpValue(Inst->getSrc()),
1490-
getOpValue(Inst->getDest()), Inst->isTakeOfSrc(),
1491-
Inst->isInitializationOfDest()));
1487+
if (!getBuilder().hasOwnership()) {
1488+
recordClonedInstruction(
1489+
Inst, getBuilder().createCopyAddr(
1490+
getOpLocation(Inst->getLoc()), getOpValue(Inst->getSrc()),
1491+
getOpValue(Inst->getDest()), Inst->isTakeOfSrc(),
1492+
Inst->isInitializationOfDest()));
1493+
} else {
1494+
// preserve the explicit_*
1495+
recordClonedInstruction(
1496+
Inst, getBuilder().createExplicitCopyAddr(
1497+
getOpLocation(Inst->getLoc()), getOpValue(Inst->getSrc()),
1498+
getOpValue(Inst->getDest()), Inst->isTakeOfSrc(),
1499+
Inst->isInitializationOfDest()));
1500+
}
14921501
}
14931502

14941503
template <typename ImplClass>

‎lib/AST/ASTContext.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6370,3 +6370,8 @@ ASTContext::lookupExecutablePluginByModuleName(Identifier moduleName) {
63706370

63716371
return plugin.get();
63726372
}
6373+
6374+
bool ASTContext::supportsMoveOnlyTypes() const {
6375+
// currently the only thing holding back whether the types can appear is this.
6376+
return SILOpts.LexicalLifetimes != LexicalLifetimesOption::Off;
6377+
}

‎lib/SIL/Verifier/SILVerifier.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2822,6 +2822,18 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
28222822
"'MoveOnly' types can only be copied in Raw SIL?!");
28232823
}
28242824

2825+
void checkExplicitCopyAddrInst(ExplicitCopyAddrInst *ecai) {
2826+
require(F.hasOwnership(), "explicit_copy_* is only valid in OSSA.");
2827+
require(ecai->getSrc()->getType().isAddress(),
2828+
"Src value should be lvalue");
2829+
require(ecai->getDest()->getType().isAddress(),
2830+
"Dest address should be lvalue");
2831+
requireSameType(ecai->getDest()->getType(), ecai->getSrc()->getType(),
2832+
"Store operand type and dest type mismatch");
2833+
require(F.isTypeABIAccessible(ecai->getDest()->getType()),
2834+
"cannot directly copy type with inaccessible ABI");
2835+
}
2836+
28252837
void checkMarkUnresolvedMoveAddrInst(MarkUnresolvedMoveAddrInst *SI) {
28262838
require(F.hasOwnership(), "Only valid in OSSA.");
28272839
require(F.getModule().getStage() == SILStage::Raw, "Only valid in Raw SIL");
@@ -2861,6 +2873,14 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
28612873
"'MoveOnly' types can only be copied in Raw SIL?!");
28622874
}
28632875

2876+
void checkExplicitCopyValueInst(ExplicitCopyValueInst *I) {
2877+
require(F.hasOwnership(), "explicit_copy_* is only valid in OSSA.");
2878+
require(I->getOperand()->getType().isObject(),
2879+
"Source value should be an object value");
2880+
require(!I->getOperand()->getType().isTrivial(*I->getFunction()),
2881+
"Source value should be non-trivial");
2882+
}
2883+
28642884
void checkDestroyValueInst(DestroyValueInst *I) {
28652885
require(I->getOperand()->getType().isObject(),
28662886
"Source value should be an object value");

‎lib/SILGen/SILGenDecl.cpp

Lines changed: 29 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -620,8 +620,7 @@ class LetValueInitialization : public Initialization {
620620
public:
621621
LetValueInitialization(VarDecl *vd, SILGenFunction &SGF) : vd(vd) {
622622
const TypeLowering *lowering = nullptr;
623-
if (SGF.getASTContext().LangOpts.Features.count(Feature::MoveOnly) &&
624-
vd->isNoImplicitCopy()) {
623+
if (vd->isNoImplicitCopy()) {
625624
lowering = &SGF.getTypeLowering(
626625
SILMoveOnlyWrappedType::get(vd->getType()->getCanonicalType()));
627626
} else {
@@ -661,8 +660,7 @@ class LetValueInitialization : public Initialization {
661660

662661
// Make sure that we have a non-address only type when binding a
663662
// @_noImplicitCopy let.
664-
if (SGF.getASTContext().LangOpts.Features.count(Feature::MoveOnly) &&
665-
lowering->isAddressOnly() && vd->isNoImplicitCopy()) {
663+
if (lowering->isAddressOnly() && vd->isNoImplicitCopy()) {
666664
auto d = diag::noimplicitcopy_used_on_generic_or_existential;
667665
diagnose(SGF.getASTContext(), vd->getLoc(), d);
668666
}
@@ -740,10 +738,6 @@ class LetValueInitialization : public Initialization {
740738
SILValue value, bool wasPlusOne) {
741739
// If we have none...
742740
if (value->getOwnershipKind() == OwnershipKind::None) {
743-
// If we don't have move only features enabled, just return, we are done.
744-
if (!SGF.getASTContext().LangOpts.Features.count(Feature::MoveOnly))
745-
return value;
746-
747741
// Then check if we have a pure move only type. In that case, we need to
748742
// insert a no implicit copy
749743
if (value->getType().isPureMoveOnly()) {
@@ -768,15 +762,6 @@ class LetValueInitialization : public Initialization {
768762
MarkMustCheckInst::CheckKind::ConsumableAndAssignable);
769763
}
770764

771-
// Then if we don't have move only, just perform a lexical borrow if the
772-
// lifetime is lexical.
773-
if (!SGF.getASTContext().LangOpts.Features.count(Feature::MoveOnly)) {
774-
if (SGF.F.getLifetime(vd, value->getType()).isLexical())
775-
return SGF.B.createBeginBorrow(PrologueLoc, value, /*isLexical*/ true);
776-
else
777-
return value;
778-
}
779-
780765
// Otherwise, we need to perform some additional processing. First, if we
781766
// have an owned moveonly value that had a cleanup, then create a move_value
782767
// that acts as a consuming use of the value. The reason why we want this is
@@ -2136,35 +2121,22 @@ void SILGenFunction::destroyLocalVariable(SILLocation silLoc, VarDecl *vd) {
21362121
return;
21372122
}
21382123

2139-
if (getASTContext().LangOpts.hasFeature(Feature::MoveOnly)) {
2140-
if (auto *mvi = dyn_cast<MarkMustCheckInst>(Val.getDefiningInstruction())) {
2141-
if (mvi->hasMoveCheckerKind()) {
2142-
if (auto *cvi = dyn_cast<CopyValueInst>(mvi->getOperand())) {
2143-
if (auto *bbi = dyn_cast<BeginBorrowInst>(cvi->getOperand())) {
2144-
if (bbi->isLexical()) {
2145-
B.emitDestroyValueOperation(silLoc, mvi);
2146-
B.createEndBorrow(silLoc, bbi);
2147-
B.emitDestroyValueOperation(silLoc, bbi->getOperand());
2148-
return;
2149-
}
2150-
}
2151-
}
2152-
2153-
if (auto *copyToMove = dyn_cast<CopyableToMoveOnlyWrapperValueInst>(
2154-
mvi->getOperand())) {
2155-
if (auto *cvi = dyn_cast<CopyValueInst>(copyToMove->getOperand())) {
2156-
if (auto *bbi = dyn_cast<BeginBorrowInst>(cvi->getOperand())) {
2157-
if (bbi->isLexical()) {
2158-
B.emitDestroyValueOperation(silLoc, mvi);
2159-
B.createEndBorrow(silLoc, bbi);
2160-
B.emitDestroyValueOperation(silLoc, bbi->getOperand());
2161-
return;
2162-
}
2163-
}
2124+
if (auto *mvi = dyn_cast<MarkMustCheckInst>(Val.getDefiningInstruction())) {
2125+
if (mvi->hasMoveCheckerKind()) {
2126+
if (auto *cvi = dyn_cast<CopyValueInst>(mvi->getOperand())) {
2127+
if (auto *bbi = dyn_cast<BeginBorrowInst>(cvi->getOperand())) {
2128+
if (bbi->isLexical()) {
2129+
B.emitDestroyValueOperation(silLoc, mvi);
2130+
B.createEndBorrow(silLoc, bbi);
2131+
B.emitDestroyValueOperation(silLoc, bbi->getOperand());
2132+
return;
21642133
}
21652134
}
2135+
}
21662136

2167-
if (auto *cvi = dyn_cast<ExplicitCopyValueInst>(mvi->getOperand())) {
2137+
if (auto *copyToMove = dyn_cast<CopyableToMoveOnlyWrapperValueInst>(
2138+
mvi->getOperand())) {
2139+
if (auto *cvi = dyn_cast<CopyValueInst>(copyToMove->getOperand())) {
21682140
if (auto *bbi = dyn_cast<BeginBorrowInst>(cvi->getOperand())) {
21692141
if (bbi->isLexical()) {
21702142
B.emitDestroyValueOperation(silLoc, mvi);
@@ -2174,15 +2146,26 @@ void SILGenFunction::destroyLocalVariable(SILLocation silLoc, VarDecl *vd) {
21742146
}
21752147
}
21762148
}
2149+
}
21772150

2178-
// Handle trivial arguments.
2179-
if (auto *move = dyn_cast<MoveValueInst>(mvi->getOperand())) {
2180-
if (move->isLexical()) {
2151+
if (auto *cvi = dyn_cast<ExplicitCopyValueInst>(mvi->getOperand())) {
2152+
if (auto *bbi = dyn_cast<BeginBorrowInst>(cvi->getOperand())) {
2153+
if (bbi->isLexical()) {
21812154
B.emitDestroyValueOperation(silLoc, mvi);
2155+
B.createEndBorrow(silLoc, bbi);
2156+
B.emitDestroyValueOperation(silLoc, bbi->getOperand());
21822157
return;
21832158
}
21842159
}
21852160
}
2161+
2162+
// Handle trivial arguments.
2163+
if (auto *move = dyn_cast<MoveValueInst>(mvi->getOperand())) {
2164+
if (move->isLexical()) {
2165+
B.emitDestroyValueOperation(silLoc, mvi);
2166+
return;
2167+
}
2168+
}
21862169
}
21872170
}
21882171

‎lib/SILOptimizer/Mandatory/ConsumeOperatorCopyableAddressesChecker.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2400,7 +2400,7 @@ class ConsumeOperatorCopyableAddressesCheckerPass
24002400
auto &astContext = fn->getASTContext();
24012401

24022402
// Only run this pass if the move only language feature is enabled.
2403-
if (!astContext.LangOpts.Features.contains(Feature::MoveOnly))
2403+
if (!astContext.supportsMoveOnlyTypes())
24042404
return;
24052405

24062406
// Don't rerun diagnostics on deserialized functions.

‎lib/SILOptimizer/Mandatory/ConsumeOperatorCopyableValuesChecker.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -527,7 +527,7 @@ class ConsumeOperatorCopyableValuesCheckerPass : public SILFunctionTransform {
527527
auto *fn = getFunction();
528528

529529
// Only run this pass if the move only language feature is enabled.
530-
if (!fn->getASTContext().LangOpts.Features.contains(Feature::MoveOnly))
530+
if (!fn->getASTContext().supportsMoveOnlyTypes())
531531
return;
532532

533533
// Don't rerun diagnostics on deserialized functions.

‎lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerTester.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ class MoveOnlyAddressCheckerTesterPass : public SILFunctionTransform {
7979
auto *fn = getFunction();
8080

8181
// Only run this pass if the move only language feature is enabled.
82-
if (!fn->getASTContext().LangOpts.Features.contains(Feature::MoveOnly))
82+
if (!fn->getASTContext().supportsMoveOnlyTypes())
8383
return;
8484

8585
// Don't rerun diagnostics on deserialized functions.

‎lib/SILOptimizer/Mandatory/MoveOnlyBorrowToDestructureTester.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ class MoveOnlyBorrowToDestructureTransformPass : public SILFunctionTransform {
7272
auto *fn = getFunction();
7373

7474
// Only run this pass if the move only language feature is enabled.
75-
if (!fn->getASTContext().LangOpts.Features.contains(Feature::MoveOnly))
75+
if (!fn->getASTContext().supportsMoveOnlyTypes())
7676
return;
7777

7878
// Don't rerun diagnostics on deserialized functions.

0 commit comments

Comments
 (0)