[PATCH] D45121: [coroutines] Add noop_coroutine to <experimental/coroutine>

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added inline comments.


================
Comment at: include/experimental/coroutine:294
+
+inline _LIBCPP_ALWAYS_INLINE
+noop_coroutine_handle noop_coroutine() _NOEXCEPT {
----------------
GorNishanov wrote:
> lewissbaker wrote:
> > EricWF wrote:
> > > This should just be `_LIBCPP_INLINE_VISIBILITY`. We try not to use `_LIBCPP_ALWAYS_INLINE` in new code.
> > Should the same change be applied to the other usages of `_LIBCPP_ALWAYS_INLINE` in this file?
> > Should some of them be marked `constexpr` to be consistent with `noop_coroutine_handle` member functions above?
> Those were added by @EricWF, so from my perspective they are immutable.
I'm not sure what I was thinking when I implemented these things with `_LIBCPP_ALWAYS_INLINE`.


================
Comment at: include/experimental/coroutine:288
+
+    coroutine_handle() {
+      this->__handle_ = __builtin_coro_noop();
----------------
GorNishanov wrote:
> CaseyCarter wrote:
> > CaseyCarter wrote:
> > > GorNishanov wrote:
> > > > EricWF wrote:
> > > > > Can `__builtin_coro_noop` produce a constant expression?
> > > > No. llvm generates this value, so from clang perspective, it is not a constant.
> > > > At llvm level it is a private per TU constant, so invocations of noop_coroutine() in different TUs linked into the same program will return you different values.
> > > If this class type is not literal - since there's no `constexpr` constructor - applying `constexpr` to the member functions on 278-283 is at best misleading and at worst ill-formed NDR due to http://eel.is/c++draft/dcl.constexpr#5.
> > This constructor should be `_NOEXCEPT`, since it's invoked by `noop_coroutine` which is `_NOEXCEPT`.
> Issue for Rapperswil? These constexprs were approved by LEWG/LWG in Jacksonville 2018
@CaseyCarter: I'm not sure this is true. Clang seems to be able to evaluate constexpr member functions of a non-literal type so long as they don't depend on the "argument value" of `this`. Example:

```
struct T {
  T() {}
  constexpr bool foo() const { return true; }
};
T t;
static_assert(t.foo(), "");
```


https://reviews.llvm.org/D45121