Merge pull request #1886 from Reflejo/optional-try-leak

[Swift 2.2.1] [SILGen] Fix for 'try?' leaking the error value

Author: tkremenek

lib/SILGen/SILGenExpr.cpp

@@ -988,7 +988,11 @@ RValue RValueEmitter::visitOptionalTryExpr(OptionalTryExpr *E, SGFContext C) {
   // If control branched to the failure block, inject .None into the
   // result type.
   SGF.B.emitBlock(catchBB);
-  (void)catchBB->createBBArg(SILType::getExceptionType(SGF.getASTContext()));
+  FullExpr catchCleanups(SGF.Cleanups, E);
+  auto *errorArg = catchBB->createBBArg(
+      SILType::getExceptionType(SGF.getASTContext()));
+  (void) SGF.emitManagedRValueWithCleanup(errorArg);
+  catchCleanups.pop();
 
   if (isByAddress) {
     SGF.emitInjectOptionalNothingInto(E, optInit->getAddress(), optTL);

test/Interpreter/errors.swift

@@ -0,0 +1,54 @@
+// RUN: %target-run-simple-swift
+// REQUIRES: executable_test
+
+//
+// Tests for error handling.
+//
+
+import StdlibUnittest
+
+// Also import modules which are used by StdlibUnittest internally. This
+// workaround is needed to link all required libraries in case we compile
+// StdlibUnittest with -sil-serialize-all.
+import SwiftPrivate
+#if _runtime(_ObjC)
+import ObjectiveC
+#endif
+
+enum Excuse : ErrorType { case CatAteHomework(LifetimeTracked) }
+
+var ErrorHandlingTests = TestSuite("ErrorHandling")
+
+func furball(b: Bool) throws -> LifetimeTracked {
+  if b {
+    throw Excuse.CatAteHomework(LifetimeTracked(0))
+  } else {
+    return LifetimeTracked(1)
+  }
+}
+
+ErrorHandlingTests.test("tryCatch") {
+  do {
+    try expectEqual(furball(false), LifetimeTracked(1))
+  } catch {
+    expectUnreachable() 
+  }
+
+  do {
+    try furball(true)
+    expectUnreachable() 
+  } catch let e {
+    if case Excuse.CatAteHomework(let c) = e {
+      expectEqual(c, LifetimeTracked(0))
+    } else {
+      expectUnreachable()
+    }
+  }
+}
+
+ErrorHandlingTests.test("tryOptional") {
+  expectEqual(LifetimeTracked(1), try? furball(false))
+  expectEqual(Optional<LifetimeTracked>.None, try? furball(true))
+}
+
+runAllTests()

test/Interpreter/failable_initializers.swift

@@ -13,35 +13,23 @@ import ObjectiveC
 
 var FailableInitTestSuite = TestSuite("FailableInit")
 
-class Canary {
-  static var count: Int = 0
-
-  init() {
-    Canary.count += 1
-  }
-
-  deinit {
-    Canary.count -= 1
-  }
-}
-
 class Bear {
-  let x: Canary
+  let x: LifetimeTracked
 
   /* Designated */
   init(n: Int) {
-    x = Canary()
+    x = LifetimeTracked(0)
   }
 
   init?(n: Int, before: Bool) {
     if before {
       return nil
     }
-    self.x = Canary()
+    self.x = LifetimeTracked(0)
   }
 
   init?(n: Int, after: Bool) {
-    self.x = Canary()
+    self.x = LifetimeTracked(0)
     if after {
       return nil
     }
@@ -51,7 +39,7 @@ class Bear {
     if before {
       return nil
     }
-    self.x = Canary()
+    self.x = LifetimeTracked(0)
     if after {
       return nil
     }
@@ -121,45 +109,45 @@ class Bear {
 }
 
 class PolarBear : Bear {
-  let y: Canary
+  let y: LifetimeTracked
 
   /* Designated */
   override init(n: Int) {
-    self.y = Canary()
+    self.y = LifetimeTracked(0)
     super.init(n: n)
   }
 
   override init?(n: Int, before: Bool) {
     if before {
       return nil
     }
-    self.y = Canary()
+    self.y = LifetimeTracked(0)
     super.init(n: n)
   }
 
   init?(n: Int, during: Bool) {
-    self.y = Canary()
+    self.y = LifetimeTracked(0)
     super.init(n: n, before: during)
   }
 
   init?(n: Int, before: Bool, during: Bool) {
-    self.y = Canary()
+    self.y = LifetimeTracked(0)
     if before {
       return nil
     }
     super.init(n: n, before: during)
   }
 
   override init?(n: Int, after: Bool) {
-    self.y = Canary()
+    self.y = LifetimeTracked(0)
     super.init(n: n)
     if after {
       return nil
     }
   }
 
   init?(n: Int, during: Bool, after: Bool) {
-    self.y = Canary()
+    self.y = LifetimeTracked(0)
     super.init(n: n, before: during)
     if after {
       return nil
@@ -170,7 +158,7 @@ class PolarBear : Bear {
     if before {
       return nil
     }
-    self.y = Canary()
+    self.y = LifetimeTracked(0)
     super.init(n: n)
     if after {
       return nil
@@ -181,7 +169,7 @@ class PolarBear : Bear {
     if before {
       return nil
     }
-    self.y = Canary()
+    self.y = LifetimeTracked(0)
     super.init(n: n, before: during)
     if after {
       return nil
@@ -190,50 +178,50 @@ class PolarBear : Bear {
 }
 
 class GuineaPig<T> : Bear {
-  let y: Canary
+  let y: LifetimeTracked
   let t: T
 
   init?(t: T, during: Bool) {
-    self.y = Canary()
+    self.y = LifetimeTracked(0)
     self.t = t
     super.init(n: 0, before: during)
   }
 }
 
 struct Chimera {
-  let x: Canary
-  let y: Canary
+  let x: LifetimeTracked
+  let y: LifetimeTracked
 
   init?(before: Bool) {
     if before {
       return nil
     }
-    x = Canary()
-    y = Canary()
+    x = LifetimeTracked(0)
+    y = LifetimeTracked(0)
   }
 
   init?(during: Bool) {
-    x = Canary()
+    x = LifetimeTracked(0)
     if during {
       return nil
     }
-    y = Canary()
+    y = LifetimeTracked(0)
   }
 
   init?(before: Bool, during: Bool) {
     if before {
       return nil
     }
-    x = Canary()
+    x = LifetimeTracked(0)
     if during {
       return nil
     }
-    y = Canary()
+    y = LifetimeTracked(0)
   }
 
   init?(after: Bool) {
-    x = Canary()
-    y = Canary()
+    x = LifetimeTracked(0)
+    y = LifetimeTracked(0)
     if after {
       return nil
     }
@@ -243,19 +231,19 @@ struct Chimera {
     if before {
       return nil
     }
-    x = Canary()
-    y = Canary()
+    x = LifetimeTracked(0)
+    y = LifetimeTracked(0)
     if after {
       return nil
     }
   }
 
   init?(during: Bool, after: Bool) {
-    x = Canary()
+    x = LifetimeTracked(0)
     if during {
       return nil
     }
-    y = Canary()
+    y = LifetimeTracked(0)
     if after {
       return nil
     }
@@ -265,11 +253,11 @@ struct Chimera {
     if before {
       return nil
     }
-    x = Canary()
+    x = LifetimeTracked(0)
     if during {
       return nil
     }
-    y = Canary()
+    y = LifetimeTracked(0)
     if after {
       return nil
     }
@@ -287,8 +275,6 @@ FailableInitTestSuite.test("FailableInitFailure_Root") {
   mustFail { Bear(n: 0, after: true) }
   mustFail { Bear(n: 0, before: true, after: false) }
   mustFail { Bear(n: 0, before: false, after: true) }
-
-  expectEqual(0, Canary.count)
 }
 
 FailableInitTestSuite.test("FailableInitFailure_Derived") {
@@ -304,14 +290,10 @@ FailableInitTestSuite.test("FailableInitFailure_Derived") {
   mustFail { PolarBear(n: 0, before: true, during: false, after: false) }
   mustFail { PolarBear(n: 0, before: false, during: true, after: false) }
   mustFail { PolarBear(n: 0, before: false, during: false, after: true) }
-
-  expectEqual(0, Canary.count)
 }
 
 FailableInitTestSuite.test("DesignatedInitFailure_DerivedGeneric") {
-  mustFail { GuineaPig<Canary>(t: Canary(), during: true) }
-
-  expectEqual(0, Canary.count)
+  mustFail { GuineaPig<LifetimeTracked>(t: LifetimeTracked(0), during: true) }
 }
 
 FailableInitTestSuite.test("ConvenienceInitFailure_Root") {
@@ -330,8 +312,6 @@ FailableInitTestSuite.test("ConvenienceInitFailure_Root") {
 
   _ = Bear(IUO: false)
   _ = Bear(force: false)
-
-  expectEqual(0, Canary.count)
 }
 
 FailableInitTestSuite.test("ConvenienceInitFailure_Derived") {
@@ -347,8 +327,6 @@ FailableInitTestSuite.test("ConvenienceInitFailure_Derived") {
   mustFail { PolarBear(before: true, during: false, after: false) }
   mustFail { PolarBear(before: false, during: true, after: false) }
   mustFail { PolarBear(before: false, during: false, after: true) }
-
-  expectEqual(0, Canary.count)
 }
 
 FailableInitTestSuite.test("InitFailure_Struct") {
@@ -364,8 +342,6 @@ FailableInitTestSuite.test("InitFailure_Struct") {
   mustFail { Chimera(before: true, during: false, after: false) }
   mustFail { Chimera(before: false, during: true, after: false) }
   mustFail { Chimera(before: false, during: false, after: true) }
-
-  expectEqual(0, Canary.count)
 }
 
 runAllTests()