Skip to content
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.

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

Upgrade clang-tidy to version 19 #166

Merged
merged 21 commits into from
Dec 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
fcbce01
Cancelling should stop everything
TrentHouliston Dec 25, 2024
2d754fd
Upgrade to clang-tidy 19
TrentHouliston Dec 25, 2024
f267881
put everything on 24.04
TrentHouliston Dec 25, 2024
915a4d4
Turns out I want 20
TrentHouliston Dec 25, 2024
9d2fa74
Turns out it is 19 but I'm doing it wrong?
TrentHouliston Dec 25, 2024
899a463
Apply some fixes from the new clang-tidy
TrentHouliston Dec 25, 2024
55a9417
Made no sense including nuclear as a system include in tests
TrentHouliston Dec 25, 2024
e3cbbee
More changes
TrentHouliston Dec 26, 2024
2d0cc4d
nolint the bug for now, fix in another PR since it might change a bit
TrentHouliston Dec 26, 2024
02eeaed
Need the headers for windows
TrentHouliston Dec 26, 2024
20c5806
Only need forward declarations here
TrentHouliston Dec 26, 2024
e998ffe
fwd?
TrentHouliston Dec 26, 2024
d824db5
Try forward declaring iovec and saying it's exported? if this doesn't…
TrentHouliston Dec 26, 2024
67a2840
swap to a block
TrentHouliston Dec 26, 2024
9c9ef96
Don't like it
TrentHouliston Dec 26, 2024
286b263
Should be all the includes fixed!
TrentHouliston Dec 26, 2024
97764fb
"handle" the exception so clang-tidy is happy
TrentHouliston Dec 26, 2024
ff1e99b
invalid, clearly this is never fired
TrentHouliston Dec 26, 2024
8c41c53
Forgot why this was important, left a comment for future me/others
TrentHouliston Dec 27, 2024
c5e54e9
ignore .DS_Store files
TrentHouliston Dec 27, 2024
e0b5998
This header shouldn't be committed
TrentHouliston Dec 27, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -70,3 +71,5 @@ CheckOptions:
value: google
- key: readability-braces-around-statements.ShortStatementLines
value: "1"
- key: misc-include-cleaner.IgnoreHeaders
value: "asm-generic/.*;bits/.*"
4 changes: 2 additions & 2 deletions .github/workflows/gcc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 }}
Expand Down
12 changes: 6 additions & 6 deletions .github/workflows/linting.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/macos.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions .github/workflows/sonarcloud.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 }}
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/windows.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,4 @@ tags
docs/doxygen
docs/doxygen_sqlite3.db
*.sublime-workspace
.DS_Store
6 changes: 3 additions & 3 deletions cmake/ClangTidy.cmake
Original file line number Diff line number Diff line change
@@ -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
Expand Down
1 change: 0 additions & 1 deletion src/Configuration.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
#ifndef NUCLEAR_CONFIGURATION_HPP
#define NUCLEAR_CONFIGURATION_HPP

#include <cstddef>
#include <thread>

namespace NUClear {
Expand Down
2 changes: 1 addition & 1 deletion src/Environment.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
#define NUCLEAR_ENVIRONMENT_HPP

#include <string>
#include <utility>

#include "LogLevel.hpp"

namespace NUClear {

Expand Down
4 changes: 3 additions & 1 deletion src/LogLevel.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 5 additions & 6 deletions src/PowerPlant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions src/PowerPlant.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 2 additions & 0 deletions src/Reactor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@

#include "Reactor.hpp"

#include "LogLevel.hpp"

namespace NUClear {

constexpr LogLevel Reactor::TRACE;
Expand Down
18 changes: 11 additions & 7 deletions src/Reactor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
4 changes: 2 additions & 2 deletions src/dsl/fusion/BindFusion.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)...);
}
};

Expand Down
4 changes: 4 additions & 0 deletions src/dsl/operation/CacheGet.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@
#include "../store/ThreadStore.hpp"

namespace NUClear {
namespace threading {
class ReactionTask; // Forward declare ReactionTask
} // namespace threading
namespace dsl {
namespace operation {

Expand All @@ -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*/) {
Expand Down
2 changes: 2 additions & 0 deletions src/dsl/operation/ChronoTask.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
3 changes: 3 additions & 0 deletions src/dsl/operation/TypeBind.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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) {
Expand Down
7 changes: 6 additions & 1 deletion src/dsl/word/Idle.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
};
Expand Down
14 changes: 11 additions & 3 deletions src/dsl/word/Shutdown.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
10 changes: 9 additions & 1 deletion src/dsl/word/Startup.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 1 addition & 2 deletions src/dsl/word/TaskScope.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/dsl/word/UDP.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {};
}
Expand Down
Loading
Loading