[libcxx] [Coroutines] Support noop_coroutine for GCC

[ChuanqiXu]

Summary

We didn't support noop_coroutine for GCC in previous conforming patch. So that GCC couldn't use noop_coroutine() defined in <coroutine>. And after this patch, GCC should be able to compile the whole <coroutine> header.

[• Quuxplusone]

The changes in <__coroutine/noop_coroutine_handle.h> might be correct, but I have no idea and assume I'll never have any idea. :)
Who knows enough about coroutine innards to review this?

libcxx/include/__coroutine/noop_coroutine_handle.h

78

The double-underscore here seems suspicious. Could you make it a single underscore? or is this specific identifier magic to GCC somehow?

libcxx/test/std/language.support/support.coroutines/coroutine.handle/coroutine.handle.noop/noop_coroutine.pass.cpp

23–24

At this point, I think you could just remove this #if and start using the usual annotations, e.g. // UNSUPPORTED: clang-13 or whatever.

80–81

Let's keep these consistent with lines 60–61. I have no preference on the order or whether they end with , "", but let's be consistent one way or the other.

[ChuanqiXu]

Address comments

[ChuanqiXu]

I added @iains who is the original author of libstdc++'s coroutine. I am not sure if Iain would take a look. But I believe the implementation might be correct since I looked at the implementation of libstdc++'s implementation.

libcxx/include/__coroutine/noop_coroutine_handle.h

78

I guess you are referring the double-underscore in the middle instead of the start, right? My bad. It is my typo.

libcxx/test/std/language.support/support.coroutines/coroutine.handle/coroutine.handle.noop/noop_coroutine.pass.cpp

23–24

I think the line // UNSUPPORTED: libcpp-no-coroutines would filter clang-13 out.

80–81

We can't do that. Since noop_coroutine_handle::done() is constexpr and std::coroutine_handle<>::done() isn't constexpr.

[• Quuxplusone]

libcxx/include/__coroutine/noop_coroutine_handle.h

78

Yep, I meant the double-underscore in the middle.

Also, please remove the trailing underscore in __dummy_resume_destroy_func_. Nobody's 100% sure whether we use trailing-underscore for data members or private members ;) but __dummy_resume_destroy_func_ is neither private nor data, so it definitely shouldn't have one. Just make it __dummy_resume_destroy_func().

libcxx/test/std/language.support/support.coroutines/coroutine.handle/coroutine.handle.noop/noop_coroutine.pass.cpp

23–24

No problem. I was just trying to guess the name of some compiler that might fail the condition #if __has_builtin(__builtin_coro_noop) || defined(_LIBCPP_COMPILER_GCC).

80–81

Oops, I hadn't noticed that these were regular assert instead of static_assert. So they can't use , ""; but at least you can put them in the same order: i.e., please swap lines 69 and 70.

[ChuanqiXu]

libcxx/test/std/language.support/support.coroutines/coroutine.handle/coroutine.handle.noop/noop_coroutine.pass.cpp

80–81

Yeah, but is is weird to see assert(X.y() && X) since it would crash if X is null. How do you think about swap lines 58 and 59?

[• Quuxplusone]

libcxx/test/std/language.support/support.coroutines/coroutine.handle/coroutine.handle.noop/noop_coroutine.pass.cpp

80–81

Sure, swap either pair, as long as it ends up consistent.

[ChuanqiXu]

Address comments

[ChuanqiXu]

Rename __dummy_resume_destroy_func_ to __dummy_resume_destroy_func

[• Quuxplusone]

libcxx/include/__coroutine/noop_coroutine_handle.h

23–25

76–77

[ChuanqiXu]

Address comments

libcxx/docs/Status/Cxx2bIssues.csv

 "`3428 <https://wg21.link/LWG3428>`__","``single_view``'s in place constructor should be explicit","November 2020","","","|ranges|"
 "`3434 <https://wg21.link/LWG3434>`__","``ios_base`` never reclaims memory for iarray and parray","November 2020","",""
 "`3437 <https://wg21.link/LWG3437>`__","``__cpp_lib_polymorphic_allocator`` is in the wrong header","November 2020","",""
 "`3446 <https://wg21.link/LWG3446>`__","``indirectly_readable_traits`` ambiguity for types with both ``value_type`` and ``element_type``","November 2020","","","|ranges|"
 "`3448 <https://wg21.link/LWG3448>`__","``transform_view``'s ``sentinel<false>`` not comparable with ``iterator<true>``","November 2020","","","|ranges|"
 "`3449 <https://wg21.link/LWG3449>`__","``take_view`` and ``take_while_view``'s ``sentinel<false>`` not comparable with their ``const iterator``","November 2020","","","|ranges|"
 "`3453 <https://wg21.link/LWG3453>`__","Generic code cannot call ``ranges::advance(i, s)``","November 2020","","","|ranges|"
 "`3455 <https://wg21.link/LWG3455>`__","Incorrect Postconditions on ``unique_ptr`` move assignment","November 2020","|Nothing To Do|",""
-"`3460 <https://wg21.link/LWG3460>`__","Unimplementable ``noop_coroutine_handle`` guarantees","November 2020","",""
+"`3460 <https://wg21.link/LWG3460>`__","Unimplementable ``noop_coroutine_handle`` guarantees","November 2020","|Complete|","14.0"
 "`3461 <https://wg21.link/LWG3461>`__","``convertible_to``'s description mishandles cv-qualified ``void``","November 2020","",""
 "`3465 <https://wg21.link/LWG3465>`__","``compare_partial_order_fallback`` requires ``F < E``","November 2020","","","|spaceship|"
 "`3466 <https://wg21.link/LWG3466>`__","Specify the requirements for ``promise``/``future``/``shared_future`` consistently","November 2020","",""
 "`3467 <https://wg21.link/LWG3467>`__","``bool`` can't be an integer-like type","November 2020","",""
 "`3472 <https://wg21.link/LWG3472>`__","``counted_iterator`` is missing preconditions","November 2020","","","|ranges|"
 "`3473 <https://wg21.link/LWG3473>`__","Normative encouragement in non-normative note","November 2020","","","|format|"
 "`3474 <https://wg21.link/LWG3474>`__","Nesting ``join_views`` is broken because of CTAD","November 2020","","","|ranges|"
 "`3476 <https://wg21.link/LWG3476>`__","``thread`` and ``jthread`` constructors require that the parameters be move-constructible but never move construct the parameters","November 2020","",""

