Reduce the number of MSVC warnings in CI workflow builds on Windows (#7254)

afarber · web-flow · commit 51f5bdcd42e4 · 2025-10-15T08:01:30.000+02:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,7 @@
 # Unreleased
   - Changes from 6.0.0
+    - Build:
+      - FIXED: Reduce MSVC compiler warnings by suppressing informational warnings while preserving bug-indicating warnings [#7253](https://github.com/Project-OSRM/osrm-backend/issues/7253)
     - Misc:
       - CHANGED: Update fmt library to version 11.2.0 [#7238](https://github.com/Project-OSRM/osrm-backend/issues/7238)
       - CHANGED: Upgrade protozero from v1.7.1 to v1.8.1 [#7239](https://github.com/Project-OSRM/osrm-backend/pull/7239)
diff --git a/cmake/warnings.cmake b/cmake/warnings.cmake
@@ -1,7 +1,7 @@
 include (CheckCXXCompilerFlag)
 include (CheckCCompilerFlag)
 
-# Try to add -Wflag if compiler supports it
+# Try to add -Wflag if compiler supports it (GCC/Clang)
 macro (add_warning flag)
     string(REPLACE "-" "_" underscored_flag ${flag})
     string(REPLACE "+" "x" underscored_flag ${underscored_flag})
@@ -22,6 +22,37 @@ macro (add_warning flag)
     endif()
 endmacro()
 
+# MSVC warning management macros
+macro (msvc_warning_level level)
+    if (MSVC)
+        set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /W${level}")
+        set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /W${level}")
+    endif()
+endmacro()
+
+# Disable specific MSVC warning by code (e.g., 4711)
+macro (msvc_disable_warning code)
+    if (MSVC)
+        set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd${code}")
+        set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /wd${code}")
+    endif()
+endmacro()
+
+# Enable specific MSVC warning by code
+macro (msvc_enable_warning code)
+    if (MSVC)
+        set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /w1${code}")
+        set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /w1${code}")
+    endif()
+endmacro()
+
+# Disable MSVC warning for specific target
+macro (target_msvc_disable_warning target code)
+    if (MSVC)
+        target_compile_options(${target} PRIVATE "/wd${code}")
+    endif()
+endmacro()
+
 # Try to add -Wno flag if compiler supports it
 macro (no_warning flag)
     add_warning(no-${flag})
@@ -85,4 +116,57 @@ no_warning(restrict)
 no_warning(free-nonheap-object)
 if(CMAKE_CXX_COMPILER_ID MATCHES "GNU")
   no_warning(stringop-overflow)
+endif()
+
+# MSVC-specific warning configuration
+if(MSVC)
+  # Set warning level 3 (default level with reasonable warnings)
+  msvc_warning_level(3)
+
+  # Disable excessive informational warnings that don't indicate bugs
+  msvc_disable_warning(4711)  # Function selected for automatic inline expansion
+  msvc_disable_warning(4710)  # Function not inlined
+  msvc_disable_warning(4514)  # Unreferenced inline function removed
+  msvc_disable_warning(4820)  # Padding added to struct/class
+
+  # Disable implicit special member function warnings (often unavoidable in modern C++)
+  msvc_disable_warning(4626)  # Assignment operator implicitly deleted
+  msvc_disable_warning(4625)  # Copy constructor implicitly deleted
+  msvc_disable_warning(5027)  # Move assignment operator implicitly deleted
+  msvc_disable_warning(5026)  # Move constructor implicitly deleted
+  msvc_disable_warning(4623)  # Default constructor implicitly deleted
+
+  # Disable other low-value warnings
+  msvc_disable_warning(5219)  # Implicit conversion from T to bool
+  msvc_disable_warning(5045)  # Spectre mitigation
+  msvc_disable_warning(5246)  # Aggregate initialization
+  msvc_disable_warning(4686)  # Template specialization
+  msvc_disable_warning(5266)  # const qualifier discarded
+  msvc_disable_warning(4800)  # Implicit conversion to bool
+  msvc_disable_warning(4868)  # Left-to-right evaluation order
+  msvc_disable_warning(4371)  # Layout change from previous compiler version
+  msvc_disable_warning(4127)  # Conditional expression is constant
+  msvc_disable_warning(4355)  # 'this' used in member initializer list
+  msvc_disable_warning(4668)  # Preprocessor macro not defined
+  msvc_disable_warning(5039)  # Exception specification issue
+  msvc_disable_warning(4777)  # Format string mismatch
+  msvc_disable_warning(5264)  # Variable declared but not used
+
+  # KEEP these warnings enabled - they indicate potential bugs:
+  # C4365: Signed/unsigned mismatch
+  # C4267: Conversion with possible loss of data
+  # C4244: Data conversion with possible loss
+  # C4242: Data conversion with possible loss
+  # C4458: Declaration hides class member
+  # C4245: Signed/unsigned mismatch in conversion
+  # C4389: Signed/unsigned mismatch in equality
+  # C4457: Declaration hides function parameter
+  # C4146: Unary minus applied to unsigned type
+  # C4456: Declaration hides previous local declaration
+  # C4996: Deprecated function usage
+  # C4100: Unreferenced formal parameter
+  # C4101: Unreferenced local variable
+  # C4061: Switch statement case not handled
+
+  message(STATUS "MSVC warning configuration applied - suppressed informational warnings, kept bug-indicating warnings")
 endif()
diff --git a/include/contractor/contracted_edge_container.hpp b/include/contractor/contracted_edge_container.hpp
@@ -61,7 +61,7 @@ struct ContractedEdgeContainer
     void Filter(const std::vector<bool> &filter, std::size_t index)
     {
         BOOST_ASSERT(index < sizeof(MergedFlags) * CHAR_BIT);
-        const MergedFlags flag = 1 << index;
+        const MergedFlags flag = MergedFlags{1} << index;
 
         for (auto edge_index : util::irange<std::size_t>(0, edges.size()))
         {
@@ -81,7 +81,7 @@ struct ContractedEdgeContainer
     {
         BOOST_ASSERT(index < sizeof(MergedFlags) * CHAR_BIT);
 
-        const MergedFlags flag = 1 << index++;
+        const MergedFlags flag = MergedFlags{1} << index++;
 
         auto edge_iter = edges.cbegin();
         auto edge_end = edges.cend();
@@ -151,7 +151,7 @@ struct ContractedEdgeContainer
         std::vector<std::vector<bool>> filters(index);
         for (const auto flag_index : util::irange<std::size_t>(0, index))
         {
-            MergedFlags mask = 1 << flag_index;
+            MergedFlags mask = MergedFlags{1} << flag_index;
             for (const auto flag : flags)
             {
                 filters[flag_index].push_back(flag & mask);
diff --git a/include/engine/douglas_peucker.hpp b/include/engine/douglas_peucker.hpp
@@ -24,7 +24,9 @@ inline std::vector<std::uint64_t> generateThreshold(double min_pixel, unsigned n
         const double shift = (1u << zoom) * 256;
         const double b = shift / 2.0;
         const double pixel_to_deg = 180. / b;
-        const std::uint64_t min_deg = min_pixel * pixel_to_deg * COORDINATE_PRECISION;
+        // Safe conversion: geographic coordinate calculation always produces positive integers
+        const std::uint64_t min_deg =
+            static_cast<std::uint64_t>(min_pixel * pixel_to_deg * COORDINATE_PRECISION);
         thresholds[zoom] = min_deg * min_deg;
     }
 
diff --git a/include/engine/guidance/assemble_steps.hpp b/include/engine/guidance/assemble_steps.hpp
@@ -215,8 +215,9 @@ inline std::vector<RouteStep> assembleSteps(const datafacade::BaseDataFacade &fa
                 {
                     intersection.entry.push_back(entry_class.allowsEntry(idx));
                 }
+                // Safe conversion: bearing values [0,360) fit comfortably in int16_t
                 std::int16_t bearing_in_driving_direction =
-                    util::bearing::reverse(std::round(bearings.first));
+                    static_cast<std::int16_t>(util::bearing::reverse(std::round(bearings.first)));
                 maneuver = {intersection.location,
                             bearing_in_driving_direction,
                             bearings.second,
diff --git a/include/guidance/turn_bearing.hpp b/include/guidance/turn_bearing.hpp
@@ -14,7 +14,9 @@ class TurnBearing
   public:
     static constexpr double bearing_scale = 360.0 / 256.0;
     // discretizes a bearing into distinct units of 1.4 degrees
-    TurnBearing(const double value = 0) : bearing(value / bearing_scale)
+    TurnBearing(const double value = 0)
+        // Safe conversion: [0,360) / 1.40625 = [0,256) fits exactly in uint8_t
+        : bearing(static_cast<std::uint8_t>(value / bearing_scale))
     {
         BOOST_ASSERT_MSG(value >= 0 && value < 360.0,
                          "Bearing value needs to be between 0 and 360 (exclusive)");
diff --git a/include/partitioner/multi_level_partition.hpp b/include/partitioner/multi_level_partition.hpp
@@ -15,6 +15,7 @@
 #include <climits>
 #include <cmath>
 #include <cstdint>
+#include <limits>
 #include <numeric>
 #include <vector>
 
@@ -217,9 +218,10 @@ template <storage::Ownership Ownership> class MultiLevelPartitionImpl final
                                 BOOST_ASSERT(next_offset <= NUM_PARTITION_BITS);
                                 // Check offset for shift overflow. Offsets are strictly increasing,
                                 // so we only need the check on the last mask.
-                                PartitionID next_mask = next_offset == NUM_PARTITION_BITS
-                                                            ? -1ULL
-                                                            : (1ULL << next_offset) - 1ULL;
+                                PartitionID next_mask =
+                                    next_offset == NUM_PARTITION_BITS
+                                        ? std::numeric_limits<PartitionID>::max()
+                                        : (1ULL << next_offset) - 1ULL;
                                 // 001100
                                 masks[lidx++] = next_mask ^ mask;
                             });
diff --git a/include/storage/shared_datatype.hpp b/include/storage/shared_datatype.hpp
@@ -146,7 +146,7 @@ class ContiguousDataLayout final : public BaseDataLayout
     inline void *align(void *&ptr) const noexcept
     {
         const auto intptr = reinterpret_cast<std::uintptr_t>(ptr);
-        const auto aligned = (intptr - 1u + BLOCK_ALIGNMENT) & -BLOCK_ALIGNMENT;
+        const auto aligned = (intptr - 1u + BLOCK_ALIGNMENT) & ~(BLOCK_ALIGNMENT - 1u);
         return ptr = reinterpret_cast<void *>(aligned);
     }
 
diff --git a/include/util/xor_fast_hash.hpp b/include/util/xor_fast_hash.hpp
@@ -44,10 +44,10 @@ template <std::size_t MaxNumElements = (1u << 16u)> class XORFastHash
     {
         std::mt19937 generator(1); // impl. defined but deterministic default seed
 
-        std::iota(begin(table1), end(table1), 0u);
+        std::iota(begin(table1), end(table1), std::uint16_t{0});
         std::shuffle(begin(table1), end(table1), generator);
 
-        std::iota(begin(table2), end(table2), 0u);
+        std::iota(begin(table2), end(table2), std::uint16_t{0});
         std::shuffle(begin(table2), end(table2), generator);
     }
 

Original file line number	Diff line number	Diff line change
`@@ -61,7 +61,7 @@ struct ContractedEdgeContainer`
`61`	`61`	`void Filter(const std::vector<bool> &filter, std::size_t index)`
`62`	`62`	`{`
`63`	`63`	`BOOST_ASSERT(index < sizeof(MergedFlags) * CHAR_BIT);`
`64`		`- const MergedFlags flag = 1 << index;`
	`64`	`+ const MergedFlags flag = MergedFlags{1} << index;`
`65`	`65`
`66`	`66`	`for (auto edge_index : util::irange<std::size_t>(0, edges.size()))`
`67`	`67`	`{`
`@@ -81,7 +81,7 @@ struct ContractedEdgeContainer`
`81`	`81`	`{`
`82`	`82`	`BOOST_ASSERT(index < sizeof(MergedFlags) * CHAR_BIT);`
`83`	`83`
`84`		`- const MergedFlags flag = 1 << index++;`
	`84`	`+ const MergedFlags flag = MergedFlags{1} << index++;`
`85`	`85`
`86`	`86`	`auto edge_iter = edges.cbegin();`
`87`	`87`	`auto edge_end = edges.cend();`
`@@ -151,7 +151,7 @@ struct ContractedEdgeContainer`
`151`	`151`	`std::vector<std::vector<bool>> filters(index);`
`152`	`152`	`for (const auto flag_index : util::irange<std::size_t>(0, index))`
`153`	`153`	`{`
`154`		`- MergedFlags mask = 1 << flag_index;`
	`154`	`+ MergedFlags mask = MergedFlags{1} << flag_index;`
`155`	`155`	`for (const auto flag : flags)`
`156`	`156`	`{`
`157`	`157`	`filters[flag_index].push_back(flag & mask);`
Original file line number	Diff line number	Diff line change
`@@ -215,8 +215,9 @@ inline std::vector<RouteStep> assembleSteps(const datafacade::BaseDataFacade &fa`
`215`	`215`	`{`
`216`	`216`	`intersection.entry.push_back(entry_class.allowsEntry(idx));`
`217`	`217`	`}`
	`218`	`+ // Safe conversion: bearing values [0,360) fit comfortably in int16_t`
`218`	`219`	`std::int16_t bearing_in_driving_direction =`
`219`		`- util::bearing::reverse(std::round(bearings.first));`
	`220`	`+ static_cast<std::int16_t>(util::bearing::reverse(std::round(bearings.first)));`
`220`	`221`	`maneuver = {intersection.location,`
`221`	`222`	`bearing_in_driving_direction,`
`222`	`223`	`bearings.second,`
Original file line number	Diff line number	Diff line change
`@@ -146,7 +146,7 @@ class ContiguousDataLayout final : public BaseDataLayout`
`146`	`146`	`inline void align(void &ptr) const noexcept`
`147`	`147`	`{`
`148`	`148`	`const auto intptr = reinterpret_cast<std::uintptr_t>(ptr);`
`149`		`- const auto aligned = (intptr - 1u + BLOCK_ALIGNMENT) & -BLOCK_ALIGNMENT;`
	`149`	`+ const auto aligned = (intptr - 1u + BLOCK_ALIGNMENT) & ~(BLOCK_ALIGNMENT - 1u);`
`150`	`150`	`return ptr = reinterpret_cast<void *>(aligned);`
`151`	`151`	`}`
`152`	`152`
Original file line number	Diff line number	Diff line change
`@@ -44,10 +44,10 @@ template <std::size_t MaxNumElements = (1u << 16u)> class XORFastHash`
`44`	`44`	`{`
`45`	`45`	`std::mt19937 generator(1); // impl. defined but deterministic default seed`
`46`	`46`
`47`		`- std::iota(begin(table1), end(table1), 0u);`
	`47`	`+ std::iota(begin(table1), end(table1), std::uint16_t{0});`
`48`	`48`	`std::shuffle(begin(table1), end(table1), generator);`
`49`	`49`
`50`		`- std::iota(begin(table2), end(table2), 0u);`
	`50`	`+ std::iota(begin(table2), end(table2), std::uint16_t{0});`
`51`	`51`	`std::shuffle(begin(table2), end(table2), generator);`
`52`	`52`	`}`
`53`	`53`