Skip to content

Remove dead pre-C++20 compatibility fallbacks#22

Open
pr-minder[bot] wants to merge 8 commits into
masterfrom
claude/cpp20-cleanup
Open

Remove dead pre-C++20 compatibility fallbacks#22
pr-minder[bot] wants to merge 8 commits into
masterfrom
claude/cpp20-cleanup

Conversation

@pr-minder

@pr-minder pr-minder Bot commented Jul 21, 2026

Copy link
Copy Markdown

Removes compatibility fallbacks for pre-C++20 toolchains that no CI compiler needs anymore, and fixes thread_pool teardown (the final commit, last section below). Every change was verified locally with g++-12 and clang++-15/libc++-15, each in header-only and compiled-library mode, running the full Catch2 suite (the same matrix CI runs, minus g++-13/clang++-14 which are covered by CI itself).

MH_CONSTEVAL macro removed

MH_CONSTEVAL expanded to consteval when __cpp_consteval was defined and constexpr otherwise. Both definition sites (cpp/include/mh/variant.hpp, cpp/include/mh/text/multi_char.hpp) are gone; all five uses now say consteval directly. This also removes the macro leak from multi_char.hpp, which never #undefd it.

Note: clang 15 does not define __cpp_consteval, so the libc++ CI legs previously compiled these functions as constexpr; plain consteval compiles fine there (all four local configs build and pass, test counts unchanged), and every existing caller was already a compile-time context.

Unconditional #include <coroutine>

cpp/include/mh/coroutine/coroutine_include.hpp selected between <coroutine> (aliasing detail::coro = std) and <experimental/coroutine> (aliasing std::experimental). Every supported standard library ships <coroutine> (libstdc++ since GCC 10, libc++ since LLVM 14, MSVC since VS 2019 16.8), and libc++ 14 removed <experimental/coroutine> outright — the fallback branch was unreachable on every CI leg (coroutine tests run and pass on all of them, which requires the <coroutine> branch). The __INTELLISENSE__ special case existed only to force the non-experimental branch and is subsumed. MH_COROUTINES_SUPPORTED (21 references across 15 files) and the detail::coro alias (~38 uses) keep their current definitions; retiring them is left as a follow-up.

std::countl_zero in uint128

cpp/include/mh/math/uint128.hpp carried a hand-rolled countl_zero (explicit zero check plus __builtin_clzl/__builtin_clz, with a generic bit loop for other compilers) behind a __cpp_lib_bitops gate. libc++ 15 does not define __cpp_lib_bitops, but std::countl_zero itself is present and compiles on every CI standard library, and its semantics match the fallback exactly (returns the digit count for zero). The wrapper is deleted; the three call sites (leading_zeros(), the to_string digit skip) call std::countl_zero directly, and #include <bit> is unconditional.

std::numbers::pi in math/angles.hpp

The magic literals 0.01745329251994329577 (pi/180) and 57.2957795130823208768 (180/pi) are now derived from std::numbers::pi (<numbers> is present on every CI standard library). The factors are still computed in double precision and the derived values are bit-identical to the old literals — verified by static_assert on both libstdc++ and libc++ — so every instantiation returns exactly the same result as before. (Deliberately not pi_v<TResult> arithmetic: computing 180/pi directly in float yields a less accurate value than converting the double-precision factor, and would have changed results.) The header previously had no tests; a small test/math_angles_test.cpp pins the factors at compile time and checks a round trip (+1 test case on every leg).

mh::erase and mh::sort removed

mh::erase (erase–remove idiom) duplicates C++20 std::erase, and both mh::sort overloads were thin pass-throughs to std::sort with no added behavior. Neither had any callers outside their own tests. Deliberate test removals in test/algorithm_algorithm_test.cpp: the erase, sort, and sort with comparator cases (plus the comparator_probe helper, which existed only to prove mh::sort never moved from a caller-owned comparator — a property with no subject once the wrapper is gone). Coverage of the remaining algorithms in that header (find_or_add*, contains) is unchanged. Net test-case count: −3 per leg. (Callers were not migrated to std::ranges::sort: libc++ 15 has no ranges algorithms, so std::sort/std::erase are the correct replacements on this support matrix.)

Pre-C++20 <=> and concepts guards removed

