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.

‎lib/SILOptimizer/Mandatory/MoveOnlyChecker.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ class MoveOnlyCheckerPass : public SILFunctionTransform {
147147
auto *fn = getFunction();
148148

149149
// Only run this pass if the move only language feature is enabled.
150-
if (!fn->getASTContext().LangOpts.Features.contains(Feature::MoveOnly))
150+
if (!fn->getASTContext().supportsMoveOnlyTypes())
151151
return;
152152

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

‎lib/SILOptimizer/Mandatory/MoveOnlyObjectCheckerTester.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ class MoveOnlyObjectCheckerTesterPass : public SILFunctionTransform {
7676
auto *fn = getFunction();
7777

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

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

‎lib/SILOptimizer/Mandatory/MoveOnlyUtils.cpp

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -116,9 +116,12 @@ bool swift::siloptimizer::cleanupNonCopyableCopiesAfterEmittingDiagnostic(
116116
auto *inst = &*ii;
117117
++ii;
118118

119-
// Convert load [copy] -> load_borrow + explicit_copy_value.
119+
// Convert load [copy] *MoveOnly -> load_borrow + explicit_copy_value.
120120
if (auto *li = dyn_cast<LoadInst>(inst)) {
121121
if (li->getOwnershipQualifier() == LoadOwnershipQualifier::Copy) {
122+
if (!li->getType().isMoveOnly())
123+
continue;
124+
122125
SILBuilderWithScope builder(li);
123126
auto *lbi = builder.createLoadBorrow(li->getLoc(), li->getOperand());
124127
auto *cvi = builder.createExplicitCopyValue(li->getLoc(), lbi);
@@ -129,10 +132,13 @@ bool swift::siloptimizer::cleanupNonCopyableCopiesAfterEmittingDiagnostic(
129132
}
130133
}
131134

132-
// Convert copy_addr !take of src to its explicit value form so we don't
133-
// error.
135+
// Convert copy_addr !take MoveOnly ... -> explicit_copy_addr ...same...
136+
// so we don't error.
134137
if (auto *copyAddr = dyn_cast<CopyAddrInst>(inst)) {
135138
if (!copyAddr->isTakeOfSrc()) {
139+
if (!copyAddr->getSrc()->getType().isMoveOnly())
140+
continue;
141+
136142
SILBuilderWithScope builder(copyAddr);
137143
builder.createExplicitCopyAddr(
138144
copyAddr->getLoc(), copyAddr->getSrc(), copyAddr->getDest(),
@@ -143,10 +149,11 @@ bool swift::siloptimizer::cleanupNonCopyableCopiesAfterEmittingDiagnostic(
143149
}
144150
}
145151

146-
// Convert any copy_value of move_only type to explicit copy value.
152+
// Convert any copy_value of MoveOnly type -> explicit_copy_value.
147153
if (auto *cvi = dyn_cast<CopyValueInst>(inst)) {
148154
if (!cvi->getOperand()->getType().isMoveOnly())
149155
continue;
156+
150157
SILBuilderWithScope b(cvi);
151158
auto *expCopy =
152159
b.createExplicitCopyValue(cvi->getLoc(), cvi->getOperand());

‎lib/Sema/MiscDiagnostics.cpp

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -428,31 +428,13 @@ static void diagSyntacticUseRestrictions(const Expr *E, const DeclContext *DC,
428428
}
429429

430430
void checkMoveExpr(MoveExpr *moveExpr) {
431-
// Make sure the MoveOnly feature is set. If not, error.
432-
// This should not currently be reached because the parse should ignore
433-
// the _move keyword unless the feature flag is set.
434-
if (!Ctx.LangOpts.hasFeature(Feature::MoveOnly)) {
435-
auto error =
436-
diag::experimental_moveonly_feature_can_only_be_used_when_enabled;
437-
Ctx.Diags.diagnose(moveExpr->getLoc(), error);
438-
}
439-
440431
if (!isa<DeclRefExpr>(moveExpr->getSubExpr())) {
441432
Ctx.Diags.diagnose(moveExpr->getLoc(),
442433
diag::move_expression_not_passed_lvalue);
443434
}
444435
}
445436

446437
void checkBorrowExpr(BorrowExpr *borrowExpr) {
447-
// Make sure the MoveOnly feature is set. If not, error.
448-
// This should not currently be reached because the parse should ignore
449-
// the _move keyword unless the feature flag is set.
450-
if (!Ctx.LangOpts.hasFeature(Feature::MoveOnly)) {
451-
auto error =
452-
diag::experimental_moveonly_feature_can_only_be_used_when_enabled;
453-
Ctx.Diags.diagnose(borrowExpr->getLoc(), error);
454-
}
455-
456438
// Allow for a chain of member_ref exprs that end in a decl_ref expr.
457439
auto *subExpr = borrowExpr->getSubExpr();
458440
while (auto *memberRef = dyn_cast<MemberRefExpr>(subExpr))
@@ -6177,11 +6159,6 @@ bool swift::diagnoseUnhandledThrowsInAsyncContext(DeclContext *dc,
61776159

61786160
void swift::diagnoseCopyableTypeContainingMoveOnlyType(
61796161
NominalTypeDecl *copyableNominalType) {
6180-
// If we don't have move only enabled, bail early.
6181-
if (!copyableNominalType->getASTContext().LangOpts.Features.contains(
6182-
Feature::MoveOnly))
6183-
return;
6184-
61856162
// If we already have a move only type, just bail, we have no further work to
61866163
// do.
61876164
if (copyableNominalType->isMoveOnly())

‎lib/Sema/TypeCheckAttr.cpp

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2161,12 +2161,8 @@ void AttributeChecker::visitFinalAttr(FinalAttr *attr) {
21612161
}
21622162

21632163
void AttributeChecker::visitMoveOnlyAttr(MoveOnlyAttr *attr) {
2164-
if (!D->getASTContext().LangOpts.hasFeature(Feature::MoveOnly)) {
2165-
auto error =
2166-
diag::experimental_moveonly_feature_can_only_be_used_when_enabled;
2167-
diagnoseAndRemoveAttr(attr, error);
2168-
return;
2169-
}
2164+
if (!D->getASTContext().supportsMoveOnlyTypes())
2165+
D->diagnose(diag::moveOnly_requires_lexical_lifetimes);
21702166

21712167
if (isa<StructDecl>(D) || isa<EnumDecl>(D))
21722168
return;

0 commit comments

Comments
 (0)