Skip to content

MoveOnlyChecker: Properly insert cleanup for dead try_apply def. #69887

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 1 commit into from
Nov 15, 2023
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
10 changes: 8 additions & 2 deletions lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2984,8 +2984,14 @@ void MoveOnlyAddressCheckerPImpl::insertDestroysOnBoundary(
InitableButNotConsumable)
continue;

auto *insertPt = inst->getNextInstruction();
assert(insertPt && "def instruction was a terminator");
SILInstruction *insertPt;
if (auto tryApply = dyn_cast<TryApplyInst>(inst)) {
// The dead def is only valid on the success return path.
insertPt = &tryApply->getNormalBB()->front();
} else {
insertPt = inst->getNextInstruction();
assert(insertPt && "instruction is a terminator that wasn't handled?");
}
insertDestroyBeforeInstruction(addressUseState, insertPt,
liveness.getRootValue(), defPair.second,
consumes);
Expand Down
29 changes: 29 additions & 0 deletions test/Interpreter/Inputs/moveonly_resilient_type.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
struct MyError: Error {}

public struct Resilient: ~Copyable {
static var nextValue: Int = 0

private(set) public var value: Int
public init(nonthrowing: ()) {
value = Self.nextValue
Self.nextValue += 1
}
deinit { print("resilient deinit \(value)") }

public init(throwing: Bool) throws {
if throwing {
throw MyError()
}
self = .init(nonthrowing: ())
}
public init(throwingAfterInit: Bool) throws {
self = .init(nonthrowing: ())
if throwingAfterInit {
throw MyError()
}
}

public static func instanceCount() -> Int {
return nextValue
}
}
88 changes: 88 additions & 0 deletions test/Interpreter/moveonly_resilient_deinit_on_throw.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
// RUN: %empty-directory(%t)
// RUN: %target-build-swift -emit-module-path %t -enable-library-evolution -module-name moveonly_resilient_type -parse-as-library %S/Inputs/moveonly_resilient_type.swift -c -o %t/moveonly_resilient_type.o
// RUN: %target-build-swift -enable-library-evolution -module-name moveonly_resilient_type -parse-as-library %S/Inputs/moveonly_resilient_type.swift -c -o %t/moveonly_resilient_type.o
// RUN: %target-build-swift -o %t/a.out -I %t %s %t/moveonly_resilient_type.o
// RUN: %target-run %t/a.out | %FileCheck %s

// REQUIRES: executable_test

// CHECK: starting

import moveonly_resilient_type

func makeItem1() throws -> Resilient {
return Resilient(nonthrowing: ())
}

func test1a() throws {
// CHECK-NEXT: resilient deinit 0
_ = try makeItem1()
}
func test1b() throws {
// CHECK-NEXT: resilient deinit 1
let x = try makeItem1()
}

func makeItem2(throwing: Bool) throws -> Resilient {
return try Resilient(throwing: throwing)
}

func test2aa() throws {
// CHECK-NEXT: resilient deinit 2
_ = try makeItem2(throwing: false)
}

func test2ab() throws {
_ = try makeItem2(throwing: true)
}

func test2ba() throws {
// CHECK-NEXT: resilient deinit 3
let x = try makeItem2(throwing: false)
}

func test2bb() throws {
let x = try makeItem2(throwing: true)
}

func makeItem3(throwing: Bool) throws -> Resilient {
return try Resilient(throwingAfterInit: throwing)
}

func test3aa() throws {
// CHECK-NEXT: resilient deinit 4
_ = try makeItem3(throwing: false)
}

func test3ab() throws {
// CHECK-NEXT: resilient deinit 5
_ = try makeItem3(throwing: true)
}

func test3ba() throws {
// CHECK-NEXT: resilient deinit 6
let x = try makeItem3(throwing: false)
}

func test3bb() throws {
// CHECK-NEXT: resilient deinit 7
let x = try makeItem3(throwing: true)
}

func main() {
print("starting")
_ = try? test1a()
Copy link
Contributor

Choose a reason for hiding this comment

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

@jckarter can you add in a main only PR, a test case that validates the SILGen we emit here?

_ = try? test1b()
_ = try? test2aa()
_ = try? test2ab()
_ = try? test2ba()
_ = try? test2bb()
_ = try? test3aa()
_ = try? test3ab()
_ = try? test3ba()
_ = try? test3bb()

// CHECK-NEXT: 8 instances in total
print("\(Resilient.instanceCount()) instances in total")
}
main()