From ab53d51c6fcf2b80ba73973cc5d5ab524192825d Mon Sep 17 00:00:00 2001 From: Tyler Veness Date: Thu, 25 Sep 2025 21:28:04 -0700 Subject: [PATCH] Fix or suppress clang-tidy warnings (#8254) --- .clang-tidy | 2 ++ benchmark/src/main/native/cpp/Main.cpp | 2 ++ cscore/examples/usbcvstream/usbcvstream.cpp | 1 - cscore/src/main/native/cpp/cscore_c.cpp | 4 ++-- .../src/main/native/windows/UsbCameraImpl.cpp | 3 ++- .../src/lib/native/cpp/other/PIDController.cpp | 7 ++----- .../native/cpp/other/ProfiledPIDController.cpp | 7 ++----- ntcore/.clang-tidy | 1 + ntcore/manualTests/native/client.cpp | 17 +++++++---------- ntcore/manualTests/native/server.cpp | 13 +++++-------- ntcore/src/main/native/cpp/Value_internal.h | 1 + ntcore/src/main/native/cpp/server/Functions.h | 2 ++ .../include/networktables/ProtobufTopic.h | 4 +--- ntcore/src/test/native/cpp/StructTest.cpp | 2 +- processstarter/src/main/native/linux/main.cpp | 5 +++-- .../src/test/native/cpp/DSCommPacketTest.cpp | 6 ++++-- wpical/.styleguide | 1 - wpical/src/main/native/cpp/WPIcal.cpp | 5 +++-- wpical/src/main/native/cpp/tagpose.cpp | 3 ++- wpical/src/main/native/include/fieldmap.h | 3 ++- wpical/src/main/native/include/fmap.h | 6 +++--- .../include/frc2/command/sysid/SysIdRoutine.h | 2 +- wpilibc/src/main/native/cpp/ADIS16470_IMU.cpp | 6 +++--- wpilibc/src/test/native/cpp/AlertTest.cpp | 2 +- .../cpp/commands/TeleopArcadeDrive.cpp | 6 ++++-- .../src/main/native/cpp/DIOLoopTest.cpp | 3 ++- wpiutil/src/main/native/include/wpi/jni_util.h | 1 + wpiutil/src/test/native/cpp/StringMapTest.cpp | 3 +++ 28 files changed, 62 insertions(+), 56 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index e8fe57b952..33d48c6044 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -45,6 +45,8 @@ Checks: -clang-diagnostic-#warnings, -clang-diagnostic-pedantic, clang-analyzer-*, + -clang-analyzer-optin.cplusplus.UninitializedObject, + -clang-analyzer-security.FloatLoopCounter, cppcoreguidelines-slicing, google-build-namespaces, google-explicit-constructor, diff --git a/benchmark/src/main/native/cpp/Main.cpp b/benchmark/src/main/native/cpp/Main.cpp index e332345a88..be193dc554 100644 --- a/benchmark/src/main/native/cpp/Main.cpp +++ b/benchmark/src/main/native/cpp/Main.cpp @@ -22,6 +22,7 @@ void BM_Transform(benchmark::State& state) { auto transform = pose2 - pose1; return units::math::hypot(transform.X(), transform.Y()).value(); }}; + // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores) for (auto _ : state) { traveler.Solve(poses, iterations); } @@ -33,6 +34,7 @@ void BM_Twist(benchmark::State& state) { auto twist = pose1.Log(pose2); return units::math::hypot(twist.dx, twist.dy).value(); }}; + // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores) for (auto _ : state) { traveler.Solve(poses, iterations); } diff --git a/cscore/examples/usbcvstream/usbcvstream.cpp b/cscore/examples/usbcvstream/usbcvstream.cpp index ae37d3726c..7a438e2c86 100644 --- a/cscore/examples/usbcvstream/usbcvstream.cpp +++ b/cscore/examples/usbcvstream/usbcvstream.cpp @@ -5,7 +5,6 @@ #include #include -#include "cscore.h" #include "cscore_cv.h" int main() { diff --git a/cscore/src/main/native/cpp/cscore_c.cpp b/cscore/src/main/native/cpp/cscore_c.cpp index b55d697995..9f4a2c38c2 100644 --- a/cscore/src/main/native/cpp/cscore_c.cpp +++ b/cscore/src/main/native/cpp/cscore_c.cpp @@ -44,7 +44,7 @@ static O* ConvertToC(std::vector&& in, int* count) { // retain vector at end of returned array alignas(T) unsigned char buf[sizeof(T)]; new (buf) T(std::move(in)); - std::memcpy(out + size * sizeof(O), buf, sizeof(T)); + std::memcpy(out + size, buf, sizeof(T)); return out; } @@ -392,7 +392,7 @@ void CS_FreeEvents(CS_Event* arr, int count) { // destroy vector saved at end of array using T = std::vector; alignas(T) unsigned char buf[sizeof(T)]; - std::memcpy(buf, arr + count * sizeof(CS_Event), sizeof(T)); + std::memcpy(buf, arr + count, sizeof(T)); reinterpret_cast(buf)->~T(); std::free(arr); diff --git a/cscore/src/main/native/windows/UsbCameraImpl.cpp b/cscore/src/main/native/windows/UsbCameraImpl.cpp index 1f180a142c..516e33baaa 100644 --- a/cscore/src/main/native/windows/UsbCameraImpl.cpp +++ b/cscore/src/main/native/windows/UsbCameraImpl.cpp @@ -707,8 +707,9 @@ void UsbCameraImpl::DeviceCacheProperty( } NotifyPropertyCreated(*rawIndex, *rawPropPtr); - if (perPropPtr && perIndex) + if (perPropPtr && perIndex) { NotifyPropertyCreated(*perIndex, *perPropPtr); + } } CS_StatusValue UsbCameraImpl::DeviceProcessCommand( diff --git a/glass/src/lib/native/cpp/other/PIDController.cpp b/glass/src/lib/native/cpp/other/PIDController.cpp index 8aa5e1f99d..bfcac82f5a 100644 --- a/glass/src/lib/native/cpp/other/PIDController.cpp +++ b/glass/src/lib/native/cpp/other/PIDController.cpp @@ -4,11 +4,8 @@ #include "glass/other/PIDController.h" -#include - #include -#include "glass/Context.h" #include "glass/DataSource.h" using namespace glass; @@ -34,8 +31,8 @@ void glass::DisplayPIDController(PIDControllerModel* m) { [flag](const char* name, double* v, std::function callback) { ImGui::SetNextItemWidth(ImGui::GetFontSize() * 4); - if (ImGui::InputScalar(name, ImGuiDataType_Double, v, NULL, NULL, - "%.3f", flag)) { + if (ImGui::InputScalar(name, ImGuiDataType_Double, v, nullptr, + nullptr, "%.3f", flag)) { callback(*v); } }; diff --git a/glass/src/lib/native/cpp/other/ProfiledPIDController.cpp b/glass/src/lib/native/cpp/other/ProfiledPIDController.cpp index f9d37b9001..49e414b85d 100644 --- a/glass/src/lib/native/cpp/other/ProfiledPIDController.cpp +++ b/glass/src/lib/native/cpp/other/ProfiledPIDController.cpp @@ -4,11 +4,8 @@ #include "glass/other/ProfiledPIDController.h" -#include - #include -#include "glass/Context.h" #include "glass/DataSource.h" using namespace glass; @@ -34,8 +31,8 @@ void glass::DisplayProfiledPIDController(ProfiledPIDControllerModel* m) { [flag](const char* name, double* v, std::function callback) { ImGui::SetNextItemWidth(ImGui::GetFontSize() * 4); - if (ImGui::InputScalar(name, ImGuiDataType_Double, v, NULL, NULL, - "%.3f", flag)) { + if (ImGui::InputScalar(name, ImGuiDataType_Double, v, nullptr, + nullptr, "%.3f", flag)) { callback(*v); } }; diff --git a/ntcore/.clang-tidy b/ntcore/.clang-tidy index 41c15bba7d..59a5681cb7 100644 --- a/ntcore/.clang-tidy +++ b/ntcore/.clang-tidy @@ -44,6 +44,7 @@ Checks: -clang-diagnostic-#warnings, -clang-diagnostic-pedantic, clang-analyzer-*, + -clang-analyzer-optin.cplusplus.UninitializedObject, cppcoreguidelines-slicing, google-build-namespaces, google-explicit-constructor, diff --git a/ntcore/manualTests/native/client.cpp b/ntcore/manualTests/native/client.cpp index f9466738e6..23bd40d3f9 100644 --- a/ntcore/manualTests/native/client.cpp +++ b/ntcore/manualTests/native/client.cpp @@ -11,20 +11,17 @@ int main() { auto inst = nt::GetDefaultInstance(); - nt::AddLogger( - inst, - [](const nt::LogMessage& msg) { - std::fputs(msg.message.c_str(), stderr); - std::fputc('\n', stderr); - }, - 0, UINT_MAX); - nt::StartClient(inst, "127.0.0.1", 10000); + nt::AddLogger(inst, 0, UINT_MAX, [](const nt::Event& event) { + std::fputs(event.GetLogMessage()->message.c_str(), stderr); + std::fputc('\n', stderr); + }); + nt::StartClient4(inst, "127.0.0.1"); std::this_thread::sleep_for(std::chrono::seconds(2)); auto foo = nt::GetEntry(inst, "/foo"); auto foo_val = nt::GetEntryValue(foo); - if (foo_val && foo_val->IsDouble()) { - std::printf("Got foo: %g\n", foo_val->GetDouble()); + if (foo_val && foo_val.IsDouble()) { + std::printf("Got foo: %g\n", foo_val.GetDouble()); } auto bar = nt::GetEntry(inst, "/bar"); diff --git a/ntcore/manualTests/native/server.cpp b/ntcore/manualTests/native/server.cpp index 08259602bc..5ef58d161f 100644 --- a/ntcore/manualTests/native/server.cpp +++ b/ntcore/manualTests/native/server.cpp @@ -11,14 +11,11 @@ int main() { auto inst = nt::GetDefaultInstance(); - nt::AddLogger( - inst, - [](const nt::LogMessage& msg) { - std::fputs(msg.message.c_str(), stderr); - std::fputc('\n', stderr); - }, - 0, UINT_MAX); - nt::StartServer(inst, "persistent.ini", "", 10000); + nt::AddLogger(inst, 0, UINT_MAX, [](const nt::Event& event) { + std::fputs(event.GetLogMessage()->message.c_str(), stderr); + std::fputc('\n', stderr); + }); + nt::StartServer(inst, "persistent.ini", "", 10000, 10001); std::this_thread::sleep_for(std::chrono::seconds(1)); auto foo = nt::GetEntry(inst, "/foo"); diff --git a/ntcore/src/main/native/cpp/Value_internal.h b/ntcore/src/main/native/cpp/Value_internal.h index fd633f1300..768b9034ce 100644 --- a/ntcore/src/main/native/cpp/Value_internal.h +++ b/ntcore/src/main/native/cpp/Value_internal.h @@ -177,6 +177,7 @@ static_assert(ValidType); static_assert(ValidType>); template +// NOLINTNEXTLINE(google-readability-casting) constexpr bool IsNTType = TypeInfo>::kType == type; static_assert(IsNTType); diff --git a/ntcore/src/main/native/cpp/server/Functions.h b/ntcore/src/main/native/cpp/server/Functions.h index a1b4ac65db..89b7756863 100644 --- a/ntcore/src/main/native/cpp/server/Functions.h +++ b/ntcore/src/main/native/cpp/server/Functions.h @@ -4,6 +4,8 @@ #pragma once +#include + #include #include diff --git a/ntcore/src/main/native/include/networktables/ProtobufTopic.h b/ntcore/src/main/native/include/networktables/ProtobufTopic.h index 7a56759f67..43baa87927 100644 --- a/ntcore/src/main/native/include/networktables/ProtobufTopic.h +++ b/ntcore/src/main/native/include/networktables/ProtobufTopic.h @@ -7,9 +7,7 @@ #include #include -#include #include -#include #include #include @@ -207,7 +205,7 @@ class ProtobufPublisher : public Publisher { ProtobufPublisher(ProtobufPublisher&& rhs) : Publisher{std::move(rhs)}, m_msg{std::move(rhs.m_msg)}, - m_schemaPublished{rhs.m_schemaPublished} {} + m_schemaPublished{rhs.m_schemaPublished.load()} {} ProtobufPublisher& operator=(ProtobufPublisher&& rhs) { Publisher::operator=(std::move(rhs)); diff --git a/ntcore/src/test/native/cpp/StructTest.cpp b/ntcore/src/test/native/cpp/StructTest.cpp index 6842edb480..3c66482822 100644 --- a/ntcore/src/test/native/cpp/StructTest.cpp +++ b/ntcore/src/test/native/cpp/StructTest.cpp @@ -157,7 +157,7 @@ namespace nt { class StructTest : public ::testing::Test { public: StructTest() { inst = nt::NetworkTableInstance::Create(); } - ~StructTest() { nt::NetworkTableInstance::Destroy(inst); } + ~StructTest() override { nt::NetworkTableInstance::Destroy(inst); } nt::NetworkTableInstance inst; }; diff --git a/processstarter/src/main/native/linux/main.cpp b/processstarter/src/main/native/linux/main.cpp index 1d2847967e..0ee1bc6b4e 100644 --- a/processstarter/src/main/native/linux/main.cpp +++ b/processstarter/src/main/native/linux/main.cpp @@ -72,8 +72,9 @@ int StartJavaTool(std::filesystem::path& exePath) { std::string data = jarPath; std::string jarArg = "-jar"; - char* const arguments[] = {Java.generic_string().data(), jarArg.data(), - data.data(), nullptr}; + auto javaGenericStr = Java.generic_string(); + char* const arguments[] = {javaGenericStr.data(), jarArg.data(), data.data(), + nullptr}; int status = posix_spawn(&pid, Java.c_str(), nullptr, nullptr, arguments, environ); diff --git a/simulation/halsim_ds_socket/src/test/native/cpp/DSCommPacketTest.cpp b/simulation/halsim_ds_socket/src/test/native/cpp/DSCommPacketTest.cpp index 1c7bfc5dc9..1e2e48ed76 100644 --- a/simulation/halsim_ds_socket/src/test/native/cpp/DSCommPacketTest.cpp +++ b/simulation/halsim_ds_socket/src/test/native/cpp/DSCommPacketTest.cpp @@ -69,10 +69,12 @@ TEST_F(DSCommPacketTest, MainJoystickTag) { std::array _buttons{{0, 1, 0, 0, 1, 1, 0, 1, 0, 1, 0, 1}}; std::array _button_bytes{{0, 0}}; - for (int btn = 0; btn < 8; btn++) + for (int btn = 0; btn < 8; btn++) { _button_bytes[1] |= _buttons[btn] << btn; - for (int btn = 8; btn < 12; btn++) + } + for (int btn = 8; btn < 12; btn++) { _button_bytes[0] |= _buttons[btn] << (btn - 8); + } // 5 for base, 4 joystick, 12 buttons (2 bytes) 3 povs uint8_t arr[5 + 4 + 2 + 6] = {// Size, Tag diff --git a/wpical/.styleguide b/wpical/.styleguide index 19766ff2c6..e2401dc33d 100644 --- a/wpical/.styleguide +++ b/wpical/.styleguide @@ -31,7 +31,6 @@ includeOtherLibs { ^mrcal_wrapper\.h$ ^opencv2\.h$ ^portable-file-dialogs\.h$ - ^tagpose\.h$ ^wpi/ ^wpigui } diff --git a/wpical/src/main/native/cpp/WPIcal.cpp b/wpical/src/main/native/cpp/WPIcal.cpp index f73105a50d..4f26625bba 100644 --- a/wpical/src/main/native/cpp/WPIcal.cpp +++ b/wpical/src/main/native/cpp/WPIcal.cpp @@ -19,10 +19,11 @@ #include #include #include -#include #include #include +#include "tagpose.h" + namespace gui = wpi::gui; const char* GetWPILibVersion(); @@ -140,7 +141,7 @@ static bool EmitEntryTarget(int tag_id, std::string& file) { if (ImGui::BeginDragDropTarget()) { if (const ImGuiPayload* payload = ImGui::AcceptDragDropPayload("FieldCalibration")) { - file = *(std::string*)payload->Data; + file = *static_cast(payload->Data); rv = true; } ImGui::EndDragDropTarget(); diff --git a/wpical/src/main/native/cpp/tagpose.cpp b/wpical/src/main/native/cpp/tagpose.cpp index ec871d5651..9a78479655 100644 --- a/wpical/src/main/native/cpp/tagpose.cpp +++ b/wpical/src/main/native/cpp/tagpose.cpp @@ -2,7 +2,8 @@ // Open Source Software; you can modify and/or share it under the terms of // the WPILib BSD license file in the root directory of this project. -#include +#include "tagpose.h" + #include WPI_IGNORE_DEPRECATED diff --git a/wpical/src/main/native/include/fieldmap.h b/wpical/src/main/native/include/fieldmap.h index 5e581c7322..4008bb0f96 100644 --- a/wpical/src/main/native/include/fieldmap.h +++ b/wpical/src/main/native/include/fieldmap.h @@ -7,9 +7,10 @@ #include #include -#include #include +#include "tagpose.h" + class Fieldmap { public: Fieldmap() = default; diff --git a/wpical/src/main/native/include/fmap.h b/wpical/src/main/native/include/fmap.h index 6757b7cada..38a3127e9a 100644 --- a/wpical/src/main/native/include/fmap.h +++ b/wpical/src/main/native/include/fmap.h @@ -4,11 +4,11 @@ #pragma once -#include - -#include #include +#include "fieldmap.h" +#include "tagpose.h" + namespace fmap { wpi::json singleTag(int tag, const tag::Pose& tagpose); wpi::json convertfmap(const wpi::json& json); diff --git a/wpilibNewCommands/src/main/native/include/frc2/command/sysid/SysIdRoutine.h b/wpilibNewCommands/src/main/native/include/frc2/command/sysid/SysIdRoutine.h index 4ccfd41328..3315f73d31 100644 --- a/wpilibNewCommands/src/main/native/include/frc2/command/sysid/SysIdRoutine.h +++ b/wpilibNewCommands/src/main/native/include/frc2/command/sysid/SysIdRoutine.h @@ -53,7 +53,7 @@ class Config { std::optional stepVoltage, std::optional timeout, std::function recordState) - : m_recordState{recordState} { + : m_recordState{std::move(recordState)} { if (rampRate) { m_rampRate = rampRate.value(); } diff --git a/wpilibc/src/main/native/cpp/ADIS16470_IMU.cpp b/wpilibc/src/main/native/cpp/ADIS16470_IMU.cpp index 2ff191376e..99ae3a53ea 100644 --- a/wpilibc/src/main/native/cpp/ADIS16470_IMU.cpp +++ b/wpilibc/src/main/native/cpp/ADIS16470_IMU.cpp @@ -79,9 +79,9 @@ ADIS16470_IMU::ADIS16470_IMU(IMUAxis yaw_axis, IMUAxis pitch_axis, "IMUAxis.kZ as arguments."); REPORT_ERROR( "Constructing ADIS with default axes. (IMUAxis.kZ is defined as Yaw)"); - yaw_axis = kZ; - pitch_axis = kY; - roll_axis = kX; + m_yaw_axis = kZ; + m_pitch_axis = kY; + m_roll_axis = kX; } if (m_simDevice) { diff --git a/wpilibc/src/test/native/cpp/AlertTest.cpp b/wpilibc/src/test/native/cpp/AlertTest.cpp index b19f0dd0c4..5f233e3342 100644 --- a/wpilibc/src/test/native/cpp/AlertTest.cpp +++ b/wpilibc/src/test/native/cpp/AlertTest.cpp @@ -22,7 +22,7 @@ using namespace frc; using enum Alert::AlertType; class AlertsTest : public ::testing::Test { public: - ~AlertsTest() { + ~AlertsTest() override { // test all destructors Update(); EXPECT_EQ(GetSubscriberForType(kError).Get().size(), 0ul); diff --git a/wpilibcExamples/src/main/cpp/examples/RomiReference/cpp/commands/TeleopArcadeDrive.cpp b/wpilibcExamples/src/main/cpp/examples/RomiReference/cpp/commands/TeleopArcadeDrive.cpp index e457af9c43..f444d2918b 100644 --- a/wpilibcExamples/src/main/cpp/examples/RomiReference/cpp/commands/TeleopArcadeDrive.cpp +++ b/wpilibcExamples/src/main/cpp/examples/RomiReference/cpp/commands/TeleopArcadeDrive.cpp @@ -4,14 +4,16 @@ #include "commands/TeleopArcadeDrive.h" +#include + #include "subsystems/Drivetrain.h" TeleopArcadeDrive::TeleopArcadeDrive( Drivetrain* subsystem, std::function xaxisSpeedSupplier, std::function zaxisRotateSuppplier) : m_drive{subsystem}, - m_xaxisSpeedSupplier{xaxisSpeedSupplier}, - m_zaxisRotateSupplier{zaxisRotateSuppplier} { + m_xaxisSpeedSupplier{std::move(xaxisSpeedSupplier)}, + m_zaxisRotateSupplier{std::move(zaxisRotateSuppplier)} { AddRequirements(subsystem); } diff --git a/wpilibcIntegrationTests/src/main/native/cpp/DIOLoopTest.cpp b/wpilibcIntegrationTests/src/main/native/cpp/DIOLoopTest.cpp index a810be92e7..31c6db8d72 100644 --- a/wpilibcIntegrationTests/src/main/native/cpp/DIOLoopTest.cpp +++ b/wpilibcIntegrationTests/src/main/native/cpp/DIOLoopTest.cpp @@ -171,8 +171,9 @@ TEST_F(DIOLoopTest, SynchronousInterruptWorks) { timer.Start(); interrupt.WaitForInterrupt(kSynchronousInterruptTime + 1_s); auto time = timer.Get().value(); - if (thr.joinable()) + if (thr.joinable()) { thr.join(); + } EXPECT_NEAR(kSynchronousInterruptTime.value(), time, kSynchronousInterruptTimeTolerance.value()); } diff --git a/wpiutil/src/main/native/include/wpi/jni_util.h b/wpiutil/src/main/native/include/wpi/jni_util.h index 4e451a6367..cd3eeedd64 100644 --- a/wpiutil/src/main/native/include/wpi/jni_util.h +++ b/wpiutil/src/main/native/include/wpi/jni_util.h @@ -322,6 +322,7 @@ class JSpanBase { } } + // NOLINTNEXTLINE(google-explicit-constructor) operator std::span() const { return array(); } std::span array() const { diff --git a/wpiutil/src/test/native/cpp/StringMapTest.cpp b/wpiutil/src/test/native/cpp/StringMapTest.cpp index 0086660923..69da187c5a 100644 --- a/wpiutil/src/test/native/cpp/StringMapTest.cpp +++ b/wpiutil/src/test/native/cpp/StringMapTest.cpp @@ -335,6 +335,7 @@ TEST_F(StringMapTest, MoveConstruct) { StringMap A; A["x"] = 42; StringMap B = std::move(A); + // NOLINTNEXTLINE(clang-analyzer-cplusplus.Move) ASSERT_EQ(A.size(), 0u); ASSERT_EQ(B.size(), 1u); ASSERT_EQ(B["x"], 42); @@ -348,8 +349,10 @@ TEST_F(StringMapTest, MoveAssignment) { B["y"] = 117; A = std::move(B); ASSERT_EQ(A.size(), 1u); + // NOLINTNEXTLINE(clang-analyzer-cplusplus.Move) ASSERT_EQ(B.size(), 0u); ASSERT_EQ(A["y"], 117); + // NOLINTNEXTLINE(clang-analyzer-cplusplus.Move) ASSERT_EQ(B.count("x"), 0u); }