Skip to content

[6.0] Disallow consuming self in a noncopyable deinit again. #75567

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: 2 additions & 0 deletions include/swift/AST/DiagnosticsSIL.def
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,8 @@ ERROR(sil_movechecking_discard_missing_consume_self, none,
"must consume 'self' before exiting method that discards self", ())
ERROR(sil_movechecking_reinit_after_discard, none,
"cannot reinitialize 'self' after 'discard self'", ())
ERROR(sil_movechecking_consume_during_deinit, none,
"'self' cannot be consumed during 'deinit'", ())

NOTE(sil_movechecking_discard_self_here, none,
"discarded self here", ())
Expand Down
1 change: 1 addition & 0 deletions include/swift/Basic/Features.def
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ EXPERIMENTAL_FEATURE(OldOwnershipOperatorSpellings, true)
EXPERIMENTAL_FEATURE(MoveOnlyEnumDeinits, true)
EXPERIMENTAL_FEATURE(MoveOnlyTuples, true)
EXPERIMENTAL_FEATURE(MoveOnlyPartialReinitialization, true)
EXPERIMENTAL_FEATURE(ConsumeSelfInDeinit, true)

EXPERIMENTAL_FEATURE(OneWayClosureParameters, false)
EXPERIMENTAL_FEATURE(LayoutPrespecialization, true)
Expand Down
1 change: 1 addition & 0 deletions lib/AST/FeatureSet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -695,6 +695,7 @@ UNINTERESTING_FEATURE(BorrowingSwitch)

UNINTERESTING_FEATURE(ClosureIsolation)
UNINTERESTING_FEATURE(Extern)
UNINTERESTING_FEATURE(ConsumeSelfInDeinit)

