Skip to content

Clang21 build fix #10998

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+5 −3
Open

Clang21 build fix #10998

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
272967a
UCS/BITMAP/TEST: Fix build with >= Clang 21
listout Nov 9, 2025
2c07e61
TEST/IODEMO: Use std::shuffle when available
listout Nov 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion config/m4/compiler.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# Initialize CFLAGS
#
BASE_CFLAGS="-g -Wall -Werror"
BASE_CFLAGS="-g -Wall"
Copy link

@coderabbitai coderabbitai bot Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ 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:

  1. 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;}]])],
                               [],
                               [])

  2. 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.

Copy link
Contributor

@iyastreb iyastreb Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Author

@listout listout Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@yosefe yosefe Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


# Prevent libtool from suppression of warnings
LT_CFLAGS="-no-suppress"
Expand Down
6 changes: 4 additions & 2 deletions test/apps/iodemo/io_demo.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <csignal>
#include <cerrno>
#include <vector>
#include <random>
#include <map>
#include <queue>
#include <algorithm>
Expand Down Expand Up @@ -2999,8 +3000,9 @@ static int do_client(options_t& test_opts)
LOG << "random seed: " << test_opts.random_seed;

// randomize servers to optimize startup
std::random_shuffle(test_opts.servers.begin(), test_opts.servers.end(),
IoDemoRandom::urand<size_t>);
std::random_device rd;
std::mt19937 rng(rd());
std::shuffle(test_opts.servers.begin(), test_opts.servers.end(), rng);

UcxLog vlog(LOG_PREFIX, test_opts.verbose);
vlog << "List of servers:";
Expand Down
Loading