system/serenity
system/serenity
1
0
Fork 0
mirror of https://github.com/SerenityOS/serenity synced 2024-10-01 13:44:21 +00:00
Code Issues Packages Projects Releases Wiki Activity

AK+LibTest: Choose definition of CO_TRY and CO_TRY_OR_FAIL more robustly

Browse source 
There are three compiler bugs that influence this decision:

 - Clang writing to (validly) destroyed coroutine frame with -O0 and
   -fsanitize=null,address under some conditions
   (https://godbolt.org/z/17Efq5Ma5) (AK_COROUTINE_DESTRUCTION_BROKEN);

 - GCC being unable to handle statement expressions in coroutines
   (AK_COROUTINE_STATEMENT_EXPRS_BROKEN);

 - GCC being unable to deduce template type parameter for TryAwaiter
   with nested CO_TRYs (AK_COROUTINE_TYPE_DEDUCTION_BROKEN).

Instead of growing an ifdef soup in AK/Coroutine.h and
LibTest/AsyncTestCase.h, define three macros in AK/Platform.h that
correspond to these bugs and use them accordingly in the said files.
This commit is contained in:
Dan Klishch 2024-06-29 20:27:44 -04:00 committed by Andrew Kaster
parent 6fc3908818
commit c03cca7b2f
5 changed files with 62 additions and 31 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

37
AK/Coroutine.h
View file

@ -14,8 +14,12 @@ namespace AK {
namespace Detail {
// FIXME: GCC ICEs when a simpler implementation of CO_TRY_OR_FAIL is used. See also LibTest/AsyncTestCase.h.
#ifdef AK_COMPILER_GCC
// FIXME: GCC ICEs when an implementation of CO_TRY_OR_FAIL with statement expressions is used. See also LibTest/AsyncTestCase.h.
#if defined(AK_COROUTINE_STATEMENT_EXPRS_BROKEN) && !defined(AK_COROUTINE_DESTRUCTION_BROKEN)
# define AK_USE_TRY_OR_FAIL_AWAITER
#endif
#ifdef AK_USE_TRY_OR_FAIL_AWAITER
namespace Test {
template<typename T>
@ -123,7 +127,7 @@ private:
template<typename U>
friend struct Detail::TryAwaiter;
#ifdef AK_COMPILER_GCC
#ifdef AK_USE_TRY_OR_FAIL_AWAITER
template<typename U>
friend struct AK::Detail::Test::TryOrFailAwaiter;
#endif
@ -189,6 +193,7 @@ T must_sync(Coroutine<ErrorOr<T>>&& coroutine)
return object.release_value();
}
#ifndef AK_COROUTINE_DESTRUCTION_BROKEN
namespace Detail {
template<typename T>
struct TryAwaiter {
@ -243,13 +248,9 @@ struct TryAwaiter {
};
}
#ifdef AK_COMPILER_CLANG
# define CO_TRY(expression) (co_await ::AK::Detail::TryAwaiter { (expression) })
#else
// GCC cannot handle CO_TRY(...CO_TRY(...)...), this hack ensures that it always has the right type information available.
// FIXME: Remove this once GCC can correctly infer the result type of `co_await TryAwaiter { ... }`.
# define CO_TRY(expression) static_cast<decltype(AK::Detail::declval_coro_result(expression).release_value())>(co_await ::AK::Detail::TryAwaiter { (expression) })
# ifndef AK_COROUTINE_TYPE_DEDUCTION_BROKEN
# define CO_TRY(expression) (co_await ::AK::Detail::TryAwaiter { (expression) })
# else
namespace Detail {
template<typename T>
auto declval_coro_result(Coroutine<T>&&) -> T;
@ -257,7 +258,23 @@ template<typename T>
auto declval_coro_result(T&&) -> T;
}
// GCC cannot handle CO_TRY(...CO_TRY(...)...), this hack ensures that it always has the right type information available.
// FIXME: Remove this once GCC can correctly infer the result type of `co_await TryAwaiter { ... }`.
# define CO_TRY(expression) static_cast<decltype(AK::Detail::declval_coro_result(expression).release_value())>(co_await ::AK::Detail::TryAwaiter { (expression) })
# endif
#elifndef AK_COROUTINE_STATEMENT_EXPRS_BROKEN
# define CO_TRY(expression) \
({ \
AK_IGNORE_DIAGNOSTIC("-Wshadow", \
auto _temporary_result = (expression)); \
if (_temporary_result.is_error()) [[unlikely]] \
co_return _temporary_result.release_error(); \
_temporary_result.release_value(); \
})
#else
# error Unable to work around compiler bugs in definiton of CO_TRY.
#endif
}
#ifdef USING_AK_GLOBALLY

10
AK/Platform.h
View file

@ -158,6 +158,16 @@
# define AK_BUILTIN_SUBC_BROKEN
#endif
#if defined(AK_COMPILER_CLANG) && __clang_major__ < 19
# define AK_COROUTINE_DESTRUCTION_BROKEN
#endif
#ifdef AK_COMPILER_GCC
// FIXME: Reduce and report these to GCC.
# define AK_COROUTINE_STATEMENT_EXPRS_BROKEN
# define AK_COROUTINE_TYPE_DEDUCTION_BROKEN
#endif
#ifdef ALWAYS_INLINE
# undef ALWAYS_INLINE
#endif

6
Tests/AK/TestCoroutine.cpp
View file

@ -238,18 +238,22 @@ namespace {
Coroutine<ErrorOr<void>> co_try_success()
{
auto c = CO_TRY(co_await return_class_3());
#ifndef AK_COROUTINE_DESTRUCTION_BROKEN
// 1. Construct temporary as an argument for the constructor of a temporary ErrorOr<Class>.
// 2. Move temporary ErrorOr<Class> into Coroutine.
// -. Some magic is done in TryAwaiter.
// 3. Move Class from ErrorOr<Class> inside Coroutine to c.
EXPECT_EQ(c.cookie(), 3);
#else
EXPECT_EQ(c.cookie(), 4);
#endif
co_return {};
}
Coroutine<ErrorOr<void>> co_try_fail()
{
ErrorOr<void> error = Error::from_string_literal("ERROR!");
CO_TRY(error);
CO_TRY(move(error));
co_return {};
}

2
Userland/Libraries/LibHTTP/Http11Connection.h
View file

@ -90,7 +90,7 @@ public:
if (response->is_open()) {
auto close_result = co_await response->close();
if (!result.is_error()) // Preserve callback error in case it has failed.
CO_TRY(close_result);
CO_TRY(move(close_result));
}
co_return result;
}

38
Userland/Libraries/LibTest/AsyncTestCase.h
View file

@ -10,7 +10,21 @@
#include <LibCore/EventLoop.h>
#include <LibTest/TestCase.h>
#ifdef AK_COMPILER_GCC
#ifndef AK_COROUTINE_STATEMENT_EXPRS_BROKEN
# define CO_TRY_OR_FAIL(expression) \
({ \
/* Ignore -Wshadow to allow nesting the macro. */ \
AK_IGNORE_DIAGNOSTIC("-Wshadow", \
auto _temporary_result = (expression)); \
static_assert(!::AK::Detail::IsLvalueReference<decltype(_temporary_result.release_value())>, \
"Do not return a reference from a fallible expression"); \
if (_temporary_result.is_error()) [[unlikely]] { \
FAIL(_temporary_result.release_error()); \
co_return; \
} \
_temporary_result.release_value(); \
})
#elifndef AK_COROUTINE_DESTRUCTION_BROKEN
namespace AK::Detail::Test {
// FIXME: Straight-forward way to implement CO_TRY_OR_FAIL we use for Clang causes GCC to ICE.
@ -71,6 +85,10 @@ struct TryOrFailAwaiter {
};
}
# define CO_TRY_OR_FAIL(expression) (co_await ::AK::Detail::Test::TryOrFailAwaiter { (expression) })
#else
# error Unable to work around compiler bugs in definiton of CO_TRY_OR_FAIL.
#endif
namespace Test {
@ -95,22 +113,4 @@ namespace Test {
static struct __TESTCASE_TYPE(x) __TESTCASE_TYPE(x); \
static Coroutine<void> __ASYNC_TESTCASE_FUNC(x)()
#ifdef AK_COMPILER_GCC
# define CO_TRY_OR_FAIL(expression) (co_await ::AK::Detail::Test::TryOrFailAwaiter { (expression) })
#else
# define CO_TRY_OR_FAIL(expression) \
({ \
/* Ignore -Wshadow to allow nesting the macro. */ \
AK_IGNORE_DIAGNOSTIC("-Wshadow", \
auto _temporary_result = (expression)); \
static_assert(!::AK::Detail::IsLvalueReference<decltype(_temporary_result.release_value())>, \
"Do not return a reference from a fallible expression"); \
if (_temporary_result.is_error()) [[unlikely]] { \
FAIL(_temporary_result.release_error()); \
co_return; \
} \
_temporary_result.release_value(); \
})
#endif
}