Clang21 build fix #10998

listout · 2025-11-09T07:20:29Z

What?

Currently UCX is failing to build with newer compiler (for example Clang 21). This PR fixes that.

Why?

This is cause due to Clang enabling default-const-init-var-unsafe by default,
and -Werror makes it treat as an warning. This is better fixed by upstream
developer so -Werror commit mostly a RFC from upstream.

Also std::random_shuffle has been removed since C++17 (deprecated in C++14). PR would make use of recommended std::shuffle.

How?

Not needed in this case I guess :)

Summary by CodeRabbit

Refactor
- Modernized test randomization to use a current C++ randomization approach for more robust shuffling.
Chores
- Relaxed compiler flags so warnings no longer fail the build (no longer treated as errors).

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2025-11-09T07:20:41Z

Walkthrough

Replaces deprecated C++ std::random_shuffle with std::shuffle using <random> and an mt19937 RNG; removes -Werror from BASE_CFLAGS in build configuration.

Changes

Cohort / File(s)	Summary
C++ modernization `test/apps/iodemo/io_demo.cc`	Adds `<random>` header and replaces `std::random_shuffle` with `std::shuffle`, introducing `std::random_device` and `std::mt19937` to shuffle `test_opts.servers`.
Compiler configuration `config/m4/compiler.m4`	Updates `BASE_CFLAGS` from `"-g -Wall -Werror"` to `"-g -Wall"`, removing the `-Werror` flag.

Sequence Diagram(s)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Pay attention to seeding choice for mt19937 (uses std::random_device) and ensure consistent build/test behavior.
Confirm no remaining uses of deprecated std::random_shuffle elsewhere.

Poem

🐰 I shuffled the servers with fresh RNG light,

Gone is the old call that kept warnings in sight.
Flags loosened, builds breathe, hop into the day —
Code sniffs the meadow and merrily plays.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Clang21 build fix' accurately reflects the main purpose of the PR: addressing build failures with Clang 21 by replacing deprecated std::random_shuffle and removing the -Werror flag.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e29e5f5 and 56456ab.

📒 Files selected for processing (2)

src/ucs/datastruct/static_bitmap.h (2 hunks)
test/apps/iodemo/io_demo.cc (3 hunks)

🔇 Additional comments (4)

test/apps/iodemo/io_demo.cc (2)

23-23: LGTM!

The <random> header is correctly added to support std::random_device and std::mt19937 used in the C++17+ code path.

3003-3013: Good modernization for C++17 compatibility.

The conditional compilation correctly addresses the removal of std::random_shuffle in C++17. The implementation properly uses std::shuffle with a proper random number generator for C++17+ while maintaining backward compatibility.

src/ucs/datastruct/static_bitmap.h (2)

229-237: LGTM!

Zero-initializing _r_##_uid properly addresses Clang 21's -Wdefault-const-init-var-unsafe warning. The variable is immediately populated by the bitmap operation, so this initialization has no runtime impact beyond satisfying the compiler check.

254-265: LGTM!

Zero-initializing _r_##_uid properly addresses Clang 21's -Wdefault-const-init-var-unsafe warning. The variable is immediately populated by the bitmap binary operation, so this initialization has no runtime impact beyond satisfying the compiler check.

test/apps/iodemo/io_demo.cc

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6f11c87 and 9ac4c2c.

📒 Files selected for processing (2)

