[SILGen] Make sure enum element is always cleaned up.

lib/SILGen/SILGenPattern.cpp

@@ -2048,9 +2048,12 @@ void PatternMatchEmission::emitEnumElementDispatch(
     break;
   }
 
+  CleanupCloner srcValueCleanupCloner(SGF, src.getFinalManagedValue());
+
   // Emit the switch_enum_addr instruction.
-  SILValue srcValue = src.getFinalManagedValue().forward(SGF);
-  SGF.B.createSwitchEnumAddr(loc, srcValue, blocks.getDefaultBlock(),
+  ManagedValue srcValue =
+      ManagedValue::forUnmanaged(src.getFinalManagedValue().forward(SGF));
+  SGF.B.createSwitchEnumAddr(loc, srcValue.getValue(), blocks.getDefaultBlock(),
                              blocks.getCaseBlocks(), blocks.getCounts(),
                              defaultCaseCount);
 
@@ -2076,6 +2079,13 @@ void PatternMatchEmission::emitEnumElementDispatch(
       hasElt = !eltTy.getASTType()->isVoid();
     }
 
+    // If the element type is trivial, the enum won't be consumed. In this case,
+    // make sure we emit a cleanup for the enum itself (not the data, which is
+    // trivial).
+    if (!hasElt || eltTy.isTrivial(SGF.getFunction())) {
+      srcValueCleanupCloner.clone(srcValue.getValue());
+    }
+
     ConsumableManagedValue eltCMV, origCMV;
 
     // Empty cases.  Try to avoid making an empty tuple value if it's
@@ -2124,8 +2134,8 @@ void PatternMatchEmission::emitEnumElementDispatch(
       // nondestructively from an enum.
       switch (eltConsumption) {
       case CastConsumptionKind::TakeAlways:
-        eltValue =
-            SGF.B.createUncheckedTakeEnumDataAddr(loc, srcValue, elt, eltTy);
+        eltValue = SGF.B.createUncheckedTakeEnumDataAddr(
+            loc, srcValue.getValue(), elt, eltTy);
         break;
       case CastConsumptionKind::BorrowAlways:
         // If we reach this point, we know that we have a loadable
@@ -2135,9 +2145,14 @@ void PatternMatchEmission::emitEnumElementDispatch(
         // address only types do not support BorrowAlways.
         llvm_unreachable("not allowed");
       case CastConsumptionKind::CopyOnSuccess: {
-        auto copy = SGF.emitTemporaryAllocation(loc, srcValue->getType());
-        SGF.B.createCopyAddr(loc, srcValue, copy, IsNotTake, IsInitialization);
-        // We can always take from the copy.
+        auto copy = SGF.emitTemporaryAllocation(loc, srcValue.getType());
+        // If the element is trivial it won't be destroyed so we need to make
+        // sure we destroy the enum here.
+        if (eltTL->isTrivial())
+          copy = SGF.emitManagedBufferWithCleanup(copy).getValue();
+        SGF.B.createCopyAddr(loc, srcValue.getValue(), copy, IsNotTake,
+                             IsInitialization);
+        // We can always take from the managedCopy.
         eltConsumption = CastConsumptionKind::TakeAlways;
         eltValue = SGF.B.createUncheckedTakeEnumDataAddr(loc, copy, elt, eltTy);
         break;

test/Interpreter/switch.swift

@@ -282,4 +282,24 @@ SwitchTestSuite.test("Enum Initialization Leaks") {
   }
 }
 
+SwitchTestSuite.test("Destroy all elements: protocol and trivial") {
+  enum ProtocolAndTrivial {
+    case a(P)
+    case b(Int)
+    case c(LifetimeTracked)
+  }
+
+  func testEnumWithProtocolAndTrivial(_ e: ProtocolAndTrivial) -> Bool {
+    switch (e) {
+    case .a(_): return true
+    case .b(_): return true
+    case .c(_): return true
+    }
+  }
+
+  do {
+    testEnumWithProtocolAndTrivial(.c(LifetimeTracked(0)))
+  }
+}
+
 runAllTests()

test/SILGen/destroy_enum_in_all_cases.swift

@@ -0,0 +1,45 @@
+// RUN: %target-swift-emit-silgen -module-name switch %s > %t.sil
+// RUN: %target-sil-opt %t.sil -sil-combine -enable-sil-verify-all | %FileCheck %s
+
+protocol P { }
+
+enum ProtocolAndTrivial {
+    case a(P)
+    case b(Int)
+}
+
+// This test is mainly checking the verifier doesn't error but, just in case,
+// there's also a filecheck test.
+// CHECK-LABEL: @$s6switch4testyyAA18ProtocolAndTrivialO_ADtF
+// CHECK: [[A1:%[0-9]+]] = alloc_stack $ProtocolAndTrivial
+// CHECK: [[A2:%[0-9]+]] = alloc_stack $ProtocolAndTrivial
+// CHECK: [[C1:%[0-9]+]] = alloc_stack $(ProtocolAndTrivial, ProtocolAndTrivial)
+
+// Make sure we destroy both arg temp allocs.
+// CHECK: copy_addr [take] [[A1]]
+// CHECK: copy_addr [take] [[A2]]
+
+// CHECK: [[CA:%[0-9]+]] = tuple_element_addr [[C1]] : $*(ProtocolAndTrivial, ProtocolAndTrivial), 0
+// CHECK: [[CB:%[0-9]+]] = tuple_element_addr [[C1]] : $*(ProtocolAndTrivial, ProtocolAndTrivial), 1
+
+// CHECK: switch_enum_addr [[CA]] : $*ProtocolAndTrivial, case #ProtocolAndTrivial.a!enumelt: [[BBA:bb[0-9]+]], case #ProtocolAndTrivial.b!enumelt: [[BBB:bb[0-9]+]]
+
+// We're only testing the trivial case.
+// CHECK: [[BBB]]:
+// CHECK: [[TMP:%[0-9]+]] = alloc_stack $ProtocolAndTrivial
+// CHECK: switch_enum_addr [[CB]] : $*ProtocolAndTrivial, case #ProtocolAndTrivial.b!enumelt: [[BOTH_B:bb[0-9]+]], default [[DEFAULT:bb[0-9]+]]
+
+// Make sure that we destroy everything.
+// CHECK: [[BOTH_B]]:
+// CHECK: destroy_addr [[CB]]
+// CHECK: destroy_addr [[TMP]]
+// CHECK: destroy_addr [[CA]]
+
+// CHECK-LABEL: end sil function '$s6switch4testyyAA18ProtocolAndTrivialO_ADtF'
+func test(_ a : ProtocolAndTrivial, _ b : ProtocolAndTrivial) {
+    switch ((a, b)) {
+    case (.a(_), .a(_)): _ = ()
+    case (.b(_), .b(_)): _ = ()
+    default: _ = ()
+    }
+}

test/SILGen/enum_resilience.swift

@@ -15,9 +15,11 @@ import resilient_enum
 // CHECK-NEXT:    [[METATYPE:%.+]] = value_metatype $@thick Medium.Type, [[BOX]] : $*Medium
 // CHECK-NEXT:    switch_enum_addr [[BOX]] : $*Medium, case #Medium.Paper!enumelt: bb1, case #Medium.Canvas!enumelt: bb2, case #Medium.Pamphlet!enumelt: bb3, case #Medium.Postcard!enumelt: bb4, default bb5
 // CHECK:       bb1:
+// CHECK-NEXT:    destroy_addr [[BOX]]
 // CHECK-NEXT:    dealloc_stack [[BOX]]
 // CHECK-NEXT:    br bb6
 // CHECK:       bb2:
+// CHECK-NEXT:    destroy_addr [[BOX]]
 // CHECK-NEXT:    dealloc_stack [[BOX]]
 // CHECK-NEXT:    br bb6
 // CHECK:       bb3:

test/SILGen/enum_resilience_testable.swift

@@ -20,9 +20,11 @@
 // CHECK-NEXT:    [[METATYPE:%.+]] = value_metatype $@thick Medium.Type, [[BOX]] : $*Medium
 // CHECK-NEXT:    switch_enum_addr [[BOX]] : $*Medium, case #Medium.Paper!enumelt: bb1, case #Medium.Canvas!enumelt: bb2, case #Medium.Pamphlet!enumelt: bb3, case #Medium.Postcard!enumelt: bb4, default bb5
 // CHECK:       bb1:
+// CHECK-NEXT:    destroy_addr [[BOX]]
 // CHECK-NEXT:    dealloc_stack [[BOX]]
 // CHECK-NEXT:    br bb6
 // CHECK:       bb2:
+// CHECK-NEXT:    destroy_addr [[BOX]]
 // CHECK-NEXT:    dealloc_stack [[BOX]]
 // CHECK-NEXT:    br bb6
 // CHECK:       bb3:

test/SILGen/switch.swift

@@ -1605,3 +1605,26 @@ struct rdar49990484Struct {
     }
   }
 }
+
+enum ProtocolAndTrivial {
+    case a(P)
+    case b(Int)
+}
+
+// Make sure "e" is destroyed in both cases.
+// CHECK-LABEL: sil hidden [ossa] @$s6switch30testEnumWithProtocolAndTrivialySbAA0efG0OF
+// CHECK: bb0(
+// CHECK: [[TMP:%[0-9]+]] = alloc_stack
+// CHECK: bb1:
+// CHECK: [[P:%[0-9]+]] = unchecked_take_enum_data_addr
+// CHECK: destroy_addr [[P]]
+// CHECK: bb2:
+// CHECK: destroy_addr [[TMP]]
+// CHECK-LABEL: end sil function '$s6switch30testEnumWithProtocolAndTrivialySbAA0efG0OF'
+func testEnumWithProtocolAndTrivial(_ e: ProtocolAndTrivial) -> Bool {
+    switch (e) {
+    case .a(_): return true
+    case .b(_): return true
+    }
+}
+