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

Eric Fiselier via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 4 11:18:25 PDT 2018


EricWF added inline comments.


================
Comment at: include/experimental/coroutine:294
+
+inline _LIBCPP_ALWAYS_INLINE
+noop_coroutine_handle noop_coroutine() _NOEXCEPT {
----------------
GorNishanov wrote:
> EricWF wrote:
> > 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`.
> @EricWF would you like me to do wholesale search-and-replace in coroutine header and make everything _LIBCPP_INLINE_VISIBILITY. Though, strategic use of always_inline in coroutine_handle and related classes can allow heap allocation elision to work even in -O0
`_LIBCPP_INLINE_VISIBILITY` implies always inline most of the time. I wouldn't mind you replacing all instances in the header.


https://reviews.llvm.org/D45121





More information about the cfe-commits mailing list