Repository navigation
[WTF] OSAllocatorPOSIX: test BUN_MACOSX with defined() - #568
Conversation
BUN_MACOSX is only #define'd under OS(DARWIN); evaluating the bare identifier in #if on other platforms is well-defined (0) but trips -Wundef, which WTF builds with, on every non-Darwin compile of this file. Use defined(), as the other use in this file already does.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — one-token preprocessor fix with no behavioral change.
What was reviewed:
- Confirmed
BUN_MACOSXis only ever#defined to1inside#if OS(DARWIN)(line 62), sodefined(BUN_MACOSX)and the bare token evaluate identically on every platform. - Matches the existing
#ifdef BUN_MACOSXguard at line 66; no other uses in the file. - No CODEOWNERS entry covers
Source/WTF/wtf/posix/.
Extended reasoning...
Overview
This PR changes a single preprocessor token in Source/WTF/wtf/posix/OSAllocatorPOSIX.cpp, line 197: || BUN_MACOSX becomes || defined(BUN_MACOSX) inside the #if guard that selects the mach-vm-based aligned reservation path in OSAllocator::tryReserveUncommittedAligned. The macro BUN_MACOSX is a file-local sentinel defined to 1 at line 62, itself inside #if OS(DARWIN). On non-Darwin builds the bare identifier was undefined and evaluated to 0 in the preprocessor conditional — behaviorally correct, but a -Wundef trigger.
Security risks
None. This is a preprocessor guard adjustment that does not alter which code path is taken on any platform: on Darwin the macro is defined to 1 so both forms are truthy; elsewhere it is undefined so both forms are falsy. No memory, permission, or allocation semantics change.
Level of scrutiny
Low. The change is a one-token warning-hygiene fix that mirrors the existing #ifdef BUN_MACOSX at line 66 in the same file. There is no scenario in which BUN_MACOSX is defined to 0 (it is only set once, locally, to 1), so defined() cannot diverge from the bare-token evaluation. The exit reason was dry_streak, and no candidate issues were surfaced or ruled out beyond the direct verification above.
Other factors
Checked .github/CODEOWNERS: no pattern matches Source/WTF/wtf/posix/OSAllocatorPOSIX.cpp, so no owner sign-off is required for this path. The PR conversation timeline shows only the initial commit with no outstanding third-party reviews or objections. The root CLAUDE.md contains no review rules that bear on this change.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (1)
Included review availability: 2 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 3 reviews per hour. WalkthroughThe macOS-specific allocator condition now explicitly checks whether ChangesAllocator guard update
Merge Risk: ⚪ Minimal · up to The macOS allocator guard now avoids undefined-macro warnings on non-Darwin builds without a supported indication of changed allocation behavior or remaining merge risk. 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Full details: Description checkResolution Add the bug title and Bugzilla URL, include “Reviewed by NOBODY (OOPS!).” or the applicable review information, provide the required bug explanation, and list the changed file and relevant function or class sections using the repository template.
Warning Git: CodeRabbit could not clone the repository, so clone-backed analysis was skipped and this review may be incomplete. Verify repository clone access, such as SSH credentials, before requesting another full review. If clone access is intentionally unavailable, use Comment |
Preview Builds
|
1 participant
BUN_MACOSXis only#defined underOS(DARWIN)(line 62). Line 197 evaluates the bare identifier in#if PLATFORM(MAC) || USE(APPLE_INTERNAL_SDK) || BUN_MACOSX; on every other OS that is an undefined identifier in a preprocessor conditional — value 0, so behavior is correct, but it triggers-Wundef(part of WTF's warning set) on each non-Darwin compile of this file. The other use in the file (line 66) already uses#ifdef.One-line change:
|| defined(BUN_MACOSX). No functional change on any platform.