Skip to content

Commit e8baca6

Browse files
committed
Fix GCC builds
It turns out that GCC needs `5 * sizeof(void*)` space for the inlined coroutine frame where Clang only needs `4 * sizeof(void*)`; I haven't figured out why, yet, but this diff reserves more space when compiling with GCC. It also seems to me there's a GCC bug being tickled in the previous commit that prevents direct initialization of the stored awaiter and/or awaitable from the result of `_get_awaitable` and/or `__get_awaiter`, respectively, when the result is immovable. I think this is a failure to implement C++17's mandatory copy elision, but I'm not sure. I've worked around it with unions and in-place `new`. I also added test cases to cover the immovable awaiter/awaitable cases.
1 parent 7261caf commit e8baca6

File tree

2 files changed

+150
-43
lines changed

2 files changed

+150
-43
lines changed

include/stdexec/__detail/__connect_awaitable.hpp

Lines changed: 79 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,10 @@ namespace STDEXEC
3838
// __connect_await
3939
namespace __connect_await
4040
{
41-
static constexpr std::size_t __storage_size = 4 * sizeof(void*) - 1;
41+
// four pointers' worth of space when compiling with Clang or MSVC; five with GCC
42+
static constexpr std::size_t __num_pointers = 4 * (STDEXEC_CLANG() + STDEXEC_MSVC())
43+
+ 5 * STDEXEC_GCC();
44+
static constexpr std::size_t __storage_size = __num_pointers * sizeof(void*) - 1;
4245
static constexpr std::size_t __storage_align = __STDCPP_DEFAULT_NEW_ALIGNMENT__;
4346

4447
// clang-format off
@@ -123,14 +126,36 @@ namespace STDEXEC
123126
{
124127
__state(_Awaitable&& __source, __std::coroutine_handle<_Promise> __coro)
125128
noexcept(__is_nothrow)
126-
: __awaitable_(__get_awaitable(static_cast<_Awaitable&&>(__source), __coro.promise()))
127-
, __awaiter_(__get_awaiter(static_cast<__awaitable_t&&>(__awaitable_)))
128-
{}
129+
{
130+
// GCC doesn't like initializing __awaitable_ or __awaiter_ in the member initializer
131+
// clause when the result of __get_awaitable or __get_awaiter is immovable; it *seems*
132+
// like direct initialization of a member with the result of a function ought to trigger
133+
// C++17's mandatory copy elision, and both Clang and MSVC accept that code, but using
134+
// a union with in-place new works around the issue.
135+
new (static_cast<void*>(std::addressof(__awaitable_)))
136+
__awaitable_t(__get_awaitable(static_cast<_Awaitable&&>(__source), __coro.promise()));
137+
new (static_cast<void*>(std::addressof(__awaiter_)))
138+
__awaiter_t(__get_awaiter(static_cast<__awaitable_t&&>(__awaitable_)));
139+
}
129140

130-
[[no_unique_address]]
131-
__awaitable_t __awaitable_;
132-
[[no_unique_address]]
133-
__awaiter_t __awaiter_;
141+
~__state()
142+
{
143+
// make sure to destroy in the reverse order of construction
144+
std::destroy_at(std::addressof(__awaiter_));
145+
std::destroy_at(std::addressof(__awaitable_));
146+
}
147+
148+
union
149+
{
150+
[[no_unique_address]]
151+
__awaitable_t __awaitable_;
152+
};
153+
154+
union
155+
{
156+
[[no_unique_address]]
157+
__awaiter_t __awaiter_;
158+
};
134159
};
135160

136161
[[no_unique_address]]
@@ -146,9 +171,9 @@ namespace STDEXEC
146171
explicit __awaitable_state(_A&& __awaitable)
147172
noexcept(__nothrow_constructible_from<_Awaitable, _A>)
148173
: __source_awaitable_(static_cast<_A&&>(__awaitable))
149-
{}
174+
{ }
150175

151-
~__awaitable_state() {}
176+
~__awaitable_state() { }
152177

153178
constexpr void construct(__std::coroutine_handle<_Promise> __coro) noexcept(__is_nothrow)
154179
{
@@ -178,17 +203,32 @@ namespace STDEXEC
178203
{
179204
__state(_Awaitable&& __source, __std::coroutine_handle<_Promise> __coro)
180205
noexcept(__is_nothrow)
181-
: __awaiter_(__get_awaiter(static_cast<_Awaitable&&>(__source)))
182206
{
207+
// GCC doesn't like initializing __awaiter_ in the member initializer clause when the
208+
// result of __get_awaiter is immovable; it *seems* like direct initialization of a
209+
// member with the result of a function ought to trigger C++17's mandatory copy elision,
210+
// and both Clang and MSVC accept that code, but using a union with in-place new works
211+
// around the issue.
212+
new (static_cast<void*>(std::addressof(__awaiter_)))
213+
__awaiter_t(__get_awaiter(static_cast<_Awaitable&&>(__source)));
214+
183215
[[maybe_unused]]
184216
auto&& __awaitable = __get_awaitable(static_cast<_Awaitable&&>(__source),
185217
__coro.promise());
186218

187219
STDEXEC_ASSERT(std::addressof(__awaitable) == std::addressof(__source));
188220
}
189221

190-
[[no_unique_address]]
191-
__awaiter_t __awaiter_;
222+
~__state()
223+
{
224+
std::destroy_at(std::addressof(__awaiter_));
225+
}
226+
227+
union
228+
{
229+
[[no_unique_address]]
230+
__awaiter_t __awaiter_;
231+
};
192232
};
193233

194234
[[no_unique_address]]
@@ -204,9 +244,9 @@ namespace STDEXEC
204244
explicit __awaitable_state(_A&& __awaitable)
205245
noexcept(__nothrow_constructible_from<_Awaitable, _A>)
206246
: __source_awaitable_(static_cast<_A&&>(__awaitable))
207-
{}
247+
{ }
208248

209-
~__awaitable_state() {}
249+
~__awaitable_state() { }
210250

211251
constexpr void construct(__std::coroutine_handle<_Promise> __coro) noexcept(__is_nothrow)
212252
{
@@ -236,15 +276,29 @@ namespace STDEXEC
236276
{
237277
__state(_Awaitable&& __source, __std::coroutine_handle<_Promise> __coro)
238278
noexcept(__is_nothrow)
239-
: __awaiter_(__get_awaitable(static_cast<_Awaitable&&>(__source), __coro.promise()))
240279
{
280+
// GCC doesn't like initializing __awaiter_ in the member initializer clause when the
281+
// result of __get_awaitable is immovable; it *seems* like direct initialization of a
282+
// member with the result of a function ought to trigger C++17's mandatory copy elision,
283+
// and both Clang and MSVC accept that code, but using a union with in-place new works
284+
// around the issue.
285+
new (static_cast<void*>(std::addressof(__awaiter_)))
286+
__awaiter_t(__get_awaitable(static_cast<_Awaitable&&>(__source), __coro.promise()));
287+
241288
[[maybe_unused]]
242289
auto&& __awaiter = __get_awaiter(static_cast<__awaiter_t&&>(__awaiter_));
243290
STDEXEC_ASSERT(std::addressof(__awaiter) == std::addressof(__awaiter_));
244291
}
245292

246-
[[no_unique_address]]
247-
__awaiter_t __awaiter_;
293+
~__state()
294+
{
295+
std::destroy_at(std::addressof(__awaiter_));
296+
}
297+
298+
[[no_unique_address]] union
299+
{
300+
__awaiter_t __awaiter_;
301+
};
248302
};
249303

250304
[[no_unique_address]]
@@ -260,9 +314,9 @@ namespace STDEXEC
260314
explicit __awaitable_state(_A&& __awaitable)
261315
noexcept(__nothrow_constructible_from<_Awaitable, _A>)
262316
: __source_awaitable_(static_cast<_A&&>(__awaitable))
263-
{}
317+
{ }
264318

265-
~__awaitable_state() {}
319+
~__awaitable_state() { }
266320

267321
constexpr void construct(__std::coroutine_handle<_Promise> __coro) noexcept(__is_nothrow)
268322
{
@@ -292,7 +346,7 @@ namespace STDEXEC
292346
explicit __awaitable_state(_A&& __awaitable)
293347
noexcept(__nothrow_constructible_from<_Awaitable, _A>)
294348
: __awaiter_(static_cast<_A&&>(__awaitable))
295-
{}
349+
{ }
296350

297351
static constexpr void construct(__std::coroutine_handle<_Promise>) noexcept
298352
{
@@ -459,7 +513,7 @@ namespace STDEXEC
459513
noexcept(__nothrow_move_constructible<_Awaitable>)
460514
: __rcvr_(static_cast<_Receiver&&>(__rcvr))
461515
, __awaiter_(static_cast<_Awaitable&&>(__awaitable))
462-
{}
516+
{ }
463517

464518
__opstate(__opstate&&) = delete;
465519

@@ -611,15 +665,15 @@ namespace STDEXEC
611665
{
612666
template <class, class>
613667
struct __promise
614-
{};
668+
{ };
615669

616670
template <class>
617671
struct __with_await_transform
618-
{};
672+
{ };
619673
} // namespace __connect_await
620674

621675
struct __connect_awaitable_t
622-
{};
676+
{ };
623677

624678
#endif
625679

test/stdexec/cpos/test_cpo_connect_awaitable.cpp

Lines changed: 71 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,11 @@ namespace
3333
public:
3434
explicit constexpr ready_awaitable(T&& t) noexcept
3535
: t_(std::move(t))
36-
{}
36+
{ }
3737

3838
explicit constexpr ready_awaitable(T const & t)
3939
: t_(t)
40-
{}
40+
{ }
4141

4242
static constexpr bool await_ready() noexcept
4343
{
@@ -76,7 +76,7 @@ namespace
7676
FAIL_CHECK("this awaitable should never suspend");
7777
}
7878

79-
static constexpr void await_resume() noexcept {}
79+
static constexpr void await_resume() noexcept { }
8080

8181
ready_awaitable& base() noexcept
8282
{
@@ -91,7 +91,7 @@ namespace
9191

9292
explicit constexpr awaitable_ref(Awaitable& awaitable) noexcept
9393
: awaitable_(&awaitable)
94-
{}
94+
{ }
9595

9696
constexpr auto await_ready() const noexcept(noexcept(awaitable_->await_ready()))
9797
requires requires(Awaitable& a) {
@@ -192,7 +192,7 @@ namespace
192192
explicit constexpr conditionally_suspending_awaitable(bool suspend, U&&... u) noexcept
193193
: suspending_awaitable<T>(std::forward<U>(u)...)
194194
, suspend_(suspend)
195-
{}
195+
{ }
196196

197197
constexpr bool await_suspend(std::coroutine_handle<> coro) noexcept
198198
{
@@ -236,20 +236,20 @@ namespace
236236
}
237237
};
238238

239-
template <class Awaitable>
239+
template <class Awaitable, template <class> class Wrapper = std::type_identity_t>
240240
struct with_as_awaitable
241241
{
242242
template <class... T>
243243
requires std::constructible_from<Awaitable, T...>
244244
explicit(sizeof...(T) == 1) with_as_awaitable(T&&... t)
245245
noexcept(std::is_nothrow_constructible_v<Awaitable, T...>)
246246
: awaitable_(std::forward<T>(t)...)
247-
{}
247+
{ }
248248

249249
template <class Promise>
250-
awaitable_ref<Awaitable> as_awaitable(Promise&) noexcept
250+
Wrapper<awaitable_ref<Awaitable>> as_awaitable(Promise&) noexcept
251251
{
252-
return awaitable_ref(awaitable_);
252+
return Wrapper<awaitable_ref<Awaitable>>(awaitable_);
253253
}
254254

255255
auto& base() noexcept
@@ -261,19 +261,19 @@ namespace
261261
Awaitable awaitable_;
262262
};
263263

264-
template <class Awaitable>
264+
template <class Awaitable, template <class> class Wrapper = std::type_identity_t>
265265
struct with_member_co_await
266266
{
267267
template <class... T>
268268
requires std::constructible_from<Awaitable, T...>
269269
explicit(sizeof...(T) == 1) with_member_co_await(T&&... t)
270270
noexcept(std::is_nothrow_constructible_v<Awaitable, T...>)
271271
: awaitable_(std::forward<T>(t)...)
272-
{}
272+
{ }
273273

274-
constexpr awaitable_ref<Awaitable> operator co_await() noexcept
274+
constexpr Wrapper<awaitable_ref<Awaitable>> operator co_await() noexcept
275275
{
276-
return awaitable_ref(awaitable_);
276+
return Wrapper<awaitable_ref<Awaitable>>(awaitable_);
277277
}
278278

279279
auto& base() noexcept
@@ -285,15 +285,15 @@ namespace
285285
Awaitable awaitable_;
286286
};
287287

288-
template <class Awaitable>
288+
template <class Awaitable, template <class> class Wrapper = std::type_identity_t>
289289
struct with_friend_co_await
290290
{
291291
template <class... T>
292292
requires std::constructible_from<Awaitable, T...>
293293
explicit(sizeof...(T) == 1) with_friend_co_await(T&&... t)
294294
noexcept(std::is_nothrow_constructible_v<Awaitable, T...>)
295295
: awaitable_(std::forward<T>(t)...)
296-
{}
296+
{ }
297297

298298
auto& base() noexcept
299299
{
@@ -305,9 +305,9 @@ namespace
305305

306306
template <class Self>
307307
requires std::same_as<std::remove_cvref_t<Self>, with_friend_co_await>
308-
friend constexpr awaitable_ref<Awaitable> operator co_await(Self&& self) noexcept
308+
friend constexpr Wrapper<awaitable_ref<Awaitable>> operator co_await(Self&& self) noexcept
309309
{
310-
return awaitable_ref(self.awaitable_);
310+
return Wrapper<awaitable_ref<Awaitable>>(self.awaitable_);
311311
}
312312
};
313313

@@ -700,12 +700,65 @@ namespace
700700
return coro.promise().unhandled_stopped();
701701
}
702702

703-
static constexpr void await_resume() noexcept {}
703+
static constexpr void await_resume() noexcept { }
704704
};
705705

706706
TEST_CASE("promise().unhandled_stopped() invokes set_stopped", "[cpo][cpo_connect_awaitable]")
707707
{
708708
auto op = ex::connect(stop_on_suspend{}, expect_stopped_receiver{});
709709
op.start();
710710
}
711+
712+
template <class Awaitable>
713+
struct as_immovable : Awaitable
714+
{
715+
using Awaitable::Awaitable;
716+
717+
as_immovable(as_immovable&&) = delete;
718+
719+
as_immovable& base() noexcept
720+
{
721+
return *this;
722+
}
723+
};
724+
725+
TEST_CASE("can connect and start immovable awaiters", "[cpo][cpo_connect_awaitable]")
726+
{
727+
{
728+
// .as_awaitable(promise) returns an immovable value
729+
auto op = ex::connect(with_as_awaitable<ready_awaitable<void>, as_immovable>{},
730+
expect_void_receiver{});
731+
op.start();
732+
}
733+
{
734+
// .operator co_await() returns an immovable value
735+
auto op = ex::connect(with_member_co_await<ready_awaitable<void>, as_immovable>{},
736+
expect_void_receiver{});
737+
op.start();
738+
}
739+
{
740+
// operator co_await(awaitable) returns an immovable value
741+
auto op = ex::connect(with_friend_co_await<ready_awaitable<void>, as_immovable>{},
742+
expect_void_receiver{});
743+
op.start();
744+
}
745+
{
746+
// both .as_awaitable(promise) and .as_awaitable(promise).operator co_await() return
747+
// immovable values
748+
auto op =
749+
ex::connect(with_as_awaitable<with_member_co_await<ready_awaitable<void>, as_immovable>,
750+
as_immovable>{},
751+
expect_void_receiver{});
752+
op.start();
753+
}
754+
{
755+
// both .as_awaitable(promise) and operator co_await(as_awaitable(promise)) return
756+
// immovable values
757+
auto op =
758+
ex::connect(with_as_awaitable<with_friend_co_await<ready_awaitable<void>, as_immovable>,
759+
as_immovable>{},
760+
expect_void_receiver{});
761+
op.start();
762+
}
763+
}
711764
} // namespace

0 commit comments

Comments
 (0)