mirror of
https://github.com/apple/swift.git
synced 2025-12-14 20:36:38 +01:00
3de8263b26
For the following discussion, let OP be the switch's operand. This is implemented by: * Modeling switch_enum_addr as not forwarding OP and instead delegate the forwarding of OP to be done by each enum case. This matches what SILGen is actually doing since the actual taking of the address in SIL is done in each enum case block by an unchecked_take_enum_data_addr. * In each enum case, I treat OP as being forwarded into an irrefutable sub-tree. I follow the pattern of other places this is done by creating a CleanupStateRestorationScope and using forwardIntoIrrefutableSubTree. This ensures that if I forward OP in the enum case, it just becomes dormant instead of being thrown away. * Inside each case, there is a bunch of code that does some final preparations to src before dispatching to the inner dispatch. This code was written using old low-level SILValue APIs where ownership is done by hand. I replaced all of that by hand ownership with higher level Managed Value APIs that automatically handle ownership for the user. This simplified the implementation and ensured correctness via SILGenBuilder API invariants. The end result of all of these together is that: 1. The cleanup on OP is still live when we emit the default case later than the cleanups. This eliminates the leak that I am fixing. 2. We have greater correctness since the SILGenBuilder APIs automatically handle ownership for us. 3. We have eliminated some brittle logic that could in the future introduce bugs. Specifically, I noticed that if we were ever given a ConsumableManagedValue that was CopyOnSuccess or BorrowAlways but its ManagedValue was a +1 value, we would leak. I could not figure out how to create a Swift test case that would go down this code path though = (. But that being said, it is one new language feature away from being broken. I added some asserts to ConsumableManagedValue that ensures this invariant, so we are safe. If it is inconvenient, we can also cause ConsumableManagedValue to create a new ManagedValue when it detects this condition without the cleanup. But lets see how difficult it is to keep this invariant. In terms of testing, I put in both a SILGen test and also an end<->end interpreter test to ensure this doesn't break again. rdar://71992652 SR-13926
29 lines
569 B
Swift
29 lines
569 B
Swift
// RUN: %target-run-simple-swift
|
|
|
|
// REQUIRES: executable_test
|
|
|
|
import StdlibUnittest
|
|
|
|
var SwitchDefaultTestSuite = TestSuite("Switch.Default")
|
|
defer { runAllTests() }
|
|
|
|
class Klass {}
|
|
protocol Protocol {}
|
|
|
|
enum Enum {
|
|
case value1(LifetimeTracked)
|
|
case value2(Protocol)
|
|
}
|
|
|
|
SwitchDefaultTestSuite.test("do not leak default case payload") {
|
|
// We are passing in Enum.value1 so we go down the default and leak our
|
|
// lifetime tracked.
|
|
func f(_ e: Enum?) {
|
|
switch (e) {
|
|
case .value2: return
|
|
default: return
|
|
}
|
|
}
|
|
f(.value1(LifetimeTracked(0)))
|
|
}
|