Skip to content

bazel: Switch over to clang-22 #30012

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

Merged
StephanDollberg merged 9 commits into from
Apr 8, 2026
Merged

bazel: Switch over to clang-22 #30012

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ce09779
bazel: Switch over to clang-22
StephanDollberg Mar 31, 2026
e007b06
bazel: Remove deprecated header in clang-tidy module
StephanDollberg Mar 31, 2026
29978ac
bazel: Remove old system clang versions
StephanDollberg Mar 31, 2026
efb2171
bazel: Reformat for clang-format-22
StephanDollberg Apr 7, 2026
c0ac70c
git: Ignore clang-format-22 commit
StephanDollberg Mar 31, 2026
e66402a
clang-tidy: clang-tidy-22 fixes
StephanDollberg Mar 31, 2026
043416b
clang-tidy: Pass -Wno-error in clang-tidy
StephanDollberg Apr 1, 2026
c91dd7f
clang-tidy: Adapt Regex
StephanDollberg Apr 1, 2026
ef3fac5
clang-tidy: Fix various C++ errors
StephanDollberg Apr 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
12 changes: 0 additions & 12 deletions .bazelrc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,6 @@ common --@toolchains_llvm//toolchain/config:compiler-rt=False
build --linkopt --unwindlib=libgcc
build --linkopt -stdlib=libc++

common:clang-21 --extra_toolchains=@next_llvm_toolchain//:all

# llvm 22.1.0
common:clang-22 --extra_toolchains=@clang-22_llvm_toolchain//:all

# Use Bash instead of system python for finding the hermetic interpreter
# within Bazel due to the `python3` binary not being present on CI
# (it has `python` version 3 but Bazel uses the `python3` executable).
Expand All @@ -31,13 +26,6 @@ build:system-clang --extra_toolchains=@local_config_cc_toolchains//:all
build:system-clang --action_env=BAZEL_COMPILER=clang
build:system-clang --cxxopt=-std=c++23 --host_cxxopt=-std=c++23
build:system-clang --linkopt -fuse-ld=lld
# use a compiler name that doesn't symlink to ccache
build:system-clang-19 --config=system-clang
build:system-clang-19 --action_env=CC=clang-19 --host_action_env=CC=clang-19
build:system-clang-19 --action_env=CXX=clang++-19 --host_action_env=CXX=clang++-19
build:system-clang-20 --config=system-clang
build:system-clang-20 --action_env=CC=clang-20 --host_action_env=CC=clang-20
build:system-clang-20 --action_env=CXX=clang++-20 --host_action_env=CXX=clang++-20

# Use new proto toolchain resolution
build --incompatible_enable_proto_toolchain_resolution
Expand Down
3 changes: 2 additions & 1 deletion .clang-tidy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
---
Checks: 'redpanda-*,clang-diagnostic-*,clang-analyzer-*,cert-*,cppcoreguidelines-*,hicpp-*,modernize-*,performance-*,misc-*,bugprone-*,-modernize-use-trailing-return-type,-modernize-use-nodiscard,-hicpp-named-parameter,-misc-include-cleaner,-clang-analyzer-optin.core.EnumCastOutOfRange,-cppcoreguidelines-avoid-reference-coroutine-parameters'
WarningsAsErrors: 'redpanda-*,bugprone-use-after-move,bugprone-assert-side-effect,bugprone-assignment-in-if-condition,bugprone-dangling-handle,bugprone-sizeof-container,bugprone-stringview-nullptr,bugprone-unused-return-value,bugprone-suspicious-string-compare,misc-unused-using-decls,misc-unused-alias-decls,modernize-redundant-void-arg,performance-implicit-conversion-in-loop,performance-trivially-destructible,performance-no-automatic-move,performance-move-const-arg,bugprone-unused-local-non-trivial-variable'
HeaderFilterRegex: '^(?!external/.*).*'
HeaderFilterRegex: '.*'
ExcludeHeaderFilterRegex: '(^|/)external/|\.json\.hh$'
FormatStyle: file
CheckOptions:
- key: cert-dcl16-c.NewSuffixes
Expand Down
5 changes: 4 additions & 1 deletion .git-blame-ignore-revs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,7 @@ b079582532df982b81190a34fd0b1f9621ba8213
e3c23b96330a88fba88b6f1bd4f1fa68086e53fb

# ruff check . --select F401 --fix
77f53ca22fb2ed2fe6cfba40ca6923928e8d8988
77f53ca22fb2ed2fe6cfba40ca6923928e8d8988