static bool usesFeatureConformanceSuppression(Decl *decl) {
auto *nominal = dyn_cast<NominalTypeDecl>(decl);
Expand Down
14 changes: 14 additions & 0 deletions lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1816,7 +1816,21 @@ shouldEmitPartialMutationError(UseState &useState, PartialMutation::Kind kind,
LLVM_DEBUG(llvm::dbgs() << " Iter Type: " << iterType << '\n'
<< " Target Type: " << targetType << '\n');

// Allowing full object consumption in a deinit is still not allowed.
if (iterType == targetType && !isa<DropDeinitInst>(user)) {
// Don't allow whole-value consumption of `self` from a `deinit`.
if (!fn->getModule().getASTContext().LangOpts
.hasFeature(Feature::ConsumeSelfInDeinit)
&& kind == PartialMutation::Kind::Consume
&& useState.sawDropDeinit
// TODO: Revisit this when we introduce deinits on enums.
&& !targetType.getEnumOrBoundGenericEnum()) {
LLVM_DEBUG(llvm::dbgs() << " IterType is TargetType in deinit! "
"Not allowed yet");

return {PartialMutationError::consumeDuringDeinit(iterType)};
}

LLVM_DEBUG(llvm::dbgs() << " IterType is TargetType! Exiting early "
"without emitting error!\n");
return {};
Expand Down
6 changes: 6 additions & 0 deletions lib/SILOptimizer/Mandatory/MoveOnlyDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -889,5 +889,11 @@ void DiagnosticEmitter::emitCannotPartiallyMutateError(
registerDiagnosticEmitted(address);
return;
}
case PartialMutationError::Kind::ConsumeDuringDeinit: {
astContext.Diags.diagnose(user->getLoc().getSourceLoc(),
diag::sil_movechecking_consume_during_deinit);
return;
}
}
llvm_unreachable("unhandled case");
}
18 changes: 15 additions & 3 deletions lib/SILOptimizer/Mandatory/MoveOnlyTypeUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,10 @@ class PartialMutationError {
struct NonfrozenUsableFromInlineType {
NominalTypeDecl &nominal;
};
struct ConsumeDuringDeinit {
};
using Payload = TaggedUnion<FeatureDisabled, HasDeinit, NonfrozenImportedType,
NonfrozenUsableFromInlineType>;
NonfrozenUsableFromInlineType, ConsumeDuringDeinit>;
Payload payload;

PartialMutationError(SILType type, Payload payload)
Expand Down Expand Up @@ -212,6 +214,8 @@ class PartialMutationError {
/// with library evolution, prevent the aggregate from being partially
/// mutated.
NonfrozenUsableFromInlineType,
/// The value was fully consumed in a `deinit`.
ConsumeDuringDeinit,
};

operator Kind() {
Expand All @@ -221,8 +225,12 @@ class PartialMutationError {
return Kind::HasDeinit;
else if (payload.isa<NonfrozenImportedType>())
return Kind::NonfrozenImportedType;
assert(payload.isa<NonfrozenUsableFromInlineType>());
return Kind::NonfrozenUsableFromInlineType;
else if (payload.isa<NonfrozenUsableFromInlineType>())
return Kind::NonfrozenUsableFromInlineType;
else if (payload.isa<ConsumeDuringDeinit>())
return Kind::ConsumeDuringDeinit;

llvm_unreachable("unhandled tag");
}

static PartialMutationError featureDisabled(SILType type,
Expand All @@ -245,6 +253,10 @@ class PartialMutationError {
return PartialMutationError(type, NonfrozenUsableFromInlineType{nominal});
}

static PartialMutationError consumeDuringDeinit(SILType type) {
return PartialMutationError(type, ConsumeDuringDeinit{});
}

PartialMutation::Kind getKind() {
return payload.get<FeatureDisabled>().kind;
}
Expand Down
104 changes: 104 additions & 0 deletions test/SILOptimizer/deinit_total_consumption.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
// RUN: %target-swift-frontend -DEMPTY -emit-sil -verify %s
// RUN: %target-swift-frontend -DTRIVIAL -emit-sil -verify %s
// RUN: %target-swift-frontend -DLOADABLE -emit-sil -verify %s
// RUN: %target-swift-frontend -DADDRESS_ONLY -emit-sil -verify %s

// Don't allow whole-value consumption or mutation of a value in its own
// `deinit`, but allow consumption of its individual fields.

struct T1: ~Copyable {
#if EMPTY
#elseif TRIVIAL
var _x: Int
#elseif LOADABLE
var _x: AnyObject
#elseif ADDRESS_ONLY
var _x: Any
#else
#error("pick one")
#endif
deinit {
self.foo() // expected-error{{'self' cannot be consumed during 'deinit'}}
}

consuming func foo() {}
}

struct T2: ~Copyable {
#if EMPTY
#elseif TRIVIAL
var _x: Int
#elseif LOADABLE
var _x: AnyObject
#elseif ADDRESS_ONLY
var _x: Any
#else
#error("pick one")
#endif

deinit {
let myself = self // expected-error{{'self' cannot be consumed during 'deinit'}}
_ = myself
}
}

func consume<T: ~Copyable>(_: consuming T) {}

struct NC<T: ~Copyable>: ~Copyable {
var x: T
}

struct T3: ~Copyable {
#if EMPTY
var _x: NC<()>
#elseif TRIVIAL
var _x: NC<Int>
#elseif LOADABLE
var _x: NC<AnyObject>
#elseif ADDRESS_ONLY
var _x: NC<Any>
#else
#error("pick one")
#endif

// consuming the single field of a one-field type is OK
deinit {
consume(_x)
}
}

struct T4: ~Copyable {
#if EMPTY
#elseif TRIVIAL
var _x: Int
#elseif LOADABLE
var _x: AnyObject
#elseif ADDRESS_ONLY
var _x: Any
#else
#error("pick one")
#endif

// the implicit cleanup of the value is OK
deinit {
}
}

struct T5: ~Copyable {
#if EMPTY
var _x: NC<()>
#elseif TRIVIAL
var _x: NC<Int>
#elseif LOADABLE
var _x: NC<AnyObject>
#elseif ADDRESS_ONLY
var _x: NC<Any>
#else
#error("pick one")
#endif

// the implicit cleanup of the value is OK
deinit {
}
}

2 changes: 1 addition & 1 deletion test/SILOptimizer/moveonly_deinits.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// RUN: %target-swift-frontend -sil-verify-all -verify -emit-sil -enable-experimental-feature MoveOnlyEnumDeinits %s
// RUN: %target-swift-frontend -sil-verify-all -verify -emit-sil -enable-experimental-feature MoveOnlyEnumDeinits -enable-experimental-feature ConsumeSelfInDeinit %s

class Klass {}

Expand Down
2 changes: 1 addition & 1 deletion test/SILOptimizer/moveonly_discard.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// RUN: %target-swift-frontend -sil-verify-all -verify -emit-sil -enable-experimental-feature MoveOnlyEnumDeinits %s
// RUN: %target-swift-frontend -sil-verify-all -verify -emit-sil -enable-experimental-feature MoveOnlyEnumDeinits -enable-experimental-feature ConsumeSelfInDeinit %s


func posix_close(_ t: Int) {}
Expand Down