Guards on __cpp_impl_three_way_comparison >= 201907 and __cpp_concepts >= 201907 (and their __has_include(<compare>) / __has_include(<concepts>) companions) are gone: both macros are guaranteed in C++20 mode, and every CI compiler already took the guarded branch, so the compiled code is identical on every leg. Covered:

  • math/uint128.hpp — the operator<=> family is unconditional; the pre-spaceship operator< / operator>= fallback block (dead on every leg) is deleted; <compare> is included unconditionally.
  • error/status.hpp (defaulted operator<=> on value_type), memory/checked_ptr.hpp (pointer <=> overloads, plus the mirrored guard around the TEST_CASE in test/memory_checked_ptr_test.cpp), memory/unique_object.hpp (member and free <=>, the UniqueObjectTraits concept and its requires clause).
  • text/multi_char.hpp — the <=> overloads move into the main namespace block; the separate #if __has_include(<compare>) section at the bottom of the file is gone.
  • Whole-header __cpp_concepts gates around raii/scope_exit.hpp, error/expected.hpp, and reflection/struct.hpp (plus the mirrored whole-file gate in test/reflection_struct_test.cpp), and the partial gates in error/ensure.hpp (StreamInsertable; can_print_value_impl() loses the return false fallback no supported compiler could reach).

Guards that also tested __cpp_lib_three_way_comparison were not deleted: io/getopt.hpp, memory/buffer.hpp/.inl, and test/memory_buffer_test.cpp keep their guards with the condition reduced to just the library-side check. libc++ 14/15 still lack library <=>, so the clang legs still compile the #else bool-comparison path in getopt.hpp and still skip buffer's operator<=> — exactly as before.

Unconditional std::is_constant_evaluated

__cpp_lib_is_constant_evaluated gates are removed from data/bit_float.hpp, math/uint128.hpp, data/bits.hpp, text/fmtstr.hpp, and math/interpolation.hpp; the feature is present on every CI standard library (probed on both toolchains). uint128.hpp's detail::uint128_hpp::is_constant_evaluated() wrapper — whose fallback returned true, i.e. permanently disabled the runtime __uint128_t fast paths on pre-C++20 libraries — is deleted, and its ten call sites use std::is_constant_evaluated() directly. bit_float.hpp's equivalent wrapper had no callers and is deleted outright. In bits.hpp, three gates guarded memcpy/unaligned-read fast paths that are themselves disabled unless MH_BITS_ENABLE_SMALL_MEMCPY / MH_BITS_ENABLE_UNALIGNED_INTEGERS are defined (both are commented out at the top of the header); those #ifs now test just the MH_BITS_ENABLE_* macros, so the (currently non-existent) fast paths only cost anything if those experiments are re-enabled.

Unconditional std::bit_cast — undefined-behavior fallback deleted

data/bit_float.hpp's detail::bit_float_hpp::bit_cast fallback was *reinterpret_cast<const To*>(&from) — a strict-aliasing violation (undefined behavior) on any compiler that ever took that branch. The wrapper is deleted; bits_to_native / native_to_bits call std::bit_cast directly, and <bit> is included unconditionally. math/uint128.hpp's get_u128 / set_u128 similarly lose their __cpp_lib_bit_cast gates and shift/or fallbacks (well-defined, just redundant). std::bit_cast was probed on both CI toolchains, including the std::array<uint64_t, 2>__uint128_t conversion these functions perform.

With no feature-test macros left to consult, the #if __has_include(<version>) includes at the top of uint128.hpp and bit_float.hpp serve no purpose and are deleted. The debug() helpers in uint128.hpp / bits.hpp stay (they are referenced from the non-platform operator/ path and the bit-copy slow paths) but lose the dead feature gate inside their bodies.

thread_pool joins its workers on destruction

~thread_pool() used to detach() its workers, so they raced program teardown: threads either kept running pool work for up to a second after the pool was gone, or stayed parked on the dispatcher's condition variable until their current 1s wait window expired. The destructor now closes the pool's dispatcher and join()s every worker, and the constructor joins already-started threads if spawning a later one throws.

The mechanism is a new dispatcher::close() (plus is_closed() and an untimed wait_tasks()), with these shutdown semantics — documented on close() and ~thread_pool():

  • Accepted work is never dropped. Tasks that are already runnable (queued tasks, expired delays) stay runnable and are drained by the workers before they exit; the joins only wait on work that is actually executing.
  • Waits whose condition never arrived complete exceptionally. close() flushes parked delay/fd waits into the ready queue; their awaiters observe the closed dispatcher on resume and throw. Destroying or abandoning the handles instead is not an option — task objects may still reference the frames (destroy would be use-after-free) and an abandoned frame leaks and hangs its waiters forever — so every coroutine reaches a terminal state and anyone wait()ing on its task wakes up.
  • co_await after close() throws instead of suspending. The internal add functions check the closed flag under the same lock that guards the queues and reject the handle, closing the add-vs-close race; nothing is ever parked in a queue that will never run again.

The worker loop is now purely condition-variable-driven — while (wait_tasks()) run() instead of the old 1s poll cycle — so both shutdown and delay-task wakeup are prompt: close() wakes every thread blocked in the wait_tasks family via notify_all, and wait_tasks() re-derives its deadline from the delay-heap front on every wakeup, like the timed variants. Standalone dispatchers that never call close() behave exactly as before.

