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 to clang-tidy 18 #6919

Merged
merged 29 commits into from
Jun 6, 2024
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
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
13 changes: 13 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ Checks: >
-bugprone-forward-declaration-namespace,
-bugprone-sizeof-expression,
-bugprone-throw-keyword-missing,
-bugprone-chained-comparison,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many of these warnings seem to be useful - I'll create an issue to fix some of them before merging it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-bugprone-incorrect-enable-if,
-bugprone-switch-missing-default-case,
-bugprone-empty-catch,
-bugprone-unused-return-value,
-clang-analyzer-*,
-clang-diagnostic-deprecated-declarations,
-clang-diagnostic-constant-conversion,
Expand Down Expand Up @@ -49,11 +54,15 @@ Checks: >
-misc-misplaced-const,
-misc-definitions-in-headers,
-misc-unused-parameters,
-misc-include-cleaner,
modernize-concat-nested-namespaces,
modernize-use-using,
performance-*,
-performance-noexcept-move-constructor,
-performance-noexcept-swap,
-performance-no-int-to-ptr,
-performance-enum-size,
-performance-avoid-endl,
readability-*,
-readability-avoid-const-params-in-decls,
-readability-braces-around-statements,
Expand Down Expand Up @@ -82,6 +91,10 @@ Checks: >
-readability-make-member-function-const,
-readability-redundant-string-init,
-readability-non-const-parameter,
-readability-redundant-inline-specifier,
-readability-avoid-nested-conditional-operator,
-readability-avoid-return-with-void-value,
-readability-redundant-casting,
-readability-static-accessed-through-instance
WarningsAsErrors: '*'
Expand Down
10 changes: 5 additions & 5 deletions .github/workflows/osrm-backend.yml
Original file line number Diff line number Diff line change
Expand Up @@ -192,14 +192,14 @@ jobs:
CXXCOMPILER: clang++-15
CUCUMBER_TIMEOUT: 60000

- name: clang-15-debug-clang-tidy
- name: clang-18-debug-clang-tidy
continue-on-error: false
node: 18
runs-on: ubuntu-22.04
runs-on: ubuntu-24.04
BUILD_TOOLS: ON
BUILD_TYPE: Debug
CCOMPILER: clang-15
CXXCOMPILER: clang++-15
CCOMPILER: clang-18
CXXCOMPILER: clang++-18
CUCUMBER_TIMEOUT: 60000
ENABLE_CLANG_TIDY: ON

Expand Down Expand Up @@ -703,7 +703,7 @@ jobs:
run: |
ccache -p
ccache -s

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit pick: stray whitespace 😉

ci-complete:
runs-on: ubuntu-22.04
needs: [build-test-publish, docker-image, windows-release-node, benchmarks]
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
- ADDED: Add support for opposite approach request parameter. [#6842](https://github.com/Project-OSRM/osrm-backend/pull/6842)
- ADDED: Add support for accessing edge flags in `process_segment` [#6658](https://github.com/Project-OSRM/osrm-backend/pull/6658)
- Build:
- CHANGED: Upgrade clang-format to version 15. [#6919](https://github.com/Project-OSRM/osrm-backend/pull/6919)
- CHANGED: Use Debian Bookworm as base Docker image [#6904](https://github.com/Project-OSRM/osrm-backend/pull/6904)
- CHANGED: Upgrade CI actions to latest versions [#6893](https://github.com/Project-OSRM/osrm-backend/pull/6893)
- CHANGED: Remove outdated warnings #6894 [#6894](https://github.com/Project-OSRM/osrm-backend/pull/6894)
Expand Down
1 change: 0 additions & 1 deletion include/extractor/serialization.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,6 @@ inline void read(storage::tar::FileReader &reader,
const std::string &name,
detail::NameTableImpl<Ownership> &name_table)
{
std::string buffer;
util::serialization::read(reader, name, name_table.indexed_data);
}
} // namespace osrm::extractor::serialization
Expand Down
2 changes: 1 addition & 1 deletion include/storage/shared_memory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class SharedMemory
{
shm = boost::interprocess::xsi_shared_memory(boost::interprocess::open_only, key);

util::Log(logDEBUG) << "opening " << (int)shm.get_shmid() << " from id " << (int)id;
util::Log(logDEBUG) << "opening " << shm.get_shmid() << " from id " << (int)id;

region = boost::interprocess::mapped_region(shm, boost::interprocess::read_only);
}
Expand Down
16 changes: 14 additions & 2 deletions third_party/sol2/include/sol/sol.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -19416,7 +19416,13 @@ namespace sol { namespace function_detail {
}

template <bool is_yielding, bool no_trampoline>
static int call(lua_State* L) noexcept(std::is_nothrow_copy_assignable_v<T>) {
static int call(lua_State* L)
#if SOL_IS_ON(SOL_COMPILER_CLANG)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Took this workaround from ThePhD/sol2#1581 (comment)

I don't see another way to build with newer clang unfortunately. I'll try to make a PR to have this workaround in sol2 mainline...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a link as a comment right in the source. This way one can trace the origin of this bandaid, and eventually remove it in the future.

// apparent regression in clang 18 - llvm/llvm-project#91362
#else
noexcept(std::is_nothrow_copy_assignable_v<T>)
#endif
{
int nr;
if constexpr (no_trampoline) {
nr = real_call(L);
Expand Down Expand Up @@ -19456,7 +19462,13 @@ namespace sol { namespace function_detail {
}

template <bool is_yielding, bool no_trampoline>
static int call(lua_State* L) noexcept(std::is_nothrow_copy_assignable_v<T>) {
static int call(lua_State* L)
#if SOL_IS_ON(SOL_COMPILER_CLANG)
// apparent regression in clang 18 - llvm/llvm-project#91362
#else
noexcept(std::is_nothrow_copy_assignable_v<T>)
#endif
{
int nr;
if constexpr (no_trampoline) {
nr = real_call(L);
Expand Down
1 change: 0 additions & 1 deletion unit_tests/library/route.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,6 @@ void test_route_same_coordinates(bool use_json_only_api)
BOOST_CHECK(((void)name, true));

// nothing can be said about mode, contains mode of transportation
const auto mode = std::get<json::String>(step_object.values.at("mode")).value;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment above looks like it can be removed, too.

BOOST_CHECK(!name.empty());

const auto &maneuver =
Expand Down
5 changes: 4 additions & 1 deletion unit_tests/partitioner/bisection_graph_view.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,15 @@ using namespace osrm::util;

BOOST_AUTO_TEST_SUITE(graph_view)

static void shuffle(std::vector<EdgeWithSomeAdditionalData> &grid_edges)
namespace
{
void shuffle(std::vector<EdgeWithSomeAdditionalData> &grid_edges)
{
std::random_device rd;
std::mt19937 rng(rd());
std::shuffle(grid_edges.begin(), grid_edges.end(), rng);
}
} // namespace

BOOST_AUTO_TEST_CASE(separate_top_bottom)
{
Expand Down
2 changes: 0 additions & 2 deletions unit_tests/util/static_rtree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -235,8 +235,6 @@ auto make_rtree(const boost::filesystem::path &path, FixtureT &fixture)
template <typename RTreeT = TestStaticRTree, typename FixtureT>
void construction_test(const std::string &path, FixtureT &fixture)
{
std::string leaves_path;
std::string nodes_path;
auto rtree = make_rtree<RTreeT>(path, fixture);
LinearSearchNN<TestData> lsnn(fixture.coords, fixture.edges);

Expand Down
Loading