Skip to content
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
37 changes: 24 additions & 13 deletions lib/SILOptimizer/Mandatory/DIMemoryUseCollector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,17 +39,22 @@ static bool isVariableOrResult(MarkUninitializedInst *MUI) {
static void gatherDestroysOfContainer(const DIMemoryObjectInfo &memoryInfo,
DIElementUseInfo &useInfo) {
auto *uninitMemory = memoryInfo.getUninitializedValue();
auto origUninitMemory = uninitMemory;

if (auto mui = dyn_cast<MarkUnresolvedNonCopyableValueInst>(uninitMemory)) {
origUninitMemory = cast<SingleValueInstruction>(mui->getOperand());
}

// The uninitMemory must be used on an alloc_box, alloc_stack, or global_addr.
// If we have an alloc_stack or a global_addr, there is nothing further to do.
if (isa<AllocStackInst>(uninitMemory->getOperand(0)) ||
isa<GlobalAddrInst>(uninitMemory->getOperand(0)) ||
isa<SILArgument>(uninitMemory->getOperand(0)) ||
if (isa<AllocStackInst>(origUninitMemory->getOperand(0)) ||
isa<GlobalAddrInst>(origUninitMemory->getOperand(0)) ||
isa<SILArgument>(origUninitMemory->getOperand(0)) ||
// FIXME: We only support pointer to address here to not break LLDB. It is
// important that long term we get rid of this since this is a situation
// where LLDB is breaking SILGen/DI invariants by not creating a new
// independent stack location for the pointer to address.
isa<PointerToAddressInst>(uninitMemory->getOperand(0))) {
isa<PointerToAddressInst>(origUninitMemory->getOperand(0))) {
return;
}

Expand All @@ -58,8 +63,8 @@ static void gatherDestroysOfContainer(const DIMemoryObjectInfo &memoryInfo,
//
// TODO: This should really be tracked separately from other destroys so that
// we distinguish the lifetime of the container from the value itself.
assert(isa<ProjectBoxInst>(uninitMemory));
auto value = uninitMemory->getOperand(0);
assert(isa<ProjectBoxInst>(origUninitMemory));
auto value = origUninitMemory->getOperand(0);
if (auto *bbi = dyn_cast<BeginBorrowInst>(value)) {
value = bbi->getOperand();
}
Expand Down Expand Up @@ -181,17 +186,23 @@ SILInstruction *DIMemoryObjectInfo::getFunctionEntryPoint() const {

static SingleValueInstruction *
getUninitializedValue(MarkUninitializedInst *MemoryInst) {
SILValue inst = MemoryInst;
if (auto *bbi = MemoryInst->getSingleUserOfType<BeginBorrowInst>()) {
inst = bbi;
SingleValueInstruction *inst = MemoryInst;
SILValue projectBoxUser = inst;

// Check for a project_box instruction (possibly via a borrow).
if (auto *bbi = projectBoxUser->getSingleUserOfType<BeginBorrowInst>()) {
projectBoxUser = bbi;
}
if (auto pbi = projectBoxUser->getSingleUserOfType<ProjectBoxInst>()) {
inst = pbi;
}

if (SingleValueInstruction *svi =
inst->getSingleUserOfType<ProjectBoxInst>()) {
return svi;
// Access move-only values through their marker.
if (auto mui = inst->getSingleUserOfType<MarkUnresolvedNonCopyableValueInst>()) {
inst = mui;
}

return MemoryInst;
return inst;
}

SingleValueInstruction *DIMemoryObjectInfo::getUninitializedValue() const {
Expand Down
40 changes: 40 additions & 0 deletions test/Interpreter/moveonly_throw_partial_init.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// RUN: %target-run-simple-swift
// REQUIRES: executable_test

struct Butt: Error {}

struct Inside: ~Copyable {
init() throws { throw Butt() }
}

class C {
deinit { print("destroying") }
}

struct Outside: ~Copyable {
let instance: Inside
var c: C

init() throws {
c = C()
let instance = try Inside()
self.instance = instance
}
}

// CHECK: begin
// CHECK-NEXT: destroying
// CHECK-NEXT: caught
// CHECK-NEXT: end

func test() {
print("begin")
do {
_ = try Outside()
} catch {
print("caught")
}
print("end")
}
test()

40 changes: 40 additions & 0 deletions test/SILOptimizer/definite_init_moveonly.sil
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// RUN: %target-sil-opt -module-name definite_init_moveonly -enable-sil-verify-all -definite-init %s | %FileCheck %s
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you look into why the verifier did not flag the double destroy before this change? is that a separate problem?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I filed another bug to look into that.


import Swift

class C {
init()
}

struct S: ~Copyable {
var c1: C
var c2: C
}

// Ensure that, when DI introduces cleanups for partially initialized move-
// only values, the cleanups are applied under the `mark_unresolved`
// instruction, so that the move checker properly accounts for the cleanups.
// #85063 | rdar://163194098

// CHECK-LABEL: sil [ossa] @$test
sil [ossa] @$test : $@convention(thin) (@owned C) -> () {
entry(%c : @owned $C):
%s = alloc_stack [lexical] [var_decl] $S, var, name "butts", type $S
%s_marked = mark_uninitialized [rootself] %s
// CHECK: [[S_MOVEONLY:%.*]] = mark_unresolved_non_copyable_value
%s_moveonly = mark_unresolved_non_copyable_value [consumable_and_assignable] %s_marked
%s_access = begin_access [modify] [static] %s_moveonly
%c1 = struct_element_addr %s_access, #S.c1
assign %c to %c1
end_access %s_access
// CHECK: br [[NEXT:bb[0-9]+]]
br next

// CHECK: [[NEXT]]:
// CHECK: [[FIELD_TO_DESTROY:%.*]] = struct_element_addr [[S_MOVEONLY]], #S.c1
// CHECK: destroy_addr [[FIELD_TO_DESTROY]]
next:
destroy_addr %s_moveonly
dealloc_stack %s
return undef : $()
}
73 changes: 73 additions & 0 deletions test/SILOptimizer/moveonly_partial_consumption_inlinable.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
// RUN: %target-swift-frontend -enable-library-evolution -emit-sil -verify %s

public struct Consumable: ~Copyable {}

public func consume(_: consuming Consumable) {}

public struct NotFrozen: ~Copyable {
public var field: Consumable

public consuming func notInlinable() {
consume(field)
}

@inlinable
public consuming func inlinable() {
// expected-error @+1 {{consumed but not reinitialized}}
consume(field) // expected-note{{}}
}

@_alwaysEmitIntoClient
public consuming func aeic() {
// expected-error @+1 {{consumed but not reinitialized}}
consume(field) // expected-note{{}}
}

@_transparent
public consuming func transparent() {
// expected-error @+1 {{consumed but not reinitialized}}
consume(field) // expected-note{{}}
}
}

@frozen
public struct Frozen: ~Copyable {
public var field: Consumable

public consuming func notInlinable() {
consume(field)
}

@inlinable
public consuming func inlinable() {
consume(field)
}
}

@usableFromInline
internal struct NotFrozenUFI: ~Copyable {
public var field: Consumable

public consuming func notInlinable() {
consume(field)
}

@inlinable
public consuming func inlinable() {
// expected-error @+1 {{consumed but not reinitialized}}
consume(field) // expected-note{{}}
}

@_alwaysEmitIntoClient
public consuming func aeic() {
// expected-error @+1 {{consumed but not reinitialized}}
consume(field) // expected-note{{}}
}

@_transparent
public consuming func transparent() {
// expected-error @+1 {{consumed but not reinitialized}}
consume(field) // expected-note{{}}
}
}