Upgrade clang-tidy to version 19 (#166)

Upgrading clang-tidy to version 19 as well as using ubuntu 24.04 for the
GHA runners

Most fixes were as a result of include cleaner adding includes that were used

Second most was making code that was only used within a cpp file have internal linkage.

Beyond that there were a few minor fixes that it found as well as one legitimate bug that will be fixed/tested in a future PR

.clang-tidy

@@ -34,6 +34,7 @@ Checks: >
   -readability-function-size,
   -readability-identifier-length,
   -readability-magic-numbers,
+  -readability-math-missing-parentheses,
   -readability-uppercase-literal-suffix,
   modernize-*,
   -modernize-use-trailing-return-type,
@@ -70,3 +71,5 @@ CheckOptions:
     value: google
   - key: readability-braces-around-statements.ShortStatementLines
     value: "1"
+  - key: misc-include-cleaner.IgnoreHeaders
+    value: "asm-generic/.*;bits/.*"

.github/workflows/gcc.yaml

@@ -36,7 +36,7 @@ jobs:
             version: "7"
 
     name: Linux GCC-${{ matrix.toolchain.version }}
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-24.04
     continue-on-error: true
 
     # Use the container for this specific version of gcc
@@ -96,7 +96,7 @@ jobs:
         run: ctest --output-on-failure -E "dsl/UDP"
 
       - name: Upload Traces
-        if: always()
+        if: ${{ !cancelled() }}
         uses: actions/upload-artifact@v4
         with:
           name: traces-gcc-${{ matrix.toolchain.version }}

.github/workflows/linting.yaml

@@ -20,19 +20,19 @@ concurrency:
 jobs:
   linux-clang-tidy:
     name: Linux Clang-Tidy
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-24.04
 
     steps:
       - name: Checkout Code
         uses: actions/checkout@v4
 
-      - name: Install clang-tidy-15
+      - name: Install clang-tidy
         run: |
           wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | sudo apt-key add -
-          echo "deb http://apt.llvm.org/jammy/ llvm-toolchain-jammy-15 main" | sudo tee /etc/apt/sources.list.d/llvm-15
-          echo "deb http://apt.llvm.org/jammy/ llvm-toolchain-jammy-15 main" | sudo tee -a /etc/apt/sources.list.d/llvm-15
+          echo "deb http://apt.llvm.org/noble/ llvm-toolchain-noble-19 main" | sudo tee /etc/apt/sources.list.d/llvm-19.list
+          echo "deb-src http://apt.llvm.org/noble/ llvm-toolchain-noble-19 main" | sudo tee -a /etc/apt/sources.list.d/llvm-19.list
           sudo apt-get update
-          sudo apt-get install -y clang-tidy-15
+          sudo apt-get install -y clang-tidy-19
 
       # Download and install cmake
       - name: Install CMake
@@ -62,7 +62,7 @@ jobs:
 
           mkdir -p ${{ github.workspace }}/.ctcache
 
-          echo CTCACHE_CLANG_TIDY='/usr/bin/clang-tidy-15' >> "$GITHUB_ENV"
+          echo CTCACHE_CLANG_TIDY='/usr/bin/clang-tidy-19' >> "$GITHUB_ENV"
           echo CTCACHE_LOCAL=1 >> "$GITHUB_ENV"
           echo CTCACHE_SAVE_OUTPUT=1 >> "$GITHUB_ENV"
           echo CTCACHE_DIR='${{github.workspace}}/.ctcache' >> "$GITHUB_ENV"

.github/workflows/macos.yaml

@@ -64,7 +64,7 @@ jobs:
         run: ctest --output-on-failure
 
       - name: Upload Traces
-        if: always()
+        if: ${{ !cancelled() }}
         uses: actions/upload-artifact@v4
         with:
           name: traces-macos

.github/workflows/sonarcloud.yaml

@@ -67,11 +67,11 @@ jobs:
         run: ctest --output-on-failure
 
       - name: Collect coverage into one XML report
-        if: always()
+        if: ${{ !cancelled() }}
         run: gcovr --gcov-ignore-parse-errors=negative_hits.warn_once_per_file --exclude-unreachable-branches --exclude-noncode-lines --sonarqube > coverage.xml
 
       - name: Upload coverage report
-        if: always()
+        if: ${{ !cancelled() }}
         uses: actions/upload-artifact@v4
         with:
           name: coverage-sonar
@@ -80,7 +80,7 @@ jobs:
           overwrite: true
 
       - name: Run sonar-scanner
-        if: always()
+        if: ${{ !cancelled() }}
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
           SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
@@ -90,7 +90,7 @@ jobs:
             --define sonar.coverageReportPaths=coverage.xml
 
       - name: Upload Traces
-        if: always()
+        if: ${{ !cancelled() }}
         uses: actions/upload-artifact@v4
         with:
           name: traces-sonar

.github/workflows/windows.yaml

@@ -85,7 +85,7 @@ jobs:
         run: ctest --output-on-failure
 
       - name: Upload Traces
-        if: always()
+        if: ${{ !cancelled() }}
         uses: actions/upload-artifact@v4
         with:
           name: traces-windows-${{ matrix.toolchain.name }}

.gitignore

@@ -27,3 +27,4 @@ tags
 docs/doxygen
 docs/doxygen_sqlite3.db
 *.sublime-workspace
+.DS_Store

cmake/ClangTidy.cmake

@@ -1,10 +1,10 @@
 # Default not to run the clang-tidy checks, default to whatever our CI_BUILD is
 option(ENABLE_CLANG_TIDY "Enable building with clang-tidy checks.")
 if(ENABLE_CLANG_TIDY)
-  # Search for clang-tidy-15 first as this is the version installed in CI
-  find_program(CLANG_TIDY_EXECUTABLE NAMES clang-tidy-15 clang-tidy)
+  # Search for the same version of clang-tidy as is used in CI first
+  find_program(CLANG_TIDY_EXECUTABLE NAMES clang-tidy-19 clang-tidy)
   if(NOT CLANG_TIDY_EXECUTABLE)
-    message(FATAL_ERROR "clang-tidy-15 not found.")
+    message(FATAL_ERROR "clang-tidy not found.")
   endif()
 
   # Report clang-tidy executable details

src/Configuration.hpp

@@ -23,7 +23,6 @@
 #ifndef NUCLEAR_CONFIGURATION_HPP
 #define NUCLEAR_CONFIGURATION_HPP
 
-#include <cstddef>
 #include <thread>
 
 namespace NUClear {

src/Environment.hpp

@@ -24,8 +24,8 @@
 #define NUCLEAR_ENVIRONMENT_HPP
 
 #include <string>
+#include <utility>
 
-#include "LogLevel.hpp"
 
 namespace NUClear {
 

src/LogLevel.hpp

@@ -23,7 +23,9 @@
 #ifndef NUCLEAR_LOGLEVEL_HPP
 #define NUCLEAR_LOGLEVEL_HPP
 
-#include <ostream>
+#include <cstdint>
+#include <iosfwd>
+#include <string>
 
 // Why do we need to include platform.hpp here?
 // Because windows defines a bunch of things for legacy reasons, one of which is a #define for ERROR as blank

src/PowerPlant.cpp

@@ -22,19 +22,18 @@
 
 #include "PowerPlant.hpp"
 
-#include <exception>
-#include <tuple>
+#include <memory>
+#include <utility>
 
+#include "Configuration.hpp"
 #include "Reactor.hpp"
 #include "dsl/store/DataStore.hpp"
 #include "dsl/word/Shutdown.hpp"
 #include "dsl/word/Startup.hpp"
 #include "dsl/word/emit/Inline.hpp"
-#include "extension/ChronoController.hpp"
-#include "extension/IOController.hpp"
-#include "extension/NetworkController.hpp"
+#include "id.hpp"
 #include "message/CommandLineArguments.hpp"
-#include "message/LogMessage.hpp"
+#include "threading/Reaction.hpp"
 #include "threading/ReactionTask.hpp"
 
 namespace NUClear {

src/PowerPlant.hpp

@@ -23,18 +23,18 @@
 #ifndef NUCLEAR_POWER_PLANT_HPP
 #define NUCLEAR_POWER_PLANT_HPP
 
-#include <atomic>
-#include <functional>
 #include <memory>
-#include <sstream>
+#include <tuple>
+#include <type_traits>
 #include <utility>
 #include <vector>
 
 // Utilities
-#include "Configuration.hpp"
+#include "Configuration.hpp"  // IWYU pragma: export
 #include "Environment.hpp"
 #include "LogLevel.hpp"
 #include "id.hpp"
+#include "threading/Reaction.hpp"
 #include "threading/ReactionTask.hpp"
 #include "threading/scheduler/Scheduler.hpp"
 #include "util/FunctionFusion.hpp"

src/Reactor.cpp

@@ -22,6 +22,8 @@
 
 #include "Reactor.hpp"
 
+#include "LogLevel.hpp"
+
 namespace NUClear {
 
 constexpr LogLevel Reactor::TRACE;

src/Reactor.hpp

@@ -24,18 +24,22 @@
 #define NUCLEAR_REACTOR_HPP
 
 #include <chrono>
-#include <functional>
+#include <cstddef>
+#include <memory>
 #include <regex>
 #include <string>
-#include <typeindex>
+#include <tuple>
+#include <utility>
 #include <vector>
 
-#include "Environment.hpp"
-#include "LogLevel.hpp"
+#include "Environment.hpp"  // IWYU pragma: export
+#include "LogLevel.hpp"     // IWYU pragma: export
+#include "clock.hpp"        // IWYU pragma: export
 #include "dsl/Parse.hpp"
-#include "threading/Reaction.hpp"
-#include "threading/ReactionHandle.hpp"
-#include "threading/ReactionIdentifiers.hpp"
+#include "id.hpp"                             // IWYU pragma: export
+#include "threading/Reaction.hpp"             // IWYU pragma: export
+#include "threading/ReactionHandle.hpp"       // IWYU pragma: export
+#include "threading/ReactionIdentifiers.hpp"  // IWYU pragma: export
 #include "util/CallbackGenerator.hpp"
 #include "util/Sequence.hpp"
 #include "util/demangle.hpp"

src/dsl/fusion/BindFusion.hpp

@@ -79,13 +79,13 @@ namespace dsl {
                                                    reaction,
                                                    std::forward<Arguments>(args)...))>::value,
                                                NoReturn,
-                                               Return>::template call(reaction, std::forward<Arguments>(args)...)) {
+                                               Return>::call(reaction, std::forward<Arguments>(args)...)) {
 
                 return std::conditional_t<
                     std::is_void<decltype(Function::template bind<DSL>(reaction,
                                                                        std::forward<Arguments>(args)...))>::value,
                     NoReturn,
-                    Return>::template call(reaction, std::forward<Arguments>(args)...);
+                    Return>::call(reaction, std::forward<Arguments>(args)...);
             }
         };
 

src/dsl/operation/CacheGet.hpp

@@ -27,6 +27,9 @@
 #include "../store/ThreadStore.hpp"
 
 namespace NUClear {
+namespace threading {
+    class ReactionTask;  // Forward declare ReactionTask
+}  // namespace threading
 namespace dsl {
     namespace operation {
 
@@ -41,6 +44,7 @@ namespace dsl {
          */
         template <typename DataType>
         struct CacheGet {
+            CacheGet() = delete;  // Ensure that DSL words are not instantiated
 
             template <typename DSL, typename T = DataType>
             static std::shared_ptr<const T> get(const threading::ReactionTask& /*task*/) {

src/dsl/operation/ChronoTask.hpp

@@ -23,6 +23,8 @@
 #ifndef NUCLEAR_DSL_OPERATION_CHRONO_TASK_HPP
 #define NUCLEAR_DSL_OPERATION_CHRONO_TASK_HPP
 
+#include <functional>
+
 #include "../../clock.hpp"
 #include "../../id.hpp"
 

src/dsl/operation/TypeBind.hpp

@@ -23,6 +23,8 @@
 #ifndef NUCLEAR_DSL_OPERATION_TYPE_BIND_HPP
 #define NUCLEAR_DSL_OPERATION_TYPE_BIND_HPP
 
+#include <algorithm>
+
 #include "../store/TypeCallbackStore.hpp"
 
 namespace NUClear {
@@ -56,6 +58,7 @@ namespace dsl {
          */
         template <typename DataType>
         struct TypeBind {
+            TypeBind() = delete;  // Ensure that DSL words are not instantiated
 
             template <typename DSL>
             static void bind(const std::shared_ptr<threading::Reaction>& reaction) {

src/dsl/word/Idle.hpp

@@ -71,7 +71,12 @@ namespace dsl {
             static void bind(const std::shared_ptr<threading::Reaction>& reaction) {
 
                 // Make a fake task to use for finding an appropriate descriptor
-                threading::ReactionTask task(reaction, false, DSL::priority, DSL::run_inline, DSL::pool, DSL::group);
+                const threading::ReactionTask task(reaction,
+                                                   false,
+                                                   DSL::priority,
+                                                   DSL::run_inline,
+                                                   DSL::pool,
+                                                   DSL::group);
                 bind_idle(reaction, PoolType::template pool<DSL>(task));
             }
         };

src/dsl/word/Shutdown.hpp

@@ -52,11 +52,19 @@ namespace dsl {
          * @par Implements
          *  Bind
          */
-        struct Shutdown
-            : operation::TypeBind<Shutdown>
-            , Priority::IDLE {};
+        struct Shutdown {};
 
     }  // namespace word
+
+    namespace operation {
+
+        template <>
+        struct DSLProxy<word::Shutdown>
+            : operation::TypeBind<word::Shutdown>
+            , word::Priority::IDLE {};
+
+    }  // namespace operation
+
 }  // namespace dsl
 }  // namespace NUClear
 

src/dsl/word/Startup.hpp

@@ -39,9 +39,17 @@ namespace dsl {
          * @par Implements
          *  Bind
          */
-        struct Startup : operation::TypeBind<Startup> {};
+        struct Startup {};
 
     }  // namespace word
+
+    namespace operation {
+
+        template <>
+        struct DSLProxy<word::Startup> : operation::TypeBind<word::Startup> {};
+
+    }  // namespace operation
+
 }  // namespace dsl
 }  // namespace NUClear
 

src/dsl/word/TaskScope.hpp

@@ -21,7 +21,6 @@
  */
 #ifndef NUCLEAR_DSL_WORD_TASK_SCOPE_HPP
 #define NUCLEAR_DSL_WORD_TASK_SCOPE_HPP
-#include <iostream>
 
 #include "../../id.hpp"
 #include "../../threading/ReactionTask.hpp"
@@ -76,7 +75,7 @@ namespace dsl {
             template <typename DSL>
             static Lock scope(const threading::ReactionTask& task) {
                 // Store the old task id
-                NUClear::id_t old_id = current_task_id;
+                const NUClear::id_t old_id = current_task_id;
                 // Set the current task id to the word
                 current_task_id = task.id;
                 // Return a lock that will restore the old task id

src/dsl/word/UDP.hpp

@@ -367,7 +367,7 @@ namespace dsl {
                 mh.msg_iovlen     = 1;
 
                 // Receive our message
-                ssize_t received = recvmsg(event.fd, &mh, MSG_DONTWAIT);
+                const ssize_t received = recvmsg(event.fd, &mh, MSG_DONTWAIT);
                 if (received < 0) {
                     return {};
                 }