MemPoolTypes: portable thread-id cast for logging (pointer pthread_t)#3040
Open
tomjn wants to merge 1 commit into
Open
MemPoolTypes: portable thread-id cast for logging (pointer pthread_t)#3040tomjn wants to merge 1 commit into
tomjn wants to merge 1 commit into
Conversation
Collaborator
|
Looks ok but I think the |
tomjn
force-pushed
the
fix/mempool-pointer-threadid
branch
from
2e7948c to
f143b15
Compare
Collaborator
|
Looks more ok than previous, but this kind of helper sounds like it belongs in the threading class (GetAsU32 or something) |
StablePosAllocator::Allocate logs the current thread id. NativeThreadId is
pthread_t, an integer on Linux and a pointer (_opaque_pthread_t*) on macOS,
so static_cast<uint32_t>(GetCurrentThreadId()) failed to compile under GCC
on macOS:
MemPoolTypes.h: error: invalid 'static_cast' from type
'Threading::NativeThreadId' {aka '_opaque_pthread_t*'} to type 'uint32_t'
Add the conversion next to NativeThreadId in Threading: a small templated
ThreadIdAsU32() (so `if constexpr` actually selects a valid cast per target
-- it would not be discarded in a non-template function), plus an inline
GetCurrentThreadIdAsU32() wrapper. MemPoolTypes now calls that instead of
casting inline. reinterpret_cast for the pointer case, the original
static_cast otherwise; no codegen change on Linux/Windows.
Assisted by Claude Code; verified the cast compiles for pointer and
integer thread-id types on macOS GCC.
tomjn
force-pushed
the
fix/mempool-pointer-threadid
branch
from
f143b15 to
69c26de
Compare
sprunk
approved these changes
Jun 21, 2026
sprunk
left a comment
sprunk
left a comment
Collaborator
There was a problem hiding this comment.
Looks ok. I feel like it could be cleaner still but let's not bother for something that is ultimately just printing a log.
sprunk
added
refactor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
StablePosAllocator::Allocatelogs the current thread id:NativeThreadIdispthread_t— an integer on Linux but a pointer (_opaque_pthread_t*) on macOS. The directstatic_casttouint32_tis therefore invalid on macOS and fails to compile under GCC:This casts via
uintptr_tfirst (valid for both pointer and integer thread-id representations), then narrows touint32_tfor the log line.Notes
reportWorkisfalse), but the template body still has to compile, so the cast must be valid on every platform.uintptr_t(integer)is just a widening cast there.unitsynctarget on macOS with GCC.