Always suspend coroutine before destruction

Summary:
Suspending the coroutine before it's destroyed prevents a dangling
reference to its value.

This dangling reference is exposed when running tests that were
built with `-O3` with ASAN. To run tests at this level, split the
tests up: move tests that cannot be compiled with ASAN at `-O3`,
due to https://bugs.llvm.org/show_bug.cgi?id=34289, into their
own file. The remaining tests stay where they are, with `-O3`
re-enabled.

Reviewed By: yfeldblum

Differential Revision: D7130814

fbshipit-source-id: bf3e2b8f4a086f1786ab46f453c3dd9171d018e8

folly/Expected.h

@@ -1473,7 +1473,7 @@ struct Promise {
   std::experimental::suspend_never initial_suspend() const noexcept {
     return {};
   }
-  std::experimental::suspend_never final_suspend() const {
+  std::experimental::suspend_always final_suspend() const {
     return {};
   }
   template <typename U>

folly/Optional.h

@@ -602,7 +602,7 @@ struct OptionalPromise {
   std::experimental::suspend_never initial_suspend() const noexcept {
     return {};
   }
-  std::experimental::suspend_never final_suspend() const {
+  std::experimental::suspend_always final_suspend() const {
     return {};
   }
   template <typename U>

folly/test/ExpectedCoroutinesPR34289Test.cpp

+/*
+ * Copyright 2017-present Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <folly/Expected.h>
+#include <folly/Portability.h>
+#include <folly/ScopeGuard.h>
+#include <folly/portability/GTest.h>
+
+using namespace folly;
+
+namespace {
+
+struct Exn {};
+
+// not default-constructible, thereby preventing Expected<T, Err> from being
+// default-constructible, forcing our implementation to handle such cases
+class Err {
+ private:
+  enum class Type { Bad, Badder, Baddest };
+
+  Type type_;
+
+  constexpr Err(Type type) : type_(type) {}
+
+ public:
+  Err(Err const&) = default;
+  Err(Err&&) = default;
+  Err& operator=(Err const&) = default;
+  Err& operator=(Err&&) = default;
+
+  friend bool operator==(Err a, Err b) {
+    return a.type_ == b.type_;
+  }
+  friend bool operator!=(Err a, Err b) {
+    return a.type_ != b.type_;
+  }
+
+  static constexpr Err bad() {
+    return Type::Bad;
+  }
+  static constexpr Err badder() {
+    return Type::Badder;
+  }
+  static constexpr Err baddest() {
+    return Type::Baddest;
+  }
+};
+
+Expected<int, Err> f1() {
+  return 7;
+}
+
+Expected<double, Err> f2(int x) {
+  return 2.0 * x;
+}
+
+// move-only type
+Expected<std::unique_ptr<int>, Err> f3(int x, double y) {
+  return std::make_unique<int>(int(x + y));
+}
+
+} // namespace
+
+#if FOLLY_HAS_COROUTINES
+
+TEST(Expected, CoroutineSuccess) {
+  auto r0 = []() -> Expected<int, Err> {
+    auto x = co_await f1();
+    EXPECT_EQ(7, x);
+    auto y = co_await f2(x);
+    EXPECT_EQ(2.0 * 7, y);
+    auto z = co_await f3(x, y);
+    EXPECT_EQ(int(2.0 * 7 + 7), *z);
+    co_return* z;
+  }();
+  EXPECT_TRUE(r0.hasValue());
+  EXPECT_EQ(21, *r0);
+}
+
+#endif

folly/test/ExpectedCoroutinesTest.cpp

@@ -67,11 +67,6 @@ Expected<double, Err> f2(int x) {
   return 2.0 * x;
 }
 
-// move-only type
-Expected<std::unique_ptr<int>, Err> f3(int x, double y) {
-  return std::make_unique<int>(int(x + y));
-}
-
 // error result
 Expected<int, Err> f4(int, double, Err err) {
   return makeUnexpected(err);
@@ -86,20 +81,6 @@ Expected<int, Err> throws() {
 
 #if FOLLY_HAS_COROUTINES
 
-TEST(Expected, CoroutineSuccess) {
-  auto r0 = []() -> Expected<int, Err> {
-    auto x = co_await f1();
-    EXPECT_EQ(7, x);
-    auto y = co_await f2(x);
-    EXPECT_EQ(2.0 * 7, y);
-    auto z = co_await f3(x, y);
-    EXPECT_EQ(int(2.0 * 7 + 7), *z);
-    co_return* z;
-  }();
-  EXPECT_TRUE(r0.hasValue());
-  EXPECT_EQ(21, *r0);
-}
-
 TEST(Expected, CoroutineFailure) {
   auto r1 = []() -> Expected<int, Err> {
     auto x = co_await f1();
@@ -137,4 +118,17 @@ TEST(Expected, CoroutineCleanedUp) {
   EXPECT_EQ(1, count_dest);
 }
 
+Expected<int, Err> f5(int x) {
+  return makeExpected<Err>(x * 3);
+}
+
+Expected<int, Err> f6(int n) {
+  auto x = co_await f5(n);
+  co_return x + 9;
+}
+
+TEST(Optional, CoroutineDanglingReference) {
+  EXPECT_EQ(*f6(1), 12);
+}
+
 #endif

folly/test/OptionalCoroutinesPR34289Test.cpp

+/*
+ * Copyright 2017-present Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <folly/Optional.h>
+#include <folly/Portability.h>
+#include <folly/ScopeGuard.h>
+#include <folly/portability/GTest.h>
+
+#if FOLLY_HAS_COROUTINES
+using folly::Optional;
+
+Optional<int> f1() {
+  return 7;
+}
+Optional<double> f2(int x) {
+  return 2.0 * x;
+}
+
+// move-only type
+Optional<std::unique_ptr<int>> f3(int x, double y) {
+  return std::make_unique<int>((int)(x + y));
+}
+
+TEST(Optional, CoroutineSuccess) {
+  auto r0 = []() -> Optional<int> {
+    auto x = co_await f1();
+    EXPECT_EQ(7, x);
+    auto y = co_await f2(x);
+    EXPECT_EQ(2.0 * 7, y);
+    auto z = co_await f3(x, y);
+    EXPECT_EQ((int)(2.0 * 7 + 7), *z);
+    co_return* z;
+  }();
+  EXPECT_TRUE(r0.hasValue());
+  EXPECT_EQ(21, *r0);
+}
+
+#endif

folly/test/OptionalCoroutinesTest.cpp

@@ -34,20 +34,6 @@ Optional<std::unique_ptr<int>> f3(int x, double y) {
   return std::make_unique<int>((int)(x + y));
 }
 
-TEST(Optional, CoroutineSuccess) {
-  auto r0 = []() -> Optional<int> {
-    auto x = co_await f1();
-    EXPECT_EQ(7, x);
-    auto y = co_await f2(x);
-    EXPECT_EQ(2.0 * 7, y);
-    auto z = co_await f3(x, y);
-    EXPECT_EQ((int)(2.0 * 7 + 7), *z);
-    co_return* z;
-  }();
-  EXPECT_TRUE(r0.hasValue());
-  EXPECT_EQ(21, *r0);
-}
-
 Optional<int> f4(int, double) {
   return folly::none;
 }
@@ -98,4 +84,17 @@ TEST(Optional, CoroutineCleanedUp) {
   EXPECT_EQ(1, count_dest);
 }
 
+folly::Optional<int> f5(int x) {
+  return folly::make_optional(x * 3);
+}
+
+folly::Optional<int> f6(int n) {
+  auto x = co_await f5(n);
+  co_return x + 9;
+}
+
+TEST(Optional, CoroutineDanglingReference) {
+  EXPECT_EQ(*f6(1), 12);
+}
+
 #endif