Improve error message if you co_await a non-awaitable (#702)

Author: oldnewthing

strings/base_coroutine_threadpool.h

@@ -107,115 +107,41 @@ namespace winrt::impl
     }
 
     template <typename T>
-    class has_awaitable_member
+    class awaiter_finder
     {
-        template <typename U, typename = decltype(std::declval<U>().await_ready())> static constexpr bool get_value(int) { return true; }
-        template <typename> static constexpr bool get_value(...) { return false; }
+        template <typename U, typename = decltype(std::declval<U>().await_ready())> static constexpr bool find_awaitable_member(int) { return true; }
+        template <typename> static constexpr bool find_awaitable_member(...) { return false; }
 
-    public:
-
-        static constexpr bool value = get_value<T>(0);
-    };
+        template <typename U, typename = decltype(std::declval<U>().operator co_await())> static constexpr bool find_co_await_member(int) { return true; }
+        template <typename> static constexpr bool find_co_await_member(...) { return false; }
 
-    template <typename T>
-    class has_awaitable_free
-    {
-        template <typename U, typename = decltype(await_ready(std::declval<U>()))> static constexpr bool get_value(int) { return true; }
-        template <typename> static constexpr bool get_value(...) { return false; }
+        template <typename U, typename = decltype(operator co_await(std::declval<U>()))> static constexpr bool find_co_await_free(int) { return true; }
+        template <typename> static constexpr bool find_co_await_free(...) { return false; }
 
     public:
 
-        static constexpr bool value = get_value<T>(0);
+        static constexpr bool has_awaitable_member = find_awaitable_member<T>(0);
+        static constexpr bool has_co_await_member = find_co_await_member<T&&>(0);
+        static constexpr bool has_co_await_free = find_co_await_free<T&&>(0);
     };
 
     template <typename T>
-    struct free_await_adapter_impl
+    decltype(auto) get_awaiter(T&& value) noexcept
     {
-        T&& awaitable;
-
-        bool ready()
-        {
-            return await_ready(awaitable);
-        }
-
-        template <typename U>
-        auto suspend(std::experimental::coroutine_handle<U> handle)
+        if constexpr (awaiter_finder<T>::has_co_await_member)
         {
-            return await_suspend(awaitable, handle);
+            static_assert(!awaiter_finder<T>::has_co_await_free, "Ambiguous operator co_await (as both member and free function).");
+            return static_cast<T&&>(value).operator co_await();
         }
-
-        auto resume()
+        else if constexpr (awaiter_finder<T>::has_co_await_free)
         {
-            return await_resume(awaitable);
+            return operator co_await(static_cast<T&&>(value));
         }
-    };
-
-    template <typename T>
-    struct free_await_adapter
-    {
-        T&& awaitable;
-
-        bool await_ready()
-        {
-            return free_await_adapter_impl<T>{ static_cast<T&&>(awaitable) }.ready();
-        }
-
-        template <typename U>
-        auto await_suspend(std::experimental::coroutine_handle<U> handle)
-        {
-            return free_await_adapter_impl<T>{ static_cast<T&&>(awaitable) }.suspend(handle);
-        }
-
-        decltype(auto) await_resume()
-        {
-            return free_await_adapter_impl<T>{ static_cast<T&&>(awaitable) }.resume();
-        }
-    };
-
-    template <typename T>
-    struct member_await_adapter
-    {
-        T&& awaitable;
-
-        bool await_ready()
-        {
-            return awaitable.await_ready();
-        }
-
-        template <typename U>
-        auto await_suspend(std::experimental::coroutine_handle<U> handle)
-        {
-            return awaitable.await_suspend(handle);
-        }
-
-        decltype(auto) await_resume()
+        else
         {
-            return awaitable.await_resume();
+            static_assert(awaiter_finder<T>::has_awaitable_member, "Not an awaitable type");
+            return static_cast<T&&>(value);
         }
-    };
-
-    template <typename T>
-    auto get_awaiter(T&& value) noexcept -> decltype(static_cast<T&&>(value).operator co_await())
-    {
-        return static_cast<T&&>(value).operator co_await();
-    }
-
-    template <typename T>
-    auto get_awaiter(T&& value) noexcept -> decltype(operator co_await(static_cast<T&&>(value)))
-    {
-        return operator co_await(static_cast<T&&>(value));
-    }
-
-    template <typename T, std::enable_if_t<has_awaitable_member<T>::value, int> = 0>
-    auto get_awaiter(T&& value) noexcept
-    {
-        return member_await_adapter<T>{ static_cast<T&&>(value) };
-    }
-
-    template <typename T, std::enable_if_t<has_awaitable_free<T>::value, int> = 0>
-    auto get_awaiter(T&& value) noexcept
-    {
-        return free_await_adapter<T>{ static_cast<T&&>(value) };
     }
 
     template <typename T>

