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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
