From a9f0f8d99c9a720add376c74a4e0a6da3d29d06f Mon Sep 17 00:00:00 2001 From: Alvin Wong Date: Mon, 5 Dec 2022 21:59:35 +0800 Subject: [PATCH 01/31] Hack around GCC issue with using winrt::impl::bind_in. Upstream bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61663 --- strings/base_string.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/strings/base_string.h b/strings/base_string.h index 13f3b9dce..2a6023a26 100644 --- a/strings/base_string.h +++ b/strings/base_string.h @@ -488,11 +488,23 @@ namespace winrt::impl T const& object; +#if !defined(__GNUC__) || defined(__clang__) template operator R const& () const noexcept { return reinterpret_cast(object); } +#else + // HACK: GCC does not handle template deduction of const T& conversion + // function according to CWG issue 976. To make this compile on GCC, + // we have to intentionally drop the const qualifier. + // Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61663 + template + operator R& () const noexcept + { + return const_cast(reinterpret_cast(object)); + } +#endif }; template From 87d2dd000c7270937bdcfa43fe196cbbc675b036 Mon Sep 17 00:00:00 2001 From: Alvin Wong Date: Mon, 5 Dec 2022 21:45:22 +0800 Subject: [PATCH 02/31] Fix WINRT_IMPL_LINK weak symbol being discarded on GCC GNU assembler treats symbols starting with uppercase `L` as local symbols and discard them, unless they have been defined as global. --- strings/base_extern.h | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/strings/base_extern.h b/strings/base_extern.h index 52b72bea5..e8a065710 100644 --- a/strings/base_extern.h +++ b/strings/base_extern.h @@ -93,9 +93,15 @@ extern "C" #endif #elif defined(__GNUC__) #if defined(__i386__) -#define WINRT_IMPL_LINK(function, count) __asm__(".weak _WINRT_IMPL_" #function "@" #count "\n.set _WINRT_IMPL_" #function "@" #count ", _" #function "@" #count); +#define WINRT_IMPL_LINK(function, count) __asm__( \ + ".globl _" #function "@" #count "\n\t" \ + ".weak _WINRT_IMPL_" #function "@" #count "\n\t" \ + ".set _WINRT_IMPL_" #function "@" #count ", _" #function "@" #count); #else -#define WINRT_IMPL_LINK(function, count) __asm__(".weak WINRT_IMPL_" #function "\n.set WINRT_IMPL_" #function ", " #function); +#define WINRT_IMPL_LINK(function, count) __asm__( \ + ".globl " #function "\n\t" \ + ".weak WINRT_IMPL_" #function "\n\t" \ + ".set WINRT_IMPL_" #function ", " #function); #endif #endif From 244df2e14eb1c5f7c0edd2c36f562736257b80f0 Mon Sep 17 00:00:00 2001 From: Alvin Wong Date: Mon, 5 Dec 2022 22:46:59 +0800 Subject: [PATCH 03/31] Work around GCC wanting move constructor with awaitable When calling co_await with the return value of resume_after and resume_on_signal, GCC seems to require a move. This may be an upstream bug, but work around it for now by providing the move constructor for GCC. This might be related to upstream bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99575 --- strings/base_coroutine_threadpool.h | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/strings/base_coroutine_threadpool.h b/strings/base_coroutine_threadpool.h index 415857850..9861b8294 100644 --- a/strings/base_coroutine_threadpool.h +++ b/strings/base_coroutine_threadpool.h @@ -328,6 +328,19 @@ WINRT_EXPORT namespace winrt { } +#if defined(__GNUC__) && !defined(__clang__) + // HACK: GCC seems to require a move when calling operator co_await + // on the return value of resume_after. + // This might be related to upstream bug: + // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99575 + awaitable(awaitable &&other) noexcept : + m_timer{std::move(other.m_timer)}, + m_duration{std::move(other.m_duration)}, + m_handle{std::move(other.m_handle)}, + m_state{other.m_state.load()} + {} +#endif + void enable_cancellation(cancellable_promise* promise) { promise->set_canceller([](void* context) @@ -434,6 +447,21 @@ WINRT_EXPORT namespace winrt m_handle(handle) {} +#if defined(__GNUC__) && !defined(__clang__) + // HACK: GCC seems to require a move when calling operator co_await + // on the return value of resume_on_signal. + // This might be related to upstream bug: + // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99575 + awaitable(awaitable &&other) noexcept : + m_wait{std::move(other.m_wait)}, + m_timeout{std::move(other.m_timeout)}, + m_handle{std::move(other.m_handle)}, + m_result{std::move(other.m_result)}, + m_resume{std::move(other.m_resume)}, + m_state{other.m_state.load()} + {} +#endif + void enable_cancellation(cancellable_promise* promise) { promise->set_canceller([](void* context) From e87b24965d022a33d1dcb8a7d1591988965450d6 Mon Sep 17 00:00:00 2001 From: Alvin Wong Date: Tue, 6 Dec 2022 06:12:58 +0800 Subject: [PATCH 04/31] Work around GCC wanting copy constructor in thread_pool Make thread_pool copyable by making m_pool a std::shared_ptr, but only for GCC. This might be related to upstream bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103963 --- strings/base_coroutine_threadpool.h | 37 ++++++++++++++++++++++++++--- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/strings/base_coroutine_threadpool.h b/strings/base_coroutine_threadpool.h index 9861b8294..43a598d5e 100644 --- a/strings/base_coroutine_threadpool.h +++ b/strings/base_coroutine_threadpool.h @@ -556,18 +556,35 @@ WINRT_EXPORT namespace winrt return awaitable{ handle, timeout }; } + // HACK: GCC does not compile co_await on thread_pool because it wants + // a copy constructor. Since m_pool is not copyable, in order to provide + // a workaround we have to wrap it in a shared_ptr. Apply this workaround + // only for GCC so we don't affect other compilers. + // This might be related to upstream bug: + // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103963 struct thread_pool { thread_pool() : +#if defined(__GNUC__) && !defined(__clang__) + m_pool(std::make_shared>( + check_pointer(WINRT_IMPL_CreateThreadpool(nullptr)))) +#else m_pool(check_pointer(WINRT_IMPL_CreateThreadpool(nullptr))) +#endif { - m_environment.Pool = m_pool.get(); + m_environment.Pool = get_pool(); } +#if defined(__GNUC__) && !defined(__clang__) + // HACK: See above + thread_pool(thread_pool const&) = default; + thread_pool &operator=(thread_pool const&) = delete; +#endif + void thread_limits(uint32_t const high, uint32_t const low) { - WINRT_IMPL_SetThreadpoolThreadMaximum(m_pool.get(), high); - check_bool(WINRT_IMPL_SetThreadpoolThreadMinimum(m_pool.get(), low)); + WINRT_IMPL_SetThreadpoolThreadMaximum(get_pool(), high); + check_bool(WINRT_IMPL_SetThreadpoolThreadMinimum(get_pool(), low)); } bool await_ready() const noexcept @@ -632,7 +649,21 @@ WINRT_EXPORT namespace winrt uint32_t Size{ sizeof(environment) }; }; + pool_traits::type get_pool() const + { +#if defined(__GNUC__) && !defined(__clang__) + return m_pool->get(); +#else + return m_pool.get(); +#endif + } + +#if defined(__GNUC__) && !defined(__clang__) + // HACK: See above + std::shared_ptr> m_pool; +#else handle_type m_pool; +#endif environment m_environment; }; From 1406d1fdc867570aeef397eb84a2ddd5373ac2de Mon Sep 17 00:00:00 2001 From: Alvin Wong Date: Tue, 6 Dec 2022 06:15:00 +0800 Subject: [PATCH 05/31] cmake: Stop adding Clang-specific flag when using GCC --- test/test_cpp20/CMakeLists.txt | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test/test_cpp20/CMakeLists.txt b/test/test_cpp20/CMakeLists.txt index 8b787b696..34895411f 100644 --- a/test/test_cpp20/CMakeLists.txt +++ b/test/test_cpp20/CMakeLists.txt @@ -1,7 +1,10 @@ set(CMAKE_CXX_STANDARD 20) -# std::format, std::ranges::is_heap, std::views::reverse, std::ranges::max -# are experimental in libc++ as of Clang 15. -set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fexperimental-library") +if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang") + # std::format, std::ranges::is_heap, std::views::reverse, std::ranges::max + # are experimental in libc++ as of Clang 15. + # FIXME: Should probably use compile test instead? + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fexperimental-library") +endif() if(ENABLE_TEST_SANITIZERS) # As of LLVM 15, custom_error.cpp doesn't build with ASAN due to: From f3233712f10458cce0bf414303485b7aab43e841 Mon Sep 17 00:00:00 2001 From: Alvin Wong Date: Tue, 6 Dec 2022 06:16:06 +0800 Subject: [PATCH 06/31] Fix custom_error.cpp tests on GCC --- test/test/custom_error.cpp | 11 +++++++++++ test/test_cpp20/custom_error.cpp | 4 ++++ 2 files changed, 15 insertions(+) diff --git a/test/test/custom_error.cpp b/test/test/custom_error.cpp index f186ee970..453514810 100644 --- a/test/test/custom_error.cpp +++ b/test/test/custom_error.cpp @@ -102,10 +102,21 @@ TEST_CASE("custom_error_logger") // Validate that handler translated exception REQUIRE_THROWS_AS(check_hresult(0x80000018), hresult_illegal_delegate_assignment); REQUIRE(s_loggerCalled); +#ifndef __cpp_lib_source_location // In C++17 these fields cannot be filled in so they are expected to be empty. REQUIRE(s_loggerArgs.lineNumber == 0); REQUIRE(s_loggerArgs.fileName == nullptr); REQUIRE(s_loggerArgs.functionName == nullptr); +#else + // GCC can only compile these tests in C++20 so these fields will be filled in. + REQUIRE(s_loggerArgs.lineNumber == 103); + const auto fileNameSv = std::string_view(s_loggerArgs.fileName); + REQUIRE(!fileNameSv.empty()); + REQUIRE(fileNameSv.find("custom_error.cpp") != std::string::npos); + const auto functionNameSv = std::string_view(s_loggerArgs.functionName); + REQUIRE(!functionNameSv.empty()); + // Don't check the function name here. It is tested in `test_cpp20/custom_error.cpp`. +#endif REQUIRE(s_loggerArgs.returnAddress); REQUIRE(s_loggerArgs.result == 0x80000018); // E_ILLEGAL_DELEGATE_ASSIGNMENT) diff --git a/test/test_cpp20/custom_error.cpp b/test/test_cpp20/custom_error.cpp index 620ffc755..707088cd9 100644 --- a/test/test_cpp20/custom_error.cpp +++ b/test/test_cpp20/custom_error.cpp @@ -59,7 +59,11 @@ TEST_CASE("custom_error_logger") REQUIRE(fileNameSv.find("custom_error.cpp") != std::string::npos); const auto functionNameSv = std::string_view(s_loggerArgs.functionName); REQUIRE(!functionNameSv.empty()); +#if defined(__GNUC__) && !defined(__clang__) + REQUIRE(functionNameSv == "void {anonymous}::FailOnLine15()"); +#else REQUIRE(functionNameSv == "FailOnLine15"); +#endif REQUIRE(s_loggerArgs.returnAddress); REQUIRE(s_loggerArgs.result == 0x80000018); // E_ILLEGAL_DELEGATE_ASSIGNMENT) From 089ae3e6747e652d7d24b365291f0624a3c91d47 Mon Sep 17 00:00:00 2001 From: Alvin Wong Date: Fri, 9 Dec 2022 03:46:45 +0800 Subject: [PATCH 07/31] Disable code in weak.cpp test that doesn't build with GCC --- test/old_tests/UnitTests/weak.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/old_tests/UnitTests/weak.cpp b/test/old_tests/UnitTests/weak.cpp index 6724b71ad..55c40dbe6 100644 --- a/test/old_tests/UnitTests/weak.cpp +++ b/test/old_tests/UnitTests/weak.cpp @@ -50,8 +50,8 @@ namespace } }; -// FIXME: Fail to compile with Clang due to incomplete type. -#if !defined(__clang__) +// FIXME: Fail to compile with Clang and GCC due to incomplete type. +#if !defined(__clang__) && !defined(__GNUC__) struct WeakWithSelfReference : implements { winrt::weak_ref weak_self = get_weak(); @@ -447,8 +447,8 @@ TEST_CASE("weak,assignment") // Not constructible from L"" (because Uri constructor is explicit) static_assert(!std::is_constructible_v, const wchar_t*>); -// FIXME: WeakWithSelfReference fails to compile with Clang. -#if !defined(__clang__) +// FIXME: WeakWithSelfReference fails to compile with Clang and GCC. +#if !defined(__clang__) && !defined(__GNUC__) // Constructible from com_ptr because com_ptr is // implicitly convertible to com_ptr. struct Derived : WeakWithSelfReference {}; @@ -487,8 +487,8 @@ TEST_CASE("weak,no_module_lock") REQUIRE(get_module_lock() == object_count); } -// FIXME: WeakWithSelfReference fails to compile with Clang. -#if !defined(__clang__) +// FIXME: WeakWithSelfReference fails to compile with Clang and GCC. +#if !defined(__clang__) && !defined(__GNUC__) TEST_CASE("weak,self") { // The REQUIRE statements are in the WeakWithSelfReference class itself. From 47222ab35c2d96ea0a8a5ce912668b51f4928dcc Mon Sep 17 00:00:00 2001 From: Alvin Wong Date: Fri, 9 Dec 2022 17:42:57 +0800 Subject: [PATCH 08/31] Fix `async, resume_on_signal` test The expansion of the REQUIRE macro ends up causing the co_await expression inside to be run twice when compiled with GCC, which causes the test to fail. Now this may be a compiler bug, but the workaround is simple. --- test/old_tests/UnitTests/async.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/old_tests/UnitTests/async.cpp b/test/old_tests/UnitTests/async.cpp index e5c468144..979c2aac7 100644 --- a/test/old_tests/UnitTests/async.cpp +++ b/test/old_tests/UnitTests/async.cpp @@ -1566,10 +1566,12 @@ namespace co_await resume_on_signal(signal); // should not suspend because already signaled REQUIRE(caller == GetCurrentThreadId()); // still on calling thread - REQUIRE(false == co_await resume_on_signal(signal, 1us)); // should suspend but timeout + bool suspend_but_timeout_result = co_await resume_on_signal(signal, 1us); + REQUIRE(false == suspend_but_timeout_result); // should suspend but timeout REQUIRE(caller != GetCurrentThreadId()); // now on background thread - REQUIRE(true == co_await resume_on_signal(signal, 1s)); // should eventually succeed + bool suspend_and_succeed_result = co_await resume_on_signal(signal, 1s); + REQUIRE(true == suspend_and_succeed_result); // should eventually succeed } } From d6b56d69719b81ec427106d1e856198e7ce118b3 Mon Sep 17 00:00:00 2001 From: Alvin Wong Date: Fri, 9 Dec 2022 18:42:35 +0800 Subject: [PATCH 09/31] GCC: Fix clock::from_time_t with libstdc++ libstdc++ uses nanoseconds for system_clock, which causes signed overflow when offset by the epoch inside clock::from_sys. The fix is to reduce its resolution before passing it to clock::from_sys, instead of after. --- strings/base_chrono.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/strings/base_chrono.h b/strings/base_chrono.h index 151246d46..78f740134 100644 --- a/strings/base_chrono.h +++ b/strings/base_chrono.h @@ -47,7 +47,7 @@ WINRT_EXPORT namespace winrt static time_point from_time_t(time_t time) noexcept { - return std::chrono::time_point_cast(from_sys(std::chrono::system_clock::from_time_t(time))); + return from_sys(std::chrono::time_point_cast(std::chrono::system_clock::from_time_t(time))); } static file_time to_file_time(time_point const& time) noexcept From 36bb18a11a17e24add09c2cee0cee0a00ebfcfd0 Mon Sep 17 00:00:00 2001 From: Alvin Wong Date: Fri, 9 Dec 2022 18:46:30 +0800 Subject: [PATCH 10/31] cmake: Remove the update step for winmd ...so that it won't rebuild cppwinrt every single time. --- CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index e8ef5fa4a..b5f547fd5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -182,6 +182,7 @@ ExternalProject_Add(winmd CONFIGURE_COMMAND "" BUILD_COMMAND "" INSTALL_COMMAND "" + UPDATE_COMMAND "" ) add_dependencies(cppwinrt winmd) ExternalProject_Get_Property(winmd SOURCE_DIR) From f3f838d623fd2379a1e3ee21624a226205e7e48c Mon Sep 17 00:00:00 2001 From: Alvin Wong Date: Sat, 10 Dec 2022 06:46:11 +0800 Subject: [PATCH 11/31] GCC: Work around out_params_bad.cpp miscompilation --- test/test/CMakeLists.txt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/test/CMakeLists.txt b/test/test/CMakeLists.txt index c14fc7c00..75b363ca7 100644 --- a/test/test/CMakeLists.txt +++ b/test/test/CMakeLists.txt @@ -63,6 +63,12 @@ set_source_files_properties( PROPERTIES SKIP_PRECOMPILE_HEADERS true ) +if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") + # GCC seems to miscompile out_params_bad.cpp if -fdevirtualize is enabled. + # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108040 + set_source_files_properties(out_params_bad.cpp PROPERTIES COMPILE_OPTIONS "-fno-devirtualize") +endif() + add_dependencies(test-vanilla build-cppwinrt-projection) add_test( From f90f7547f0a07de7f436fd39f764a4e77e863940 Mon Sep 17 00:00:00 2001 From: Alvin Wong Date: Sun, 11 Dec 2022 02:51:32 +0800 Subject: [PATCH 12/31] GCC: Work around multi_threaded_vector.cpp miscompilation --- test/test/CMakeLists.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/test/CMakeLists.txt b/test/test/CMakeLists.txt index 75b363ca7..aea445868 100644 --- a/test/test/CMakeLists.txt +++ b/test/test/CMakeLists.txt @@ -67,6 +67,11 @@ if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") # GCC seems to miscompile out_params_bad.cpp if -fdevirtualize is enabled. # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108040 set_source_files_properties(out_params_bad.cpp PROPERTIES COMPILE_OPTIONS "-fno-devirtualize") + + if(CMAKE_BUILD_TYPE STREQUAL "Release") + # GCC miscompiles multi_threaded_vector.cpp with -O3 + set_source_files_properties(multi_threaded_vector.cpp PROPERTIES COMPILE_OPTIONS "-O2") + endif() endif() add_dependencies(test-vanilla build-cppwinrt-projection) From 1f4edaede8d49707170e380c294d4f2d9b4c9f12 Mon Sep 17 00:00:00 2001 From: Alvin Wong Date: Sun, 11 Dec 2022 16:05:22 +0800 Subject: [PATCH 13/31] GCC: Disable test_old/agile_ref that randomly crashes --- test/old_tests/UnitTests/agile_ref.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/old_tests/UnitTests/agile_ref.cpp b/test/old_tests/UnitTests/agile_ref.cpp index 2bf6dd637..13f76e17f 100644 --- a/test/old_tests/UnitTests/agile_ref.cpp +++ b/test/old_tests/UnitTests/agile_ref.cpp @@ -39,6 +39,9 @@ IAsyncAction test_agile_ref() #if defined(__clang__) && defined(_MSC_VER) && (defined(_M_IX86) || defined(__i386__)) // FIXME: Test is known to crash from calling invalid address on x86 when built with Clang. TEST_CASE("agile_ref", "[.clang-crash]") +#elif defined(__GNUC__) && !defined(__clang__) +// FIXME: Test is known to randomly crash or abort when built with GCC (segfaults under appverifier). +TEST_CASE("agile_ref", "[.gcc-crash]") #else TEST_CASE("agile_ref") #endif From 2b37b0a2374087cc486b4afc75b81c3db283e106 Mon Sep 17 00:00:00 2001 From: Alvin Wong Date: Sun, 11 Dec 2022 16:09:33 +0800 Subject: [PATCH 14/31] Remove use-after-free in hstring test --- test/old_tests/UnitTests/hstring.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/test/old_tests/UnitTests/hstring.cpp b/test/old_tests/UnitTests/hstring.cpp index 1369fd229..2abd9769f 100644 --- a/test/old_tests/UnitTests/hstring.cpp +++ b/test/old_tests/UnitTests/hstring.cpp @@ -272,7 +272,6 @@ TEST_CASE("hstring,operator,std::wstring_view") REQUIRE(L"abc" == ws); hs.clear(); - REQUIRE(L"abc" == ws); ws = hs; REQUIRE(ws.empty()); } From 5a5fc0948a622059d73d1e2bde2f8de0fad8f13e Mon Sep 17 00:00:00 2001 From: Alvin Wong Date: Fri, 9 Dec 2022 02:07:19 +0800 Subject: [PATCH 15/31] CI: Add GCC tests using MSYS2 --- .github/workflows/ci.yml | 45 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dca6c31bc..552d609aa 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -303,6 +303,51 @@ jobs: $env:UBSAN_OPTIONS = "print_stacktrace=1" ctest --verbose + test-msys2-gcc-cppwinrt: + name: 'gcc/msys2: Build and test (${{ matrix.sys }}, ${{ matrix.config }})' + strategy: + fail-fast: false + matrix: + include: + - { sys: mingw32, arch: i686, config: Debug } + - { sys: mingw32, arch: i686, config: Release } + - { sys: mingw64, arch: x86_64, config: Release } + - { sys: ucrt64, arch: x86_64, config: Release } + runs-on: windows-latest + defaults: + run: + shell: msys2 {0} + steps: + - uses: msys2/setup-msys2@v2 + with: + msystem: ${{matrix.sys}} + pacboy: >- + crt:p + gcc:p + binutils:p + cmake:p + ninja:p + + - uses: actions/checkout@v3 + + - name: Build cppwinrt + run: | + mkdir build + cd build + cmake ../ -GNinja -DCMAKE_BUILD_TYPE=${{ matrix.config }} \ + -DDOWNLOAD_WINDOWSNUMERICS=TRUE + cmake --build . --target cppwinrt + + - name: Build tests + run: | + cd build + cmake --build . --target test-vanilla test_cpp20 test_win7 test_old + + - name: Run tests + run: | + cd build + ctest --verbose + build-linux-cross-cppwinrt: name: 'cross: Cross-build from Linux' strategy: From 8cdacaab7ac8830d7b069fc39f54537a0a0ba0c5 Mon Sep 17 00:00:00 2001 From: Alvin Wong Date: Fri, 9 Dec 2022 03:03:37 +0800 Subject: [PATCH 16/31] CI/cmake: Enable colour test output --- .github/workflows/ci.yml | 12 ++++++++---- test/CMakeLists.txt | 7 +++++++ test/old_tests/UnitTests/CMakeLists.txt | 2 +- test/test/CMakeLists.txt | 2 +- test/test_cpp20/CMakeLists.txt | 2 +- test/test_win7/CMakeLists.txt | 2 +- 6 files changed, 19 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 552d609aa..e9ff442db 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -277,6 +277,7 @@ jobs: } cmake ../ -G"MinGW Makefiles" -DCMAKE_BUILD_TYPE=${{ matrix.config }} ` -DDOWNLOAD_WINDOWSNUMERICS=TRUE ` + -DUSE_ANSI_COLOR=TRUE ` -DENABLE_TEST_SANITIZERS=$sanitizers cmake --build . -j2 --target cppwinrt @@ -335,7 +336,8 @@ jobs: mkdir build cd build cmake ../ -GNinja -DCMAKE_BUILD_TYPE=${{ matrix.config }} \ - -DDOWNLOAD_WINDOWSNUMERICS=TRUE + -DDOWNLOAD_WINDOWSNUMERICS=TRUE \ + -DUSE_ANSI_COLOR=TRUE cmake --build . --target cppwinrt - name: Build tests @@ -412,7 +414,8 @@ jobs: cd build cmake ../test -G"MinGW Makefiles" -DCMAKE_BUILD_TYPE=Debug ` -DCPPWINRT_PROJECTION_INCLUDE_DIR="../.test/out" ` - -DDOWNLOAD_WINDOWSNUMERICS=TRUE + -DDOWNLOAD_WINDOWSNUMERICS=TRUE ` + -DUSE_ANSI_COLOR=TRUE cmake --build . -j2 - name: Run tests @@ -453,7 +456,8 @@ jobs: -DCMAKE_BUILD_TYPE=Debug \ -DCMAKE_CXX_FLAGS="-static" \ -DCPPWINRT_PROJECTION_INCLUDE_DIR=/tmp/cppwinrt \ - -DDOWNLOAD_WINDOWSNUMERICS=TRUE + -DDOWNLOAD_WINDOWSNUMERICS=TRUE \ + -DUSE_ANSI_COLOR=TRUE cmake --build build/cross-tests -j2 - name: Upload built tests @@ -484,7 +488,7 @@ jobs: $has_failed_tests = 0 foreach ($test_exe in $test_exes) { echo "::group::Run '$test_exe'" - & .\$test_exe + & .\$test_exe --use-colour yes echo "::endgroup::" if ($LastExitCode -ne 0) { echo "::error::Test '$test_exe' failed!" diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 6cf1aa3ca..aad6605ea 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -94,6 +94,13 @@ if(ENABLE_TEST_SANITIZERS) endif() +set(USE_ANSI_COLOR FALSE CACHE BOOL "Enable ANSI colour output for Catch2 test runner.") +if(USE_ANSI_COLOR) + add_compile_definitions(CATCH_CONFIG_COLOUR_ANSI) + set(TEST_COLOR_ARG "--use-colour yes") +endif() + + add_subdirectory(test) add_subdirectory(test_cpp20) add_subdirectory(test_win7) diff --git a/test/old_tests/UnitTests/CMakeLists.txt b/test/old_tests/UnitTests/CMakeLists.txt index 7042aa63b..9adf9a615 100644 --- a/test/old_tests/UnitTests/CMakeLists.txt +++ b/test/old_tests/UnitTests/CMakeLists.txt @@ -57,5 +57,5 @@ add_dependencies(test_old build-cppwinrt-projection) add_test( NAME test_old - COMMAND "$" + COMMAND "$" ${TEST_COLOR_ARG} ) diff --git a/test/test/CMakeLists.txt b/test/test/CMakeLists.txt index aea445868..f2cfe2cc5 100644 --- a/test/test/CMakeLists.txt +++ b/test/test/CMakeLists.txt @@ -78,5 +78,5 @@ add_dependencies(test-vanilla build-cppwinrt-projection) add_test( NAME test - COMMAND "$" + COMMAND "$" ${TEST_COLOR_ARG} ) diff --git a/test/test_cpp20/CMakeLists.txt b/test/test_cpp20/CMakeLists.txt index 34895411f..cbf1b799d 100644 --- a/test/test_cpp20/CMakeLists.txt +++ b/test/test_cpp20/CMakeLists.txt @@ -42,5 +42,5 @@ add_dependencies(test_cpp20 build-cppwinrt-projection) add_test( NAME test_cpp20 - COMMAND "$" + COMMAND "$" ${TEST_COLOR_ARG} ) diff --git a/test/test_win7/CMakeLists.txt b/test/test_win7/CMakeLists.txt index 7ada247c4..901681d17 100644 --- a/test/test_win7/CMakeLists.txt +++ b/test/test_win7/CMakeLists.txt @@ -54,5 +54,5 @@ add_dependencies(test_win7 build-cppwinrt-projection) add_test( NAME test_win7 - COMMAND "$" + COMMAND "$" ${TEST_COLOR_ARG} ) From 3ea3663c371c8cc0a050dfa3cbccdf6a8c77c7d6 Mon Sep 17 00:00:00 2001 From: Alvin Wong Date: Fri, 9 Dec 2022 02:43:44 +0800 Subject: [PATCH 17/31] CI: Enable CMake colour diagnostics --- .github/workflows/ci.yml | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e9ff442db..66a5a70c8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -257,6 +257,9 @@ jobs: arch: [i686, x86_64] config: [Debug, Release] runs-on: windows-latest + env: + CMAKE_COLOR_DIAGNOSTICS: 1 + CLICOLOR_FORCE: 1 steps: - uses: actions/checkout@v3 @@ -278,6 +281,7 @@ jobs: cmake ../ -G"MinGW Makefiles" -DCMAKE_BUILD_TYPE=${{ matrix.config }} ` -DDOWNLOAD_WINDOWSNUMERICS=TRUE ` -DUSE_ANSI_COLOR=TRUE ` + -DCMAKE_CXX_FLAGS="-fansi-escape-codes" ` -DENABLE_TEST_SANITIZERS=$sanitizers cmake --build . -j2 --target cppwinrt @@ -315,6 +319,9 @@ jobs: - { sys: mingw64, arch: x86_64, config: Release } - { sys: ucrt64, arch: x86_64, config: Release } runs-on: windows-latest + env: + CMAKE_COLOR_DIAGNOSTICS: 1 + CLICOLOR_FORCE: 1 defaults: run: shell: msys2 {0} @@ -356,6 +363,9 @@ jobs: matrix: arch: [i686, x86_64] runs-on: ubuntu-22.04 + env: + CMAKE_COLOR_DIAGNOSTICS: 1 + CLICOLOR_FORCE: 1 steps: - uses: actions/checkout@v3 @@ -426,6 +436,9 @@ jobs: build-linux-native-cppwinrt: name: 'linux: GCC native build + llvm-mingw cross-build tests' runs-on: ubuntu-22.04 + env: + CMAKE_COLOR_DIAGNOSTICS: 1 + CLICOLOR_FORCE: 1 steps: - uses: actions/checkout@v3 From 2ad00efc6bbaafbd8b38be0fe113a2266c44f3a5 Mon Sep 17 00:00:00 2001 From: Alvin Wong Date: Fri, 9 Dec 2022 22:13:39 +0800 Subject: [PATCH 18/31] CI: Update msys2 packages --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 66a5a70c8..406f63ea4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -329,6 +329,7 @@ jobs: - uses: msys2/setup-msys2@v2 with: msystem: ${{matrix.sys}} + update: true pacboy: >- crt:p gcc:p From bb78632e2525e827fb34226baa284ae23545b907 Mon Sep 17 00:00:00 2001 From: Alvin Wong Date: Fri, 9 Dec 2022 21:24:45 +0800 Subject: [PATCH 19/31] CI/msys2: Add mingw64 build --- .github/workflows/ci.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 406f63ea4..c4e679afc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -316,6 +316,7 @@ jobs: include: - { sys: mingw32, arch: i686, config: Debug } - { sys: mingw32, arch: i686, config: Release } + - { sys: mingw64, arch: x86_64, config: Debug } - { sys: mingw64, arch: x86_64, config: Release } - { sys: ucrt64, arch: x86_64, config: Release } runs-on: windows-latest @@ -351,7 +352,7 @@ jobs: - name: Build tests run: | cd build - cmake --build . --target test-vanilla test_cpp20 test_win7 test_old + cmake --build . -j2 --target test-vanilla test_cpp20 test_win7 test_old - name: Run tests run: | From 9c9627b5b18861fdf7425f26349e5fd02a9ae890 Mon Sep 17 00:00:00 2001 From: Alvin Wong Date: Sun, 11 Dec 2022 02:57:46 +0800 Subject: [PATCH 20/31] CI: Skip building large PCH for i686 --- .github/workflows/ci.yml | 6 +++++- test/CMakeLists.txt | 3 +++ test/old_tests/UnitTests/CMakeLists.txt | 12 +++++++----- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c4e679afc..f60f550ae 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -344,9 +344,13 @@ jobs: run: | mkdir build cd build + if [[ "${{ matrix.arch }}" = "i686" ]]; then + skip_large_pch_arg="-DSKIP_LARGE_PCH=TRUE" + fi cmake ../ -GNinja -DCMAKE_BUILD_TYPE=${{ matrix.config }} \ -DDOWNLOAD_WINDOWSNUMERICS=TRUE \ - -DUSE_ANSI_COLOR=TRUE + -DUSE_ANSI_COLOR=TRUE \ + $skip_large_pch_arg cmake --build . --target cppwinrt - name: Build tests diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index aad6605ea..d4bb3e125 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -101,6 +101,9 @@ if(USE_ANSI_COLOR) endif() +set(SKIP_LARGE_PCH FALSE CACHE BOOL "Skip building large precompiled headers.") + + add_subdirectory(test) add_subdirectory(test_cpp20) add_subdirectory(test_win7) diff --git a/test/old_tests/UnitTests/CMakeLists.txt b/test/old_tests/UnitTests/CMakeLists.txt index 9adf9a615..908691653 100644 --- a/test/old_tests/UnitTests/CMakeLists.txt +++ b/test/old_tests/UnitTests/CMakeLists.txt @@ -47,11 +47,13 @@ endforeach() add_executable(test_old Main.cpp ${TEST_SRCS}) target_link_libraries(test_old runtimeobject synchronization) -target_precompile_headers(test_old PRIVATE pch.h) -set_source_files_properties( - conditional_implements_pure.cpp - PROPERTIES SKIP_PRECOMPILE_HEADERS true -) +if(NOT SKIP_LARGE_PCH) + target_precompile_headers(test_old PRIVATE pch.h) + set_source_files_properties( + conditional_implements_pure.cpp + PROPERTIES SKIP_PRECOMPILE_HEADERS true + ) +endif() add_dependencies(test_old build-cppwinrt-projection) From e7d0d09546cea939bf13139a610ad81a8172ce6b Mon Sep 17 00:00:00 2001 From: Alvin Wong Date: Sun, 11 Dec 2022 16:52:39 +0800 Subject: [PATCH 21/31] CI: Remove msys2 i686 Debug build The test doesn't build due to meomory limitation. --- .github/workflows/ci.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f60f550ae..90c9d705b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -314,7 +314,6 @@ jobs: fail-fast: false matrix: include: - - { sys: mingw32, arch: i686, config: Debug } - { sys: mingw32, arch: i686, config: Release } - { sys: mingw64, arch: x86_64, config: Debug } - { sys: mingw64, arch: x86_64, config: Release } From 0f0a3ef771bd509be78c8cfda5131590eb158d12 Mon Sep 17 00:00:00 2001 From: Alvin Wong Date: Sun, 11 Dec 2022 22:09:37 +0800 Subject: [PATCH 22/31] GCC: Fix clock::to_time_t with libstdc++ libstdc++ uses nanoseconds for system_clock, which causes signed overflow when offset by the epoch inside clock::to_sys. The fix is to do time_point_ca *after* passing it to clock::to_sys, instead of before. --- strings/base_chrono.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/strings/base_chrono.h b/strings/base_chrono.h index 78f740134..d48cd703b 100644 --- a/strings/base_chrono.h +++ b/strings/base_chrono.h @@ -42,7 +42,7 @@ WINRT_EXPORT namespace winrt static time_t to_time_t(time_point const& time) noexcept { - return static_cast(std::chrono::system_clock::to_time_t(to_sys(std::chrono::time_point_cast(time)))); + return static_cast(std::chrono::system_clock::to_time_t(std::chrono::time_point_cast(to_sys(time)))); } static time_point from_time_t(time_t time) noexcept From 0129567a307a401dc00c4ffe2c6a191fc654c82a Mon Sep 17 00:00:00 2001 From: Alvin Wong Date: Sun, 11 Dec 2022 17:38:44 +0800 Subject: [PATCH 23/31] Include directxmath.h only if it exists This is not available in older mingw-w64 headers. --- strings/base_includes.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/strings/base_includes.h b/strings/base_includes.h index 54edca841..ef4336862 100644 --- a/strings/base_includes.h +++ b/strings/base_includes.h @@ -27,8 +27,10 @@ #if __has_include() #define WINRT_IMPL_NUMERICS +#if __has_include() #include #endif +#endif #ifndef WINRT_LEAN_AND_MEAN #include From 24cd40f8ddfa10e8c3255b4a8ce2bea38eb31624 Mon Sep 17 00:00:00 2001 From: Alvin Wong Date: Sun, 11 Dec 2022 17:26:35 +0800 Subject: [PATCH 24/31] CI: Add cross-built tests with GCC --- .github/workflows/ci.yml | 37 ++++++++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 90c9d705b..459ec822e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -439,14 +439,33 @@ jobs: ctest --verbose build-linux-native-cppwinrt: - name: 'linux: GCC native build + llvm-mingw cross-build tests' + name: 'linux: GCC native build + mingw-w64 cross-build tests' + strategy: + fail-fast: false + matrix: + cross_toolchain: [gcc, llvm-mingw] + cross_arch: [i686, x86_64] + include: + - cross_toolchain: gcc + container: + image: archlinux:base-devel runs-on: ubuntu-22.04 + container: ${{ matrix.container }} + defaults: + run: + shell: bash env: CMAKE_COLOR_DIAGNOSTICS: 1 CLICOLOR_FORCE: 1 steps: - uses: actions/checkout@v3 + - name: Install build tools + if: matrix.cross_toolchain == 'gcc' + run: | + pacman --noconfirm -Suuy + pacman --needed --noconfirm -S cmake ninja git + - name: Build cppwinrt run: | cmake -S . -B build/native/ \ @@ -465,12 +484,18 @@ jobs: - id: setup-llvm name: Set up llvm-mingw + if: matrix.cross_toolchain == 'llvm-mingw' uses: ./.github/actions/setup-llvm-mingw + - name: Install GCC cross compiler + if: matrix.cross_toolchain == 'gcc' + run: | + pacman --needed --noconfirm -S mingw-w64-gcc + - name: Cross-build tests using projection run: | cmake -S test -B build/cross-tests --toolchain "$PWD/cross-mingw-toolchain.cmake" \ - -DCMAKE_SYSTEM_PROCESSOR=x86_64 \ + -DCMAKE_SYSTEM_PROCESSOR=${{ matrix.cross_arch }} \ -DCMAKE_BUILD_TYPE=Debug \ -DCMAKE_CXX_FLAGS="-static" \ -DCPPWINRT_PROJECTION_INCLUDE_DIR=/tmp/cppwinrt \ @@ -481,15 +506,17 @@ jobs: - name: Upload built tests uses: actions/upload-artifact@v3 with: - name: linux-native-cppwinrt-cross-build-tests-x86_64-bin + name: linux-native-cppwinrt-cross-build-tests-${{ matrix.cross_toolchain }}-${{ matrix.cross_arch }}-bin path: build/cross-tests/*.exe test-linux-native-cppwinrt-cross-tests: name: 'linux: Run llvm-mingw cross-build tests' needs: build-linux-native-cppwinrt strategy: + fail-fast: false matrix: - arch: [x86_64] + cross_toolchain: [gcc, llvm-mingw] + cross_arch: [i686, x86_64] runs-on: windows-latest steps: - uses: actions/checkout@v3 @@ -497,7 +524,7 @@ jobs: - name: Fetch test executables uses: actions/download-artifact@v3 with: - name: linux-native-cppwinrt-cross-build-tests-${{ matrix.arch }}-bin + name: linux-native-cppwinrt-cross-build-tests-${{ matrix.cross_toolchain }}-${{ matrix.cross_arch }}-bin path: ./ - name: Run tests From ed9d296807400a020814b845cae55731671e7d9e Mon Sep 17 00:00:00 2001 From: Alvin Wong Date: Sun, 11 Dec 2022 23:04:17 +0800 Subject: [PATCH 25/31] CI: Disable GCC cross build (mingw-w64 headers are still too old) --- .github/workflows/ci.yml | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 459ec822e..1a3b121b7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -443,12 +443,14 @@ jobs: strategy: fail-fast: false matrix: - cross_toolchain: [gcc, llvm-mingw] + # TODO: Enable gcc build once Arch Linux gets more recent mingw-w64 headers (ver. 11 perhaps?) + # cross_toolchain: [gcc, llvm-mingw] + cross_toolchain: [llvm-mingw] cross_arch: [i686, x86_64] - include: - - cross_toolchain: gcc - container: - image: archlinux:base-devel + # include: + # - cross_toolchain: gcc + # container: + # image: archlinux:base-devel runs-on: ubuntu-22.04 container: ${{ matrix.container }} defaults: @@ -515,7 +517,9 @@ jobs: strategy: fail-fast: false matrix: - cross_toolchain: [gcc, llvm-mingw] + # TODO: Enable gcc build test when it is buildable + # cross_toolchain: [gcc, llvm-mingw] + cross_toolchain: [llvm-mingw] cross_arch: [i686, x86_64] runs-on: windows-latest steps: From e2cd4df94d0014708d6d4fbc5383457aa066aeb3 Mon Sep 17 00:00:00 2001 From: Alvin Wong Date: Fri, 9 Dec 2022 02:36:35 +0800 Subject: [PATCH 26/31] test: Tag async_propagate_cancel test as mayfail Ref: https://github.com/microsoft/cppwinrt/issues/1243 --- test/test/async_propagate_cancel.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/test/async_propagate_cancel.cpp b/test/test/async_propagate_cancel.cpp index a3e5af749..b2e331929 100644 --- a/test/test/async_propagate_cancel.cpp +++ b/test/test/async_propagate_cancel.cpp @@ -132,7 +132,8 @@ namespace // FIXME: Test is known to segfault when built with Clang. TEST_CASE("async_propagate_cancel", "[.clang-crash]") #else -TEST_CASE("async_propagate_cancel") +// FIXME: mayfail because of https://github.com/microsoft/cppwinrt/issues/1243 +TEST_CASE("async_propagate_cancel", "[!mayfail]") #endif { Check(Action); From 9645cf0a1be0eae7d02187fea567490e1cbbba20 Mon Sep 17 00:00:00 2001 From: Alvin Wong Date: Mon, 12 Dec 2022 18:33:52 +0800 Subject: [PATCH 27/31] Revert "Include directxmath.h only if it exists" This reverts commit 0129567a307a401dc00c4ffe2c6a191fc654c82a. --- strings/base_includes.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/strings/base_includes.h b/strings/base_includes.h index ef4336862..54edca841 100644 --- a/strings/base_includes.h +++ b/strings/base_includes.h @@ -27,10 +27,8 @@ #if __has_include() #define WINRT_IMPL_NUMERICS -#if __has_include() #include #endif -#endif #ifndef WINRT_LEAN_AND_MEAN #include From cc37b3f49e76bad39e44d1cb983efb91e2d983a9 Mon Sep 17 00:00:00 2001 From: Alvin Wong Date: Tue, 13 Dec 2022 00:40:40 +0800 Subject: [PATCH 28/31] Revert "Work around GCC wanting copy constructor in thread_pool" This reverts commit e87b24965d022a33d1dcb8a7d1591988965450d6. --- strings/base_coroutine_threadpool.h | 37 +++-------------------------- 1 file changed, 3 insertions(+), 34 deletions(-) diff --git a/strings/base_coroutine_threadpool.h b/strings/base_coroutine_threadpool.h index 43a598d5e..9861b8294 100644 --- a/strings/base_coroutine_threadpool.h +++ b/strings/base_coroutine_threadpool.h @@ -556,35 +556,18 @@ WINRT_EXPORT namespace winrt return awaitable{ handle, timeout }; } - // HACK: GCC does not compile co_await on thread_pool because it wants - // a copy constructor. Since m_pool is not copyable, in order to provide - // a workaround we have to wrap it in a shared_ptr. Apply this workaround - // only for GCC so we don't affect other compilers. - // This might be related to upstream bug: - // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103963 struct thread_pool { thread_pool() : -#if defined(__GNUC__) && !defined(__clang__) - m_pool(std::make_shared>( - check_pointer(WINRT_IMPL_CreateThreadpool(nullptr)))) -#else m_pool(check_pointer(WINRT_IMPL_CreateThreadpool(nullptr))) -#endif { - m_environment.Pool = get_pool(); + m_environment.Pool = m_pool.get(); } -#if defined(__GNUC__) && !defined(__clang__) - // HACK: See above - thread_pool(thread_pool const&) = default; - thread_pool &operator=(thread_pool const&) = delete; -#endif - void thread_limits(uint32_t const high, uint32_t const low) { - WINRT_IMPL_SetThreadpoolThreadMaximum(get_pool(), high); - check_bool(WINRT_IMPL_SetThreadpoolThreadMinimum(get_pool(), low)); + WINRT_IMPL_SetThreadpoolThreadMaximum(m_pool.get(), high); + check_bool(WINRT_IMPL_SetThreadpoolThreadMinimum(m_pool.get(), low)); } bool await_ready() const noexcept @@ -649,21 +632,7 @@ WINRT_EXPORT namespace winrt uint32_t Size{ sizeof(environment) }; }; - pool_traits::type get_pool() const - { -#if defined(__GNUC__) && !defined(__clang__) - return m_pool->get(); -#else - return m_pool.get(); -#endif - } - -#if defined(__GNUC__) && !defined(__clang__) - // HACK: See above - std::shared_ptr> m_pool; -#else handle_type m_pool; -#endif environment m_environment; }; From 9d4583fc6ee1e7405234203a8aac149ffcc7a5b6 Mon Sep 17 00:00:00 2001 From: Alvin Wong Date: Tue, 13 Dec 2022 00:47:47 +0800 Subject: [PATCH 29/31] test: Disable thread_pool.cpp tests for GCC GCC does not compile co_await on thread_pool because it wants a copy constructor. Disabling this test for now. This might be related to upstream bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103963 --- test/test/CMakeLists.txt | 10 ++++++++++ test/test_win7/CMakeLists.txt | 10 ++++++++++ 2 files changed, 20 insertions(+) diff --git a/test/test/CMakeLists.txt b/test/test/CMakeLists.txt index f2cfe2cc5..ba174d41b 100644 --- a/test/test/CMakeLists.txt +++ b/test/test/CMakeLists.txt @@ -39,6 +39,16 @@ list(APPEND BROKEN_TESTS when ) +if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") + # FIXME: GCC does not compile co_await on thread_pool because it wants + # a copy constructor. Disabling this test for now. + # This might be related to upstream bug: + # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103963 + list(APPEND BROKEN_TESTS + thread_pool + ) +endif() + # Exclude broken tests foreach(TEST_SRCS_EXCLUDE_ITEM IN LISTS BROKEN_TESTS) list(FILTER TEST_SRCS EXCLUDE REGEX "/${TEST_SRCS_EXCLUDE_ITEM}\\.cpp") diff --git a/test/test_win7/CMakeLists.txt b/test/test_win7/CMakeLists.txt index 901681d17..d1c4120ea 100644 --- a/test/test_win7/CMakeLists.txt +++ b/test/test_win7/CMakeLists.txt @@ -32,6 +32,16 @@ list(APPEND BROKEN_TESTS when ) +if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") + # FIXME: GCC does not compile co_await on thread_pool because it wants + # a copy constructor. Disabling this test for now. + # This might be related to upstream bug: + # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103963 + list(APPEND BROKEN_TESTS + thread_pool + ) +endif() + # Exclude broken tests foreach(TEST_SRCS_EXCLUDE_ITEM IN LISTS BROKEN_TESTS) list(FILTER TEST_SRCS EXCLUDE REGEX "/${TEST_SRCS_EXCLUDE_ITEM}\\.cpp") From 75d5c6e3930fb379ed91fc7d8beb86def5c65033 Mon Sep 17 00:00:00 2001 From: Alvin Wong Date: Tue, 13 Dec 2022 03:18:03 +0800 Subject: [PATCH 30/31] test: Remove redundant checks in test/custom_error.cpp --- test/test/custom_error.cpp | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/test/test/custom_error.cpp b/test/test/custom_error.cpp index 453514810..76cb123a7 100644 --- a/test/test/custom_error.cpp +++ b/test/test/custom_error.cpp @@ -108,14 +108,9 @@ TEST_CASE("custom_error_logger") REQUIRE(s_loggerArgs.fileName == nullptr); REQUIRE(s_loggerArgs.functionName == nullptr); #else - // GCC can only compile these tests in C++20 so these fields will be filled in. - REQUIRE(s_loggerArgs.lineNumber == 103); - const auto fileNameSv = std::string_view(s_loggerArgs.fileName); - REQUIRE(!fileNameSv.empty()); - REQUIRE(fileNameSv.find("custom_error.cpp") != std::string::npos); - const auto functionNameSv = std::string_view(s_loggerArgs.functionName); - REQUIRE(!functionNameSv.empty()); - // Don't check the function name here. It is tested in `test_cpp20/custom_error.cpp`. + // GCC/Clang can only compile these tests in C++20 mode. If source_location + // is available these fields will be filled in. Don't do any cheks here + // because these are already tested in `test_cpp20/custom_error.cpp`. #endif REQUIRE(s_loggerArgs.returnAddress); From fb9d1055509eccf86c7b8a4b535e9ca48ce53983 Mon Sep 17 00:00:00 2001 From: Alvin Wong Date: Tue, 13 Dec 2022 05:41:30 +0800 Subject: [PATCH 31/31] Fix typo --- test/test/custom_error.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test/custom_error.cpp b/test/test/custom_error.cpp index 76cb123a7..8e1855e31 100644 --- a/test/test/custom_error.cpp +++ b/test/test/custom_error.cpp @@ -109,7 +109,7 @@ TEST_CASE("custom_error_logger") REQUIRE(s_loggerArgs.functionName == nullptr); #else // GCC/Clang can only compile these tests in C++20 mode. If source_location - // is available these fields will be filled in. Don't do any cheks here + // is available these fields will be filled in. Don't do any checks here // because these are already tested in `test_cpp20/custom_error.cpp`. #endif