.*: Enable clang-tidy and fix all warnings
Change-Id: Ia27779c20129aacbe48c623bb1a1235201c961a6
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/experimental/+/276954
Commit-Queue: Ted Pudlik <tpudlik@google.com>
Reviewed-by: Wyatt Hepler <hepler@google.com>
Lint: Lint 🤖 <android-build-ayeaye@system.gserviceaccount.com>
GitOrigin-RevId: c169d25ef507056f45bf6db95f30bc0f6264612e
diff --git a/.bazelrc b/.bazelrc
index 0917b31..c7173a1 100644
--- a/.bazelrc
+++ b/.bazelrc
@@ -61,6 +61,22 @@
common:remote_cache --remote_instance_name=projects/pigweed-rbe-open/instances/default-instance
common:remote_cache --remote_upload_local_results=false
+# clang-tidy configuration
+build:clang-tidy --aspects @bazel_clang_tidy//clang_tidy:clang_tidy.bzl%clang_tidy_aspect
+build:clang-tidy --output_groups=report
+build:clang-tidy --@bazel_clang_tidy//:clang_tidy_config=//:clang_tidy_config
+# Use the clang-tidy executable from Pigweed's toolchain, and include
+# our sysroot headers.
+build:clang-tidy --@bazel_clang_tidy//:clang_tidy_executable=@pigweed//pw_toolchain/host_clang:copy_clang_tidy
+build:clang-tidy --@bazel_clang_tidy//:clang_tidy_additional_deps=@pigweed//pw_toolchain/host_clang:sysroot_root
+# Skip any targets with tags = ["noclangtidy"]. This allows a gradual
+# rollout.
+build:clang-tidy --build_tag_filters=-noclangtidy
+# We need to disable this warning to avoid spurious "#pragma once in main file"
+# warnings for header-only libraries. For another approach, see
+# https://github.com/mongodb-forks/bazel_clang_tidy/pull/2
+build:clang-tidy --copt=-Wno-pragma-once-outside-header
+
# User bazelrc file; see
# https://bazel.build/configure/best-practices#bazelrc-file
#
diff --git a/.clang-tidy b/.clang-tidy
new file mode 100644
index 0000000..3658be4
--- /dev/null
+++ b/.clang-tidy
@@ -0,0 +1,85 @@
+# Copyright 2025 The Pigweed Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may not
+# use this file except in compliance with the License. You may obtain a copy of
+# the License at
+#
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations under
+# the License.
+
+---
+UseColor: true
+
+Checks: >
+ bugprone-argument-comment,
+ bugprone-assert-side-effect,
+ bugprone-bool-pointer-implicit-conversion,
+ bugprone-dangling-handle,
+ bugprone-fold-init-type,
+ bugprone-forwarding-reference-overload,
+ bugprone-forward-declaration-namespace,
+ bugprone-inaccurate-erase,
+ bugprone-macro-repeated-side-effects,
+ bugprone-move-forwarding-reference,
+ bugprone-multiple-statement-macro,
+ bugprone-string-constructor,
+ bugprone-suspicious-memset-usage,
+ bugprone-swapped-arguments,
+ bugprone-undefined-memory-manipulation,
+ bugprone-undelegated-constructor,
+ bugprone-unused-raii,
+ bugprone-use-after-move,
+ clang-diagnostic-*,
+ -clang-analyzer-*,
+ darwin-avoid-spinlock,
+ google-build-explicit-make-pair,
+ google-build-namespaces,
+ google-default-arguments,
+ google-global-names-in-headers,
+ google-readability-function-size,
+ google-readability-namespace-comments,
+ google-runtime-operator,
+ misc-static-assert,
+ misc-unconventional-assign-operator,
+ misc-unused-using-decls,
+ modernize-avoid-bind,
+ modernize-deprecated-ios-base-aliases,
+ modernize-make-shared,
+ modernize-make-unique,
+ modernize-replace-auto-ptr,
+ modernize-replace-disallow-copy-and-assign-macro,
+ modernize-replace-random-shuffle,
+ modernize-shrink-to-fit,
+ modernize-unary-static-assert,
+ modernize-use-bool-literals,
+ modernize-use-equals-delete,
+ modernize-use-noexcept,
+ modernize-use-nullptr,
+ modernize-use-override,
+ modernize-use-transparent-functors,
+ modernize-use-uncaught-exceptions,
+ performance-faster-string-find,
+ performance-for-range-copy,
+ performance-implicit-conversion-in-loop,
+ performance-inefficient-algorithm,
+ performance-inefficient-vector-operation,
+ performance-move-constructor-init,
+ readability-container-size-empty,
+ readability-inconsistent-declaration-parameter-name,
+ readability-misleading-indentation,
+ readability-redundant-control-flow,
+ readability-redundant-smartptr-get,
+ readability-string-compare,
+WarningsAsErrors: >
+ *,
+ -clang-diagnostic-deprecated-declarations,
+ -clang-diagnostic-unused-command-line-argument
+HeaderFilterRegex: '.*'
+ExcludeHeaderFilterRegex: 'external/.*'
+...
+
diff --git a/BUILD.bazel b/BUILD.bazel
index d7c1fab..cda64a7 100644
--- a/BUILD.bazel
+++ b/BUILD.bazel
@@ -12,4 +12,7 @@
# License for the specific language governing permissions and limitations under
# the License.
-# We need a root-level BUILD file, even if it's empty.
+filegroup(
+ name = "clang_tidy_config",
+ srcs = [".clang-tidy"],
+)
diff --git a/MODULE.bazel b/MODULE.bazel
index 0479085..f570557 100644
--- a/MODULE.bazel
+++ b/MODULE.bazel
@@ -25,6 +25,17 @@
path = "third_party/pigweed",
)
+git_repository = use_repo_rule(
+ "@bazel_tools//tools/build_defs/repo:git.bzl",
+ "git_repository",
+)
+
+git_repository(
+ name = "bazel_clang_tidy",
+ commit = "db677011c7363509a288a9fb3bf0a50830bbf791",
+ remote = "https://github.com/erenon/bazel_clang_tidy.git",
+)
+
register_toolchains(
"@pigweed//pw_toolchain/host_clang:host_cc_toolchain_linux",
"@pigweed//pw_toolchain/host_clang:host_cc_toolchain_macos",
diff --git a/pw_display_driver_st7789/display_driver.cc b/pw_display_driver_st7789/display_driver.cc
index 9a467eb..fd9b679 100644
--- a/pw_display_driver_st7789/display_driver.cc
+++ b/pw_display_driver_st7789/display_driver.cc
@@ -194,17 +194,17 @@
return OkStatus();
}
-void DisplayDriverST7789::WriteFramebuffer(Framebuffer frame_buffer,
+void DisplayDriverST7789::WriteFramebuffer(Framebuffer framebuffer,
WriteCallback write_callback) {
- PW_ASSERT(frame_buffer.pixel_format() == PixelFormat::RGB565);
+ PW_ASSERT(framebuffer.pixel_format() == PixelFormat::RGB565);
if (config_.pixel_pusher) {
// Write the pixel data.
- config_.pixel_pusher->WriteFramebuffer(std::move(frame_buffer),
+ config_.pixel_pusher->WriteFramebuffer(std::move(framebuffer),
std::move(write_callback));
return;
}
- // Write the frame_buffer using pw_spi.
+ // Write the framebuffer using pw_spi.
// Let controller know a write is coming.
Status s;
{
@@ -212,7 +212,7 @@
ChipSelectBehavior::kPerWriteRead);
s = WriteCommand(transaction, {ST7789_RAMWR, kEmptyArray});
if (!s.ok()) {
- write_callback(std::move(frame_buffer), s);
+ write_callback(std::move(framebuffer), s);
return;
}
}
@@ -220,11 +220,11 @@
// Write the pixel data.
auto transaction = config_.spi_device_16_bit.StartTransaction(
ChipSelectBehavior::kPerWriteRead);
- const uint16_t* fb_data = static_cast<const uint16_t*>(frame_buffer.data());
- const int num_pixels = frame_buffer.size().width * frame_buffer.size().height;
+ const uint16_t* fb_data = static_cast<const uint16_t*>(framebuffer.data());
+ const int num_pixels = framebuffer.size().width * framebuffer.size().height;
s = transaction.Write(
ConstByteSpan(reinterpret_cast<const byte*>(fb_data), num_pixels));
- write_callback(std::move(frame_buffer), s);
+ write_callback(std::move(framebuffer), s);
}
Status DisplayDriverST7789::WriteRow(span<uint16_t> row_pixels,
diff --git a/pw_graphics/pw_color/public/pw_color/colors_endesga32.h b/pw_graphics/pw_color/public/pw_color/colors_endesga32.h
index 94fd8b3..df9168d 100644
--- a/pw_graphics/pw_color/public/pw_color/colors_endesga32.h
+++ b/pw_graphics/pw_color/public/pw_color/colors_endesga32.h
@@ -15,9 +15,11 @@
#include <cinttypes>
+#include "pw_color/color.h"
+
namespace pw::color {
-const color_rgb565_t colors_endesga32_rgb565[] = {
+inline const color_rgb565_t colors_endesga32_rgb565[] = {
0xba45, // #be4a2f
0xd3a8, // #d77643
0xeeb5, // #ead4aa
@@ -52,7 +54,7 @@
0xc42d, // #c28569
};
-const color_rgba8888_t colors_endesga32_rgba8888[] = {
+inline const color_rgba8888_t colors_endesga32_rgba8888[] = {
0xff2f4abe, // #be4a2f
0xff4376d7, // #d77643
0xffaad4ea, // #ead4aa
diff --git a/pw_graphics/pw_color/public/pw_color/colors_pico8.h b/pw_graphics/pw_color/public/pw_color/colors_pico8.h
index 3a4f5f9..5a4e335 100644
--- a/pw_graphics/pw_color/public/pw_color/colors_pico8.h
+++ b/pw_graphics/pw_color/public/pw_color/colors_pico8.h
@@ -22,7 +22,7 @@
/// @ingroup pw_color
///
/// RGB565 values for the Pico-8 color palette.
-constexpr color_rgb565_t kColorsPico8Rgb565[] = {
+inline constexpr color_rgb565_t kColorsPico8Rgb565[] = {
0x0000, // #000000 0 Black
0x194a, // #1d2b53 1 Dark blue
0x792a, // #7e2553 2 Dark purple
@@ -44,7 +44,7 @@
/// @ingroup pw_color
///
/// RGB8888 values for the Pico-8 color palette.
-constexpr color_rgba8888_t kColorsPico8Rgba8888[] = {
+inline constexpr color_rgba8888_t kColorsPico8Rgba8888[] = {
0xff000000, // #000000 0 Black
0xff532b1d, // #1d2b53 1 Dark blue
0xff53257e, // #7e2553 2 Dark purple
diff --git a/pw_pixel_pusher/BUILD.bazel b/pw_pixel_pusher/BUILD.bazel
index 240e3b4..426b1b9 100644
--- a/pw_pixel_pusher/BUILD.bazel
+++ b/pw_pixel_pusher/BUILD.bazel
@@ -26,6 +26,7 @@
"//pw_graphics/pw_framebuffer",
"//pw_graphics/pw_framebuffer_pool",
"@pigweed//pw_bytes",
+ "@pigweed//pw_function",
"@pigweed//pw_span",
"@pigweed//pw_status",
],
diff --git a/tools/pigweed_experimental_tools/presubmit_checks.py b/tools/pigweed_experimental_tools/presubmit_checks.py
index 846de46..c7a754a 100755
--- a/tools/pigweed_experimental_tools/presubmit_checks.py
+++ b/tools/pigweed_experimental_tools/presubmit_checks.py
@@ -156,6 +156,10 @@
build.bazel(ctx, 'test', '//...')
+def bazel_clang_tidy(ctx: PresubmitContext):
+ build.bazel(ctx, 'build', '--config=clang-tidy', '//...')
+
+
def check_for_git_changes(_: PresubmitContext):
"""Checks that repositories have all changes commited."""
checked_repos = (PIGWEED_ROOT, *REPOS)
@@ -223,6 +227,7 @@
# pico_build,
stm32cube_f4_build,
stm32cube_f7_build,
+ bazel_clang_tidy,
bazel_test,
)