Skip to content

Commit 52989d1

Browse files
nate-chandlernate-chandler
authored
Merge pull request #71234 from nate-chandler/partial-consumption/20240129/1/reinit-feature
[MoveChecker] Separate partial reinit feature from partial consume feature.
2 parents 6cca6e5 + 03f904a commit 52989d1

9 files changed

+106
-33
lines changed

‎include/swift/Basic/Features.def‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ EXPERIMENTAL_FEATURE(OldOwnershipOperatorSpellings, true)
151151
EXPERIMENTAL_FEATURE(MoveOnlyEnumDeinits, true)
152152
EXPERIMENTAL_FEATURE(MoveOnlyTuples, true)
153153
EXPERIMENTAL_FEATURE(MoveOnlyPartialConsumption, true)
154+
EXPERIMENTAL_FEATURE(MoveOnlyPartialReinitialization, true)
154155

155156
EXPERIMENTAL_FEATURE(OneWayClosureParameters, false)
156157
EXPERIMENTAL_FEATURE(LayoutPrespecialization, true)

‎lib/AST/ASTPrinter.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3631,6 +3631,10 @@ static bool usesFeatureMoveOnlyPartialConsumption(Decl *decl) {
36313631
return false;
36323632
}
36333633

3634+
static bool usesFeatureMoveOnlyPartialReinitialization(Decl *decl) {
3635+
return false;
3636+
}
3637+
36343638
static bool usesFeatureNoncopyableGenerics(Decl *decl) {
36353639
auto checkMarking = [](auto &marking) -> bool {
36363640
switch (marking.getInverse().getKind()) {

‎lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp‎

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1654,8 +1654,8 @@ struct CopiedLoadBorrowEliminationVisitor
16541654

16551655
/// Whether an error should be emitted in response to a partial consumption.
16561656
static llvm::Optional<PartialMutationError>
1657-
shouldEmitPartialMutationError(UseState &useState, SILInstruction *user,
1658-
SILType useType,
1657+
shouldEmitPartialMutationError(UseState &useState, PartialMutation::Kind kind,
1658+
SILInstruction *user, SILType useType,
16591659
TypeTreeLeafTypeRange usedBits) {
16601660
SILFunction *fn = useState.getFunction();
16611661

@@ -1672,18 +1672,19 @@ shouldEmitPartialMutationError(UseState &useState, SILInstruction *user,
16721672
LLVM_DEBUG(llvm::dbgs() << " Iter Type: " << iterType << '\n'
16731673
<< " Target Type: " << targetType << '\n');
16741674

1675-
if (!fn->getModule().getASTContext().LangOpts.hasFeature(
1676-
Feature::MoveOnlyPartialConsumption)) {
1677-
LLVM_DEBUG(llvm::dbgs() << " MoveOnlyPartialConsumption disabled!\n");
1678-
// If the types equal, just bail early.
1679-
if (iterType == targetType) {
1680-
LLVM_DEBUG(llvm::dbgs() << " IterType is TargetType! Exiting early "
1681-
"without emitting error!\n");
1682-
return {};
1683-
}
1675+
if (iterType == targetType) {
1676+
LLVM_DEBUG(llvm::dbgs() << " IterType is TargetType! Exiting early "
1677+
"without emitting error!\n");
1678+
return {};
1679+
}
16841680

1681+
auto feature = partialMutationFeature(kind);
1682+
if (!fn->getModule().getASTContext().LangOpts.hasFeature(feature)) {
1683+
LLVM_DEBUG(llvm::dbgs()
1684+
<< " " << getFeatureName(feature) << " disabled!\n");
1685+
// If the types equal, just bail early.
16851686
// Emit the error.
1686-
return {PartialMutationError::featureDisabled(iterType)};
1687+
return {PartialMutationError::featureDisabled(iterType, kind)};
16871688
}
16881689

16891690
LLVM_DEBUG(llvm::dbgs() << " MoveOnlyPartialConsumption enabled!\n");
@@ -1715,13 +1716,14 @@ shouldEmitPartialMutationError(UseState &useState, SILInstruction *user,
17151716

17161717
static bool checkForPartialMutation(UseState &useState,
17171718
DiagnosticEmitter &diagnosticEmitter,
1719+
PartialMutation::Kind kind,
17181720
SILInstruction *user, SILType useType,
17191721
TypeTreeLeafTypeRange usedBits,
17201722
PartialMutation partialMutateKind) {
17211723
// We walk down from our ancestor to our projection, emitting an error if
17221724
// any of our types have a deinit.
17231725
auto error =
1724-
shouldEmitPartialMutationError(useState, user, useType, usedBits);
1726+
shouldEmitPartialMutationError(useState, kind, user, useType, usedBits);
17251727
if (!error)
17261728
return false;
17271729

@@ -1762,8 +1764,9 @@ void PartialReinitChecker::performPartialReinitChecking(
17621764
initToValues.first, index,
17631765
[&](SILInstruction *consumingInst) -> bool {
17641766
return !checkForPartialMutation(
1765-
useState, diagnosticEmitter, initToValues.first,
1766-
value->getType(), TypeTreeLeafTypeRange(index, index + 1),
1767+
useState, diagnosticEmitter, PartialMutation::Kind::Reinit,
1768+
initToValues.first, value->getType(),
1769+
TypeTreeLeafTypeRange(index, index + 1),
17671770
PartialMutation::reinit(*consumingInst));
17681771
});
17691772

@@ -1797,8 +1800,9 @@ void PartialReinitChecker::performPartialReinitChecking(
17971800
reinitToValues.first, index,
17981801
[&](SILInstruction *consumingInst) -> bool {
17991802
return !checkForPartialMutation(
1800-
useState, diagnosticEmitter, reinitToValues.first,
1801-
value->getType(), TypeTreeLeafTypeRange(index, index + 1),
1803+
useState, diagnosticEmitter, PartialMutation::Kind::Reinit,
1804+
reinitToValues.first, value->getType(),
1805+
TypeTreeLeafTypeRange(index, index + 1),
18021806
PartialMutation::reinit(*consumingInst));
18031807
});
18041808
if (emittedError)
@@ -2028,7 +2032,8 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
20282032
// If we have a copy_addr, we are either going to have a take or a
20292033
// copy... in either case, this copy_addr /is/ going to be a consuming
20302034
// operation. Make sure to check if we semantically destructure.
2031-
checkForPartialMutation(useState, diagnosticEmitter, op->getUser(),
2035+
checkForPartialMutation(useState, diagnosticEmitter,
2036+
PartialMutation::Kind::Consume, op->getUser(),
20322037
op->get()->getType(), *leafRange,
20332038
PartialMutation::consume());
20342039

@@ -2223,7 +2228,8 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
22232228
} else {
22242229
// Now that we know that we are going to perform a take, perform a
22252230
// checkForDestructure.
2226-
checkForPartialMutation(useState, diagnosticEmitter, op->getUser(),
2231+
checkForPartialMutation(useState, diagnosticEmitter,
2232+
PartialMutation::Kind::Consume, op->getUser(),
22272233
op->get()->getType(), *leafRange,
22282234
PartialMutation::consume());
22292235

@@ -2280,7 +2286,8 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
22802286
// error.
22812287
unsigned numDiagnostics =
22822288
moveChecker.diagnosticEmitter.getDiagnosticCount();
2283-
checkForPartialMutation(useState, diagnosticEmitter, op->getUser(),
2289+
checkForPartialMutation(useState, diagnosticEmitter,
2290+
PartialMutation::Kind::Consume, op->getUser(),
22842291
op->get()->getType(), *leafRange,
22852292
PartialMutation::consume());
22862293
if (numDiagnostics != moveChecker.diagnosticEmitter.getDiagnosticCount()) {

‎lib/SILOptimizer/Mandatory/MoveOnlyDiagnostics.cpp‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -967,13 +967,11 @@ void DiagnosticEmitter::emitCannotPartiallyMutateError(
967967
if (!pathString.empty())
968968
varName.append(pathString);
969969

970-
bool hasPartialConsumption =
971-
astContext.LangOpts.hasFeature(Feature::MoveOnlyPartialConsumption);
972-
(void)hasPartialConsumption;
973-
974970
switch (error) {
975-
case PartialMutationError::Kind::FeatureDisabled:
976-
assert(!hasPartialConsumption);
971+
case PartialMutationError::Kind::FeatureDisabled: {
972+
assert(!astContext.LangOpts.hasFeature(
973+
partialMutationFeature(error.getKind())));
974+
977975
switch (kind) {
978976
case PartialMutation::Kind::Consume:
979977
diagnose(astContext, user, diag::sil_movechecking_cannot_destructure,
@@ -988,8 +986,12 @@ void DiagnosticEmitter::emitCannotPartiallyMutateError(
988986
}
989987
registerDiagnosticEmitted(address);
990988
return;
989+
}
991990
case PartialMutationError::Kind::HasDeinit: {
992-
assert(hasPartialConsumption);
991+
assert(
992+
astContext.LangOpts.hasFeature(Feature::MoveOnlyPartialConsumption) ||
993+
astContext.LangOpts.hasFeature(
994+
Feature::MoveOnlyPartialReinitialization));
993995
switch (kind) {
994996
case PartialMutation::Kind::Consume:
995997
diagnose(astContext, user,

‎lib/SILOptimizer/Mandatory/MoveOnlyTypeUtils.h‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#ifndef SWIFT_SILOPTIMIZER_MANDATORY_MOVEONLYTYPEUTILS_H
2121
#define SWIFT_SILOPTIMIZER_MANDATORY_MOVEONLYTYPEUTILS_H
2222

23+
#include "swift/Basic/Feature.h"
2324
#include "swift/Basic/TaggedUnion.h"
2425
#include "swift/SIL/FieldSensitivePrunedLiveness.h"
2526
#include "swift/SIL/SILBuilder.h"
@@ -127,6 +128,15 @@ class PartialMutation {
127128
}
128129
};
129130

131+
inline Feature partialMutationFeature(PartialMutation::Kind kind) {
132+
switch (kind) {
133+
case PartialMutation::Kind::Consume:
134+
return Feature::MoveOnlyPartialConsumption;
135+
case PartialMutation::Kind::Reinit:
136+
return Feature::MoveOnlyPartialReinitialization;
137+
}
138+
}
139+
130140
/// How a partial mutation (consume/reinit) is illegal for diagnostic emission.
131141
///
132142
/// Emulates the following enum with associated value:
@@ -136,7 +146,9 @@ class PartialMutation {
136146
/// case hasDeinit(SILType, NominalTypeDecl)
137147
/// }
138148
class PartialMutationError {
139-
struct FeatureDisabled {};
149+
struct FeatureDisabled {
150+
PartialMutation::Kind kind;
151+
};
140152
struct HasDeinit {
141153
NominalTypeDecl &nominal;
142154
};
@@ -176,8 +188,9 @@ class PartialMutationError {
176188
return Kind::HasDeinit;
177189
}
178190

179-
static PartialMutationError featureDisabled(SILType type) {
180-
return PartialMutationError(type, FeatureDisabled{});
191+
static PartialMutationError featureDisabled(SILType type,
192+
PartialMutation::Kind kind) {
193+
return PartialMutationError(type, FeatureDisabled{kind});
181194
}
182195

183196
static PartialMutationError hasDeinit(SILType type,
@@ -186,6 +199,7 @@ class PartialMutationError {
186199
}
187200

188201
NominalTypeDecl &getNominal() { return kind.get<HasDeinit>().nominal; };
202+
PartialMutation::Kind getKind() { return kind.get<FeatureDisabled>().kind; };
189203
};
190204
} // namespace siloptimizer
191205
} // namespace swift

‎test/SILOptimizer/moveonly_addresschecker_diagnostics.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-swift-emit-sil -O -sil-verify-all -verify -enable-experimental-feature MoveOnlyPartialConsumption -enable-experimental-feature NoImplicitCopy -enable-experimental-feature MoveOnlyClasses %s
1+
// RUN: %target-swift-emit-sil -O -sil-verify-all -verify -enable-experimental-feature MoveOnlyPartialConsumption -enable-experimental-feature MoveOnlyPartialReinitialization -enable-experimental-feature NoImplicitCopy -enable-experimental-feature MoveOnlyClasses %s
22

33
//////////////////
44
// Declarations //

‎test/SILOptimizer/moveonly_addresschecker_diagnostics_without_partial_reinit.swift‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// RUN: %target-swift-emit-sil -O -sil-verify-all -verify -enable-experimental-feature NoImplicitCopy -enable-experimental-feature MoveOnlyClasses -enable-experimental-feature MoveOnlyPartialConsumption %s
2+
3+
// Test diagnostics for partial-consumption without partial-reinitialization.
4+
5+
struct Ur : ~Copyable {
6+
deinit {}
7+
}
8+
9+
struct Agg : ~Copyable {
10+
var u1: Ur
11+
var u2: Ur
12+
}
13+
14+
func borrow(_ u1: borrowing Ur) {}
15+
func consume(_ u1: consuming Ur) {}
16+
func consume(_ u1: consuming Agg) {}
17+
func rewrite(_ u: inout Ur) {}
18+
19+
// First consumes and then reinits each field.
20+
func reinit_1(agg: inout Agg, u1: consuming Ur, u2: consuming Ur) {
21+
consume(agg.u1)
22+
consume(agg.u2)
23+
agg.u1 = u1 // expected-error{{cannot partially reinitialize 'agg' after it has been consumed; only full reinitialization is allowed}}
24+
// expected-note@-3{{consumed here}}
25+
agg.u2 = u2 // expected-error{{cannot partially reinitialize 'agg' after it has been consumed; only full reinitialization is allowed}}
26+
// expected-note@-4{{consumed here}}
27+
}
28+
29+
// Just replace fields of inout argument, no reinitialization.
30+
func not_reinit_1(agg: inout Agg, u1: consuming Ur, u2: consuming Ur) {
31+
agg.u1 = u1
32+
agg.u2 = u2
33+
}
34+
35+
// Full reinitialization is legal.
36+
func not_reinit_2(agg: inout Agg, agg2: consuming Agg) {
37+
consume(agg)
38+
agg = agg2
39+
}
40+
41+
// Passing fields as inout is allowed.
42+
func not_reinit_3(agg: inout Agg) {
43+
rewrite(&agg.u1)
44+
rewrite(&agg.u2)
45+
}

‎test/SILOptimizer/moveonly_generics_basic.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-swift-frontend %s -sil-verify-all -verify -emit-sil -enable-experimental-feature MoveOnlyPartialConsumption -enable-experimental-feature NoncopyableGenerics
1+
// RUN: %target-swift-frontend %s -sil-verify-all -verify -emit-sil -enable-experimental-feature MoveOnlyPartialConsumption -enable-experimental-feature MoveOnlyPartialReinitialization -enable-experimental-feature NoncopyableGenerics
22

33
// REQUIRES: asserts
44

‎test/SILOptimizer/moveonly_trivial_addresschecker_diagnostics.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-swift-emit-sil -enable-experimental-feature MoveOnlyPartialConsumption -sil-verify-all -verify %s
1+
// RUN: %target-swift-emit-sil -enable-experimental-feature MoveOnlyPartialConsumption -enable-experimental-feature MoveOnlyPartialReinitialization -sil-verify-all -verify %s
22

33
//////////////////
44
// Declarations //

0 commit comments

Comments
 (0)