# upgraded to clang-format 22
efb217101ea58381443956ce0e9da3678857135c
26 changes: 0 additions & 26 deletions MODULE.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,32 +157,6 @@ COMPILE_FLAGS = {

COMPILERS = {
"current": {
"tars": {
"aarch64": {
"build_date": "2025-11-14",
"sha": "f51afb38ca39cc93c6ed065a0c463d3587b975204d6028a63de33153caf3e104",
},
"x86_64": {
"build_date": "2025-11-14",
"sha": "616e3f50b1f03b61ec712f4f28fa67668d60ae3edccea8508065a3f49dbbb24a",
},
},
"version": "20.1.8",
},
"next": {
"tars": {
"aarch64": {
"build_date": "2025-11-25",
"sha": "37257804087a26b0ede7c0349d3f44db7b7439e551fc6c753f7934ead654c1b8",
},
"x86_64": {
"build_date": "2025-11-25",
"sha": "b3aa9cb72c2e46bd1cc3f17f3772b37a424e54f796253961382163eab2f4a9c0",
},
},
"version": "21.1.6",
},
"clang-22": {
"tars": {
"aarch64": {
"build_date": "2026-03-02",
Expand Down
180 changes: 2 additions & 178 deletions MODULE.bazel.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions bazel/clang_tidy/clang_tidy.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ def deps_flags(deps):
])

flags = []

for compilation_context in compilation_contexts:
# add defines
for define in compilation_context.defines.to_list():
Expand Down Expand Up @@ -266,8 +267,7 @@ def _clang_tidy_aspect_impl(target, ctx):
c_flags = toolchain_flags(ctx, ACTION_NAMES.c_compile) + rule_flags + ["-xc"]
cxx_flags = toolchain_flags(ctx, ACTION_NAMES.cpp_compile) + rule_flags + ["-xc++"]

include_headers = "no-clang-tidy-headers" not in ctx.rule.attr.tags
srcs = rule_sources(ctx.rule.attr, include_headers)
srcs = rule_sources(ctx.rule.attr, False)

outputs = [
_run_tidy(
Expand Down
19 changes: 14 additions & 5 deletions bazel/clang_tidy/clang_tidy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ def main():
yaml.safe_dump(config, final_config_file.file)

try:
extra_args = [
"--extra-arg=-Wno-error",
Copy link
Copy Markdown
Contributor

@rockwotj rockwotj Apr 8, 2026

Choose a reason for hiding this comment

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

I have a feeling there is a better way to fix this, but shouldn't block this PR

"--extra-arg=-Wno-macro-redefined",
]

verify_command = [
clang_tidy_bin,
f"--config-file={final_config_file.name}",
Expand All @@ -39,11 +44,15 @@ def main():
]
_ = subprocess.run(verify_command, check=True, capture_output=True, text=True)

run_command = [
clang_tidy_bin,
f"--config-file={final_config_file.name}",
f"--load={plugins_lib}",
] + remaining_args
run_command = (
[
clang_tidy_bin,
f"--config-file={final_config_file.name}",
f"--load={plugins_lib}",
]
+ extra_args
+ remaining_args
)

_ = subprocess.run(run_command, check=True, capture_output=True, text=True)

Expand Down
1 change: 0 additions & 1 deletion bazel/clang_tidy/plugins/redpanda_tidy_module.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#include "redpanda_noop_check.h"

#include <clang-tidy/ClangTidyModule.h>
#include <clang-tidy/ClangTidyModuleRegistry.h>

namespace clang::tidy {

Expand Down
28 changes: 16 additions & 12 deletions src/transform-sdk/cpp/examples/schema_registry.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,12 @@ constexpr std::string_view schema_def = R"({

int main() {
sr_client = sr::new_client();
if (auto res = sr_client->create_schema(
"avro-value",
sr::schema::new_avro(
std::string{schema_def.data(), schema_def.size()}));
!res.has_value()) {
if (
auto res = sr_client->create_schema(
Copy link
Copy Markdown
Member Author

@StephanDollberg StephanDollberg Mar 31, 2026

Choose a reason for hiding this comment

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

This is just odd, isn't it. Let me see whether one can disable.

Copy link
Copy Markdown
Member

@dotnwat dotnwat Mar 31, 2026

Choose a reason for hiding this comment

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

it seems kinda unfortunate that upgrading clang-format causes so much churn. are they "bugs" or just defaults changing and we can tweek our config? either way, no big deal.

Copy link
Copy Markdown
Member Author

@StephanDollberg StephanDollberg Mar 31, 2026

Choose a reason for hiding this comment

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

They are config. But it seems for this specifically it was inconsistent before so you get diffs either way. I don't care which way we go.

I'll leave default unless I hear otherwise.

Copy link
Copy Markdown
Contributor

@nvartolomei nvartolomei Mar 31, 2026

Choose a reason for hiding this comment

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

i find previous style

one cherry picked example:

  • old style it is very easy to visually scan where if expression ends and where the if block starts
  • new style: good luck since all lines are aligned to the same depth so you have to scan line ends rather than line starts
Screenshot 2026-03-31 at 16 49 40 more readable

Copy link
Copy Markdown
Member

@dotnwat dotnwat Mar 31, 2026

Choose a reason for hiding this comment

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

seems for this specifically it was inconsistent before

got it so like formatting "bugs" so yeh it makes sense then to accept all the churn?

Copy link
Copy Markdown
Contributor

@nvartolomei nvartolomei Mar 31, 2026

Choose a reason for hiding this comment

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

btw our .clang-format files has AlignAfterOpenBracket: AlwaysBreak but it seems it applied only to function parameters in prev release
Screenshot 2026-03-31 at 16 54 19

Copy link
Copy Markdown
Member Author

@StephanDollberg StephanDollberg Mar 31, 2026

Choose a reason for hiding this comment

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

there are more BreakAfterOpen* options to consider

Yes but they result in tens of thousands of line changes.

Copy link
Copy Markdown
Contributor

@rockwotj rockwotj Mar 31, 2026

Choose a reason for hiding this comment

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

Yes but they result in tens of thousands of line changes.

I mean if we can make this less painful in the future I think it's worth it.

Copy link
Copy Markdown
Member Author

@StephanDollberg StephanDollberg Mar 31, 2026

Choose a reason for hiding this comment

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

I don't really see what's painful about it right now but I also don't really see how we can avoid it.

The "problem" is that they are adding new flags for underspecified behaviour I think. So you can add explicit values for everything right now that still won't prevent future flags from causing changes.

Copy link
Copy Markdown
Member

@travisdowns travisdowns Apr 2, 2026

Choose a reason for hiding this comment

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

Agree with @StephanDollberg here, it looks like they switching from a heuristic (penalty) based breaking for these examples to more binary options which will always break after ( (for example) when it can't fit on one line. So we cannot get to zero churn.

I mean if we can make this less painful in the future I think it's worth it.

I don't think that's the case: we are just choosing between two different styles now, no reason to believe the other one will have less churn in the future. So I would vote for what we have now as the low churn option unless there is a very concrete reason we like the other way better.

"avro-value",
sr::schema::new_avro(
std::string{schema_def.data(), schema_def.size()}));
!res.has_value()) {
return 0;
}
redpanda::on_record_written(do_transform);
Expand All @@ -67,8 +68,9 @@ do_transform(redpanda::write_event event, redpanda::record_writer* writer) {
return id_e.error();
}
std::optional<sr::schema> raw_schema;
if (auto rs_e = sr_client->lookup_schema_by_id(id.value());
rs_e.has_value()) {
if (
auto rs_e = sr_client->lookup_schema_by_id(id.value());
rs_e.has_value()) {
raw_schema.emplace(std::move(rs_e).value());
} else {
return rs_e.error();
Expand All @@ -77,17 +79,19 @@ do_transform(redpanda::write_event event, redpanda::record_writer* writer) {
// auto schema = avro::schema::parse_str(raw_schema.value().schema());

std::optional<sr::subject_schema> latest_schema;
if (auto latest_e = sr_client->lookup_latest_schema("avro-value");
latest_e.has_value()) {
if (
auto latest_e = sr_client->lookup_latest_schema("avro-value");
latest_e.has_value()) {
latest_schema.emplace(std::move(latest_e).value());
} else {
return latest_e.error();
}

std::optional<sr::subject_schema> latest_direct;
if (auto latest_e = sr_client->lookup_schema_by_version(
"avro-value", latest_schema->version);
latest_e.has_value()) {
if (
auto latest_e = sr_client->lookup_schema_by_version(
"avro-value", latest_schema->version);
latest_e.has_value()) {
latest_direct.emplace(std::move(latest_e).value());
} else {
return latest_e.error();
Expand Down
Loading
Loading