Refactor CMake code related to compiler warnings, enable some additional warnings (#6355)

This commit is contained in:
Siarhei Fedartsou 2022-09-30 11:42:36 +02:00 committed by GitHub
parent 3c5d99b4cb
commit 902bfc7806
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
23 changed files with 142 additions and 93 deletions

View File

@ -335,7 +335,6 @@ jobs:
BUILD_TYPE: Release
CCOMPILER: clang-6.0
CXXCOMPILER: clang++-6.0
ENABLE_GLIBC_WORKAROUND: ON
ENABLE_CONAN: ON
NODE_PACKAGE_TESTS_ONLY: ON
@ -347,7 +346,6 @@ jobs:
BUILD_TYPE: Debug
CCOMPILER: clang-6.0
CXXCOMPILER: clang++-6.0
ENABLE_GLIBC_WORKAROUND: ON
ENABLE_CONAN: ON
NODE_PACKAGE_TESTS_ONLY: ON
@ -359,7 +357,6 @@ jobs:
BUILD_TYPE: Release
CCOMPILER: clang-6.0
CXXCOMPILER: clang++-6.0
ENABLE_GLIBC_WORKAROUND: ON
ENABLE_CONAN: ON
NODE_PACKAGE_TESTS_ONLY: ON
@ -371,7 +368,6 @@ jobs:
BUILD_TYPE: Debug
CCOMPILER: clang-6.0
CXXCOMPILER: clang++-6.0
ENABLE_GLIBC_WORKAROUND: ON
ENABLE_CONAN: ON
NODE_PACKAGE_TESTS_ONLY: ON
@ -384,7 +380,6 @@ jobs:
BUILD_TYPE: Release
CCOMPILER: clang-6.0
CXXCOMPILER: clang++-6.0
ENABLE_GLIBC_WORKAROUND: ON
ENABLE_CONAN: ON
NODE_PACKAGE_TESTS_ONLY: ON
@ -396,7 +391,6 @@ jobs:
BUILD_TYPE: Debug
CCOMPILER: clang-6.0
CXXCOMPILER: clang++-6.0
ENABLE_GLIBC_WORKAROUND: ON
ENABLE_CONAN: ON
NODE_PACKAGE_TESTS_ONLY: ON
@ -433,7 +427,6 @@ jobs:
BUILD_TYPE: Release
CCOMPILER: clang-6.0
CXXCOMPILER: clang++-6.0
ENABLE_GLIBC_WORKAROUND: ON
ENABLE_CONAN: ON
NODE_PACKAGE_TESTS_ONLY: ON
@ -445,7 +438,6 @@ jobs:
BUILD_TYPE: Debug
CCOMPILER: clang-6.0
CXXCOMPILER: clang++-6.0
ENABLE_GLIBC_WORKAROUND: ON
ENABLE_CONAN: ON
NODE_PACKAGE_TESTS_ONLY: ON
@ -482,7 +474,6 @@ jobs:
BUILD_TYPE: Release
CCOMPILER: clang-6.0
CXXCOMPILER: clang++-6.0
ENABLE_GLIBC_WORKAROUND: ON
ENABLE_CONAN: ON
NODE_PACKAGE_TESTS_ONLY: ON
@ -494,7 +485,6 @@ jobs:
BUILD_TYPE: Debug
CCOMPILER: clang-6.0
CXXCOMPILER: clang++-6.0
ENABLE_GLIBC_WORKAROUND: ON
ENABLE_CONAN: ON
NODE_PACKAGE_TESTS_ONLY: ON
@ -513,7 +503,6 @@ jobs:
ENABLE_ASSERTIONS: ${{ matrix.ENABLE_ASSERTIONS }}
ENABLE_CLANG_TIDY: ${{ matrix.ENABLE_CLANG_TIDY }}
ENABLE_COVERAGE: ${{ matrix.ENABLE_COVERAGE }}
ENABLE_GLIBC_WORKAROUND: ${{ matrix.ENABLE_GLIBC_WORKAROUND }}
ENABLE_CONAN: ${{ matrix.ENABLE_CONAN }}
ENABLE_SANITIZER: ${{ matrix.ENABLE_SANITIZER }}
NODE_PACKAGE_TESTS_ONLY: ${{ matrix.NODE_PACKAGE_TESTS_ONLY }}
@ -671,8 +660,8 @@ jobs:
-DBUILD_TOOLS=${BUILD_TOOLS:-OFF} \
-DENABLE_CCACHE=ON \
-DCMAKE_INSTALL_PREFIX=${OSRM_INSTALL_DIR} \
-DENABLE_GLIBC_WORKAROUND=${ENABLE_GLIBC_WORKAROUND:-OFF} \
"${APPLE_SILICON_FLAGS[@]}"
make --jobs=${JOBS}
if [[ "${NODE_PACKAGE_TESTS_ONLY}" != "ON" && "${ENABLE_APPLE_SILICON}" != "ON" ]]; then

View File

@ -35,7 +35,6 @@ option(ENABLE_LTO "Use LTO if available" OFF)
option(ENABLE_FUZZING "Fuzz testing using LLVM's libFuzzer" OFF)
option(ENABLE_GOLD_LINKER "Use GNU gold linker if available" ON)
option(ENABLE_NODE_BINDINGS "Build NodeJs bindings" OFF)
option(ENABLE_GLIBC_WORKAROUND "Workaround GLIBC symbol exports" OFF)
option(ENABLE_CLANG_TIDY "Enables clang-tidy checks" OFF)
if (ENABLE_CLANG_TIDY)
@ -337,9 +336,9 @@ if (ENABLE_SANITIZER)
endif()
# Configuring compilers
set(OSRM_WARNING_FLAGS "-Werror=all -Werror=extra -Werror=uninitialized -Werror=unreachable-code -Werror=unused-variable -Werror=unreachable-code -Wno-error=cpp -Wpedantic")
include(cmake/warnings.cmake)
if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${OSRM_WARNING_FLAGS} -Werror=strict-overflow=2 -Wno-error=unused-local-typedef -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fPIC -fcolor-diagnostics -ftemplate-depth=1024 -Wno-unused-command-line-argument")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fPIC -fcolor-diagnostics -ftemplate-depth=1024")
elseif(CMAKE_CXX_COMPILER_ID MATCHES "GNU")
set(COLOR_FLAG "-fdiagnostics-color=auto")
check_cxx_compiler_flag("-fdiagnostics-color=auto" HAS_COLOR_FLAG)
@ -347,7 +346,7 @@ elseif(CMAKE_CXX_COMPILER_ID MATCHES "GNU")
set(COLOR_FLAG "")
endif()
# using GCC
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${OSRM_WARNING_FLAGS} -Werror=strict-overflow=1 -Wno-error=maybe-uninitialized -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 ${COLOR_FLAG} -fPIC -ftemplate-depth=1024")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 ${COLOR_FLAG} -fPIC -ftemplate-depth=1024")
if(WIN32) # using mingw
add_dependency_defines(-DWIN32)
@ -440,6 +439,8 @@ include_directories(SYSTEM ${CHEAPRULER_INCLUDE_DIR})
add_library(MICROTAR OBJECT "${CMAKE_CURRENT_SOURCE_DIR}/third_party/microtar/src/microtar.c")
set_property(TARGET MICROTAR PROPERTY POSITION_INDEPENDENT_CODE ON)
target_no_warning(MICROTAR unused-variable)
target_no_warning(MICROTAR format)
set(PROTOZERO_INCLUDE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/third_party/protozero/include")
include_directories(SYSTEM ${PROTOZERO_INCLUDE_DIR})
@ -861,10 +862,6 @@ add_custom_target(uninstall
add_subdirectory(unit_tests)
add_subdirectory(src/benchmarks)
if (ENABLE_GLIBC_WORKAROUND)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DGLIBC_WORKAROUND")
endif()
if (ENABLE_NODE_BINDINGS)
add_subdirectory(src/nodejs)
endif()

93
cmake/warnings.cmake Normal file
View File

@ -0,0 +1,93 @@
include (CheckCXXCompilerFlag)
include (CheckCCompilerFlag)
# Try to add -Wflag if compiler supports it
macro (add_warning flag)
string(REPLACE "-" "_" underscored_flag ${flag})
string(REPLACE "+" "x" underscored_flag ${underscored_flag})
check_cxx_compiler_flag("-W${flag}" SUPPORTS_CXXFLAG_${underscored_flag})
check_c_compiler_flag("-W${flag}" SUPPORTS_CFLAG_${underscored_flag})
if (SUPPORTS_CXXFLAG_${underscored_flag})
set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -W${flag}")
else()
message (STATUS "Flag -W${flag} is unsupported")
endif()
if (SUPPORTS_CFLAG_${underscored_flag})
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -W${flag}")
else()
message(STATUS "Flag -W${flag} is unsupported")
endif()
endmacro()
# Try to add -Wno flag if compiler supports it
macro (no_warning flag)
add_warning(no-${flag})
endmacro ()
# The same but only for specified target.
macro (target_add_warning target flag)
string (REPLACE "-" "_" underscored_flag ${flag})
string (REPLACE "+" "x" underscored_flag ${underscored_flag})
check_cxx_compiler_flag("-W${flag}" SUPPORTS_CXXFLAG_${underscored_flag})
if (SUPPORTS_CXXFLAG_${underscored_flag})
target_compile_options (${target} PRIVATE "-W${flag}")
else ()
message (STATUS "Flag -W${flag} is unsupported")
endif ()
endmacro ()
macro (target_no_warning target flag)
target_add_warning(${target} no-${flag})
endmacro ()
add_warning(all)
add_warning(extra)
add_warning(pedantic)
add_warning(error) # treat all warnings as errors
if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
add_warning(strict-overflow=2)
elseif(CMAKE_CXX_COMPILER_ID MATCHES "GNU")
add_warning(strict-overflow=1)
endif()
add_warning(suggest-override)
add_warning(suggest-destructor-override)
add_warning(unused)
add_warning(unreachable-code)
add_warning(delete-incomplete)
add_warning(duplicated-cond)
add_warning(disabled-optimization)
add_warning(init-self)
add_warning(bool-compare)
add_warning(logical-not-parentheses)
add_warning(logical-op)
add_warning(maybe-uninitialized)
add_warning(misleading-indentation)
# `no-` prefix is part of warning name(i.e. doesn't mean we are disabling it)
add_warning(no-return-local-addr)
add_warning(odr)
add_warning(pointer-arith)
add_warning(redundant-decls)
add_warning(reorder)
add_warning(shift-negative-value)
add_warning(sizeof-array-argument)
add_warning(switch-bool)
add_warning(tautological-compare)
add_warning(trampolines)
no_warning(c++17-extensions)
# TODO: these warnings are not enabled by default, but we consider them as useful and good to enable in the future
no_warning(implicit-int-conversion)
no_warning(implicit-float-conversion)
no_warning(unused-member-function)
no_warning(old-style-cast)
no_warning(non-virtual-dtor)
no_warning(float-conversion)
no_warning(sign-conversion)
no_warning(shorten-64-to-32)
no_warning(padded)
no_warning(missing-noreturn)

View File

@ -56,7 +56,7 @@ inline auto contractExcludableGraph(ContractorGraph contractor_graph_,
// By not contracting all contractable nodes we avoid creating
// a very dense core. This increases the overall graph sizes a little bit
// but increases the final CH quality and contraction speed.
constexpr float BASE_CORE = 0.9;
constexpr float BASE_CORE = 0.9f;
is_shared_core =
contractGraph(contractor_graph, std::move(always_allowed), node_weights, BASE_CORE);

View File

@ -123,9 +123,9 @@ class BaseAPI
const auto &snapped_location = candidatesSnappedLocation(candidates);
const auto &input_location = candidatesInputLocation(candidates);
auto location =
fbresult::Position(static_cast<double>(util::toFloating(snapped_location.lon)),
static_cast<double>(util::toFloating(snapped_location.lat)));
auto location = fbresult::Position(
static_cast<float>(static_cast<double>(util::toFloating(snapped_location.lon))),
static_cast<float>(static_cast<double>(util::toFloating(snapped_location.lat))));
const auto toName = [this](const auto &phantom) {
return facade.GetNameForID(facade.GetNameIndex(phantom.forward_segment_id.id))
@ -156,8 +156,8 @@ class BaseAPI
auto waypoint = std::make_unique<fbresult::WaypointBuilder>(*builder);
waypoint->add_location(&location);
waypoint->add_distance(
util::coordinate_calculation::greatCircleDistance(snapped_location, input_location));
waypoint->add_distance(static_cast<float>(
util::coordinate_calculation::greatCircleDistance(snapped_location, input_location)));
waypoint->add_name(name_string);
if (parameters.generate_hints)
{

View File

@ -83,7 +83,7 @@ template <typename Algorithm> class Engine final : public EngineInterface
Engine(const Engine &) = delete;
Engine &operator=(const Engine &) = delete;
virtual ~Engine() = default;
virtual ~Engine() override = default;
Status Route(const api::RouteParameters &params, api::ResultT &result) const override final
{

View File

@ -72,7 +72,7 @@ template <typename Algorithm> class RoutingAlgorithms final : public RoutingAlgo
{
}
virtual ~RoutingAlgorithms() = default;
virtual ~RoutingAlgorithms() override = default;
InternalManyRoutesResult
AlternativePathSearch(const PhantomEndpointCandidates &endpoint_candidates,

View File

@ -75,7 +75,7 @@ template <typename Data> struct SharedMonitor
region = bi::mapped_region(shmem, bi::read_write);
}
catch (const bi::interprocess_exception &exception)
catch (const bi::interprocess_exception &)
{
auto message = boost::format("No shared memory block '%1%' found, have you forgotten "
"to run osrm-datastore?") %
@ -124,7 +124,7 @@ template <typename Data> struct SharedMonitor
bi::shared_memory_object shmem_open =
bi::shared_memory_object(bi::open_only, Data::name, bi::read_only);
}
catch (const bi::interprocess_exception &exception)
catch (const bi::interprocess_exception &)
{
return false;
}

View File

@ -106,7 +106,7 @@ class RuntimeError : public exception
// This function exists to 'anchor' the class, and stop the compiler from
// copying vtable and RTTI info into every object file that includes
// this header. (Caught by -Wweak-vtables under Clang.)
virtual void anchor() const;
virtual void anchor() const override;
const ErrorCode code;
static std::string BuildMessage(const std::string &message,

View File

@ -87,11 +87,11 @@ template <typename Integer, typename Filter> class filtered_range
// convenience function to construct an integer range with type deduction
template <typename Integer, typename Filter>
filtered_range<Integer, Filter>
filtered_irange(const Integer first,
const Integer last,
const Filter &filter,
typename std::enable_if<std::is_integral<Integer>::value>::type * = 0) noexcept
filtered_range<Integer, Filter> filtered_irange(
const Integer first,
const Integer last,
const Filter &filter,
typename std::enable_if<std::is_integral<Integer>::value>::type * = nullptr) noexcept
{
return filtered_range<Integer, Filter>(first, last, filter);
}

View File

@ -89,8 +89,8 @@ struct DiyFp
h++;
return DiyFp(h, e + rhs.e + 64);
#elif (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6)) && defined(__x86_64__)
unsigned __int128 p =
static_cast<unsigned __int128>(f) * static_cast<unsigned __int128>(rhs.f);
__extension__ using uint128 = unsigned __int128;
uint128 p = static_cast<uint128>(f) * static_cast<uint128>(rhs.f);
uint64_t h = p >> 64;
uint64_t l = static_cast<uint64_t>(p);
if (l & (uint64_t(1) << 63)) // rounding

View File

@ -86,7 +86,7 @@ template <typename Integer>
range<Integer>
irange(const Integer first,
const Integer last,
typename std::enable_if<std::is_integral<Integer>::value>::type * = 0) noexcept
typename std::enable_if<std::is_integral<Integer>::value>::type * = nullptr) noexcept
{
return range<Integer>(first, last);
}

View File

@ -49,14 +49,16 @@ template <typename WordT, typename T>
inline T get_lower_half_value(WordT word,
WordT mask,
std::uint8_t offset,
typename std::enable_if_t<std::is_integral<T>::value> * = 0)
typename std::enable_if_t<std::is_integral<T>::value> * = nullptr)
{
return static_cast<T>((word & mask) >> offset);
}
template <typename WordT, typename T>
inline T
get_lower_half_value(WordT word, WordT mask, std::uint8_t offset, typename T::value_type * = 0)
inline T get_lower_half_value(WordT word,
WordT mask,
std::uint8_t offset,
typename T::value_type * = nullptr)
{
return T{static_cast<typename T::value_type>((word & mask) >> offset)};
}
@ -65,14 +67,16 @@ template <typename WordT, typename T>
inline T get_upper_half_value(WordT word,
WordT mask,
std::uint8_t offset,
typename std::enable_if_t<std::is_integral<T>::value> * = 0)
typename std::enable_if_t<std::is_integral<T>::value> * = nullptr)
{
return static_cast<T>((word & mask) << offset);
}
template <typename WordT, typename T>
inline T
get_upper_half_value(WordT word, WordT mask, std::uint8_t offset, typename T::value_type * = 0)
inline T get_upper_half_value(WordT word,
WordT mask,
std::uint8_t offset,
typename T::value_type * = nullptr)
{
static_assert(std::is_unsigned<WordT>::value, "Only unsigned word types supported for now.");
return T{static_cast<typename T::value_type>((word & mask) << offset)};

View File

@ -63,8 +63,6 @@ template <typename NodeID, typename Key> class ArrayStorage
public:
explicit ArrayStorage(std::size_t size) : positions(size, 0) {}
~ArrayStorage() {}
Key &operator[](NodeID node) { return positions[node]; }
Key peek_index(const NodeID node) const { return positions[node]; }

View File

@ -313,8 +313,8 @@ class StaticGraph
}
protected:
NodeIterator number_of_nodes;
EdgeIterator number_of_edges;
NodeIterator number_of_nodes = 0;
EdgeIterator number_of_edges = 0;
Vector<NodeArrayEntry> node_array;
Vector<EdgeArrayEntry> edge_array;

View File

@ -118,7 +118,7 @@ static const EdgeDuration MAXIMAL_EDGE_DURATION = std::numeric_limits<EdgeDurati
static const EdgeDistance MAXIMAL_EDGE_DISTANCE = std::numeric_limits<EdgeDistance>::max();
static const TurnPenalty INVALID_TURN_PENALTY = std::numeric_limits<TurnPenalty>::max();
static const EdgeDistance INVALID_EDGE_DISTANCE = std::numeric_limits<EdgeDistance>::max();
static const EdgeDistance INVALID_FALLBACK_SPEED = std::numeric_limits<double>::max();
static const EdgeDistance INVALID_FALLBACK_SPEED = std::numeric_limits<EdgeDistance>::max();
using DatasourceID = std::uint8_t;

View File

@ -667,7 +667,7 @@ void ExtractionContainers::PrepareEdges(ScriptingEnvironment &scripting_environm
auto &edge = edge_iterator->result;
edge.weight = std::max<EdgeWeight>(1, std::round(segment.weight * weight_multiplier));
edge.duration = std::max<EdgeWeight>(1, std::round(segment.duration * 10.));
edge.distance = accurate_distance;
edge.distance = static_cast<float>(accurate_distance);
// assign new node id
const auto node_id = mapExternalToInternalNodeID(

View File

@ -17,6 +17,9 @@ add_nodejs_module(node_osrm node_osrm.cpp)
set_target_properties(node_osrm PROPERTIES CXX_STANDARD 17)
# TODO: we disable clang-tidy for this target, because it causes errors in third-party NodeJs related headers
set_target_properties(node_osrm PROPERTIES CXX_CLANG_TIDY "")
# TODO: we turn off some warnings for this target, because it causes errors in third-party NodeJs related headers
target_no_warning(node_osrm suggest-destructor-override)
target_no_warning(node_osrm suggest-override)
target_link_libraries(node_osrm osrm)

View File

@ -141,15 +141,14 @@ inline void async(const Nan::FunctionCallbackInfo<v8::Value> &info,
struct Worker final : Nan::AsyncWorker
{
using Base = Nan::AsyncWorker;
Worker(std::shared_ptr<osrm::OSRM> osrm_,
ParamPtr params_,
ServiceMemFn service,
Nan::Callback *callback,
PluginParameters pluginParams_)
: Base(callback, "osrm:async"), osrm{std::move(osrm_)}, service{std::move(service)},
params{std::move(params_)}, pluginParams{std::move(pluginParams_)}
: Nan::AsyncWorker(callback, "osrm:async"), osrm{std::move(osrm_)},
service{std::move(service)}, params{std::move(params_)}, pluginParams{
std::move(pluginParams_)}
{
}
@ -244,14 +243,12 @@ inline void asyncForTiles(const Nan::FunctionCallbackInfo<v8::Value> &info,
struct Worker final : Nan::AsyncWorker
{
using Base = Nan::AsyncWorker;
Worker(std::shared_ptr<osrm::OSRM> osrm_,
ParamPtr params_,
ServiceMemFn service,
Nan::Callback *callback,
PluginParameters pluginParams_)
: Base(callback, "osrm:asyncForTiles"), osrm{std::move(osrm_)},
: Nan::AsyncWorker(callback, "osrm:asyncForTiles"), osrm{std::move(osrm_)},
service{std::move(service)}, params{std::move(params_)}, pluginParams{
std::move(pluginParams_)}
{

View File

@ -249,7 +249,6 @@ catch (const osrm::RuntimeError &e)
{
util::DumpMemoryStats();
util::Log(logERROR) << e.what();
return EXIT_FAILURE;
return e.GetCode();
}
catch (const std::bad_alloc &e)

View File

@ -1,31 +0,0 @@
#ifdef GLIBC_WORKAROUND
#include <stdexcept>
// https://github.com/bitcoin/bitcoin/pull/4042
// allows building against libstdc++-dev-4.9 while avoiding
// GLIBCXX_3.4.20 dep
// This is needed because libstdc++ itself uses this API - its not
// just an issue of your code using it, ughhh
// Note: only necessary on Linux
#ifdef __linux__
#define _ENABLE_GLIBC_WORKAROUND
#warning building with workaround
#else
#warning not building with workaround
#endif
#ifdef _ENABLE_GLIBC_WORKAROUND
namespace std
{
void __throw_out_of_range_fmt(const char *, ...) __attribute__((__noreturn__));
void __throw_out_of_range_fmt(const char *err, ...)
{
// Safe and over-simplified version. Ignore the format and print it as-is.
__throw_out_of_range(err);
}
} // namespace std
#endif // _ENABLE_GLIBC_WORKAROUND
#endif // GLIBC_WORKAROUND

View File

@ -119,7 +119,7 @@ class ContiguousInternalMemoryDataFacade<routing_algorithms::offline::Algorithm>
ContiguousInternalMemoryDataFacade() {}
~ContiguousInternalMemoryDataFacade() {}
~ContiguousInternalMemoryDataFacade() override {}
unsigned GetNumberOfNodes() const { return 0; }

View File

@ -28,11 +28,11 @@ BOOST_AUTO_TEST_CASE(insert_and_retrieve_packed_test)
std::mt19937 rng;
rng.seed(1337);
std::uniform_int_distribution<std::mt19937::result_type> dist(0, max_id);
std::uniform_int_distribution<std::uint64_t> dist(0, max_id);
for (std::size_t i = 0; i < num_test_cases; i++)
{
OSMNodeID r{static_cast<std::uint64_t>(dist(rng))}; // max 33-bit uint
OSMNodeID r{dist(rng)}; // max 33-bit uint
packed_ids.push_back(r);
original_ids.push_back(r);