config/m4/compiler.m4 (1 hunks)
test/apps/iodemo/io_demo.cc (2 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

test/apps/iodemo/io_demo.cc

coderabbitai · 2025-11-20T14:07:13Z

config/m4/compiler.m4

 # Initialize CFLAGS
 #
-BASE_CFLAGS="-g -Wall -Werror"
+BASE_CFLAGS="-g -Wall"


⚠️ Potential issue | 🟠 Major

Consider a more targeted approach than removing -Werror entirely.

Removing -Werror from BASE_CFLAGS disables error-on-warning for the entire build, which reduces code quality enforcement. This means all warnings—not just the Clang 21 -Wdefault-const-init-var-unsafe issue—will be silently ignored, potentially masking future bugs.

Recommended alternatives:

Selectively disable the problematic warning (preferred):

-BASE_CFLAGS="-g -Wall" +BASE_CFLAGS="-g -Wall -Werror"

Then add after line 302:

# # Disable Clang 21 default-const-init-var-unsafe warning # ADD_COMPILER_FLAG_IF_SUPPORTED([-Wno-error=default-const-init-var-unsafe], [-Wno-error=default-const-init-var-unsafe], [AC_LANG_SOURCE([[int main(int argc, char** argv){return 0;}]])], [], [])

Version-specific handling (if the warning only affects Clang 21+):
Detect the compiler version and conditionally apply flags only when needed.

The file already provides ADD_COMPILER_FLAG_IF_SUPPORTED and CHECK_COMPILER_FLAG macros (lines 252, 226) specifically for this purpose. Using -Wno-error=default-const-init-var-unsafe would downgrade only this specific warning to non-fatal while preserving -Werror for all other warnings.

🤖 Prompt for AI Agents

In config/m4/compiler.m4 around line 13, do not remove -Werror from BASE_CFLAGS; instead retain it and add a targeted suppression for the Clang 21 warning after line 302 by calling ADD_COMPILER_FLAG_IF_SUPPORTED to add -Wno-error=default-const-init-var-unsafe when supported (use the same AC_LANG_SOURCE probe pattern used elsewhere). Alternatively, if you prefer compiler-aware handling, detect Clang and its version and conditionally append that -Wno-error flag only for affected Clang versions so -Werror remains active for all other warnings.

First caught on Gentoo LLVM with Clang 21.1.5 and LLVM 21.1.5 Build with fail with the following error message core/ucp_context.c:2673:47: error: default initialization of an object of type 'typeof (*tl_bitmap_super)' (aka 'const ucp_tl_bitmap_t') leaves the object uninitialized [-Werror,-Wdefault-const-init-var-unsafe] /var/tmp/portage/sys-cluster/ucx-1.19.0/work/ucx-1.19.0/src/ucs/datastruct/static_bitmap.h:250:5: note: expanded from macro 'UCS_STATIC_BITMAP_NOT' 250 | UCS_STATIC_BITMAP_UNARY_OP(_bitmap, not, UCS_PP_UNIQUE_ID) | ^ /var/tmp/portage/sys-cluster/ucx-1.19.0/work/ucx-1.19.0/src/ucs/datastruct/static_bitmap.h:239:5: note: expanded from macro 'UCS_STATIC_BITMAP_UNARY_OP' 239 | _UCS_STATIC_BITMAP_UNARY_OP(_bitmap, _op_name, _uid) | ^ /var/tmp/portage/sys-cluster/ucx-1.19.0/work/ucx-1.19.0/src/ucs/datastruct/static_bitmap.h:232:29: note: expanded from macro '_UCS_STATIC_BITMAP_UNARY_OP' 232 | ucs_typeof(_bitmap) _r_##_uid; \ | ^ <scratch space>:54:1: note: expanded from here 54 | _r_0 | ^ core/ucp_context.c:2672:25: error: default initialization of an object of type 'typeof (*tl_bitmap)' (aka 'const ucp_tl_bitmap_t') leaves the object uninitialized [-Werror,-Wdefault-const-init-var-unsafe] 2672 | ucp_tl_bitmap_t b = UCS_STATIC_BITMAP_AND(*tl_bitmap, | ^ This is cause due to Clang enabling default-const-init-var-unsafe by default, and -Werror makes it treat as an warning. This is better fixed by upstream developer so this commit mostly a RFC from upstream. Signed-off-by: Brahmajit Das <[email protected]>

random_shuffle was deprecated in C++14 and completely removed in C++17. With newer compilers like Clang 21, the build fails. This commit should preserve older behavior and use std::shuffle when available. Signed-off-by: Brahmajit Das <[email protected]>

iyastreb · 2025-11-21T06:55:03Z

config/m4/compiler.m4

 # Initialize CFLAGS
 #
-BASE_CFLAGS="-g -Wall -Werror"
+BASE_CFLAGS="-g -Wall"


Removing -Werror seems too broad.
How many errors do we see with default-const-init-var-unsafe? Maybe we can change the code on those places to make it silent?

So that was the idea before, but thought -Werror should be enabled in dev builds only. You can see what I initially did to suppress the build failure with clang here:

https://github.com/openucx/ucx/compare/6f11c87850dd7b401e975e56e915a6a771478351

if we can fix the code to avoid the warning - this is preferred.
if not possible to fix the code - we can disable a specific warning, but IMO we should keep -Werror

coderabbitai bot reviewed Nov 9, 2025

View reviewed changes

test/apps/iodemo/io_demo.cc Outdated Show resolved Hide resolved

listout force-pushed the clang21-build-fix branch 2 times, most recently from 5d2109e to a950e7a Compare November 9, 2025 07:29

listout force-pushed the clang21-build-fix branch 2 times, most recently from 6f11c87 to 9ac4c2c Compare November 20, 2025 14:05

coderabbitai bot reviewed Nov 20, 2025

View reviewed changes

listout added 2 commits November 20, 2025 19:37

listout force-pushed the clang21-build-fix branch from 9ac4c2c to 2c07e61 Compare November 20, 2025 14:07

iyastreb reviewed Nov 21, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Clang21 build fix #10998

Clang21 build fix #10998

Uh oh!

listout commented Nov 9, 2025 •

edited

Loading

Uh oh!

coderabbitai bot commented Nov 9, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Uh oh!

coderabbitai bot Nov 20, 2025

Uh oh!

iyastreb Nov 21, 2025

Uh oh!

listout Nov 21, 2025

Uh oh!

yosefe Nov 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Clang21 build fix #10998

Are you sure you want to change the base?

Clang21 build fix #10998

Uh oh!

Conversation

listout commented Nov 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What?

Why?

How?

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Nov 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

iyastreb Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

listout Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

yosefe Nov 23, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

listout commented Nov 9, 2025 •

edited

Loading

coderabbitai bot commented Nov 9, 2025 •

edited

Loading