[libcxx-commits] [PATCH] D116144: [libcxx] [Coroutines] Support noop_coroutine for GCC

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added a comment.

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?



================
Comment at: libcxx/include/__coroutine/noop_coroutine_handle.h:79
+    // frame for noop_coroutine from source instead.
+    struct __noop_coroutine__frame_ty_ {
+        static void __dummy_resume_destroy_func_() { }
----------------
The double-underscore here seems suspicious. Could you make it a single underscore? or is this //specific identifier// magic to GCC somehow?


================
Comment at: libcxx/test/std/language.support/support.coroutines/coroutine.handle/coroutine.handle.noop/noop_coroutine.pass.cpp:24
 
-#if __has_builtin(__builtin_coro_noop)
+#if __has_builtin(__builtin_coro_noop) || defined(_LIBCPP_COMPILER_GCC)
 
----------------
At this point, I think you could just remove this `#if` and start using the usual annotations, e.g. `// UNSUPPORTED: clang-13` or whatever.


================
Comment at: libcxx/test/std/language.support/support.coroutines/coroutine.handle/coroutine.handle.noop/noop_coroutine.pass.cpp:71-72
+  base();
+  assert(base);
+  assert(base.done() == false);
+
----------------
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116144/new/

https://reviews.llvm.org/D116144