Skip to content

Commit 05e54a3

Browse files
gottesmmgottesmm
committed
[move-only] Emit an error if we /ever/ partially consume a noncopyable type.
The reason why I am doing this is that this was not part of the original evolution proposal (it was called an extension) and after some discussion it was realized that partial consumption would benefit from discussion on the forums. rdar://111353459
1 parent e18c14f commit 05e54a3

20 files changed

+307
-37
lines changed

‎include/swift/AST/DiagnosticsSIL.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -785,6 +785,9 @@ ERROR(sil_movechecking_notconsumable_but_assignable_was_consumed, none,
785785
ERROR(sil_movechecking_cannot_destructure_has_deinit, none,
786786
"cannot partially consume '%0' when it has a deinitializer",
787787
(StringRef))
788+
ERROR(sil_movechecking_cannot_destructure, none,
789+
"cannot partially consume '%0'",
790+
(StringRef))
788791
ERROR(sil_movechecking_discard_missing_consume_self, none,
789792
"must consume 'self' before exiting method that discards self", ())
790793
ERROR(sil_movechecking_reinit_after_discard, none,

‎include/swift/Basic/Features.def

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ EXPERIMENTAL_FEATURE(OldOwnershipOperatorSpellings, true)
134134
EXPERIMENTAL_FEATURE(MoveOnlyEnumDeinits, true)
135135
EXPERIMENTAL_FEATURE(MoveOnlyTuples, true)
136136
EXPERIMENTAL_FEATURE(MoveOnlyResilientTypes, true)
137+
EXPERIMENTAL_FEATURE(MoveOnlyPartialConsumption, true)
137138

138139
EXPERIMENTAL_FEATURE(OneWayClosureParameters, false)
139140
EXPERIMENTAL_FEATURE(TypeWitnessSystemInference, false)

‎lib/AST/ASTPrinter.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3310,6 +3310,11 @@ static bool usesFeatureMoveOnlyResilientTypes(Decl *decl) {
33103310
return false;
33113311
}
33123312

3313+
static bool usesFeatureMoveOnlyPartialConsumption(Decl *decl) {
3314+
// Partial consumption does not affect declarations directly.
3315+
return false;
3316+
}
3317+
33133318
static bool usesFeatureOneWayClosureParameters(Decl *decl) {
33143319
return false;
33153320
}

‎lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

Lines changed: 50 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1452,10 +1452,12 @@ struct CopiedLoadBorrowEliminationVisitor final
14521452
// MARK: DestructureThroughDeinit Checking
14531453
//===----------------------------------------------------------------------===//
14541454

1455-
static void
1456-
checkForDestructureThroughDeinit(MarkMustCheckInst *rootAddress, Operand *use,
1457-
TypeTreeLeafTypeRange usedBits,
1458-
DiagnosticEmitter &diagnosticEmitter) {
1455+
/// When partial consumption is enabled, we only allow for destructure through
1456+
/// deinits. When partial consumption is disabled, we error on /all/ partial
1457+
/// consumption.
1458+
static void checkForDestructure(MarkMustCheckInst *rootAddress, Operand *use,
1459+
TypeTreeLeafTypeRange usedBits,
1460+
DiagnosticEmitter &diagnosticEmitter) {
14591461
LLVM_DEBUG(llvm::dbgs() << " DestructureNeedingUse: " << *use->getUser());
14601462

14611463
SILFunction *fn = rootAddress->getFunction();
@@ -1473,6 +1475,29 @@ checkForDestructureThroughDeinit(MarkMustCheckInst *rootAddress, Operand *use,
14731475
if (iterType.isMoveOnlyWrapped())
14741476
return;
14751477

1478+
// If we are not allowing for any partial consumption, just emit an error
1479+
// immediately.
1480+
if (!rootAddress->getModule().getASTContext().LangOpts.hasFeature(
1481+
Feature::MoveOnlyPartialConsumption)) {
1482+
// If the types equal, just bail early.
1483+
if (iterType == targetType)
1484+
return;
1485+
1486+
// Otherwise, build up the path string and emit the error.
1487+
SmallString<128> pathString;
1488+
auto rootType = rootAddress->getType();
1489+
if (iterType != rootType) {
1490+
llvm::raw_svector_ostream os(pathString);
1491+
pair.constructPathString(iterType, {rootType, fn}, rootType, fn, os);
1492+
}
1493+
1494+
diagnosticEmitter.emitCannotDestructureNominalError(
1495+
rootAddress, pathString, nullptr /*nominal*/, use->getUser(),
1496+
false /*is for deinit error*/);
1497+
return;
1498+
}
1499+
1500+
// Otherwise, walk the type looking for the deinit.
14761501
while (iterType != targetType) {
14771502
// If we have a nominal type as our parent type, see if it has a
14781503
// deinit. We know that it must be non-copyable since copyable types
@@ -1491,8 +1516,9 @@ checkForDestructureThroughDeinit(MarkMustCheckInst *rootAddress, Operand *use,
14911516
pair.constructPathString(iterType, {rootType, fn}, rootType, fn, os);
14921517
}
14931518

1494-
diagnosticEmitter.emitCannotDestructureDeinitNominalError(
1495-
rootAddress, pathString, nom, use->getUser());
1519+
diagnosticEmitter.emitCannotDestructureNominalError(
1520+
rootAddress, pathString, nom, use->getUser(),
1521+
true /*is for deinit error*/);
14961522
break;
14971523
}
14981524
}
@@ -1702,8 +1728,10 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
17021728

17031729
// TODO: Add borrow checking here like below.
17041730

1705-
// TODO: Add destructure deinit checking here once address only checking is
1706-
// completely brought up.
1731+
// If we have a copy_addr, we are either going to have a take or a
1732+
// copy... in either case, this copy_addr /is/ going to be a consuming
1733+
// operation. Make sure to check if we semantically destructure.
1734+
checkForDestructure(markedValue, op, *leafRange, diagnosticEmitter);
17071735

17081736
if (copyAddr->isTakeOfSrc()) {
17091737
LLVM_DEBUG(llvm::dbgs() << "Found take: " << *user);
@@ -1771,17 +1799,6 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
17711799
return false;
17721800
}
17731801

1774-
checkForDestructureThroughDeinit(markedValue, op, *leafRange,
1775-
diagnosticEmitter);
1776-
1777-
// If we emitted an error diagnostic, do not transform further and instead
1778-
// mark that we emitted an early diagnostic and return true.
1779-
if (numDiagnostics != moveChecker.diagnosticEmitter.getDiagnosticCount()) {
1780-
LLVM_DEBUG(llvm::dbgs()
1781-
<< "Emitting destructure through deinit error!\n");
1782-
return true;
1783-
}
1784-
17851802
// Canonicalize the lifetime of the load [take], load [copy].
17861803
LLVM_DEBUG(llvm::dbgs() << "Running copy propagation!\n");
17871804
moveChecker.changed |= moveChecker.canonicalizer.canonicalize();
@@ -1882,6 +1899,19 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
18821899
useState.recordLivenessUse(dvi, *leafRange);
18831900
}
18841901
} else {
1902+
// Now that we know that we are going to perform a take, perform a
1903+
// checkForDestructure.
1904+
checkForDestructure(markedValue, op, *leafRange, diagnosticEmitter);
1905+
1906+
// If we emitted an error diagnostic, do not transform further and instead
1907+
// mark that we emitted an early diagnostic and return true.
1908+
if (numDiagnostics !=
1909+
moveChecker.diagnosticEmitter.getDiagnosticCount()) {
1910+
LLVM_DEBUG(llvm::dbgs()
1911+
<< "Emitting destructure through deinit error!\n");
1912+
return true;
1913+
}
1914+
18851915
// If we had a load [copy], store this into the copy list. These are the
18861916
// things that we must merge into destroy_addr or reinits after we are
18871917
// done checking. The load [take] are already complete and good to go.
@@ -1926,8 +1956,7 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
19261956
// error.
19271957
unsigned numDiagnostics =
19281958
moveChecker.diagnosticEmitter.getDiagnosticCount();
1929-
checkForDestructureThroughDeinit(markedValue, op, *leafRange,
1930-
diagnosticEmitter);
1959+
checkForDestructure(markedValue, op, *leafRange, diagnosticEmitter);
19311960
if (numDiagnostics != moveChecker.diagnosticEmitter.getDiagnosticCount()) {
19321961
LLVM_DEBUG(llvm::dbgs()
19331962
<< "Emitting destructure through deinit error!\n");

‎lib/SILOptimizer/Mandatory/MoveOnlyDiagnostics.cpp

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -856,3 +856,38 @@ void DiagnosticEmitter::emitCannotDestructureDeinitNominalError(
856856
deinitedNominal->getValueTypeDestructor()->getLoc(/*SerializedOK=*/false))
857857
astContext.Diags.diagnose(deinitLoc, diag::sil_movechecking_deinit_here);
858858
}
859+
860+
void DiagnosticEmitter::emitCannotDestructureNominalError(
861+
MarkMustCheckInst *markedValue, StringRef pathString,
862+
NominalTypeDecl *nominal, SILInstruction *consumingUser,
863+
bool isDueToDeinit) {
864+
auto &astContext = fn->getASTContext();
865+
866+
SmallString<64> varName;
867+
getVariableNameForValue(markedValue, varName);
868+
869+
if (!pathString.empty())
870+
varName.append(pathString);
871+
872+
assert(
873+
astContext.LangOpts.hasFeature(Feature::MoveOnlyPartialConsumption) ==
874+
isDueToDeinit &&
875+
"Should only emit deinit error if we have partial consumption enabled");
876+
if (isDueToDeinit) {
877+
diagnose(astContext, consumingUser,
878+
diag::sil_movechecking_cannot_destructure_has_deinit, varName);
879+
} else {
880+
diagnose(astContext, consumingUser,
881+
diag::sil_movechecking_cannot_destructure, varName);
882+
}
883+
registerDiagnosticEmitted(markedValue);
884+
885+
if (!isDueToDeinit)
886+
return;
887+
888+
// Point to the deinit if we know where it is.
889+
assert(nominal);
890+
if (auto deinitLoc =
891+
nominal->getValueTypeDestructor()->getLoc(/*SerializedOK=*/false))
892+
astContext.Diags.diagnose(deinitLoc, diag::sil_movechecking_deinit_here);
893+
}

‎lib/SILOptimizer/Mandatory/MoveOnlyDiagnostics.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,11 @@ class DiagnosticEmitter {
169169
StringRef pathString,
170170
NominalTypeDecl *deinitedNominal,
171171
SILInstruction *consumingUser);
172+
void emitCannotDestructureNominalError(MarkMustCheckInst *markedValue,
173+
StringRef pathString,
174+
NominalTypeDecl *nominal,
175+
SILInstruction *consumingUser,
176+
bool isDueToDeinit);
172177

173178
private:
174179
/// Emit diagnostics for the final consuming uses and consuming uses needing

‎test/Interpreter/moveonly_address_maximize.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
// RUN: %target-run-simple-swift(-Xfrontend -sil-verify-all) | %FileCheck %s
2-
// RUN: %target-run-simple-swift(-O -Xfrontend -sil-verify-all) | %FileCheck %s
1+
// RUN: %target-run-simple-swift(-Xfrontend -sil-verify-all -enable-experimental-feature MoveOnlyPartialConsumption) | %FileCheck %s
2+
// RUN: %target-run-simple-swift(-O -Xfrontend -sil-verify-all -enable-experimental-feature MoveOnlyPartialConsumption) | %FileCheck %s
33

44
// REQUIRES: executable_test
55

‎test/SILGen/moveonly_library_evolution.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
// RUN: %target-swift-emit-silgen -enable-experimental-feature NoImplicitCopy -enable-experimental-feature MoveOnlyResilientTypes -enable-library-evolution %s | %FileCheck %s
2-
// RUN: %target-swift-emit-sil -O -sil-verify-all -enable-experimental-feature NoImplicitCopy -enable-experimental-feature MoveOnlyResilientTypes -enable-library-evolution %s
1+
// RUN: %target-swift-emit-silgen -enable-experimental-feature MoveOnlyPartialConsumption -enable-experimental-feature NoImplicitCopy -enable-experimental-feature MoveOnlyResilientTypes -enable-library-evolution %s | %FileCheck %s
2+
// RUN: %target-swift-emit-sil -O -sil-verify-all -enable-experimental-feature MoveOnlyPartialConsumption -enable-experimental-feature NoImplicitCopy -enable-experimental-feature MoveOnlyResilientTypes -enable-library-evolution %s
33

44
////////////////////////
55
// MARK: Declarations //

‎test/SILOptimizer/moveonly_addresschecker.sil

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-sil-opt -module-name moveonly_addresschecker -sil-move-only-address-checker -enable-experimental-feature MoveOnlyClasses -enable-sil-verify-all %s | %FileCheck %s
1+
// RUN: %target-sil-opt -module-name moveonly_addresschecker -sil-move-only-address-checker -enable-experimental-feature MoveOnlyClasses -enable-experimental-feature MoveOnlyPartialConsumption -enable-sil-verify-all %s | %FileCheck %s
22

33
sil_stage raw
44

‎test/SILOptimizer/moveonly_addresschecker_destructure_through_deinit_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 -sil-verify-all -verify -enable-experimental-feature MoveOnlyClasses -enable-experimental-feature MoveOnlyTuples %s
1+
// RUN: %target-swift-emit-sil -enable-experimental-feature MoveOnlyPartialConsumption -sil-verify-all -verify -enable-experimental-feature MoveOnlyClasses -enable-experimental-feature MoveOnlyTuples %s
22

33
// This test validates that we properly emit errors if we partially invalidate
44
// through a type with a deinit.

‎test/SILOptimizer/moveonly_addresschecker_diagnostics.sil

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
// RUN: %target-sil-opt -sil-move-only-address-checker -enable-experimental-feature MoveOnlyClasses -enable-sil-verify-all %s -verify
2-
// RUN: %target-sil-opt -sil-move-only-address-checker -enable-experimental-feature MoveOnlyClasses -enable-sil-verify-all -move-only-diagnostics-silently-emit-diagnostics %s | %FileCheck %s
1+
// RUN: %target-sil-opt -sil-move-only-address-checker -enable-experimental-feature MoveOnlyPartialConsumption -enable-experimental-feature MoveOnlyClasses -enable-sil-verify-all %s -verify
2+
// RUN: %target-sil-opt -sil-move-only-address-checker -enable-experimental-feature MoveOnlyPartialConsumption -enable-experimental-feature MoveOnlyClasses -enable-sil-verify-all -move-only-diagnostics-silently-emit-diagnostics %s | %FileCheck %s
33

44
// TODO: Add FileCheck
55

‎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 NoImplicitCopy -enable-experimental-feature MoveOnlyClasses %s
1+
// RUN: %target-swift-emit-sil -O -sil-verify-all -verify -enable-experimental-feature MoveOnlyPartialConsumption -enable-experimental-feature NoImplicitCopy -enable-experimental-feature MoveOnlyClasses %s
22

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

‎test/SILOptimizer/moveonly_addresschecker_maximize.sil

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-sil-opt -sil-move-only-address-checker -enable-sil-verify-all %s | %FileCheck %s
1+
// RUN: %target-sil-opt -sil-move-only-address-checker -enable-experimental-feature MoveOnlyPartialConsumption -enable-sil-verify-all %s | %FileCheck %s
22
sil_stage raw
33

44
import Builtin

‎test/SILOptimizer/moveonly_addresschecker_unmaximized.sil

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-sil-opt -module-name moveonly_addresschecker -sil-move-only-address-checker -enable-experimental-feature MoveOnlyClasses -enable-sil-verify-all %s -move-only-address-checker-disable-lifetime-extension=true | %FileCheck %s
1+
// RUN: %target-sil-opt -module-name moveonly_addresschecker -enable-experimental-feature MoveOnlyPartialConsumption -sil-move-only-address-checker -enable-experimental-feature MoveOnlyClasses -enable-sil-verify-all %s -move-only-address-checker-disable-lifetime-extension=true | %FileCheck %s
22

33
@_moveOnly
44
struct M {

‎test/SILOptimizer/moveonly_addressonly_subscript_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 -sil-verify-all -verify %s
1+
// RUN: %target-swift-emit-sil -enable-experimental-feature MoveOnlyPartialConsumption -sil-verify-all -verify %s
22

33
class CopyableKlass {}
44
protocol P {}

‎test/SILOptimizer/moveonly_loadable_subscript_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 -sil-verify-all -verify %s
1+
// RUN: %target-swift-emit-sil -enable-experimental-feature MoveOnlyPartialConsumption -sil-verify-all -verify %s
22

33
class CopyableKlass {}
44

‎test/SILOptimizer/moveonly_objectchecker_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 -sil-verify-all -verify -enable-experimental-feature MoveOnlyClasses %s
1+
// RUN: %target-swift-emit-sil -enable-experimental-feature MoveOnlyPartialConsumption -sil-verify-all -verify -enable-experimental-feature MoveOnlyClasses %s
22

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

0 commit comments

Comments
 (0)