libcxx/include/__coroutine/noop_coroutine_handle.h

 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif
 
 #if _LIBCPP_STD_VER > 17 && !defined(_LIBCPP_HAS_NO_CXX20_COROUTINES)
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-#if __has_builtin(__builtin_coro_noop)
+#if __has_builtin(__builtin_coro_noop) || defined(_LIBCPP_COMPILER_GCC)
+
 // [coroutine.noop]
 // [coroutine.promise.noop]
 struct noop_coroutine_promise {};
 
 // [coroutine.handle.noop]
 template <>
 struct _LIBCPP_TEMPLATE_VIS coroutine_handle<noop_coroutine_promise> {
 public:
     // [coroutine.handle.noop.conv], conversion
public:
     // [coroutine.handle.noop.address], address
     _LIBCPP_HIDE_FROM_ABI
     constexpr void* address() const noexcept { return __handle_; }
 
 private:
     _LIBCPP_HIDE_FROM_ABI
     friend coroutine_handle<noop_coroutine_promise> noop_coroutine() noexcept;
 
+#if __has_builtin(__builtin_coro_noop)
     _LIBCPP_HIDE_FROM_ABI coroutine_handle() noexcept { 
         this->__handle_ = __builtin_coro_noop();
     }
 
     void* __handle_ = nullptr;
+
+#elif defined(_LIBCPP_COMPILER_GCC)
+    // GCC doesn't implement __builtin_coro_noop().
+    // Construct the coroutine frame manually instead.
+    struct __noop_coroutine_frame_ty_ {
+        static void __dummy_resume_destroy_func() { }
+
+        void (*__resume_)() = __dummy_resume_destroy_func;
+        void (*__destroy_)() = __dummy_resume_destroy_func;
+        struct noop_coroutine_promise __promise_;
+    };
+
+    static __noop_coroutine_frame_ty_ __noop_coroutine_frame_;
+
+    void* __handle_ = &__noop_coroutine_frame_;
+
+    _LIBCPP_HIDE_FROM_ABI coroutine_handle() noexcept = default;
+
+#endif // __has_builtin(__builtin_coro_noop)
 };
 
 using noop_coroutine_handle = coroutine_handle<noop_coroutine_promise>;
 
+#if defined(_LIBCPP_COMPILER_GCC)
+inline noop_coroutine_handle::__noop_coroutine_frame_ty_
+    noop_coroutine_handle::__noop_coroutine_frame_{};
+#endif
+
 // [coroutine.noop.coroutine]
 inline _LIBCPP_HIDE_FROM_ABI
 noop_coroutine_handle noop_coroutine() noexcept { return noop_coroutine_handle(); }
 
-#endif // __has_builtin(__builtin_coro_noop)
+#endif // __has_builtin(__builtin_coro_noop) || defined(_LIBCPP_COMPILER_GCC)
 
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // __LIBCPP_STD_VER > 17 && !defined(_LIBCPP_HAS_NO_CXX20_COROUTINES)
 
 #endif // _LIBCPP___COROUTINE_NOOP_COROUTINE_HANDLE_H

libcxx/test/std/language.support/support.coroutines/coroutine.handle/coroutine.handle.noop/noop_coroutine.pass.cpp

 // using noop_coroutine_handle = coroutine_handle<noop_coroutine_promise>;
 // noop_coroutine_handle noop_coroutine() noexcept;
 
 #include <coroutine>
 #include <cassert>
 #include <type_traits>
 
 #include "test_macros.h"
 
-#if __has_builtin(__builtin_coro_noop)
-
 static_assert(std::is_same<std::coroutine_handle<std::noop_coroutine_promise>, std::noop_coroutine_handle>::value, "");
 static_assert(std::is_same<decltype(std::noop_coroutine()), std::noop_coroutine_handle>::value, "");
 
 // template <> struct coroutine_handle<noop_coroutine_promise> : coroutine_handle<>
 // {
 // // [coroutine.handle.noop.observers], observers
 // constexpr explicit operator bool() const noexcept;
 // constexpr bool done() const noexcept;
 
int main(int, char**)
   assert(base);
 
   assert(!h.done());
   assert(!base.done());
 
   h.resume();
   h.destroy();
   h();
-  static_assert(h.done() == false, "");
   static_assert(h, "");
+  static_assert(h.done() == false, "");
+
+  // [coroutine.handle.noop.resumption]p2
+  // Remarks: If noop_­coroutine_­handle is converted to
+  // coroutine_­handle<>, calls to operator(), resume and
+  // destroy on that handle will also have no observable
+  // effects.
+  base.resume();
+  base.destroy();
+  base();
+  assert(base);
+  assert(base.done() == false);
 
   h.promise();
   assert(h.address() == base.address());
-  assert(h==base);
+  assert(h == base);
   assert(h.address() != nullptr);
   assert(std::coroutine_handle<>::from_address(h.address()) == base);
 
   return 0;
 }
-
-#else
-
-int main(int, char**) { return 0; }
-
-#endif //  __has_builtin(__builtin_coro_noop)