Merge branch 'master' into sf-fix-windows

.clang-tidy

@@ -13,6 +13,10 @@ Checks: >
     -bugprone-forward-declaration-namespace,
     -bugprone-sizeof-expression,
     -bugprone-throw-keyword-missing,
+    -bugprone-chained-comparison,
+    -bugprone-incorrect-enable-if,
+    -bugprone-switch-missing-default-case,
+    -bugprone-empty-catch,
     -clang-analyzer-*,
     -clang-diagnostic-deprecated-declarations,
     -clang-diagnostic-constant-conversion,
@@ -49,11 +53,13 @@ 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-no-int-to-ptr,
+    -performance-enum-size,
+    -performance-avoid-endl,
     readability-*,
     -readability-avoid-const-params-in-decls,
     -readability-braces-around-statements,
@@ -82,6 +88,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: '*'

.github/workflows/osrm-backend.yml

@@ -225,17 +225,18 @@ 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
+          #   CCOMPILER: clang-18
-  #           CXXCOMPILER: clang++-15
+          #   CXXCOMPILER: clang++-18
           #   CUCUMBER_TIMEOUT: 60000
           #   ENABLE_CLANG_TIDY: ON
 
+
   #         - name: clang-14-release
   #           continue-on-error: false
   #           node: 18

CHANGELOG.md