New tests: close() promptly waking parked timed and untimed waiters; drain-accepted/reject-new semantics after close() (queued task completes normally, flushed delay throws, post-close awaits throw); a 1-thread pool destroyed with one task mid-execution and another still queued (both complete); and destruction with a 10s delay pending (returns promptly, the delay task completes exceptionally, its waiter does not hang). Beyond the suite runs in the matrix above, the two concurrency suites (dispatcher + thread_pool) were stress-run 20 times each on both toolchains, every iteration under a watchdog timeout so a shutdown hang would fail loudly — no hangs, crashes, or flaky failures — and once each under ThreadSanitizer (g++-12), which reported no data races.

Retained (verified still live)

  • cpp/include/mh/source_location.hpp fallback class: libc++ 14/15 (the clang CI legs) ship no <source_location>, so the fallback still compiles there.
  • __cpp_lib_three_way_comparison guards (io/getopt.hpp, memory/buffer.hpp/.inl, error/expected.hpp:159, and the two mirrored test guards): libc++ 14/15 do not define the macro and std::string_view has no operator<=> there, so the guarded code cannot compile unconditionally on those legs. Where these were combined with __cpp_impl_three_way_comparison, the condition is now just the library check.

C++20 guarantees __cpp_consteval; the constexpr fallback expansion is
dead on conforming compilers. Removes both macro definition sites
(variant.hpp, text/multi_char.hpp) and the leaked macro from the latter.
@pr-minder pr-minder Bot changed the title claude/cpp20-cleanup Remove MH_CONSTEVAL macro, use consteval directly Jul 21, 2026
claude added 2 commits July 21, 2026 16:33
<coroutine> exists on every supported toolchain (libstdc++ since GCC 10,
libc++ since LLVM 14, MSVC since VS 2019 16.8), and libc++ 14 removed
<experimental/coroutine> outright, so the experimental branch was
unreachable. The __INTELLISENSE__ special case is subsumed by the
unconditional include. MH_COROUTINES_SUPPORTED and the detail::coro
alias keep their current values; their removal is a separate change.
Replaces the hand-rolled countl_zero wrapper (zero-check plus
__builtin_clzl/clz or a bit loop) with std::countl_zero from <bit>,
which has identical semantics including returning the digit count for
zero. The <bit> include is now unconditional.
@PazerOP PazerOP changed the title Remove MH_CONSTEVAL macro, use consteval directly Remove dead pre-C++20 compatibility fallbacks Jul 21, 2026
@PazerOP
PazerOP marked this pull request as draft July 21, 2026 16:36
claude added 5 commits July 21, 2026 16:40
Replaces the magic pi/180 and 180/pi literals in math/angles.hpp with
expressions on std::numbers::pi. The factors are still computed in
double precision, and the derived values are bit-identical to the old
literals (checked on both libstdc++ and libc++), so every instantiation
returns exactly the same result as before. Adds a small angles test.
mh::erase duplicated what std::erase (C++20) does for any container it
was used with, and mh::sort was a thin pass-through to std::sort with
no added behavior. Neither had any callers outside their own tests;
those test cases (erase, sort, sort with comparator) are removed with
the wrappers. find_or_add* and contains remain, with their coverage.
__cpp_impl_three_way_comparison and __cpp_concepts are guaranteed in
C++20 mode. Guards that also involved __cpp_lib_three_way_comparison are
reduced to just the library-side check, which some standard libraries
still lack.
Removes the __cpp_lib_is_constant_evaluated and __cpp_lib_bit_cast
gates and their fallbacks; both features are C++20. The bit_cast
fallback in bit_float.hpp was a reinterpret_cast that violated strict
aliasing. The now-purposeless <version> includes in uint128.hpp and
bit_float.hpp are gone too.
The thread_pool destructor detach()ed its workers, so they raced program
teardown: threads either kept running pool work for up to a second after
the pool was gone or leaked blocked on the dispatcher's condition
variable until their current 1s wait window expired.

The dispatcher now has a close() operation: it wakes every thread blocked
in the wait_tasks family, keeps already-runnable work runnable so it can
be drained, immediately flushes pending delay/fd waits (which complete by
throwing on resume, since their condition never arrived), and makes
co_await after close() throw instead of suspending. Destroying handles or
abandoning them is not an option - task objects may still reference the
frames (destroy would be use-after-free) and an abandoned frame leaks and
hangs its waiters forever - so every coroutine reaches a terminal state:
normal completion for accepted work, an exceptional one otherwise.

~thread_pool() closes the dispatcher and join()s each worker. The worker
loop is now purely condition-variable-driven (while (wait_tasks()) run())
instead of polling on a 1s cycle, so both shutdown and delay-task wakeup
are prompt. The constructor also joins already-started threads if
spawning a later one throws. Standalone dispatchers that never call
close() behave exactly as before.
@PazerOP
PazerOP marked this pull request as ready for review July 21, 2026 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant