From ca272de40079e7f6abf7464db3abc70b1d3067f4 Mon Sep 17 00:00:00 2001 From: Tyler Veness Date: Sat, 2 Dec 2023 21:20:43 -0800 Subject: [PATCH] [build] Fix Gradle compile_commands.json and clang-tidy warnings (#5977) --- .github/workflows/fix_compile_commands.py | 42 +++++++++++ .github/workflows/lint-format.yml | 5 +- .../src/main/native/linux/NetworkListener.cpp | 1 + .../native/windows/WindowsMessagePump.cpp | 3 +- hal/src/main/native/include/hal/.clang-tidy | 2 - ntcore/.clang-tidy | 69 +++++++++++++++++++ .../src/main/native/cpp/DriverStationGui.cpp | 1 + .../cpp/motorcontrol/MotorController.cpp | 1 + .../include/frc/filter/SlewRateLimiter.h | 2 +- .../frc/kinematics/SwerveDriveKinematics.inc | 2 +- wpinet/src/main/native/cpp/UDPClient.cpp | 1 + 11 files changed, 123 insertions(+), 6 deletions(-) create mode 100755 .github/workflows/fix_compile_commands.py create mode 100644 ntcore/.clang-tidy diff --git a/.github/workflows/fix_compile_commands.py b/.github/workflows/fix_compile_commands.py new file mode 100755 index 0000000000..8a52830dea --- /dev/null +++ b/.github/workflows/fix_compile_commands.py @@ -0,0 +1,42 @@ +#!/usr/bin/env python3 + +import argparse +import json + + +def main(): + parser = argparse.ArgumentParser( + description="Fix compile_commands.json generated by Gradle" + ) + parser.add_argument("filename", help="compile_commands.json location") + cmd_args = parser.parse_args() + + # Read JSON + with open(cmd_args.filename) as f: + data = json.load(f) + + for obj in data: + out_args = [] + + # Filter out -isystem flags that cause false positives + iter_args = iter(obj["arguments"]) + for arg in iter_args: + if arg == "-isystem": + next_arg = next(iter_args) + + # /usr/lib/gcc/x86_64-pc-linux-gnu/13.2.1/include/xmmintrin.h:54:1: + # error: conflicting types for '_mm_prefetch' [clang-diagnostic-error] + if not next_arg.startswith("/usr/lib/gcc/"): + out_args += ["-isystem", next_arg] + else: + out_args.append(arg) + + obj["arguments"] = out_args + + # Write JSON + with open(cmd_args.filename, "w") as f: + json.dump(data, f, indent=2) + + +if __name__ == "__main__": + main() diff --git a/.github/workflows/lint-format.yml b/.github/workflows/lint-format.yml index fac88cf004..67e948efb8 100644 --- a/.github/workflows/lint-format.yml +++ b/.github/workflows/lint-format.yml @@ -68,7 +68,10 @@ jobs: - name: Install wpiformat run: pip3 install wpiformat - name: Create compile_commands.json - run: ./gradlew generateCompileCommands -Ptoolchain-optional-roboRio + run: | + ./gradlew generateCompileCommands -Ptoolchain-optional-roboRio + ./.github/workflows/fix_compile_commands.py build/TargetedCompileCommands/linuxx86-64release/compile_commands.json + ./.github/workflows/fix_compile_commands.py build/TargetedCompileCommands/linuxx86-64debug/compile_commands.json - name: List changed files run: wpiformat -list-changed-files - name: Run clang-tidy release diff --git a/cscore/src/main/native/linux/NetworkListener.cpp b/cscore/src/main/native/linux/NetworkListener.cpp index 1a0ac8b595..9771cfa20f 100644 --- a/cscore/src/main/native/linux/NetworkListener.cpp +++ b/cscore/src/main/native/linux/NetworkListener.cpp @@ -89,6 +89,7 @@ void NetworkListener::Impl::Thread::Main() { std::memset(&addr, 0, sizeof(addr)); addr.nl_family = AF_NETLINK; addr.nl_groups = RTMGRP_LINK | RTMGRP_IPV4_IFADDR; + // NOLINTNEXTLINE(modernize-avoid-bind) if (bind(sd, reinterpret_cast(&addr), sizeof(addr)) < 0) { ERROR("NetworkListener: could not create socket: {}", std::strerror(errno)); ::close(sd); diff --git a/cscore/src/main/native/windows/WindowsMessagePump.cpp b/cscore/src/main/native/windows/WindowsMessagePump.cpp index 716edd91dc..35634faa07 100644 --- a/cscore/src/main/native/windows/WindowsMessagePump.cpp +++ b/cscore/src/main/native/windows/WindowsMessagePump.cpp @@ -99,8 +99,9 @@ WindowsMessagePump::WindowsMessagePump( WindowsMessagePump::~WindowsMessagePump() { auto res = SendMessageA(hwnd, WM_CLOSE, NULL, NULL); - if (m_mainThread.joinable()) + if (m_mainThread.joinable()) { m_mainThread.join(); + } } void WindowsMessagePump::ThreadMain(HANDLE eventHandle) { diff --git a/hal/src/main/native/include/hal/.clang-tidy b/hal/src/main/native/include/hal/.clang-tidy index 4f4edda0f0..26312851e9 100644 --- a/hal/src/main/native/include/hal/.clang-tidy +++ b/hal/src/main/native/include/hal/.clang-tidy @@ -36,7 +36,6 @@ Checks: bugprone-unhandled-self-assignment, bugprone-unused-raii, bugprone-virtual-near-miss, - cert-dcl58-cpp, cert-err52-cpp, cert-err60-cpp, cert-mem57-cpp, @@ -54,7 +53,6 @@ Checks: google-readability-avoid-underscore-in-googletest-name, google-readability-casting, google-runtime-operator, - llvm-twine-local, misc-definitions-in-headers, misc-misplaced-const, misc-new-delete-overloads, diff --git a/ntcore/.clang-tidy b/ntcore/.clang-tidy new file mode 100644 index 0000000000..41c15bba7d --- /dev/null +++ b/ntcore/.clang-tidy @@ -0,0 +1,69 @@ +Checks: + 'bugprone-assert-side-effect, + bugprone-bool-pointer-implicit-conversion, + bugprone-copy-constructor-init, + bugprone-dangling-handle, + bugprone-dynamic-static-initializers, + bugprone-forwarding-reference-overload, + bugprone-inaccurate-erase, + bugprone-incorrect-roundings, + bugprone-integer-division, + bugprone-lambda-function-name, + bugprone-misplaced-operator-in-strlen-in-alloc, + bugprone-misplaced-widening-cast, + bugprone-move-forwarding-reference, + bugprone-multiple-statement-macro, + bugprone-parent-virtual-call, + bugprone-posix-return, + bugprone-sizeof-container, + bugprone-sizeof-expression, + bugprone-spuriously-wake-up-functions, + bugprone-string-constructor, + bugprone-string-integer-assignment, + bugprone-string-literal-with-embedded-nul, + bugprone-suspicious-enum-usage, + bugprone-suspicious-include, + bugprone-suspicious-memset-usage, + bugprone-suspicious-missing-comma, + bugprone-suspicious-semicolon, + bugprone-suspicious-string-compare, + bugprone-throw-keyword-missing, + bugprone-too-small-loop-variable, + bugprone-undefined-memory-manipulation, + bugprone-undelegated-constructor, + bugprone-unhandled-self-assignment, + bugprone-unused-raii, + bugprone-virtual-near-miss, + cert-err52-cpp, + cert-err60-cpp, + cert-mem57-cpp, + cert-oop57-cpp, + cert-oop58-cpp, + clang-diagnostic-*, + -clang-diagnostic-deprecated-declarations, + -clang-diagnostic-#warnings, + -clang-diagnostic-pedantic, + clang-analyzer-*, + cppcoreguidelines-slicing, + google-build-namespaces, + google-explicit-constructor, + google-global-names-in-headers, + google-readability-avoid-underscore-in-googletest-name, + google-readability-casting, + google-runtime-operator, + misc-definitions-in-headers, + misc-misplaced-const, + misc-new-delete-overloads, + misc-non-copyable-objects, + modernize-avoid-bind, + modernize-concat-nested-namespaces, + modernize-make-shared, + modernize-make-unique, + modernize-pass-by-value, + modernize-use-default-member-init, + modernize-use-noexcept, + modernize-use-nullptr, + modernize-use-override, + modernize-use-using, + readability-braces-around-statements' +FormatStyle: file diff --git a/simulation/halsim_gui/src/main/native/cpp/DriverStationGui.cpp b/simulation/halsim_gui/src/main/native/cpp/DriverStationGui.cpp index 4edaa5c770..09192f39ff 100644 --- a/simulation/halsim_gui/src/main/native/cpp/DriverStationGui.cpp +++ b/simulation/halsim_gui/src/main/native/cpp/DriverStationGui.cpp @@ -609,6 +609,7 @@ void KeyboardJoystick::EditKey(const char* label, int* key) { void KeyboardJoystick::SettingsDisplay() { if (s_keyEdit) { ImGuiIO& io = ImGui::GetIO(); + // NOLINTNEXTLINE(bugprone-sizeof-expression) for (int i = 0; i < IM_ARRAYSIZE(io.KeysDown); ++i) { if (io.KeysDown[i]) { // remove all other uses diff --git a/wpilibc/src/main/native/cpp/motorcontrol/MotorController.cpp b/wpilibc/src/main/native/cpp/motorcontrol/MotorController.cpp index 9d20144ca8..2b64da27a2 100644 --- a/wpilibc/src/main/native/cpp/motorcontrol/MotorController.cpp +++ b/wpilibc/src/main/native/cpp/motorcontrol/MotorController.cpp @@ -9,5 +9,6 @@ using namespace frc; void MotorController::SetVoltage(units::volt_t output) { + // NOLINTNEXTLINE(bugprone-integer-division) Set(output / RobotController::GetBatteryVoltage()); } diff --git a/wpimath/src/main/native/include/frc/filter/SlewRateLimiter.h b/wpimath/src/main/native/include/frc/filter/SlewRateLimiter.h index 17d0d21978..9f7c6744b9 100644 --- a/wpimath/src/main/native/include/frc/filter/SlewRateLimiter.h +++ b/wpimath/src/main/native/include/frc/filter/SlewRateLimiter.h @@ -47,7 +47,7 @@ class SlewRateLimiter { m_negativeRateLimit{negativeRateLimit}, m_prevVal{initialValue}, m_prevTime{ - units::microsecond_t(wpi::math::MathSharedStore::GetTimestamp())} {} + units::microsecond_t{wpi::math::MathSharedStore::GetTimestamp()}} {} /** * Creates a new SlewRateLimiter with the given positive rate limit and diff --git a/wpimath/src/main/native/include/frc/kinematics/SwerveDriveKinematics.inc b/wpimath/src/main/native/include/frc/kinematics/SwerveDriveKinematics.inc index db85d999e3..b9bf34016e 100644 --- a/wpimath/src/main/native/include/frc/kinematics/SwerveDriveKinematics.inc +++ b/wpimath/src/main/native/include/frc/kinematics/SwerveDriveKinematics.inc @@ -166,7 +166,7 @@ void SwerveDriveKinematics::DesaturateWheelSpeeds( auto k = units::math::max(translationalK, rotationalK); auto scale = units::math::min(k * attainableMaxModuleSpeed / realMaxSpeed, - units::scalar_t(1)); + units::scalar_t{1}); for (auto& module : states) { module.speed = module.speed * scale; } diff --git a/wpinet/src/main/native/cpp/UDPClient.cpp b/wpinet/src/main/native/cpp/UDPClient.cpp index 5963d605aa..1596b59ab0 100644 --- a/wpinet/src/main/native/cpp/UDPClient.cpp +++ b/wpinet/src/main/native/cpp/UDPClient.cpp @@ -110,6 +110,7 @@ int UDPClient::start(int port) { #endif } + // NOLINTNEXTLINE(modernize-avoid-bind) int result = bind(m_lsd, reinterpret_cast(&addr), sizeof(addr)); if (result != 0) { WPI_ERROR(m_logger, "bind() failed: {}", SocketStrerror());