@@ -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)
@@ -23,6 +24,9 @@
     - NodeJS:
       - CHANGED: Use node-api instead of NAN. [#6452](https://github.com/Project-OSRM/osrm-backend/pull/6452)
     - Misc:
+      - FIXED: Fix bugprone-unused-return-value clang-tidy warning. [#6934](https://github.com/Project-OSRM/osrm-backend/pull/6934)
+      - FIXED: Fix performance-noexcept-move-constructor clang-tidy warning. [#6931](https://github.com/Project-OSRM/osrm-backend/pull/6933)
+      - FIXED: Fix performance-noexcept-swap clang-tidy warning. [#6931](https://github.com/Project-OSRM/osrm-backend/pull/6931)
       - CHANGED: Use custom struct instead of std::pair in QueryHeap. [#6921](https://github.com/Project-OSRM/osrm-backend/pull/6921)
       - CHANGED: Use std::string_view::starts_with instead of boost::starts_with. [#6918](https://github.com/Project-OSRM/osrm-backend/pull/6918)
       - CHANGED: Get rid of boost::math::constants::* and M_PI in favor of std::numbers. [#6916](https://github.com/Project-OSRM/osrm-backend/pull/6916)

include/extractor/serialization.hpp

@@ -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

include/server/http/header.hpp

@@ -12,7 +12,7 @@ struct header
     // explicitly use default copy c'tor as adding move c'tor
     header &operator=(const header &other) = default;
     header(std::string name, std::string value) : name(std::move(name)), value(std::move(value)) {}
-    header(header &&other) : name(std::move(other.name)), value(std::move(other.value)) {}
+    header(header &&other) noexcept : name(std::move(other.name)), value(std::move(other.value)) {}
 
     void clear()
     {

include/storage/shared_memory.hpp

@@ -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);
         }

include/util/concurrent_id_map.hpp

@@ -26,7 +26,7 @@ struct ConcurrentIDMap
     mutable UpgradableMutex mutex;
 
     ConcurrentIDMap() = default;
-    ConcurrentIDMap(ConcurrentIDMap &&other)
+    ConcurrentIDMap(ConcurrentIDMap &&other) noexcept
     {
         if (this != &other)
         {
@@ -36,7 +36,7 @@ struct ConcurrentIDMap
             data = std::move(other.data);
         }
     }
-    ConcurrentIDMap &operator=(ConcurrentIDMap &&other)
+    ConcurrentIDMap &operator=(ConcurrentIDMap &&other) noexcept
     {
         if (this != &other)
         {

include/util/deallocating_vector.hpp

@@ -166,7 +166,7 @@ class DeallocatingVectorIterator
 
 template <typename ElementT> class DeallocatingVector;
 
-template <typename T> void swap(DeallocatingVector<T> &lhs, DeallocatingVector<T> &rhs);
+template <typename T> void swap(DeallocatingVector<T> &lhs, DeallocatingVector<T> &rhs) noexcept;
 
 template <typename ElementT> class DeallocatingVector
 {
@@ -204,8 +204,8 @@ template <typename ElementT> class DeallocatingVector
     }
 
     // moving is fine
-    DeallocatingVector(DeallocatingVector &&other) { swap(other); }
+    DeallocatingVector(DeallocatingVector &&other) noexcept { swap(other); }
-    DeallocatingVector &operator=(DeallocatingVector &&other)
+    DeallocatingVector &operator=(DeallocatingVector &&other) noexcept
     {
         swap(other);
         return *this;
@@ -221,9 +221,10 @@ template <typename ElementT> class DeallocatingVector
 
     ~DeallocatingVector() { clear(); }
 
-    friend void swap<>(DeallocatingVector<ElementT> &lhs, DeallocatingVector<ElementT> &rhs);
+    friend void swap<>(DeallocatingVector<ElementT> &lhs,
+                       DeallocatingVector<ElementT> &rhs) noexcept;
 
-    void swap(DeallocatingVector<ElementT> &other)
+    void swap(DeallocatingVector<ElementT> &other) noexcept
     {
         std::swap(current_size, other.current_size);
         bucket_list.swap(other.bucket_list);
@@ -342,7 +343,7 @@ template <typename ElementT> class DeallocatingVector
     }
 };
 
-template <typename T> void swap(DeallocatingVector<T> &lhs, DeallocatingVector<T> &rhs)
+template <typename T> void swap(DeallocatingVector<T> &lhs, DeallocatingVector<T> &rhs) noexcept
 {
     lhs.swap(rhs);
 }

include/util/dynamic_graph.hpp

@@ -154,7 +154,7 @@ template <typename EdgeDataT> class DynamicGraph
         return *this;
     }
 
-    DynamicGraph(DynamicGraph &&other)
+    DynamicGraph(DynamicGraph &&other) noexcept
     {
         number_of_nodes = other.number_of_nodes;
         // atomics can't be moved this is why we need an own constructor
@@ -164,7 +164,7 @@ template <typename EdgeDataT> class DynamicGraph
         edge_list = std::move(other.edge_list);
     }
 
-    DynamicGraph &operator=(DynamicGraph &&other)
+    DynamicGraph &operator=(DynamicGraph &&other) noexcept
     {
         number_of_nodes = other.number_of_nodes;
         // atomics can't be moved this is why we need an own constructor

include/util/permutation.hpp

@@ -10,9 +10,9 @@ namespace osrm::util
 
 namespace permutation_detail
 {
-template <typename T> static inline void swap(T &a, T &b) { std::swap(a, b); }
+template <typename T> static inline void swap(T &a, T &b) noexcept { std::swap(a, b); }
 
-static inline void swap(std::vector<bool>::reference a, std::vector<bool>::reference b)
+static inline void swap(std::vector<bool>::reference a, std::vector<bool>::reference b) noexcept
 {
     std::vector<bool>::swap(a, b);
 }

scripts/ci/run_benchmarks.sh

@@ -1,6 +1,18 @@
 #!/bin/bash
 set -eou pipefail
 
+function measure_peak_ram_and_time {
+    COMMAND=$1
+    OUTPUT_FILE=$2
+
+    OUTPUT=$(/usr/bin/time -f "%e %M" $COMMAND 2>&1 | tail -n 1)
+
+    TIME=$(echo $OUTPUT | awk '{print $1}')
+    PEAK_RAM_KB=$(echo $OUTPUT | awk '{print $2}')
+    PEAK_RAM_MB=$(echo "scale=2; $PEAK_RAM_KB / 1024" | bc)
+    echo "Time: ${TIME}s Peak RAM: ${PEAK_RAM_MB}MB" > $OUTPUT_FILE
+}
+
 function run_benchmarks_for_folder {
     echo "Running benchmarks for $1"
 
@@ -23,10 +35,11 @@ function run_benchmarks_for_folder {
     BINARIES_FOLDER="$FOLDER/build"
 
     cp ~/data.osm.pbf $FOLDER
-    $BINARIES_FOLDER/osrm-extract -p $FOLDER/profiles/car.lua $FOLDER/data.osm.pbf
+
-    $BINARIES_FOLDER/osrm-partition $FOLDER/data.osrm
+    measure_peak_ram_and_time "$BINARIES_FOLDER/osrm-extract -p $FOLDER/profiles/car.lua $FOLDER/data.osm.pbf" "$RESULTS_FOLDER/osrm_extract.bench"
-    $BINARIES_FOLDER/osrm-customize $FOLDER/data.osrm
+    measure_peak_ram_and_time "$BINARIES_FOLDER/osrm-partition $FOLDER/data.osrm" "$RESULTS_FOLDER/osrm_partition.bench"
-    $BINARIES_FOLDER/osrm-contract $FOLDER/data.osrm
+    measure_peak_ram_and_time "$BINARIES_FOLDER/osrm-customize $FOLDER/data.osrm" "$RESULTS_FOLDER/osrm_customize.bench"
+    measure_peak_ram_and_time "$BINARIES_FOLDER/osrm-contract $FOLDER/data.osrm" "$RESULTS_FOLDER/osrm_contract.bench"
 
     if [ -f "$FOLDER/scripts/ci/locustfile.py" ]; then
         for ALGORITHM in mld ch; do

src/server/connection.cpp

@@ -186,6 +186,7 @@ void Connection::handle_timeout(boost::system::error_code ec)
     if (ec != boost::asio::error::operation_aborted)
     {
         boost::system::error_code ignore_error;
+        // NOLINTNEXTLINE(bugprone-unused-return-value)
         TCP_socket.cancel(ignore_error);
         handle_shutdown();
     }
@@ -197,6 +198,7 @@ void Connection::handle_shutdown()
     timer.cancel();
     // Initiate graceful connection closure.
     boost::system::error_code ignore_error;
+    // NOLINTNEXTLINE(bugprone-unused-return-value)
     TCP_socket.shutdown(boost::asio::ip::tcp::socket::shutdown_both, ignore_error);
 }
 

third_party/sol2/include/sol/sol.hpp

@@ -19416,7 +19416,14 @@ 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) 
+// see https://github.com/ThePhD/sol2/issues/1581#issuecomment-2103463524
+#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);
@@ -19456,7 +19463,14 @@ 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) 
+// see https://github.com/ThePhD/sol2/issues/1581#issuecomment-2103463524		
+#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);

unit_tests/library/route.cpp

@@ -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;
                 BOOST_CHECK(!name.empty());
 
                 const auto &maneuver =

unit_tests/partitioner/bisection_graph_view.cpp

@@ -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)
 {

unit_tests/util/static_rtree.cpp

@@ -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);