test/test/notify_awaiter.cpp

@@ -5,93 +5,91 @@ using namespace Windows::Foundation;
 
 namespace
 {
-    struct free_awaitable
-    {
-    };
-    bool await_ready(free_awaitable)
-    {
-        return true;
-    }
-    void await_suspend(free_awaitable, std::experimental::coroutine_handle<>)
-    {
-    }
-    void await_resume(free_awaitable)
-    {
+    using std::experimental::suspend_never;
 
-    }
-
-    struct member_awaitable
+    // Never suspends.
+    // Allows copying, but asserts if you try.
+    struct suspend_never_but_assert_if_copied : suspend_never
     {
-        bool await_ready()
+        suspend_never_but_assert_if_copied() = default;
+        suspend_never_but_assert_if_copied(suspend_never_but_assert_if_copied const&)
         {
-            return true;
+            REQUIRE(false);
         }
-        void await_suspend(std::experimental::coroutine_handle<>)
+    };
+
+    // The bad_awaiter asserts if it ever gets used.
+    // Use it for ambiguous alternatives we don't want to pick.
+    struct bad_awaiter : suspend_never
+    {
+        void await_resume() const noexcept
         {
+            REQUIRE(false);
         }
-        void await_resume()
-        {
+    };
 
-        }
+    // If the only awaitable is the member awaitable, then use it.
+    struct member_awaitable : suspend_never_but_assert_if_copied
+    {
     };
 
-    struct free_operator_awaitable
+    // Should pick the free operator co_await over the member awaitable.
+    struct free_operator_awaitable : bad_awaiter
     {
     };
     auto operator co_await(free_operator_awaitable)
     {
-        struct awaitable
-        {
-            bool await_ready()
-            {
-                return true;
-            }
-            void await_suspend(std::experimental::coroutine_handle<>)
-            {
-            }
-            void await_resume()
-            {
-            }
-        };
-        return awaitable{};
+        return suspend_never_but_assert_if_copied{};
     }
 
-    struct member_operator_awaitable
+    // Should pick the member operator co_await over the member awaitable.
+    struct member_operator_awaitable : bad_awaiter
     {
         auto operator co_await()
         {
-            struct awaitable
-            {
-                bool await_ready()
-                {
-                    return true;
-                }
-                void await_suspend(std::experimental::coroutine_handle<>)
-                {
-                }
-                void await_resume()
-                {
-                }
-            };
-            return awaitable{};
+            return suspend_never_but_assert_if_copied{};
         }
     };
 
-    struct no_copy_awaitable
+    // Verify that we can await non-copyable objects.
+    struct no_copy_awaitable : suspend_never
     {
         no_copy_awaitable() = default;
         no_copy_awaitable(no_copy_awaitable const&) = delete;
+    };
 
-        bool await_ready()
+    // operator co_await takes precedence over member awaitable.
+    struct ambiguous_awaitable1 : bad_awaiter
+    {
+        auto operator co_await()
         {
-            return true;
+            return suspend_never_but_assert_if_copied{};
         }
-        void await_suspend(std::experimental::coroutine_handle<>)
+    };
+
+    // This awaitable supports both member co_await
+    // and free co_await, and the free co_await is a
+    // better match if invoked on an lvalue. We don't try
+    // to support this case. We just declare it as ambiguous.
+    struct ambiguous_awaitable2 : bad_awaiter
+    {
+        auto operator co_await() const
         {
+            return bad_awaiter{};
         }
-        void await_resume()
-        {
+    };
+    suspend_never_but_assert_if_copied operator co_await(ambiguous_awaitable2&)
+    {
+        return {};
+    }
 
+    // We invoke this on an lvalue, so the member co_await is unavailable.
+    // Verify that the unavailable co_await is ignored.
+    struct ambiguous_awaitable3 : suspend_never_but_assert_if_copied
+    {
+        auto operator co_await()&&
+        {
+            return bad_awaiter{};
         }
     };
 
@@ -120,18 +118,21 @@ namespace
 
     static std::vector<std::pair<void const*, notification>> watcher;
     static handle start_racing{ CreateEventW(nullptr, true, false, nullptr) };
-    constexpr size_t test_suspension_points = 12;
+    constexpr size_t test_suspension_points = 13;
 
     IAsyncAction Async()
     {
         co_await resume_on_signal(start_racing.get());
         co_await resume_background();
         co_await resume_background();
-        co_await free_awaitable{};
         co_await member_awaitable{};
         co_await free_operator_awaitable{};
         co_await member_operator_awaitable{};
         co_await no_copy_awaitable{};
+        co_await ambiguous_awaitable1{};
+        // co_await ambiguous_awaitable2{}; // does not compile
+        ambiguous_awaitable3 awaitable3;
+        co_await awaitable3;
         co_await AsyncAction();
         co_await AsyncActionWithProgress();
         co_await AsyncOperation();