Skip to content

[move-only] Ban destructuring of noncopyable address only types like we do for loadable types. #66092

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
12 changes: 12 additions & 0 deletions lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1867,6 +1867,18 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
if (!leafRange)
return false;

// Now check if we have a destructure through deinit. If we do, emit an
// error.
unsigned numDiagnostics =
moveChecker.diagnosticEmitter.getDiagnosticCount();
checkForDestructureThroughDeinit(markedValue, op, *leafRange,
diagnosticEmitter);
if (numDiagnostics != moveChecker.diagnosticEmitter.getDiagnosticCount()) {
LLVM_DEBUG(llvm::dbgs()
<< "Emitting destructure through deinit error!\n");
return true;
}

LLVM_DEBUG(llvm::dbgs() << "Pure consuming use: " << *user);
useState.takeInsts.insert({user, *leafRange});
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,44 +14,50 @@ class MoveOnlyKlass {
var value: Int = 0
}

@_moveOnly
struct KlassPair {
struct KlassPair : ~Copyable {
var lhs: Klass
var rhs: MoveOnlyKlass
}

@_moveOnly
struct AggStruct {
struct AggStruct : ~Copyable {
var pair: KlassPair
}

@_moveOnly
struct KlassPair2 {
struct KlassPair2 : ~Copyable {
var lhs: MoveOnlyKlass
var rhs: MoveOnlyKlass
}

@_moveOnly
struct AggStruct2 {
struct AggStruct2 : ~Copyable {
var lhs: MoveOnlyKlass
var pair: KlassPair2
var rhs: MoveOnlyKlass
}

@_moveOnly
struct SingleIntContainingStruct {
struct SingleIntContainingStruct : ~Copyable {
var value: Int = 0
}

struct MoveOnlyPair : ~Copyable {
var first = SingleIntContainingStruct()
var second = SingleIntContainingStruct()
}

protocol P {
static var value: Self { get }
}

func consume<T : P>(_ x: consuming T) {}
func consume(_ x: consuming SingleIntContainingStruct) {}
func consume(_ x: consuming MoveOnlyKlass) {}
func consume(_ x: consuming MoveOnlyPair) {}
func consume(_ x: consuming Klass) {}

////////////////////
// Test Top Level //
// MARK: Loadable //
////////////////////

@_moveOnly
struct DeinitStruct {
struct DeinitStruct : ~Copyable {
var first: Klass
var second: (Klass, Klass)
var third: KlassPair
Expand Down Expand Up @@ -98,12 +104,12 @@ func testConsumeNonCopyable4(_ x: consuming DeinitStruct) {
consume(x.fifth) // expected-note {{consuming use here}}
}

/////////////////
// Test Fields //
/////////////////

@_moveOnly
struct StructContainDeinitStruct {
///////////////////////////
// MARK: Loadable Fields //
///////////////////////////

struct StructContainDeinitStruct : ~Copyable {
var first: DeinitStruct
var second: (DeinitStruct, DeinitStruct)
var third: Klass
Expand Down Expand Up @@ -153,3 +159,85 @@ func testStructContainStructContainDeinitStructConsumeNonCopyable4(_ x: consumin
// expected-error @-1 {{Cannot partially consume 'x' since it contains field 'x.first' whose type 'DeinitStruct' has a user defined deinit}}
consume(x.first.fifth) // expected-note {{consuming use here}}
}

////////////////////////
// MARK: Address Only //
////////////////////////

struct AddressOnlyDeinitStruct<T : P> : ~Copyable {
var copyable: T = T.value
var moveOnly = SingleIntContainingStruct()
var moveOnlyPair = MoveOnlyPair()

deinit {}
// expected-note @-1 {{deinit declared here}}
// expected-note @-2 {{deinit declared here}}
// expected-note @-3 {{deinit declared here}}
// expected-note @-4 {{deinit declared here}}
// expected-note @-5 {{deinit declared here}}
}

func consume<T : P>(_ x: consuming AddressOnlyDeinitStruct<T>) {}

func testAddressOnlyCanConsumeEntireType<T : P>(_ x: consuming AddressOnlyDeinitStruct<T>) {
// This is ok since we are consuming a copyable value.
consume(x.copyable)
// This is ok since we are consuming the entire value.
consume(x)
}

func testAddressOnlyCannotPartialConsume<T : P>(_ x: consuming AddressOnlyDeinitStruct<T>) {
// expected-error @-1 {{Cannot partially consume 'x' since it has a user defined deinit}}
// expected-error @-2 {{Cannot partially consume 'x' since it has a user defined deinit}}
consume(x.moveOnly) // expected-note {{consuming use here}}
consume(x.moveOnlyPair.first) // expected-note {{consuming use here}}
consume(x.copyable)
}

struct AddressOnlyContainingDeinitStruct<T : P> : ~Copyable {
var a = AddressOnlyDeinitStruct<T>()
}

func testAddressOnlyCannotPartialConsumeEvenIfSingleElt<T : P>(_ x: consuming AddressOnlyContainingDeinitStruct<T>) {
// expected-error @-1 {{Cannot partially consume 'x' since it contains field 'x.a' whose type 'AddressOnlyDeinitStruct' has a user defined deinit}}

// We do not error here since we can partially consume x, but not x.a
consume(x.a)
consume(x.a.moveOnlyPair) // expected-note {{consuming use here}}
}

struct AddressOnlyContainingDeinitSingleField<T : P> : ~Copyable {
var moveOnly = SingleIntContainingStruct()
deinit {}
// expected-note @-1 {{deinit declared here}}
}

struct AddressOnlyContainingDeinitStruct3<T : P> : ~Copyable {
var a = AddressOnlyContainingDeinitSingleField<T>()
}

func consume<T : P>(_ x: consuming AddressOnlyContainingDeinitSingleField<T>) {}

func testAddressOnlyCannotPartialConsumeEvenIfSingleElt<T : P>(_ x: consuming AddressOnlyContainingDeinitStruct3<T>) {
// expected-error @-1 {{Cannot partially consume 'x' since it contains field 'x.a' whose type 'AddressOnlyContainingDeinitSingleField' has a user defined deinit}}

// We do not error here since we can partially consume x, but not x.a
consume(x.a)
consume(x.a.moveOnly) // expected-note {{consuming use here}}
}


struct AddressOnlyContainingDeinitStructPair<T : P> : ~Copyable {
var first = AddressOnlyDeinitStruct<T>()
var second = AddressOnlyDeinitStruct<T>()
}

// Make sure that if the deinit is in an intermediate element of the path that
// we still handle it appropriately.
func testAddressOnlyDeinitInMiddlePath<T : P>(_ x: consuming AddressOnlyContainingDeinitStructPair<T>) {
// expected-error @-1 {{Cannot partially consume 'x' since it contains field 'x.first' whose type 'AddressOnlyDeinitStruct' has a user defined deinit}}
// expected-error @-2 {{Cannot partially consume 'x' since it contains field 'x.first' whose type 'AddressOnlyDeinitStruct' has a user defined deinit}}
consume(x.first.moveOnly) // expected-note {{consuming use here}}
consume(x.first.moveOnlyPair.first) // expected-note {{consuming use here}}
consume(x.first.copyable)
}