Some portability fixes discovered with gcc-10 -fcoroutines

Summary:
By and large, trying to test folly::coro with gcc-10's (very green) coroutines implementation was a failure, but it did turn up a couple of issues:

* Friends of friends are not friends with gcc
* -Wshadow picks up a few more instances of shadowing
* `std::exchange` needs `#include <utility>`.

Reviewed By: lewissbaker

Differential Revision: D21190145

fbshipit-source-id: d3f3148baa26bb8a2c0c1954fc930fd8124672e1

folly/experimental/coro/Baton.cpp

@@ -21,6 +21,7 @@
 #include <folly/experimental/coro/Baton.h>
 
 #include <cassert>
+#include <utility>
 
 using namespace folly::coro;
 

folly/experimental/coro/Merge-inl.h

@@ -37,8 +37,8 @@ AsyncGenerator<Reference, Value> merge(
     folly::Executor::KeepAlive<> executor,
     AsyncGenerator<AsyncGenerator<Reference, Value>> sources) {
   struct SharedState {
-    explicit SharedState(folly::Executor::KeepAlive<> executor)
-        : executor(std::move(executor)) {}
+    explicit SharedState(folly::Executor::KeepAlive<> executor_)
+        : executor(std::move(executor_)) {}
 
     const folly::Executor::KeepAlive<> executor;
     const folly::CancellationSource cancelSource;
@@ -50,21 +50,21 @@ AsyncGenerator<Reference, Value> merge(
 
   auto makeConsumerTask =
       [](std::shared_ptr<SharedState> state,
-         AsyncGenerator<AsyncGenerator<Reference, Value>> sources)
+         AsyncGenerator<AsyncGenerator<Reference, Value>> sources_)
       -> Task<void> {
-    auto makeWorkerTask = [](std::shared_ptr<SharedState> state,
+    auto makeWorkerTask = [](std::shared_ptr<SharedState> state_,
                              AsyncGenerator<Reference, Value> generator)
         -> detail::DetachedBarrierTask {
       exception_wrapper ex;
-      auto cancelToken = state->cancelSource.getToken();
+      auto cancelToken = state_->cancelSource.getToken();
       try {
         while (auto item = co_await co_viaIfAsync(
-                   state->executor.get_alias(),
+                   state_->executor.get_alias(),
                    co_withCancellation(cancelToken, generator.next()))) {
           // We have a new value to emit in the merged stream.
           {
             auto lock = co_await co_viaIfAsync(
-                state->executor.get_alias(), state->mutex.co_scoped_lock());
+                state_->executor.get_alias(), state_->mutex.co_scoped_lock());
 
             if (cancelToken.isCancellationRequested()) {
               // Consumer has detached and doesn't want any more values.
@@ -73,17 +73,17 @@ AsyncGenerator<Reference, Value> merge(
             }
 
             // Publish the value.
-            state->record = CallbackRecord<Reference>{callback_record_value,
-                                                      *std::move(item)};
-            state->recordPublished.post();
+            state_->record = CallbackRecord<Reference>{callback_record_value,
+                                                       *std::move(item)};
+            state_->recordPublished.post();
 
             // Wait until the consumer is finished with it.
             co_await co_viaIfAsync(
-                state->executor.get_alias(), state->recordConsumed);
-            state->recordConsumed.reset();
+                state_->executor.get_alias(), state_->recordConsumed);
+            state_->recordConsumed.reset();
 
             // Clear the result before releasing the lock.
-            state->record = {};
+            state_->record = {};
           }
 
           if (cancelToken.isCancellationRequested()) {
@@ -97,14 +97,14 @@ AsyncGenerator<Reference, Value> merge(
       }
 
       if (ex) {
-        state->cancelSource.requestCancellation();
+        state_->cancelSource.requestCancellation();
 
         auto lock = co_await co_viaIfAsync(
-            state->executor.get_alias(), state->mutex.co_scoped_lock());
-        if (!state->record.hasError()) {
-          state->record =
+            state_->executor.get_alias(), state_->mutex.co_scoped_lock());
+        if (!state_->record.hasError()) {
+          state_->record =
               CallbackRecord<Reference>{callback_record_error, std::move(ex)};
-          state->recordPublished.post();
+          state_->recordPublished.post();
         }
       };
     };
@@ -113,7 +113,7 @@ AsyncGenerator<Reference, Value> merge(
 
     exception_wrapper ex;
     try {
-      while (auto item = co_await sources.next()) {
+      while (auto item = co_await sources_.next()) {
         if (state->cancelSource.isCancellationRequested()) {
           break;
         }

folly/experimental/coro/Task.h

@@ -466,6 +466,10 @@ class FOLLY_NODISCARD Task {
   class Awaiter;
   using handle_t = std::experimental::coroutine_handle<promise_type>;
 
+  void setExecutor(folly::Executor::KeepAlive<>&& e) noexcept {
+    coro_.promise().executor_ = std::move(e);
+  }
+
  public:
   Task(const Task& t) = delete;
 
@@ -492,7 +496,7 @@ class FOLLY_NODISCARD Task {
   /// task on the specified executor.
   FOLLY_NODISCARD
   TaskWithExecutor<T> scheduleOn(Executor::KeepAlive<> executor) && noexcept {
-    coro_.promise().executor_ = std::move(executor);
+    setExecutor(std::move(executor));
     return TaskWithExecutor<T>{std::exchange(coro_, {})};
   }
 
@@ -508,7 +512,7 @@ class FOLLY_NODISCARD Task {
       Executor::KeepAlive<> executor,
       Task<T>&& t) noexcept {
     // Child task inherits the awaiting task's executor
-    t.coro_.promise().executor_ = std::move(executor);
+    t.setExecutor(std::move(executor));
     return Awaiter{std::exchange(t.coro_, {})};
   }
 

folly/experimental/coro/ViaIfAsync.h

@@ -227,7 +227,8 @@ class ViaIfAsyncAwaiter {
           int> = 0>
   auto
   await_suspend(std::experimental::coroutine_handle<> continuation) noexcept(
-      noexcept(awaiter_.await_suspend(continuation))) -> Result {
+      noexcept(std::declval<Awaiter&>().await_suspend(continuation)))
+      -> Result {
     return awaiter_.await_suspend(
         viaCoroutine_.getWrappedCoroutine(continuation));
   }
@@ -239,7 +240,8 @@ class ViaIfAsyncAwaiter {
           int> = 0>
   auto
   await_suspend(std::experimental::coroutine_handle<> continuation) noexcept(
-      noexcept(awaiter_.await_suspend(continuation))) -> Result {
+      noexcept(std::declval<Awaiter&>().await_suspend(continuation)))
+      -> Result {
     return awaiter_.await_suspend(
         viaCoroutine_.getWrappedCoroutineWithSavedContext(continuation));
   }

folly/experimental/coro/test/GeneratorTest.cpp

@@ -128,10 +128,10 @@ TEST_F(GeneratorTest, DestroyedBeforeCompletion_DestructsObjectsOnStack) {
 }
 
 TEST_F(GeneratorTest, SimpleRecursiveYield) {
-  auto f = [](int n, auto& f) -> Generator<const std::uint32_t> {
+  auto f = [](int n, auto& f_) -> Generator<const std::uint32_t> {
     co_yield n;
     if (n > 0) {
-      co_yield f(n - 1, f);
+      co_yield f_(n - 1, f_);
       co_yield n;
     }
   };
@@ -188,7 +188,7 @@ TEST_F(GeneratorTest, NestedEmptyYield) {
 TEST_F(GeneratorTest, ExceptionThrownFromRecursiveCall_CanBeCaughtByCaller) {
   class SomeException : public std::exception {};
 
-  auto f = [](std::uint32_t depth, auto&& f) -> Generator<std::uint32_t> {
+  auto f = [](std::uint32_t depth, auto&& f_) -> Generator<std::uint32_t> {
     if (depth == 1u) {
       throw SomeException{};
     }
@@ -196,7 +196,7 @@ TEST_F(GeneratorTest, ExceptionThrownFromRecursiveCall_CanBeCaughtByCaller) {
     co_yield 1;
 
     try {
-      co_yield f(1, f);
+      co_yield f_(1, f_);
     } catch (const SomeException&) {
     }
 
@@ -215,14 +215,14 @@ TEST_F(GeneratorTest, ExceptionThrownFromRecursiveCall_CanBeCaughtByCaller) {
 TEST_F(GeneratorTest, ExceptionThrownFromNestedCall_CanBeCaughtByCaller) {
   class SomeException : public std::exception {};
 
-  auto f = [](std::uint32_t depth, auto&& f) -> Generator<std::uint32_t> {
+  auto f = [](std::uint32_t depth, auto&& f_) -> Generator<std::uint32_t> {
     if (depth == 4u) {
       throw SomeException{};
     } else if (depth == 3u) {
       co_yield 3;
 
       try {
-        co_yield f(4, f);
+        co_yield f_(4, f_);
       } catch (const SomeException&) {
       }
 
@@ -232,7 +232,7 @@ TEST_F(GeneratorTest, ExceptionThrownFromNestedCall_CanBeCaughtByCaller) {
     } else if (depth == 2u) {
       bool caught = false;
       try {
-        co_yield f(3, f);
+        co_yield f_(3, f_);
       } catch (const SomeException&) {
         caught = true;
       }
@@ -242,8 +242,8 @@ TEST_F(GeneratorTest, ExceptionThrownFromNestedCall_CanBeCaughtByCaller) {
       }
     } else {
       co_yield 1;
-      co_yield f(2, f);
-      co_yield f(3, f);
+      co_yield f_(2, f_);
+      co_yield f_(3, f_);
     }
   };
 

folly/experimental/coro/test/MergeTest.cpp

@@ -50,7 +50,7 @@ TEST_F(MergeTest, SimpleMerge) {
           co_yield makeGenerator(3, 2);
         }());
 
-    const std::array<int, 5> expectedValues = {0, 3, 1, 4, 2};
+    const std::array<int, 5> expectedValues = {{0, 3, 1, 4, 2}};
 
     auto item = co_await generator.next();
     for (int expectedValue : expectedValues) {